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

Accessing a FAB application with active session but deleted user leads to: AttributeError: 'NoneType' object has no attribute 'is_active' #2296

Open
fmannhardt opened this issue Jan 3, 2025 · 1 comment

Comments

@fmannhardt
Copy link

Encountering this issue in the context of using Apache Superset (v4.1.1) as reported here:
apache/superset#28188 (comment)

When, for some reason, a user is deleted while they have an active session in FAB, then this results in the following exception:

025-01-03 22:02:53 2025-01-03 21:02:53,931:ERROR:superset.views.error_handling:'NoneType' object has no attribute 'is_active'
2025-01-03 22:02:53 Traceback (most recent call last):
2025-01-03 22:02:53   File "/usr/local/lib/python3.10/site-packages/flask/app.py", line 1484, in full_dispatch_request
2025-01-03 22:02:53     rv = self.dispatch_request()
2025-01-03 22:02:53   File "/usr/local/lib/python3.10/site-packages/flask/app.py", line 1469, in dispatch_request
2025-01-03 22:02:53     return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args)
2025-01-03 22:02:53   File "/usr/local/lib/python3.10/site-packages/flask_appbuilder/security/decorators.py", line 134, in wraps
2025-01-03 22:02:53     if permission_str in self.base_permissions and self.appbuilder.sm.has_access(
2025-01-03 22:02:53   File "/usr/local/lib/python3.10/site-packages/flask_appbuilder/security/manager.py", line 1609, in has_access
2025-01-03 22:02:53     if current_user.is_authenticated:
2025-01-03 22:02:53   File "/usr/local/lib/python3.10/site-packages/werkzeug/local.py", line 318, in __get__
2025-01-03 22:02:53     obj = instance._get_current_object()
2025-01-03 22:02:53   File "/usr/local/lib/python3.10/site-packages/werkzeug/local.py", line 526, in _get_current_object
2025-01-03 22:02:53     return get_name(local())
2025-01-03 22:02:53   File "/usr/local/lib/python3.10/site-packages/flask_login/utils.py", line 25, in <lambda>
2025-01-03 22:02:53     current_user = LocalProxy(lambda: _get_user())
2025-01-03 22:02:53   File "/usr/local/lib/python3.10/site-packages/flask_login/utils.py", line 370, in _get_user
2025-01-03 22:02:53     current_app.login_manager._load_user()
2025-01-03 22:02:53   File "/usr/local/lib/python3.10/site-packages/flask_login/login_manager.py", line 364, in _load_user
2025-01-03 22:02:53     user = self._user_callback(user_id)
2025-01-03 22:02:53   File "/usr/local/lib/python3.10/site-packages/flask_appbuilder/security/manager.py", line 2163, in load_user
2025-01-03 22:02:53     if user.is_active:
2025-01-03 22:02:53 AttributeError: 'NoneType' object has no attribute 'is_active'

I think the core issue is that FAB assumes that a user provided by Flask Login will always be able to be found in the DB here:


This assumption is problematic since users may be deleted while they are logged in. The expected behavior, for me, would be to immediately logout the user.

In fact, Flask Login will simply check for existence of any user in (still valid) cookie here:
https://github.com/maxcountryman/flask-login/blob/019dbe3ae0fb95966682e769280722afb0a6b904/src/flask_login/login_manager.py#L375

For me, this happened during an update of a Superset instance in which users are auto-created on first login via OAuth. In this case, we may simply drop and re-create the Superset/FAB database and the users should be re-created on their next login. But I assume the same would happen if someone simply deletes a users that is logged in.

Probably the fix is as easy, as checking for user to be not None before calling is_active on it.

Environment

Flask-Appbuilder version: flask-appbuilder==4.5.0

pip freeze output:

# pip freeze
alembic==1.13.1
amqp==5.2.0
# Editable install with no version control (apache-superset==4.1.1)
-e /app
apispec==6.3.0
apsw==3.46.0.0
async-timeout==5.0.1
attrs==23.2.0
Authlib==1.2.0
Babel==2.15.0
backoff==2.2.1
bcrypt==4.1.3
billiard==4.2.0
blinker==1.8.2
Bottleneck==1.3.8
Brotli==1.1.0
cachelib==0.9.0
cachetools==5.3.3
cattrs==23.2.3
celery==5.4.0
certifi==2024.2.2
cffi==1.16.0
charset-normalizer==3.3.2
click==8.1.7
click-didyoumean==0.3.1
click-option-group==0.5.6
click-plugins==1.1.1
click-repl==0.3.0
colorama==0.4.6
cron-descriptor==1.4.3
croniter==2.0.5
cryptography==42.0.7
Deprecated==1.2.14
deprecation==2.1.0
dnspython==2.6.1
email_validator==2.1.1
exceptiongroup==1.2.2
Flask==2.3.3
Flask-AppBuilder==4.5.0
Flask-Babel==2.0.0
Flask-Caching==2.3.0
Flask-Compress==1.15
Flask-JWT-Extended==4.6.0
Flask-Limiter==3.7.0
Flask-Login==0.6.3
Flask-Migrate==3.1.0
Flask-Session==0.8.0
Flask-SQLAlchemy==2.5.1
flask-talisman==1.1.0
Flask-WTF==1.2.1
func_timeout==4.3.5
geographiclib==2.0
geopy==2.4.1
gevent==22.10.2
google-auth==2.29.0
greenlet==3.0.3
gunicorn==22.0.0
hashids==1.3.1
holidays==0.25
humanize==4.9.0
idna==3.7
importlib_metadata==7.1.0
importlib_resources==6.4.0
isodate==0.6.1
itsdangerous==2.2.0
Jinja2==3.1.4
jsonpath-ng==1.6.1
jsonschema==4.17.3
kombu==5.3.7
korean-lunar-calendar==0.3.1
limits==3.12.0
llvmlite==0.42.0
Mako==1.3.5
Markdown==3.6
markdown-it-py==3.0.0
MarkupSafe==2.1.5
marshmallow==3.21.2
marshmallow-sqlalchemy==0.28.2
mdurl==0.1.2
msgpack==1.0.8
msgspec==0.18.6
nh3==0.2.17
numba==0.59.1
numexpr==2.10.0
numpy==1.23.5
ordered-set==4.1.0
packaging==23.2
pandas==2.0.3
paramiko==3.4.0
parsedatetime==2.6
pgsanity==0.2.9
pillow==11.1.0
platformdirs==3.8.1
ply==3.11
polyline==2.0.2
prison==0.2.1
prompt_toolkit==3.0.44
psycopg2-binary==2.9.9
pyarrow==14.0.2
pyasn1==0.6.0
pyasn1_modules==0.4.0
pycparser==2.22
Pygments==2.18.0
PyJWT==2.8.0
PyNaCl==1.5.0
pyOpenSSL==24.1.0
pyparsing==3.1.2
pyrsistent==0.20.0
python-dateutil==2.9.0.post0
python-dotenv==1.0.1
python-geohash==0.8.5
pytz==2024.1
PyYAML==6.0.1
redis==4.6.0
requests==2.32.2
requests-cache==1.2.0
rich==13.7.1
rsa==4.9
selenium==3.141.0
shillelagh==1.2.18
shortid==0.1.2
simplejson==3.19.2
six==1.16.0
slack_sdk==3.27.2
SQLAlchemy==1.4.52
SQLAlchemy-Utils==0.38.3
sqlglot==25.24.0
sqlparse==0.5.0
sshtunnel==0.4.0
tabulate==0.8.10
typing_extensions==4.12.0
tzdata==2024.1
url-normalize==1.4.3
urllib3==1.26.18
vine==5.1.0
wcwidth==0.2.13
Werkzeug==3.0.6
wrapt==1.16.0
WTForms==3.1.2
WTForms-JSON==0.3.5
XlsxWriter==3.0.9
zipp==3.19.0
zope.event==5.0
zope.interface==7.2
zstandard==0.22.0

Describe the expected results

When deleting a user with an active session in FAB, there should not be an exception but a clean logout/destruction of that session.

Describe the actual results

The behavior I described above. Not sure how I would reproduce this in code as it concerns the whole system and authentication via cookies.

Steps to reproduce

(1) Start a Superset instance using FAB (but I guess basic FAB would also be affected)
(2) Login as a user A
(3) Remove that user A
(4) Refresh any page as user A while the session is still active

@fmannhardt
Copy link
Author

Just verified that:

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

fixes this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant