Merge branch 'master' into 1154-use-LOGOUT_REDIRECT_URL

docs/config.rst

@@ -202,8 +202,13 @@ Use config.py to configure the following parameters. By default it will use SQLL
 | AUTH_ROLE_PUBLIC                       | Special Role that holds the public         |   No      |
 |                                        | permissions, no authentication needed.     |           |
 +----------------------------------------+--------------------------------------------+-----------+
+| AUTH_STRICT_RESPONSE_CODES             | When True, protected endpoints will return |   No      |
+|                                        | HTTP 403 instead of 401. This option will  |           |
+|                                        | be removed and default to True on the next |           |
+|                                        | major release. defaults to False           |           |
++----------------------------------------+--------------------------------------------+-----------+
 | AUTH_API_LOGIN_ALLOW_MULTIPLE_PROVIDERS| Allow REST API login with alternative auth |   No      |
-| True|False                             | providers (default False)                  |           |           |
+| True|False                             | providers (default False)                  |           |
 +----------------------------------------+--------------------------------------------+-----------+
 | APP_NAME                               | The name of your application.              |   No      |
 +----------------------------------------+--------------------------------------------+-----------+

docs/security.rst

@@ -661,11 +661,11 @@ Example on your config::
     ...
 
     def custom_password_validator(password: str) -> None:
-    """
-    A simplistic example for a password validator
-    """
-    if len(password) < 8:
-        raise PasswordComplexityValidationError("Must have at least 8 characters")
+        """
+        A simplistic example for a password validator
+        """
+        if len(password) < 8:
+            raise PasswordComplexityValidationError("Must have at least 8 characters")
 
     FAB_PASSWORD_COMPLEXITY_VALIDATOR = custom_password_validator
     FAB_PASSWORD_COMPLEXITY_ENABLED = True

flask_appbuilder/security/decorators.py

@@ -1,42 +1,75 @@
 import functools
 import logging
-
-from flask import current_app, flash, jsonify, make_response, redirect, request, url_for
-from flask_jwt_extended import verify_jwt_in_request
-from flask_login import current_user
-
-from .._compat import as_unicode
-from ..const import (
+from typing import TYPE_CHECKING
+
+from flask import (
+    current_app,
+    flash,
+    jsonify,
+    make_response,
+    redirect,
+    request,
+    Response,
+    url_for,
+)
+from flask_appbuilder._compat import as_unicode
+from flask_appbuilder.const import (
     FLAMSG_ERR_SEC_ACCESS_DENIED,
     LOGMSG_ERR_SEC_ACCESS_DENIED,
     PERMISSION_PREFIX,
 )
+from flask_jwt_extended import verify_jwt_in_request
+from flask_login import current_user
+
 
 log = logging.getLogger(__name__)
 
+if TYPE_CHECKING:
+    from flask_appbuilder.api import BaseApi
+
+
+def response_unauthorized(base_class: "BaseApi") -> Response:
+    if current_app.config.get("AUTH_STRICT_RESPONSE_CODES", False):
+        return base_class.response_403()
+    return base_class.response_401()
+
+
+def response_unauthorized_mvc() -> Response:
+    status_code = 401
+    if current_app.appbuilder.sm.current_user and current_app.config.get(
+        "AUTH_STRICT_RESPONSE_CODES", False
+    ):
+        status_code = 403
+    response = make_response(
+        jsonify({"message": str(FLAMSG_ERR_SEC_ACCESS_DENIED), "severity": "danger"}),
+        status_code,
+    )
+    response.headers["Content-Type"] = "application/json"
+    return response
+
 
 def protect(allow_browser_login=False):
     """
-        Use this decorator to enable granular security permissions
-        to your API methods (BaseApi and child classes).
-        Permissions will be associated to a role, and roles are associated to users.
-
-        allow_browser_login will accept signed cookies obtained from the normal MVC app::
-
-            class MyApi(BaseApi):
-                @expose('/dosonmething', methods=['GET'])
-                @protect(allow_browser_login=True)
-                @safe
-                def do_something(self):
-                    ....
-
-                @expose('/dosonmethingelse', methods=['GET'])
-                @protect()
-                @safe
-                def do_something_else(self):
-                    ....
-
-        By default the permission's name is the methods name.
+    Use this decorator to enable granular security permissions
+    to your API methods (BaseApi and child classes).
+    Permissions will be associated to a role, and roles are associated to users.
+
+    allow_browser_login will accept signed cookies obtained from the normal MVC app::
+
+        class MyApi(BaseApi):
+            @expose('/dosonmething', methods=['GET'])
+            @protect(allow_browser_login=True)
+            @safe
+            def do_something(self):
+                ....
+
+            @expose('/dosonmethingelse', methods=['GET'])
+            @protect()
+            @safe
+            def do_something_else(self):
+                ....
+
+    By default the permission's name is the methods name.
     """
 
     def _protect(f):
@@ -47,25 +80,31 @@ def _protect(f):
 
         def wraps(self, *args, **kwargs):
             # Apply method permission name override if exists
-            permission_str = "{}{}".format(PERMISSION_PREFIX, f._permission_name)
+            permission_str = f"{PERMISSION_PREFIX}{f._permission_name}"
             if self.method_permission_name:
                 _permission_name = self.method_permission_name.get(f.__name__)
                 if _permission_name:
-                    permission_str = "{}{}".format(PERMISSION_PREFIX, _permission_name)
+                    permission_str = f"{PERMISSION_PREFIX}{_permission_name}"
             class_permission_name = self.class_permission_name
+            # Check if permission is allowed on the class
             if permission_str not in self.base_permissions:
-                return self.response_401()
+                return response_unauthorized(self)
+            # Check if the resource is public
             if current_app.appbuilder.sm.is_item_public(
                 permission_str, class_permission_name
             ):
                 return f(self, *args, **kwargs)
+            # if no browser login then verify JWT
             if not (self.allow_browser_login or allow_browser_login):
                 verify_jwt_in_request()
+            # Verify resource access
             if current_app.appbuilder.sm.has_access(
                 permission_str, class_permission_name
             ):
                 return f(self, *args, **kwargs)
+            # If browser login?
             elif self.allow_browser_login or allow_browser_login:
+                # no session cookie (but we allow it), then try JWT
                 if not current_user.is_authenticated:
                     verify_jwt_in_request()
                 if current_app.appbuilder.sm.has_access(
@@ -77,7 +116,7 @@ def wraps(self, *args, **kwargs):
                     permission_str, class_permission_name
                 )
             )
