From 5aa38b7e0ccffe050aef0ea7c8f973883ba93457 Mon Sep 17 00:00:00 2001 From: Daniel Wolf <95075445+wolfdn@users.noreply.github.com> Date: Fri, 1 Sep 2023 08:36:33 +0200 Subject: [PATCH 01/15] Use joserfc to decode Azure jwt --- flask_appbuilder/security/manager.py | 50 +++++++++------------------- setup.py | 1 + 2 files changed, 17 insertions(+), 34 deletions(-) diff --git a/flask_appbuilder/security/manager.py b/flask_appbuilder/security/manager.py index ae4fbe96a2..2f1c3631a7 100644 --- a/flask_appbuilder/security/manager.py +++ b/flask_appbuilder/security/manager.py @@ -5,6 +5,7 @@ import re from typing import Any, Dict, List, Optional, Set, Tuple, Union +import requests from flask import Flask, g, session, url_for from flask_babel import lazy_gettext as _ from flask_jwt_extended import current_user as current_user_jwt @@ -12,6 +13,8 @@ from flask_limiter import Limiter from flask_limiter.util import get_remote_address from flask_login import current_user, LoginManager +from joserfc import jwt +from joserfc.jwk import KeySet from werkzeug.security import check_password_hash, generate_password_hash from .api import SecurityApi @@ -632,11 +635,9 @@ def get_oauth_user_info(self, provider, resp): # https://docs.microsoft.com/en-us/azure/active-directory/develop/ # active-directory-protocols-oauth-code if provider == "azure": - log.debug("Azure response received : %s", resp) - id_token = resp["id_token"] - log.debug(str(id_token)) - me = self._azure_jwt_token_parse(id_token) - log.debug("Parse JWT token : %s", me) + log.debug("Azure response received:\n%s", json.dumps(resp, indent=4)) + me = self._decode_and_validate_azure_jwt(resp["id_token"]) + log.debug("Decoded JWT:\n%s", json.dumps(me, indent=4)) return { "name": me.get("name", ""), "email": me["upn"], @@ -683,36 +684,17 @@ def get_oauth_user_info(self, provider, resp): else: return {} - def _azure_parse_jwt(self, id_token): - jwt_token_parts = r"^([^\.\s]*)\.([^\.\s]+)\.([^\.\s]*)$" - matches = re.search(jwt_token_parts, id_token) - if not matches or len(matches.groups()) < 3: - log.error("Unable to parse token.") - return {} - return { - "header": matches.group(1), - "Payload": matches.group(2), - "Sig": matches.group(3), - } - - def _azure_jwt_token_parse(self, id_token): - jwt_split_token = self._azure_parse_jwt(id_token) - if not jwt_split_token: - return - - jwt_payload = jwt_split_token["Payload"] - # Prepare for base64 decoding - payload_b64_string = jwt_payload - payload_b64_string += "=" * (4 - ((len(jwt_payload) % 4))) - decoded_payload = base64.urlsafe_b64decode(payload_b64_string.encode("ascii")) - - if not decoded_payload: - log.error("Payload of id_token could not be base64 url decoded.") - return - - jwt_decoded_payload = json.loads(decoded_payload.decode("utf-8")) + def _decode_and_validate_azure_jwt(self, id_token): + keyset = KeySet.import_key_set( + requests.get("https://login.microsoftonline.com/common/discovery/keys").json() + ) + claims = jwt.decode(id_token, keyset).claims + claims_requests = jwt.JWTClaimsRegistry() + # validate basic token claims: exp, iat, nbf + # See: https://jose.authlib.org/en/dev/api/jwt/#joserfc.jwt.JWTClaimsRegistry + claims_requests.validate(claims) - return jwt_decoded_payload + return claims def register_views(self): if not self.appbuilder.app.config.get("FAB_ADD_SECURITY_VIEWS", True): diff --git a/setup.py b/setup.py index 955f0c2a62..ecdfb7c235 100644 --- a/setup.py +++ b/setup.py @@ -56,6 +56,7 @@ def desc(): "Flask-SQLAlchemy>=2.4, <3", "Flask-WTF>=0.14.2, <2", "Flask-JWT-Extended>=4.0.0, <5.0.0", + "joserfc>=0.7.0", "jsonschema>=3, <5", "marshmallow>=3.18.0, <4", "marshmallow-sqlalchemy>=0.22.0, <0.27.0", From aa9ace70ca58bf321d6b56a49229938904201bed Mon Sep 17 00:00:00 2001 From: Daniel Wolf <95075445+wolfdn@users.noreply.github.com> Date: Fri, 1 Sep 2023 09:02:01 +0200 Subject: [PATCH 02/15] Apply black formatting --- flask_appbuilder/security/manager.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/flask_appbuilder/security/manager.py b/flask_appbuilder/security/manager.py index 2f1c3631a7..1b3fa16e6b 100644 --- a/flask_appbuilder/security/manager.py +++ b/flask_appbuilder/security/manager.py @@ -686,7 +686,9 @@ def get_oauth_user_info(self, provider, resp): def _decode_and_validate_azure_jwt(self, id_token): keyset = KeySet.import_key_set( - requests.get("https://login.microsoftonline.com/common/discovery/keys").json() + requests.get( + "https://login.microsoftonline.com/common/discovery/keys" + ).json() ) claims = jwt.decode(id_token, keyset).claims claims_requests = jwt.JWTClaimsRegistry() From 8124fbf85863566a85b749ec8fdc462c7bdedcef Mon Sep 17 00:00:00 2001 From: Daniel Wolf <95075445+wolfdn@users.noreply.github.com> Date: Fri, 1 Sep 2023 09:06:57 +0200 Subject: [PATCH 03/15] Add joserfc to requirements.txt --- requirements.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/requirements.txt b/requirements.txt index ffe36d57d4..c7d847cf84 100644 --- a/requirements.txt +++ b/requirements.txt @@ -57,6 +57,8 @@ jinja2==3.0.3 # via # flask # flask-babel +joserfc==0.7.0 + # via Flask-AppBuilder (setup.py) jsonschema==3.2.0 # via Flask-AppBuilder (setup.py) limits==2.8.0 From e09b96b1c83534da9a458647bd7bdb0860b607e8 Mon Sep 17 00:00:00 2001 From: Daniel Wolf <95075445+wolfdn@users.noreply.github.com> Date: Fri, 1 Sep 2023 11:06:09 +0200 Subject: [PATCH 04/15] Remove unnecessary import statement --- flask_appbuilder/security/manager.py | 1 - 1 file changed, 1 deletion(-) diff --git a/flask_appbuilder/security/manager.py b/flask_appbuilder/security/manager.py index 1b3fa16e6b..0c4fe152ec 100644 --- a/flask_appbuilder/security/manager.py +++ b/flask_appbuilder/security/manager.py @@ -1,4 +1,3 @@ -import base64 import datetime import json import logging From 243ff93f156de53dc9c4ac110df2d68d5fa81a3a Mon Sep 17 00:00:00 2001 From: Daniel Wolf <95075445+wolfdn@users.noreply.github.com> Date: Fri, 1 Sep 2023 13:26:28 +0200 Subject: [PATCH 05/15] Fix import order --- flask_appbuilder/security/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flask_appbuilder/security/manager.py b/flask_appbuilder/security/manager.py index 0c4fe152ec..40f8caa54c 100644 --- a/flask_appbuilder/security/manager.py +++ b/flask_appbuilder/security/manager.py @@ -4,7 +4,6 @@ import re from typing import Any, Dict, List, Optional, Set, Tuple, Union -import requests from flask import Flask, g, session, url_for from flask_babel import lazy_gettext as _ from flask_jwt_extended import current_user as current_user_jwt @@ -14,6 +13,7 @@ from flask_login import current_user, LoginManager from joserfc import jwt from joserfc.jwk import KeySet +import requests from werkzeug.security import check_password_hash, generate_password_hash from .api import SecurityApi From 88dd310c31ffbd5cc17fdbb3fcc357f010b18ec2 Mon Sep 17 00:00:00 2001 From: Daniel Wolf <95075445+wolfdn@users.noreply.github.com> Date: Fri, 1 Sep 2023 14:53:32 +0200 Subject: [PATCH 06/15] Switch to authlib to stay compatible with Python 3.7 --- flask_appbuilder/security/manager.py | 13 +++++-------- requirements.txt | 4 ++-- setup.py | 2 +- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/flask_appbuilder/security/manager.py b/flask_appbuilder/security/manager.py index 40f8caa54c..18ef101366 100644 --- a/flask_appbuilder/security/manager.py +++ b/flask_appbuilder/security/manager.py @@ -4,6 +4,7 @@ import re from typing import Any, Dict, List, Optional, Set, Tuple, Union +from authlib.jose import jwt, JsonWebKey from flask import Flask, g, session, url_for from flask_babel import lazy_gettext as _ from flask_jwt_extended import current_user as current_user_jwt @@ -11,8 +12,6 @@ from flask_limiter import Limiter from flask_limiter.util import get_remote_address from flask_login import current_user, LoginManager -from joserfc import jwt -from joserfc.jwk import KeySet import requests from werkzeug.security import check_password_hash, generate_password_hash @@ -684,16 +683,14 @@ def get_oauth_user_info(self, provider, resp): return {} def _decode_and_validate_azure_jwt(self, id_token): - keyset = KeySet.import_key_set( + keyset = JsonWebKey.import_key_set( requests.get( "https://login.microsoftonline.com/common/discovery/keys" ).json() ) - claims = jwt.decode(id_token, keyset).claims - claims_requests = jwt.JWTClaimsRegistry() - # validate basic token claims: exp, iat, nbf - # See: https://jose.authlib.org/en/dev/api/jwt/#joserfc.jwt.JWTClaimsRegistry - claims_requests.validate(claims) + claims = jwt.decode(id_token, keyset) + claims.validate() + log.info("Decoded JWT:\n%s", json.dumps(claims, indent=4)) return claims diff --git a/requirements.txt b/requirements.txt index c7d847cf84..5c9df57fd4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,6 +6,8 @@ # apispec[yaml]==6.3.0 # via Flask-AppBuilder (setup.py) +authlib==1.2.1 + # via Flask-AppBuilder (setup.py) attrs==21.4.0 # via jsonschema babel==2.9.1 @@ -57,8 +59,6 @@ jinja2==3.0.3 # via # flask # flask-babel -joserfc==0.7.0 - # via Flask-AppBuilder (setup.py) jsonschema==3.2.0 # via Flask-AppBuilder (setup.py) limits==2.8.0 diff --git a/setup.py b/setup.py index ecdfb7c235..6e1aa4556f 100644 --- a/setup.py +++ b/setup.py @@ -46,6 +46,7 @@ def desc(): platforms="any", install_requires=[ "apispec[yaml]>=6.0.0, <7", + "authlib>=1.2.1", "colorama>=0.3.9, <1", "click>=8, <9", "email_validator>=1.0.5, <2", @@ -56,7 +57,6 @@ def desc(): "Flask-SQLAlchemy>=2.4, <3", "Flask-WTF>=0.14.2, <2", "Flask-JWT-Extended>=4.0.0, <5.0.0", - "joserfc>=0.7.0", "jsonschema>=3, <5", "marshmallow>=3.18.0, <4", "marshmallow-sqlalchemy>=0.22.0, <0.27.0", From 7afa57c68557bd818d7cef5697ed5903ef3e69e4 Mon Sep 17 00:00:00 2001 From: Daniel Wolf <95075445+wolfdn@users.noreply.github.com> Date: Fri, 1 Sep 2023 14:57:41 +0200 Subject: [PATCH 07/15] Fix import order --- flask_appbuilder/security/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flask_appbuilder/security/manager.py b/flask_appbuilder/security/manager.py index 18ef101366..e23053a87e 100644 --- a/flask_appbuilder/security/manager.py +++ b/flask_appbuilder/security/manager.py @@ -4,7 +4,7 @@ import re from typing import Any, Dict, List, Optional, Set, Tuple, Union -from authlib.jose import jwt, JsonWebKey +from authlib.jose import JsonWebKey, jwt from flask import Flask, g, session, url_for from flask_babel import lazy_gettext as _ from flask_jwt_extended import current_user as current_user_jwt From 86fd8b7b7d44f1e32f7a67506a4b5d81aaece130 Mon Sep 17 00:00:00 2001 From: Daniel Wolf <95075445+wolfdn@users.noreply.github.com> Date: Wed, 4 Oct 2023 13:23:08 +0200 Subject: [PATCH 08/15] Move Microsoft key set URL to constants --- flask_appbuilder/const.py | 6 ++++++ flask_appbuilder/security/manager.py | 5 ++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/flask_appbuilder/const.py b/flask_appbuilder/const.py index db3030cc9d..67af12d7aa 100644 --- a/flask_appbuilder/const.py +++ b/flask_appbuilder/const.py @@ -191,3 +191,9 @@ API_ADD_TITLE_RIS_KEY = "add_title" API_EDIT_TITLE_RIS_KEY = "edit_title" API_SHOW_TITLE_RIS_KEY = "show_title" + +# ----------------------------------- +# OAuth Provider Constants +# ----------------------------------- + +MICROSOFT_KEY_SET_URL="https://login.microsoftonline.com/common/discovery/keys" diff --git a/flask_appbuilder/security/manager.py b/flask_appbuilder/security/manager.py index e23053a87e..76746ad1f7 100644 --- a/flask_appbuilder/security/manager.py +++ b/flask_appbuilder/security/manager.py @@ -56,6 +56,7 @@ LOGMSG_WAR_SEC_NO_USER, LOGMSG_WAR_SEC_NOLDAP_OBJ, PERMISSION_PREFIX, + MICROSOFT_KEY_SET_URL, ) log = logging.getLogger(__name__) @@ -684,9 +685,7 @@ def get_oauth_user_info(self, provider, resp): def _decode_and_validate_azure_jwt(self, id_token): keyset = JsonWebKey.import_key_set( - requests.get( - "https://login.microsoftonline.com/common/discovery/keys" - ).json() + requests.get(MICROSOFT_KEY_SET_URL).json() ) claims = jwt.decode(id_token, keyset) claims.validate() From 0f8af76b10e9655d5ee4a420c5da43dc72131dd1 Mon Sep 17 00:00:00 2001 From: Daniel Wolf <95075445+wolfdn@users.noreply.github.com> Date: Wed, 4 Oct 2023 13:23:48 +0200 Subject: [PATCH 09/15] Make logging less verbose --- flask_appbuilder/security/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flask_appbuilder/security/manager.py b/flask_appbuilder/security/manager.py index 76746ad1f7..4d86a9a6f4 100644 --- a/flask_appbuilder/security/manager.py +++ b/flask_appbuilder/security/manager.py @@ -689,7 +689,7 @@ def _decode_and_validate_azure_jwt(self, id_token): ) claims = jwt.decode(id_token, keyset) claims.validate() - log.info("Decoded JWT:\n%s", json.dumps(claims, indent=4)) + log.debug("Decoded JWT:\n%s", json.dumps(claims, indent=4)) return claims From e57cab44b98d1093eabb4220a0b1d0d2234f5141 Mon Sep 17 00:00:00 2001 From: Daniel Wolf <95075445+wolfdn@users.noreply.github.com> Date: Wed, 4 Oct 2023 13:28:14 +0200 Subject: [PATCH 10/15] Add unittest --- tests/security/test_base_security_manager.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/security/test_base_security_manager.py b/tests/security/test_base_security_manager.py index 736b0b88d6..9a5cd6cc0e 100644 --- a/tests/security/test_base_security_manager.py +++ b/tests/security/test_base_security_manager.py @@ -1,8 +1,10 @@ import datetime +import json import unittest from unittest.mock import MagicMock, patch from flask_appbuilder.security.manager import BaseSecurityManager +from flask_appbuilder.security.manager import JsonWebKey, jwt @patch.object(BaseSecurityManager, "update_user") @@ -67,3 +69,15 @@ def test_subsequent_unsuccessful_auth(self, mock1, mock2): self.assertEqual(user_mock.fail_login_count, 10) self.assertEqual(user_mock.last_login, None) self.assertTrue(bsm.update_user.called_once) + + def test_azure_jwt_validated(self, mock1, mock2): + example_jwt="ExampleAzureJWT" + + JsonWebKey.import_key_set = MagicMock() + JWTClaimsMock = MagicMock() + jwt.decode = MagicMock(return_value=JWTClaimsMock) + json.dumps = MagicMock(return_value=f"Decoded{example_jwt}") + + bsm = BaseSecurityManager() + bsm._decode_and_validate_azure_jwt(example_jwt) + JWTClaimsMock.validate.assert_called() From 55a5ebe199c867760d73aa32eb0a96ee10451937 Mon Sep 17 00:00:00 2001 From: Daniel Wolf <95075445+wolfdn@users.noreply.github.com> Date: Wed, 4 Oct 2023 13:28:42 +0200 Subject: [PATCH 11/15] Remove authlib from install_requires --- setup.py | 1 - 1 file changed, 1 deletion(-) diff --git a/setup.py b/setup.py index 6e1aa4556f..955f0c2a62 100644 --- a/setup.py +++ b/setup.py @@ -46,7 +46,6 @@ def desc(): platforms="any", install_requires=[ "apispec[yaml]>=6.0.0, <7", - "authlib>=1.2.1", "colorama>=0.3.9, <1", "click>=8, <9", "email_validator>=1.0.5, <2", From 63d1e4ec1423ed81a89d2bf44f541c75167f8719 Mon Sep 17 00:00:00 2001 From: Daniel Wolf <95075445+wolfdn@users.noreply.github.com> Date: Wed, 4 Oct 2023 13:37:57 +0200 Subject: [PATCH 12/15] Fix formatting --- flask_appbuilder/const.py | 2 +- flask_appbuilder/security/manager.py | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/flask_appbuilder/const.py b/flask_appbuilder/const.py index 67af12d7aa..3ccb1bf33e 100644 --- a/flask_appbuilder/const.py +++ b/flask_appbuilder/const.py @@ -196,4 +196,4 @@ # OAuth Provider Constants # ----------------------------------- -MICROSOFT_KEY_SET_URL="https://login.microsoftonline.com/common/discovery/keys" +MICROSOFT_KEY_SET_URL = "https://login.microsoftonline.com/common/discovery/keys" diff --git a/flask_appbuilder/security/manager.py b/flask_appbuilder/security/manager.py index 4d86a9a6f4..d525750f77 100644 --- a/flask_appbuilder/security/manager.py +++ b/flask_appbuilder/security/manager.py @@ -684,9 +684,7 @@ def get_oauth_user_info(self, provider, resp): return {} def _decode_and_validate_azure_jwt(self, id_token): - keyset = JsonWebKey.import_key_set( - requests.get(MICROSOFT_KEY_SET_URL).json() - ) + keyset = JsonWebKey.import_key_set(requests.get(MICROSOFT_KEY_SET_URL).json()) claims = jwt.decode(id_token, keyset) claims.validate() log.debug("Decoded JWT:\n%s", json.dumps(claims, indent=4)) From 879edd0528db95c26e8bfc692f78376c3a30d1a3 Mon Sep 17 00:00:00 2001 From: Daniel Wolf <95075445+wolfdn@users.noreply.github.com> Date: Wed, 4 Oct 2023 15:07:25 +0200 Subject: [PATCH 13/15] Update unittest --- tests/security/test_base_security_manager.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/security/test_base_security_manager.py b/tests/security/test_base_security_manager.py index 9a5cd6cc0e..899506575e 100644 --- a/tests/security/test_base_security_manager.py +++ b/tests/security/test_base_security_manager.py @@ -6,6 +6,8 @@ from flask_appbuilder.security.manager import BaseSecurityManager from flask_appbuilder.security.manager import JsonWebKey, jwt +JWTClaimsMock = MagicMock() + @patch.object(BaseSecurityManager, "update_user") @patch.object(BaseSecurityManager, "__init__", return_value=None) @@ -70,14 +72,11 @@ def test_subsequent_unsuccessful_auth(self, mock1, mock2): self.assertEqual(user_mock.last_login, None) self.assertTrue(bsm.update_user.called_once) + @patch.object(JsonWebKey, "import_key_set", MagicMock()) + @patch.object(jwt, "decode", MagicMock(return_value=JWTClaimsMock)) + @patch.object(json, "dumps", MagicMock(return_value="DecodedExampleAzureJWT")) def test_azure_jwt_validated(self, mock1, mock2): - example_jwt="ExampleAzureJWT" - - JsonWebKey.import_key_set = MagicMock() - JWTClaimsMock = MagicMock() - jwt.decode = MagicMock(return_value=JWTClaimsMock) - json.dumps = MagicMock(return_value=f"Decoded{example_jwt}") - bsm = BaseSecurityManager() - bsm._decode_and_validate_azure_jwt(example_jwt) + + bsm._decode_and_validate_azure_jwt("ExampleAzureJWT") JWTClaimsMock.validate.assert_called() From a4e4fbb61ec3dc5179f916175f2229366227a7eb Mon Sep 17 00:00:00 2001 From: Daniel Wolf <95075445+wolfdn@users.noreply.github.com> Date: Wed, 4 Oct 2023 15:10:02 +0200 Subject: [PATCH 14/15] Fix import order --- flask_appbuilder/security/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flask_appbuilder/security/manager.py b/flask_appbuilder/security/manager.py index d525750f77..bafcae5b41 100644 --- a/flask_appbuilder/security/manager.py +++ b/flask_appbuilder/security/manager.py @@ -55,8 +55,8 @@ LOGMSG_WAR_SEC_LOGIN_FAILED, LOGMSG_WAR_SEC_NO_USER, LOGMSG_WAR_SEC_NOLDAP_OBJ, - PERMISSION_PREFIX, MICROSOFT_KEY_SET_URL, + PERMISSION_PREFIX, ) log = logging.getLogger(__name__) From 3c179986029540ae35b392540600829a1d650abc Mon Sep 17 00:00:00 2001 From: Daniel Wolf <95075445+wolfdn@users.noreply.github.com> Date: Thu, 5 Oct 2023 07:27:51 +0200 Subject: [PATCH 15/15] Move authlib to requirements-extra.txt --- requirements-extra.txt | 2 +- requirements.txt | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/requirements-extra.txt b/requirements-extra.txt index c6ddb42e4f..89ea25cb92 100644 --- a/requirements-extra.txt +++ b/requirements-extra.txt @@ -4,6 +4,6 @@ mysqlclient==2.0.1 psycopg2-binary==2.9.6 pyodbc==4.0.35 requests==2.26.0 -Authlib==0.15.4 +Authlib==1.2.1 python-ldap==3.3.1 flask-openid==1.3.0 diff --git a/requirements.txt b/requirements.txt index 5c9df57fd4..ffe36d57d4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,8 +6,6 @@ # apispec[yaml]==6.3.0 # via Flask-AppBuilder (setup.py) -authlib==1.2.1 - # via Flask-AppBuilder (setup.py) attrs==21.4.0 # via jsonschema babel==2.9.1