The alternative id feature introduced an issue with postgres/psycopg2. (

#304)

Ultimately decided that we should just say - you either use the alternative id or not
and not try to fallback if one fails.

What this means is that if an app changes from user.id to alternate id (fs_uniquifier) then all
clients will be required to log in again (their session cookie won't be valid).

closes: #298

docs/models.rst

@@ -7,7 +7,7 @@ and `Role` model. The fields on your models must follow a particular convention
 depending on the functionality your app requires. Aside from this, you're free
 to add any additional fields to your model(s) if you want.
 
-As more features are added to Flask-Security, the requirements for required fields and tables grow.
+As more features are added to Flask-Security, the list of required fields and tables grow.
 As you use these features, and therefore use these fields and tables, database migrations are required;
 which are a bit of a pain. To make things easier - Flask-Security includes mixins that
 contain ALL the fields and tables required for all features. They also contain
@@ -24,18 +24,18 @@ your `User` and `Role` model should include the following fields:
 
 **User**
 
-* ``id``
-* ``email``
-* ``password``
-* ``active``
-* ``fs_uniquifier``
+* ``id`` (primary key - integer, string, or uuid)
+* ``email`` (for most features - unique, non-nullable)
+* ``password`` (non-nullable)
+* ``active`` (boolean, non-nullable)
+* ``fs_uniquifier`` (unique, non-nullable)
 
 
 **Role**
 
-* ``id``
-* ``name``
-* ``description``
+* ``id`` (primary key - integer)
+* ``name`` (unique, non-nullable)
+* ``description`` (string)
 
 
 Additional Functionality
@@ -51,7 +51,7 @@ If you enable account confirmation by setting your application's
 `SECURITY_CONFIRMABLE` configuration value to `True`, your `User` model will
 require the following additional field:
 
-* ``confirmed_at``
+* ``confirmed_at`` (datetime)
 
 Trackable
 ^^^^^^^^^
@@ -60,11 +60,11 @@ If you enable user tracking by setting your application's `SECURITY_TRACKABLE`
 configuration value to `True`, your `User` model will require the following
 additional fields:
 
-* ``last_login_at``
-* ``current_login_at``
-* ``last_login_ip``
-* ``current_login_ip``
-* ``login_count``
+* ``last_login_at`` (datetime)
+* ``current_login_at`` (datetime)
+* ``last_login_ip`` (string)
+* ``current_login_ip`` (string)
+* ``login_count`` (integer)
 
 Two_Factor
 ^^^^^^^^^^
@@ -73,13 +73,13 @@ If you enable two-factor by setting your application's `SECURITY_TWO_FACTOR`
 configuration value to `True`, your `User` model will require the following
 additional fields:
 
-* ``tf_totp_secret``
-* ``tf_primary_method``
+* ``tf_totp_secret`` (string)
+* ``tf_primary_method`` (string)
 
 If you include 'sms' in `SECURITY_TWO_FACTOR_ENABLED_METHODS`, your `User` model
 will require the following additional field:
 
-* ``tf_phone_number``
+* ``tf_phone_number`` (string)
 
 Unified Sign In
 ^^^^^^^^^^^^^^^
@@ -93,14 +93,14 @@ additional fields:
 If you include 'sms' in :py:data:`SECURITY_US_ENABLED_METHODS`, your `User` model
 will require the following additional field:
 
-* ``us_phone_number``
+* ``us_phone_number`` (string)
 
 Permissions
 ^^^^^^^^^^^
 If you want to protect endpoints with permissions, and assign permissions to roles
 that are then assigned to users the Role model requires:
 
-* ``permissions``
+* ``permissions`` (UnicodeText)
 
 Custom User Payload
 ^^^^^^^^^^^^^^^^^^^

flask_security/core.py

@@ -437,21 +437,24 @@
 
 
 def _user_loader(user_id):
-    # Try to load based on fs_uniquifier (alternative_id) first
+    """ Try to load based on fs_uniquifier (alternative_id) if available.
+
+    Note that we don't try, and fall back to the other - primarily because some DBs
+    and drivers (psycopg2) really really hate getting mismatched types during queries.
+    They hate it enough that they abort the 'transaction' and refuse to do anything
+    in the future until the transaction is rolled-back. But we don't really control
+    that and there doesn't seem to be any way to catch the actual offensive query -
+    just next time and forever, things fail.
+    This assumes that if the app has fs_uniquifier, it is non-nullable as we specify
+    so we use that and only that.
+    """
     if hasattr(_datastore.user_model, "fs_uniquifier"):
-        try:
-            user = _security.datastore.find_user(fs_uniquifier=user_id)
-            if user and user.active:
-                return user
-        except Exception:
-            pass
-    # Load based on user.id as fallback method for upstream data
-    try:
-        user = _security.datastore.find_user(id=user_id)
-        if user and user.active:
-            return user
-    except Exception:
-        pass
+        selector = dict(fs_uniquifier=str(user_id))
+    else:
+        selector = dict(id=user_id)
+    user = _security.datastore.find_user(**selector)
+    if user and user.active:
+        return user
     return None
 
 

tests/test_datastore.py

@@ -471,3 +471,39 @@ def tear_down():
     with app.app_context():
         user = ds.get_user("matt@lp.com")
         assert not user
+
+
+def test_user_loader(app, sqlalchemy_datastore):
+    # user_loader now tries first to match fs_uniquifier then match user.id
+    # While we know that fs_uniquifier is a string - we don't know what user.id is
+    # and we know that psycopg2 is really finicky about this.
+    from flask_security.core import _user_loader
+
+    init_app_with_options(app, sqlalchemy_datastore)
+    with app.app_context():
+        jill = sqlalchemy_datastore.find_user(email="jill@lp.com")
+
+        # normal case
+        user = _user_loader(jill.fs_uniquifier)
+        assert user.email == "jill@lp.com"
+
+        # send in an int - make sure it is cast to string for check against
+        # fs_uniquifier
+        user = _user_loader(1000)
+        assert not user
+        # with psycopg2 this will lock up DB but since we pass exceptions we can
+        # check this by trying some other DB operation
+        jill = sqlalchemy_datastore.find_user(email="jill@lp.com")
+        assert jill
+
+        # This works since DBs seem to try to cast to underlying type
+        user = _user_loader("10")
+        jill = sqlalchemy_datastore.find_user(email="jill@lp.com")
+        assert jill
+
+        # Since this doesn't match an existing fs_uniqifier, and user.id is an int
+        # this will make pyscopg2 angry. This test verifies that we don't in fact
+        # try to take a string and use it to lookup an int.
+        user = _user_loader("someunknown")
+        jill = sqlalchemy_datastore.find_user(email="jill@lp.com")
+        assert jill