Fix more Basic Auth. (#371)

Basic auth didn't reject deactivated users.

Basic Auth didn't return WWW-Authenticate when request was JSON.

Introduced a new config variable SECURITY_API_ENABLED_METHODS which configures which of (basic, session, token) will be used
to authenticate Flask-Security's endpoints that require it.

change the @auth_required decorator to take a callable which can reference context variables (such as config).

Change all endpoints that require authentication to use new config variable.

Fixes: #368
Fixes: #369

Author: jwag956

docs/configuration.rst

@@ -309,6 +309,19 @@ These configuration keys are used globally across all features.
     .. deprecated:: 4.0.0
         Superseded by :py:data:`SECURITY_USER_IDENTITY_ATTRIBUTES`
 
+.. py:data:: SECURITY_API_ENABLED_METHODS
+
+    Various endpoints of Flask-Security require the caller to be authenticated.
+    This variable controls which of the methods - ``token``, ``session``, ``basic``
+    will be allowed. The default does NOT include ``basic`` since if ``basic``
+    is in the list, and if the user is NOT authenticated, then the standard/required
+    response of 401 with the ``WWW-Authenticate`` header is returned. This is
+    rarely what the client wants.
+
+    Default: ``["session", "token"]``.
+
+    .. versionadded:: 4.0.0
+
 .. py:data:: SECURITY_DEFAULT_REMEMBER_ME
 
     Specifies the default "remember me" value used when logging in a user.

docs/patterns.rst

@@ -57,6 +57,16 @@ own code. Then use Flask's ``errorhandler`` to catch that exception and create t
             raise MyForbiddenException(msg='You can only update docs you own')
 
 
+A note about Basic Auth
++++++++++++++++++++++++
+Basic Auth is supported in Flask-Security, using the @http_auth_required() decorator. If a request for an endpoint
+protected with @http_auth_required is received, and the request doesn't contain the appropriate HTTP Headers, a 401 is returned
+along with the required WWW-Authenticate header. In this case there won't be a usable session cookie returned so all future requests
+will also require credentials to be sent. Effectively the caller is temporarily 'logged in' at the beginning of each request and 'logged out' again
+at the end of the request. Most (all?) browsers intercept this response and pop up a login dialog box and remember, for the site, the entered credentials.
+This effectively bypasses any of the normal Flask-Security login forms. By default, the Flask-Security endpoints that require the caller be
+authenticated do NOT support ``basic`` - however the :py:data:`SECURITY_API_ENABLED_METHODS` can be used to override this.
+
 Freshness
 ++++++++++
 A common pattern for browser-based sites is to use sessions to manage identity. This is usually

flask_security/core.py

@@ -213,6 +213,7 @@
     "PHONE_REGION_DEFAULT": "US",
     "FRESHNESS": timedelta(hours=24),
     "FRESHNESS_GRACE_PERIOD": timedelta(hours=1),
+    "API_ENABLED_METHODS": ["session", "token"],
     "HASHING_SCHEMES": ["sha256_crypt", "hex_md5"],
     "DEPRECATED_HASHING_SCHEMES": ["hex_md5"],
     "DATETIME_FACTORY": datetime.utcnow,

flask_security/decorators.py

@@ -76,9 +76,6 @@ def default_unauthn_handler(mechanisms, headers=None):
     if config_value("BACKWARDS_COMPAT_UNAUTHN"):
         return _get_unauthenticated_response(headers=headers)
     if _security._want_json(request):
-        # Remove WWW-Authenticate from headers for JSON responses since
-        # browsers will intercept that and pop up their own login form.
-        headers.pop("WWW-Authenticate", None)
         payload = json_error_response(errors=msg)
         return _security._render_json(payload, 401, headers, None)
 
@@ -168,6 +165,8 @@ def _check_http_auth():
     if not auth.username:
         return False
     user = find_user(auth.username)
+    if user and not user.active:
+        return False
 
     if user and user.verify_and_update_password(auth.password):
         _security.datastore.commit()
@@ -283,7 +282,9 @@ def dashboard():
             return 'Dashboard'
 
     :param auth_methods: Specified mechanisms (token, basic, session). If not specified
-        then all current available mechanisms (except "basic") will be tried.
+        then all current available mechanisms (except "basic") will be tried. A callable
+        can also be passed (useful if you need app/request context). The callable
+        must return a list.
     :kwparam within: Add 'freshness' check to authentication. Is either an int
         specifying # of minutes, or a callable that returns a timedelta. For timedeltas,
         timedelta.total_seconds() is used for the calculations:
@@ -330,6 +331,9 @@ def dashboard():
     .. versionchanged:: 3.4.4
         If ``auth_methods`` isn't specified try all mechanisms EXCEPT ``basic``.
 
+    .. versionchanged:: 4.0.0
+        auth_methods can be passed as a callable.
+
     """
 
     login_mechanisms = {
@@ -338,10 +342,7 @@ def dashboard():
         "basic": lambda: _check_http_auth(),
     }
     mechanisms_order = ["token", "session", "basic"]
-    if not auth_methods:
-        auth_methods = {"session", "token"}
-    else:
-        auth_methods = [am for am in auth_methods]
+    auth_methods_arg = auth_methods
 
     def wrapper(fn):
         @wraps(fn)
@@ -360,6 +361,16 @@ def decorated_view(*args, **dkwargs):
             else:
                 grace = datetime.timedelta(minutes=grace)
 
+            if not auth_methods_arg:
+                auth_methods = {"session", "token"}
+            else:
+                auth_methods = []
+                for am in auth_methods_arg:
+                    if callable(am):
+                        auth_methods.extend(am())
+                    else:
+                        auth_methods.append(am)
+
             h = {}
             if "basic" in auth_methods:
                 r = _security.default_http_auth_realm

flask_security/unified_signin.py

@@ -404,7 +404,7 @@ def us_signin_send_code():
     )
 
 
-@auth_required()
+@auth_required(lambda: config_value("API_ENABLED_METHODS"))
 def us_verify_send_code():
     """
     Send code during verify.
@@ -557,7 +557,7 @@ def us_signin():
     )
 
 
