Simple check for null user_id on load. (#1122)

If properly configured this should never happen - but for folks upgrading from old Flask-Security its possible.
We shouldn't ever return a DB record with a null fs_uniquifier.

close #1116

Author: jwag956

.pre-commit-config.yaml

@@ -4,7 +4,7 @@ default_language_version:
     python: python3.12
 repos:
 -   repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v5.0.0
+    rev: v6.0.0
     hooks:
     -   id: trailing-whitespace
     -   id: debug-statements
@@ -20,7 +20,7 @@ repos:
     -   id: pyupgrade
         args: [--py310-plus]
 -   repo: https://github.com/psf/black
-    rev: 25.1.0
+    rev: 25.9.0
     hooks:
     -   id: black
 -   repo: https://github.com/pycqa/flake8

flask_security/core.py

@@ -693,7 +693,12 @@ class FormInfo:
 
 
 def _user_loader(user_id):
-    """Load based on fs_uniquifier (alternative_id)."""
+    """Load based on fs_uniquifier (alternative_id).
+    If the db model and db are properly configured and set there is no way we should
+    ever see a null user_id. But it is clearly wrong.
+    """
+    if not user_id:
+        return None
     user = _security.datastore.find_user(fs_uniquifier=str(user_id))
     if user and user.active:
         set_request_attr("fs_authn_via", "session")

tests/test_datastore.py

@@ -771,3 +771,18 @@ class User(Model, sqla.FsUserMixin):
     with app.app_context():
         Model.metadata.drop_all(db.engine)
         db.engine.dispose()
+
+
+def test_null_fs_uniquifier(app, client):
+    # If a record has a null fs_uniquifier - we shouldn't find it.
+    # The only way this might happen is if an app upgrades from a 3.0 Flask-Security
+    # and doesn't properly update their DB.
+    ds = app.security.datastore
+    with app.test_request_context("/"):
+        user = ds.find_user(email="gal@lp.com")
+        user.fs_uniquifier = ""
+        ds.put(user)
+        ds.commit()
+
+        user = app.security.login_manager.user_callback("")
+        assert not user

tests/test_misc.py

@@ -35,6 +35,7 @@
     populate_data,
     reset_fresh,
     get_form_input,
+    is_authenticated,
 )
 from tests.test_webauthn import HackWebauthnUtil, reg_2_keys
 
@@ -1599,3 +1600,13 @@ def test_password_required_setting(app, sqlalchemy_datastore):
     with pytest.raises(ValueError) as vex:
         Security(app=app, datastore=sqlalchemy_datastore)
     assert "SECURITY_PASSWORD_REQUIRED can only be" in str(vex.value)
+
+
+def test_null_user_id(app, client, get_message):
+    # if DB not configured correctly - make sure we catch null fs_uniquifier
+    json_authenticate(client)
+    assert is_authenticated(client, get_message)
+    with client.session_transaction() as sess:
+        sess["_user_id"] = ""
+        sess["user_id"] = ""
+    assert not is_authenticated(client, get_message)

tests/test_registerable.py

@@ -17,6 +17,7 @@
     init_app_with_options,
     json_authenticate,
     logout,
+    is_authenticated,
 )
 
 from flask_security import Security, UserMixin, user_registered, user_not_registered
@@ -578,13 +579,15 @@ def test_username(app, clients, get_message):
         "/login", json=dict(username="dude", password="awesome sunset")
     )
     assert response.status_code == 200
+    assert is_authenticated(client, get_message)
     logout(client)
 
     # login with email
     response = client.post(
         "/login", json=dict(email="dude@lp.com", password="awesome sunset")
     )
     assert response.status_code == 200
+    assert is_authenticated(client, get_message)
     logout(client)
 
     response = client.post(
@@ -621,6 +624,29 @@ def test_username(app, clients, get_message):
     )
 
 
+@pytest.mark.settings(username_enable=True)
+def test_username_not_set(app, client, get_message):
+    # login with null username - shouldn't match
+    # note that this is caught at LoginForm - it doesn't force a DB call
+    response = client.post(
+        "/register",
+        json=dict(
+            email="justemail@lp.com",
+            username="",
+            password="awesome sunset",
+            password_confirm="awesome sunset",
+        ),
+    )
+    assert response.status_code == 200
+    client.post("/logout")
+    response = client.post("/login", json=dict(username="", password="awesome sunset"))
+    assert response.status_code == 400
+    assert (
+        get_message("USER_DOES_NOT_EXIST")
+        == response.json["response"]["field_errors"][""][0].encode()
+    )
+
+
 @pytest.mark.settings(username_enable=True)
 def test_username_template(app, client):
     # verify template displays username option