chore: major bumps Flask, Click, PyJWT and flask-jwt-extended (#1817)

* chore: major bumps Flask, Click, PyJWT and flask-jwt-extended * remove python 3.6 support * breaking jwt-extended 1 * breaking jwt-extended 2 * breaking jwt-extended 3 * fix test * bump to 4.0.0 RC1 for testing * remove config key AUTH_STRICT_RESPONSE_CODES * bump more dependencies * lint and drop support for python 3.6 * fix python requires * reset version back
dpgaspar · Mar 21, 2022 · 2e2931f · 2e2931f
1 parent d341788
commit 2e2931f
Show file tree

Hide file tree

Showing 13 changed files with 58 additions and 103 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -38,7 +38,7 @@ jobs:
     runs-on: ubuntu-18.04
     strategy:
       matrix:
-        python-version: [3.6, 3.7, 3.8, 3.9.7]
+        python-version: [3.7, 3.8, 3.9.7]
     env:
       SQLALCHEMY_DATABASE_URI:
         postgresql+psycopg2://pguser:pguserpassword@127.0.0.1:15432/app
@@ -153,7 +153,7 @@ jobs:
     runs-on: ubuntu-18.04
     strategy:
       matrix:
-        python-version: [3.6, 3.7]
+        python-version: [3.7]
     services:
       mssql:
         image: mongo:4.4.1-bionic

diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
@@ -28,9 +28,14 @@ can run a subset of tests targeting only Postgres.
 
     $ docker-compose up -d
 
-
 2 - Run Postgres tests
 
+.. code-block:: bash
+
+    $ nosetests flask_appbuilder.tests
+
+You can also use tox
+
 .. code-block:: bash
 
     $ tox -e postgres
@@ -64,16 +69,14 @@ Using Postgres
 
 .. code-block:: bash
 
-    $ nosetests -v flask_appbuilder.tests.test_0_fixture
-
+    $ nosetests -v flask_appbuilder.tests.test_A_fixture
 
 4 - Run a single test
 
 .. code-block:: bash
 
     $ nosetests -v flask_appbuilder.tests.test_api:APITestCase.test_get_item_dotted_mo_notation
 
-
 .. note::
 
     If your using SQLite3, the location of the db is: ./flask_appbuilder/tests/app.db

diff --git a/docs/config.rst b/docs/config.rst
@@ -202,11 +202,6 @@ 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)                  |           |
 +----------------------------------------+--------------------------------------------+-----------+

diff --git a/flask_appbuilder/security/api.py b/flask_appbuilder/security/api.py
@@ -13,7 +13,7 @@
     create_access_token,
     create_refresh_token,
     get_jwt_identity,
-    jwt_refresh_token_required,
+    jwt_required,
 )
 from marshmallow import ValidationError
 
@@ -115,7 +115,7 @@ def login(self) -> Response:
         return self.response(200, **resp)
 
     @expose("/refresh", methods=["POST"])
-    @jwt_refresh_token_required
+    @jwt_required(refresh=True)
     @safe
     def refresh(self) -> Response:
         """

diff --git a/flask_appbuilder/security/decorators.py b/flask_appbuilder/security/decorators.py
@@ -1,6 +1,5 @@
 import functools
 import logging
-from typing import TYPE_CHECKING
 
 from flask import (
     current_app,
@@ -24,22 +23,8 @@
 
 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
+def response_unauthorized_mvc(status_code: int) -> Response:
     response = make_response(
         jsonify({"message": str(FLAMSG_ERR_SEC_ACCESS_DENIED), "severity": "danger"}),
         status_code,
@@ -88,7 +73,7 @@ def wraps(self, *args, **kwargs):
             class_permission_name = self.class_permission_name
             # Check if permission is allowed on the class
             if permission_str not in self.base_permissions:
-                return response_unauthorized(self)
+                return self.response_403()
             # Check if the resource is public
             if current_app.appbuilder.sm.is_item_public(
                 permission_str, class_permission_name
@@ -116,7 +101,7 @@ def wraps(self, *args, **kwargs):
                     permission_str, class_permission_name
                 )
             )
-            return response_unauthorized(self)
+            return self.response_403()
 
         f._permission_name = permission_str
         return functools.update_wrapper(wraps, f)
@@ -194,7 +179,9 @@ def wraps(self, *args, **kwargs):
                     permission_str, self.__class__.__name__
                 )
             )
-            return response_unauthorized_mvc()
+            if not current_user.is_authenticated:
+                return response_unauthorized_mvc(401)
+            return response_unauthorized_mvc(403)
 
     f._permission_name = permission_str
     return functools.update_wrapper(wraps, f)

diff --git a/flask_appbuilder/security/manager.py b/flask_appbuilder/security/manager.py
@@ -304,7 +304,7 @@ def create_jwt_manager(self, app) -> JWTManager:
         """
         jwt_manager = JWTManager()
         jwt_manager.init_app(app)