-@auth_required()
+@auth_required(lambda: config_value("API_ENABLED_METHODS"))
 def us_verify():
     """
     Re-authenticate to reset freshness time.
@@ -687,6 +687,7 @@ def us_verify_link():
 
 
 @auth_required(
+    lambda: config_value("API_ENABLED_METHODS"),
     within=lambda: config_value("FRESHNESS"),
     grace=lambda: config_value("FRESHNESS_GRACE_PERIOD"),
 )
@@ -790,7 +791,7 @@ def us_setup():
     )
 
 
-@auth_required()
+@auth_required(lambda: config_value("API_ENABLED_METHODS"))
 def us_setup_validate(token):
     """
     Validate new setup.
@@ -854,7 +855,7 @@ def us_setup_validate(token):
     return redirect(url_for_security("us_setup"))
 
 
-@auth_required()
+@auth_required(lambda: config_value("API_ENABLED_METHODS"))
 def us_qrcode(token):
 
     if "authenticator" not in config_value("US_ENABLED_METHODS"):

flask_security/views.py

@@ -199,7 +199,7 @@ def login():
         )
 
 
-@auth_required()
+@auth_required(lambda: config_value("API_ENABLED_METHODS"))
 def verify():
     """View function which handles a authentication verification request.
     """
@@ -610,7 +610,7 @@ def reset_password(token):
     )
 
 
-@auth_required("basic", "token", "session")
+@auth_required(lambda: config_value("API_ENABLED_METHODS"))
 def change_password():
     """View function which handles a change password request."""
 

tests/test_changeable.py