-            return self.response_401()
+            return response_unauthorized(self)
 
         f._permission_name = permission_str
         return functools.update_wrapper(wraps, f)
@@ -87,22 +126,22 @@ def wraps(self, *args, **kwargs):
 
 def has_access(f):
     """
-        Use this decorator to enable granular security permissions to your methods.
-        Permissions will be associated to a role, and roles are associated to users.
+    Use this decorator to enable granular security permissions to your methods.
+    Permissions will be associated to a role, and roles are associated to users.
 
-        By default the permission's name is the methods name.
+    By default the permission's name is the methods name.
     """
     if hasattr(f, "_permission_name"):
         permission_str = f._permission_name
     else:
         permission_str = f.__name__
 
     def wraps(self, *args, **kwargs):
-        permission_str = "{}{}".format(PERMISSION_PREFIX, f._permission_name)
+        permission_str = f"{PERMISSION_PREFIX}{f._permission_name}"
         if self.method_permission_name:
             _permission_name = self.method_permission_name.get(f.__name__)
             if _permission_name:
-                permission_str = "{}{}".format(PERMISSION_PREFIX, _permission_name)
+                permission_str = f"{PERMISSION_PREFIX}{_permission_name}"
         if permission_str in self.base_permissions and self.appbuilder.sm.has_access(
             permission_str, self.class_permission_name
         ):
@@ -127,24 +166,24 @@ def wraps(self, *args, **kwargs):
 
 def has_access_api(f):
     """
-        Use this decorator to enable granular security permissions to your API methods.
-        Permissions will be associated to a role, and roles are associated to users.
+    Use this decorator to enable granular security permissions to your API methods.
+    Permissions will be associated to a role, and roles are associated to users.
 
-        By default the permission's name is the methods name.
+    By default the permission's name is the methods name.
 
-        this will return a message and HTTP 401 is case of unauthorized access.
+    this will return a message and HTTP 403 is case of unauthorized access.
     """
     if hasattr(f, "_permission_name"):
         permission_str = f._permission_name
     else:
         permission_str = f.__name__
 
     def wraps(self, *args, **kwargs):
-        permission_str = "{}{}".format(PERMISSION_PREFIX, f._permission_name)
+        permission_str = f"{PERMISSION_PREFIX}{f._permission_name}"
         if self.method_permission_name:
             _permission_name = self.method_permission_name.get(f.__name__)
             if _permission_name:
-                permission_str = "{}{}".format(PERMISSION_PREFIX, _permission_name)
+                permission_str = f"{PERMISSION_PREFIX}{_permission_name}"
         if permission_str in self.base_permissions and self.appbuilder.sm.has_access(
             permission_str, self.class_permission_name
         ):
@@ -155,53 +194,46 @@ def wraps(self, *args, **kwargs):
                     permission_str, self.__class__.__name__
                 )
             )
