Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: don't load inactive users with sessions #2192

Merged
merged 3 commits into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions flask_appbuilder/security/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -2088,14 +2088,17 @@
raise NotImplementedError

def load_user(self, pk):
return self.get_user_by_id(int(pk))
user = self.get_user_by_id(int(pk))
if user.is_active:
return user

Check warning on line 2093 in flask_appbuilder/security/manager.py

View check run for this annotation

Codecov / codecov/patch

flask_appbuilder/security/manager.py#L2091-L2093

Added lines #L2091 - L2093 were not covered by tests

def load_user_jwt(self, _jwt_header, jwt_data):
identity = jwt_data["sub"]
user = self.load_user(identity)
# Set flask g.user to JWT user, we can't do it on before request
g.user = user
return user
if user.is_active:

Check warning on line 2098 in flask_appbuilder/security/manager.py

View check run for this annotation

Codecov / codecov/patch

flask_appbuilder/security/manager.py#L2098

Added line #L2098 was not covered by tests
# Set flask g.user to JWT user, we can't do it on before request
g.user = user
return user

Check warning on line 2101 in flask_appbuilder/security/manager.py

View check run for this annotation

Codecov / codecov/patch

flask_appbuilder/security/manager.py#L2100-L2101

Added lines #L2100 - L2101 were not covered by tests

@staticmethod
def before_request():
Expand Down
3 changes: 2 additions & 1 deletion tests/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
API_SECURITY_USERNAME_KEY,
API_SECURITY_VERSION,
)
from flask_appbuilder.security.sqla.models import User
from hiro import Timeline
import jinja2
from tests.const import (
Expand Down Expand Up @@ -130,7 +131,7 @@ def create_user(
last_name="user",
email="[email protected]",
role_names=None,
):
) -> User:
user = appbuilder.sm.find_user(username=username)
if user:
appbuilder.session.delete(user)
Expand Down
31 changes: 31 additions & 0 deletions tests/security/test_mvc_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,37 @@ def test_sec_login(self):
data = rv.data.decode("utf-8")
self.assertIn(INVALID_LOGIN_STRING, data)

def test_login_invalid_user(self):
"""
Test Security Login, Logout, invalid login, invalid access
"""
test_username = "testuser"
test_password = "password"
test_user = self.create_user(
self.appbuilder,
test_username,
test_password,
"Admin",
"user",
"user",
"[email protected]",
)
# Login and list with admin
self.browser_login(self.client, test_username, test_password)
rv = self.client.get("/model1view/list/")
self.assertEqual(rv.status_code, 200)

# Using the same session make sure the user is not allowed to access when
# the user is deactivated
test_user.active = False
self.db.session.merge(test_user)
self.db.session.commit()
rv = self.client.get("/model1view/list/")
self.assertEqual(rv.status_code, 302)

self.db.session.delete(test_user)
self.db.session.commit()

def test_db_login_no_next_url(self):
"""
Test Security no next URL
Expand Down
Loading