@@ -8,6 +8,7 @@
     :license: MIT, see LICENSE for more details.
 """
 
+import base64
 import sys
 
 import pytest
@@ -204,6 +205,32 @@ def test_token_change(app, client_nc):
     assert "authentication_token" in response.json["response"]["user"]
 
 
+@pytest.mark.settings(api_enabled_methods=["basic"])
+def test_basic_change(app, client_nc, get_message):
+    # Verify can change password using basic auth
+    data = dict(
+        password="password",
+        new_password="new strong password",
+        new_password_confirm="new strong password",
+    )
+    response = client_nc.post("/change", data=data)
+    assert b"You are not authenticated" in response.data
+    assert "WWW-Authenticate" in response.headers
+
+    response = client_nc.post(
+        "/change",
+        data=data,
+        headers={
+            "Authorization": "Basic %s"
+            % base64.b64encode(b"matt@lp.com:password").decode("utf-8")
+        },
+        follow_redirects=True,
+    )
+    assert response.status_code == 200
+    # No session so no flashing
+    assert b"Home Page" in response.data
+
+
 @pytest.mark.settings(password_complexity_checker="zxcvbn")
 def test_easy_password(app, client):
     authenticate(client)

tests/test_common.py

@@ -219,6 +219,36 @@ def test_inactive_forbids_token(app, client_nc, get_message):
     assert response.status_code == 401
 
 
+def test_inactive_forbids_basic(app, client, get_message):
+    """ Make sure that basic auth doesn't work if user deactivated
+    """
+
+    # Should properly work.
+    response = client.get(
+        "/multi_auth",
+        headers={
+            "Authorization": "Basic %s"
+            % base64.b64encode(b"joe@lp.com:password").decode("utf-8")
+        },
+    )
+    assert b"Session, Token, Basic" in response.data
+
+    # deactivate joe
+    with app.test_request_context("/"):
+        user = app.security.datastore.find_user(email="joe@lp.com")
+        app.security.datastore.deactivate_user(user)
+        app.security.datastore.commit()
+
+    response = client.get(
+        "/multi_auth",
+        headers={
+            "Authorization": "Basic %s"
+            % base64.b64encode(b"joe@lp.com:password").decode("utf-8")
+        },
+    )
+    assert b"You are not authenticated" in response.data
+
+
 def test_unset_password(client, get_message):
     response = authenticate(client, "jess@lp.com", "password")
     assert get_message("PASSWORD_NOT_SET") in response.data
@@ -472,7 +502,6 @@ def test_http_auth_no_authentication_json(client, get_message):
         "UNAUTHENTICATED"
     )
     assert response.headers["Content-Type"] == "application/json"
-    assert "WWW-Authenticate" not in response.headers
 
 
 @pytest.mark.settings(backwards_compat_unauthn=True)
@@ -491,9 +520,7 @@ def test_invalid_http_auth_invalid_username(client):
 
 @pytest.mark.settings(backwards_compat_unauthn=False)
 def test_invalid_http_auth_invalid_username_json(client, get_message):
-    # While Basic auth is allowed with JSON - we never expect a WWW-Authenticate
-    # header - since that is captured by most browsers and they pop up a
-    # login form.
+    # Even with JSON - Basic Auth required a WWW-Authenticate header response.
     response = client.get(
         "/http",
         headers={
@@ -507,7 +534,7 @@ def test_invalid_http_auth_invalid_username_json(client, get_message):
         "UNAUTHENTICATED"
     )
     assert response.headers["Content-Type"] == "application/json"
-    assert "WWW-Authenticate" not in response.headers
+    assert "WWW-Authenticate" in response.headers
 
 
 @pytest.mark.settings(backwards_compat_unauthn=True)

tests/test_unified_signin.py

@@ -9,6 +9,7 @@
 
 """
 
+import base64
 from contextlib import contextmanager
 from datetime import timedelta
 from unittest.mock import Mock
@@ -663,6 +664,21 @@ def test_setup_json_no_session(app, client_nc, get_message):
     }
     response = client_nc.get("/us-setup", headers=headers)
     assert response.status_code == 401
+    assert response.json["response"]["reauth_required"]
+    assert "WWW-Authenticate" not in response.headers
+
+
+@pytest.mark.settings(api_enabled_methods=["basic"])
+def test_setup_basic(app, client, get_message):
+    # If using Basic Auth - always fresh so should be able to setup (not sure the
+    # use case but...)
+    headers = {
+        "Authorization": "Basic %s"
+        % base64.b64encode(b"matt@lp.com:password").decode("utf-8")
+    }
+    response = client.get("/us-setup", headers=headers)
+    assert response.status_code == 200
+    assert b"Setup Unified Sign In options" in response.data
 
 
 def test_setup_bad_token(app, client, get_message):

tests/view_scaffold.py

@@ -179,6 +179,11 @@ def home():
     def basic():
         return render_template_string("Basic auth success")
 
+    @app.route("/protected")
+    @auth_required()
+    def protected():
+        return render_template_string("Protected endpoint")
+
     return app