-            response = make_response(
-                jsonify(
-                    {"message": str(FLAMSG_ERR_SEC_ACCESS_DENIED), "severity": "danger"}
-                ),
-                401,
-            )
-            response.headers["Content-Type"] = "application/json"
-            return response
+            return response_unauthorized_mvc()
 
     f._permission_name = permission_str
     return functools.update_wrapper(wraps, f)
 
 
 def permission_name(name):
     """
-        Use this decorator to override the name of the permission.
-        has_access will use the methods name has the permission name
-        if you want to override this add this decorator to your methods.
-        This is useful if you want to aggregate methods to permissions
+    Use this decorator to override the name of the permission.
+    has_access will use the methods name has the permission name
+    if you want to override this add this decorator to your methods.
+    This is useful if you want to aggregate methods to permissions
 
-        It will add '_permission_name' attribute to your method
-        that will be inspected by BaseView to collect your view's
-        permissions.
+    It will add '_permission_name' attribute to your method
+    that will be inspected by BaseView to collect your view's
+    permissions.
 
-        Note that you should use @has_access to execute after @permission_name
-        like on the following example.
+    Note that you should use @has_access to execute after @permission_name
+    like on the following example.
 
-        Use it like this to aggregate permissions for your methods::
+    Use it like this to aggregate permissions for your methods::
 
-            class MyModelView(ModelView):
-                datamodel = SQLAInterface(MyModel)
+        class MyModelView(ModelView):
+            datamodel = SQLAInterface(MyModel)
 
-                @has_access
-                @permission_name('GeneralXPTO_Permission')
-                @expose(url='/xpto')
-                def xpto(self):
-                    return "Your on xpto"
+            @has_access
+            @permission_name('GeneralXPTO_Permission')
+            @expose(url='/xpto')
+            def xpto(self):
+                return "Your on xpto"
 
-                @has_access
-                @permission_name('GeneralXPTO_Permission')
-                @expose(url='/xpto2')
-                def xpto2(self):
-                    return "Your on xpto2"
+            @has_access
+            @permission_name('GeneralXPTO_Permission')
+            @expose(url='/xpto2')
+            def xpto2(self):
+                return "Your on xpto2"
 
 
-        :param name:
-            The name of the permission to override
+    :param name:
+        The name of the permission to override
     """
 
     def wraps(f):

flask_appbuilder/security/views.py

@@ -31,7 +31,6 @@
 from ..views import expose, ModelView, SimpleFormView
 from ..widgets import ListWidget, ShowWidget
 
-
 log = logging.getLogger(__name__)
 
 
@@ -518,7 +517,10 @@ def login(self):
                 flash(as_unicode(self.invalid_login_message), "warning")
                 return redirect(self.appbuilder.get_url_for_login)
             login_user(user, remember=False)
-            return redirect(self.appbuilder.get_url_for_index)
+            next_url = request.args.get("next", "")
+            if not next_url:
+                next_url = self.appbuilder.get_url_for_index
+            return redirect(next_url)
         return self.render_template(
             self.login_template, title=self.title, form=form, appbuilder=self.appbuilder
         )
@@ -540,7 +542,10 @@ def login(self):
                 flash(as_unicode(self.invalid_login_message), "warning")
                 return redirect(self.appbuilder.get_url_for_login)
             login_user(user, remember=False)
-            return redirect(self.appbuilder.get_url_for_index)
+            next_url = request.args.get("next", "")
+            if not next_url:
+                next_url = self.appbuilder.get_url_for_index
+            return redirect(next_url)
         return self.render_template(
             self.login_template, title=self.title, form=form, appbuilder=self.appbuilder
         )
@@ -588,7 +593,10 @@ def after_login(resp):
                 session.pop("remember_me", None)
 
             login_user(user, remember=remember_me)
-            return redirect(self.appbuilder.get_url_for_index)
+            next_url = request.args.get("next", "")
+            if not next_url:
+                next_url = self.appbuilder.get_url_for_index
+            return redirect(next_url)
 
         return login_handler(self)
 

flask_appbuilder/tests/base.py

@@ -18,23 +18,19 @@
 class FABTestCase(unittest.TestCase):
     @staticmethod
     def auth_client_get(client, token, uri):
-        return client.get(uri, headers={"Authorization": "Bearer {}".format(token)})
+        return client.get(uri, headers={"Authorization": f"Bearer {token}"})
 
     @staticmethod
     def auth_client_delete(client, token, uri):
-        return client.delete(uri, headers={"Authorization": "Bearer {}".format(token)})
+        return client.delete(uri, headers={"Authorization": f"Bearer {token}"})
 
     @staticmethod
     def auth_client_put(client, token, uri, json):
-        return client.put(
-            uri, json=json, headers={"Authorization": "Bearer {}".format(token)}
-        )
+        return client.put(uri, json=json, headers={"Authorization": f"Bearer {token}"})
 
     @staticmethod
     def auth_client_post(client, token, uri, json):
-        return client.post(
-            uri, json=json, headers={"Authorization": "Bearer {}".format(token)}
-        )
+        return client.post(uri, json=json, headers={"Authorization": f"Bearer {token}"})
 
     @staticmethod
     def _login(client, username, password, refresh: bool = False):

flask_appbuilder/tests/config_api.py

@@ -6,7 +6,7 @@
     "SQLALCHEMY_DATABASE_URI"
 ) or "sqlite:///" + os.path.join(basedir, "app.db")
 
-
+AUTH_STRICT_RESPONSE_CODES = False
 SECRET_KEY = "thisismyscretkey"
 SQLALCHEMY_TRACK_MODIFICATIONS = False
 WTF_CSRF_ENABLED = False