-        jwt_manager.user_loader_callback_loader(self.load_user_jwt)
+        jwt_manager.user_lookup_loader(self.load_user_jwt)
         return jwt_manager
 
     def create_builtin_roles(self):
@@ -871,7 +871,8 @@ def auth_user_db(self, username, password):
             )
             log.info(LOGMSG_WAR_SEC_LOGIN_FAILED.format(username))
             # Balance failure and success
-            self.noop_user_update(first_user)
+            if first_user:
+                self.noop_user_update(first_user)
             return None
         elif check_password_hash(user.password, password):
             self.update_user_auth_stat(user, True)
@@ -1499,7 +1500,7 @@ def _get_user_permission_view_menus(
         result.update(pvms_names)
         return result
 
-    def has_access(self, permission_name, view_name):
+    def has_access(self, permission_name: str, view_name: str) -> bool:
         """
         Check if current user or public has access to view or menu
         """
@@ -2036,8 +2037,9 @@ def import_roles(self, path: str) -> None:
     def load_user(self, pk):
         return self.get_user_by_id(int(pk))
 
-    def load_user_jwt(self, pk):
-        user = self.load_user(pk)
+    def load_user_jwt(self, _jwt_header, jwt_data):
+        identity = jwt_data["sub"]
+        user = self.load_user(identity)
         # Set flask g.user to JWT user, we can't do it on before request
         g.user = user
         return user

diff --git a/flask_appbuilder/tests/config_api.py b/flask_appbuilder/tests/config_api.py
@@ -6,7 +6,6 @@
     "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

diff --git a/flask_appbuilder/tests/test_api.py b/flask_appbuilder/tests/test_api.py
@@ -627,14 +627,9 @@ def test_auth_authorization(self):
         pk = 1
         uri = f"api/v1/model1apirestrictedpermissions/{pk}"
 
-        self.app.config["AUTH_STRICT_RESPONSE_CODES"] = True
         rv = self.auth_client_delete(client, token, uri)
         self.assertEqual(rv.status_code, 403)
 
-        self.app.config["AUTH_STRICT_RESPONSE_CODES"] = False
-        rv = self.auth_client_delete(client, token, uri)
-        self.assertEqual(rv.status_code, 401)
-
         # Test unauthorized POST
         item = dict(
             field_string="test{}".format(MODEL1_DATA_SIZE + 1),
@@ -644,12 +639,8 @@ def test_auth_authorization(self):
         )
         uri = "api/v1/model1apirestrictedpermissions/"
 
-        self.app.config["AUTH_STRICT_RESPONSE_CODES"] = True
         rv = self.auth_client_post(client, token, uri, item)
         self.assertEqual(rv.status_code, 403)
-        self.app.config["AUTH_STRICT_RESPONSE_CODES"] = False
-        rv = self.auth_client_post(client, token, uri, item)
-        self.assertEqual(rv.status_code, 401)
 
         # Test authorized GET
         uri = f"api/v1/model1apirestrictedpermissions/{pk}"
@@ -666,7 +657,7 @@ def test_auth_builtin_roles(self):
         pk = 1
         uri = "api/v1/model1api/{}".format(pk)
         rv = self.auth_client_delete(client, token, uri)
-        self.assertEqual(rv.status_code, 401)
+        self.assertEqual(rv.status_code, 403)
 
         # Test unauthorized POST
         item = dict(
@@ -677,7 +668,7 @@ def test_auth_builtin_roles(self):
         )
         uri = "api/v1/model1api/"
         rv = self.auth_client_post(client, token, uri, item)
-        self.assertEqual(rv.status_code, 401)
+        self.assertEqual(rv.status_code, 403)
 
         # Test authorized GET
         uri = "api/v1/model1api/1"
@@ -2804,7 +2795,7 @@ class Model2PermOverride2(ModelRestApi):
         self.assertEqual(rv.status_code, 200)
         uri = "api/v1/model2permoverride2/1"
         rv = self.auth_client_delete(client, token, uri)
-        self.assertEqual(rv.status_code, 401)
+        self.assertEqual(rv.status_code, 403)
 
         # Revert test data
         self.appbuilder.get_session.delete(

diff --git a/flask_appbuilder/tests/test_mvc.py b/flask_appbuilder/tests/test_mvc.py
@@ -1342,10 +1342,7 @@ def test_api_unauthenticated(self):
         Testing unauthenticated access to MVC API
         """
         client = self.app.test_client()
-        self.app.config["AUTH_STRICT_RESPONSE_CODES"] = True
-        rv = client.get("/model1formattedview/api/read")
-        self.assertEqual(rv.status_code, 401)
-        self.app.config["AUTH_STRICT_RESPONSE_CODES"] = False
+        self.browser_logout(client)
         rv = client.get("/model1formattedview/api/read")
         self.assertEqual(rv.status_code, 401)
 
@@ -1355,21 +1352,13 @@ def test_api_unauthorized(self):
         """
         client = self.app.test_client()
         self.browser_login(client, USERNAME_READONLY, PASSWORD_READONLY)
-        self.app.config["AUTH_STRICT_RESPONSE_CODES"] = True
 
         rv = client.post(
             "/model1view/api/create",
             data=dict(field_string="zzz"),
             follow_redirects=True,
         )
         self.assertEqual(rv.status_code, 403)
-        self.app.config["AUTH_STRICT_RESPONSE_CODES"] = False
-        rv = client.post(
-            "/model1view/api/create",
-            data=dict(field_string="zzz"),
-            follow_redirects=True,
-        )
-        self.assertEqual(rv.status_code, 401)
 
     def test_api_create(self):
         """

diff --git a/flask_appbuilder/tests/test_mvc_oauth.py b/flask_appbuilder/tests/test_mvc_oauth.py
@@ -47,7 +47,7 @@ def test_oauth_login(self):
         raw_state = {}
         state = jwt.encode(raw_state, self.app.config["SECRET_KEY"], algorithm="HS256")
 
-        response = client.get(f"/oauth-authorized/google?state={state.decode('utf-8')}")
+        response = client.get(f"/oauth-authorized/google?state={state}")
         self.assertEqual(response.location, "http://localhost/")
 
     def test_oauth_login_unknown_provider(self):
@@ -61,9 +61,7 @@ def test_oauth_login_unknown_provider(self):
         raw_state = {}
         state = jwt.encode(raw_state, self.app.config["SECRET_KEY"], algorithm="HS256")
 
-        response = client.get(
-            f"/oauth-authorized/unknown_provider?state={state.decode('utf-8')}"
-        )
+        response = client.get(f"/oauth-authorized/unknown_provider?state={state}")
         self.assertEqual(response.location, "http://localhost/login/")
 
     def test_oauth_login_next(self):
@@ -77,7 +75,7 @@ def test_oauth_login_next(self):
         raw_state = {"next": ["http://localhost/users/list/"]}
         state = jwt.encode(raw_state, self.app.config["SECRET_KEY"], algorithm="HS256")
 
-        response = client.get(f"/oauth-authorized/google?state={state.decode('utf-8')}")
+        response = client.get(f"/oauth-authorized/google?state={state}")
         self.assertEqual(response.location, "http://localhost/users/list/")
 
     def test_oauth_login_next_check(self):
@@ -91,5 +89,5 @@ def test_oauth_login_next_check(self):
         raw_state = {"next": ["http://www.google.com"]}
         state = jwt.encode(raw_state, self.app.config["SECRET_KEY"], algorithm="HS256")
 
-        response = client.get(f"/oauth-authorized/google?state={state.decode('utf-8')}")
+        response = client.get(f"/oauth-authorized/google?state={state}")
         self.assertEqual(response.location, "http://localhost/")
diff --git a/requirements-extra.txt b/requirements-extra.txt
@@ -9,3 +9,4 @@ pyodbc==4.0.30
 requests==2.26.0
 Authlib==0.15.4
 python-ldap==3.3.1
+flask-openid==1.3.0