From 0d01e42f59e4ce28ea49ef9db44302b8cdcbe184 Mon Sep 17 00:00:00 2001 From: Marius Lie Winger Date: Tue, 11 Jun 2024 08:53:31 +0200 Subject: [PATCH 1/2] refactor: replace python-jose with pyjwt --- api/poetry.lock | 54 +------------------ api/pyproject.toml | 2 +- api/src/authentication/authentication.py | 20 ++++--- .../authentication/mock_token_generator.py | 3 +- 4 files changed, 14 insertions(+), 65 deletions(-) diff --git a/api/poetry.lock b/api/poetry.lock index 8c0d5a8d..3ec2cc4b 100644 --- a/api/poetry.lock +++ b/api/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.5.1 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand. [[package]] name = "annotated-types" @@ -342,24 +342,6 @@ files = [ {file = "distlib-0.3.7.tar.gz", hash = "sha256:9dafe54b34a028eafd95039d5e5d4851a13734540f1331060d31c9916e7147a8"}, ] -[[package]] -name = "ecdsa" -version = "0.18.0" -description = "ECDSA cryptographic signature library (pure python)" -optional = false -python-versions = ">=2.6, !=3.0.*, !=3.1.*, !=3.2.*" -files = [ - {file = "ecdsa-0.18.0-py2.py3-none-any.whl", hash = "sha256:80600258e7ed2f16b9aa1d7c295bd70194109ad5a30fdee0eaeefef1d4c559dd"}, - {file = "ecdsa-0.18.0.tar.gz", hash = "sha256:190348041559e21b22a1d65cee485282ca11a6f81d503fddb84d5017e9ed1e49"}, -] - -[package.dependencies] -six = ">=1.9.0" - -[package.extras] -gmpy = ["gmpy"] -gmpy2 = ["gmpy2"] - [[package]] name = "exceptiongroup" version = "1.1.3" @@ -1208,28 +1190,6 @@ files = [ [package.extras] cli = ["click (>=5.0)"] -[[package]] -name = "python-jose" -version = "3.3.0" -description = "JOSE implementation in Python" -optional = false -python-versions = "*" -files = [ - {file = "python-jose-3.3.0.tar.gz", hash = "sha256:55779b5e6ad599c6336191246e95eb2293a9ddebd555f796a65f838f07e5d78a"}, - {file = "python_jose-3.3.0-py2.py3-none-any.whl", hash = "sha256:9b1376b023f8b298536eedd47ae1089bcdb848f1535ab30555cd92002d78923a"}, -] - -[package.dependencies] -cryptography = {version = ">=3.4.0", optional = true, markers = "extra == \"cryptography\""} -ecdsa = "!=0.15" -pyasn1 = "*" -rsa = "*" - -[package.extras] -cryptography = ["cryptography (>=3.4.0)"] -pycrypto = ["pyasn1", "pycrypto (>=2.6.0,<2.7.0)"] -pycryptodome = ["pyasn1", "pycryptodome (>=3.3.1,<4.0.0)"] - [[package]] name = "pywin32" version = "306" @@ -1265,7 +1225,6 @@ files = [ {file = "PyYAML-6.0.1-cp310-cp310-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:69b023b2b4daa7548bcfbd4aa3da05b3a74b772db9e23b982788168117739938"}, {file = "PyYAML-6.0.1-cp310-cp310-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:81e0b275a9ecc9c0c0c07b4b90ba548307583c125f54d5b6946cfee6360c733d"}, {file = "PyYAML-6.0.1-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:ba336e390cd8e4d1739f42dfe9bb83a3cc2e80f567d8805e11b46f4a943f5515"}, - {file = "PyYAML-6.0.1-cp310-cp310-musllinux_1_1_x86_64.whl", hash = "sha256:326c013efe8048858a6d312ddd31d56e468118ad4cdeda36c719bf5bb6192290"}, {file = "PyYAML-6.0.1-cp310-cp310-win32.whl", hash = "sha256:bd4af7373a854424dabd882decdc5579653d7868b8fb26dc7d0e99f823aa5924"}, {file = "PyYAML-6.0.1-cp310-cp310-win_amd64.whl", hash = "sha256:fd1592b3fdf65fff2ad0004b5e363300ef59ced41c2e6b3a99d4089fa8c5435d"}, {file = "PyYAML-6.0.1-cp311-cp311-macosx_10_9_x86_64.whl", hash = "sha256:6965a7bc3cf88e5a1c3bd2e0b5c22f8d677dc88a455344035f03399034eb3007"}, @@ -1273,15 +1232,8 @@ files = [ {file = "PyYAML-6.0.1-cp311-cp311-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:42f8152b8dbc4fe7d96729ec2b99c7097d656dc1213a3229ca5383f973a5ed6d"}, {file = "PyYAML-6.0.1-cp311-cp311-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:062582fca9fabdd2c8b54a3ef1c978d786e0f6b3a1510e0ac93ef59e0ddae2bc"}, {file = "PyYAML-6.0.1-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:d2b04aac4d386b172d5b9692e2d2da8de7bfb6c387fa4f801fbf6fb2e6ba4673"}, - {file = "PyYAML-6.0.1-cp311-cp311-musllinux_1_1_x86_64.whl", hash = "sha256:e7d73685e87afe9f3b36c799222440d6cf362062f78be1013661b00c5c6f678b"}, {file = "PyYAML-6.0.1-cp311-cp311-win32.whl", hash = "sha256:1635fd110e8d85d55237ab316b5b011de701ea0f29d07611174a1b42f1444741"}, {file = "PyYAML-6.0.1-cp311-cp311-win_amd64.whl", hash = "sha256:bf07ee2fef7014951eeb99f56f39c9bb4af143d8aa3c21b1677805985307da34"}, - {file = "PyYAML-6.0.1-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:855fb52b0dc35af121542a76b9a84f8d1cd886ea97c84703eaa6d88e37a2ad28"}, - {file = "PyYAML-6.0.1-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:40df9b996c2b73138957fe23a16a4f0ba614f4c0efce1e9406a184b6d07fa3a9"}, - {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:6c22bec3fbe2524cde73d7ada88f6566758a8f7227bfbf93a408a9d86bcc12a0"}, - {file = "PyYAML-6.0.1-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:8d4e9c88387b0f5c7d5f281e55304de64cf7f9c0021a3525bd3b1c542da3b0e4"}, - {file = "PyYAML-6.0.1-cp312-cp312-win32.whl", hash = "sha256:d483d2cdf104e7c9fa60c544d92981f12ad66a457afae824d146093b8c294c54"}, - {file = "PyYAML-6.0.1-cp312-cp312-win_amd64.whl", hash = "sha256:0d3304d8c0adc42be59c5f8a4d9e3d7379e6955ad754aa9d6ab7a398b59dd1df"}, {file = "PyYAML-6.0.1-cp36-cp36m-macosx_10_9_x86_64.whl", hash = "sha256:50550eb667afee136e9a77d6dc71ae76a44df8b3e51e41b77f6de2932bfe0f47"}, {file = "PyYAML-6.0.1-cp36-cp36m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:1fe35611261b29bd1de0070f0b2f47cb6ff71fa6595c077e42bd0c419fa27b98"}, {file = "PyYAML-6.0.1-cp36-cp36m-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:704219a11b772aea0d8ecd7058d0082713c3562b4e271b849ad7dc4a5c90c13c"}, @@ -1298,7 +1250,6 @@ files = [ {file = "PyYAML-6.0.1-cp38-cp38-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:a0cd17c15d3bb3fa06978b4e8958dcdc6e0174ccea823003a106c7d4d7899ac5"}, {file = "PyYAML-6.0.1-cp38-cp38-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:28c119d996beec18c05208a8bd78cbe4007878c6dd15091efb73a30e90539696"}, {file = "PyYAML-6.0.1-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:7e07cbde391ba96ab58e532ff4803f79c4129397514e1413a7dc761ccd755735"}, - {file = "PyYAML-6.0.1-cp38-cp38-musllinux_1_1_x86_64.whl", hash = "sha256:49a183be227561de579b4a36efbb21b3eab9651dd81b1858589f796549873dd6"}, {file = "PyYAML-6.0.1-cp38-cp38-win32.whl", hash = "sha256:184c5108a2aca3c5b3d3bf9395d50893a7ab82a38004c8f61c258d4428e80206"}, {file = "PyYAML-6.0.1-cp38-cp38-win_amd64.whl", hash = "sha256:1e2722cc9fbb45d9b87631ac70924c11d3a401b2d7f410cc0e3bbf249f2dca62"}, {file = "PyYAML-6.0.1-cp39-cp39-macosx_10_9_x86_64.whl", hash = "sha256:9eb6caa9a297fc2c2fb8862bc5370d0303ddba53ba97e71f08023b6cd73d16a8"}, @@ -1306,7 +1257,6 @@ files = [ {file = "PyYAML-6.0.1-cp39-cp39-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:5773183b6446b2c99bb77e77595dd486303b4faab2b086e7b17bc6bef28865f6"}, {file = "PyYAML-6.0.1-cp39-cp39-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:b786eecbdf8499b9ca1d697215862083bd6d2a99965554781d0d8d1ad31e13a0"}, {file = "PyYAML-6.0.1-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:bc1bf2925a1ecd43da378f4db9e4f799775d6367bdb94671027b73b393a7c42c"}, - {file = "PyYAML-6.0.1-cp39-cp39-musllinux_1_1_x86_64.whl", hash = "sha256:04ac92ad1925b2cff1db0cfebffb6ffc43457495c9b3c39d3fcae417d7125dc5"}, {file = "PyYAML-6.0.1-cp39-cp39-win32.whl", hash = "sha256:faca3bdcf85b2fc05d06ff3fbc1f83e1391b3e724afa3feba7d13eeab355484c"}, {file = "PyYAML-6.0.1-cp39-cp39-win_amd64.whl", hash = "sha256:510c9deebc5c0225e8c96813043e62b680ba2f9c50a08d3724c7f28a747d1486"}, {file = "PyYAML-6.0.1.tar.gz", hash = "sha256:bfdf460b1736c775f2ba9f6a92bca30bc2095067b8a9d77876d1fad6cc3b4a43"}, @@ -1673,4 +1623,4 @@ files = [ [metadata] lock-version = "2.0" python-versions = "^3.10" -content-hash = "2912168e31d17fd2218ce1fa361ffea9f027a96d2a50a3663ede17f6921ed2c7" +content-hash = "c50b2d98a9ac249d00083406ec849e0db162e5d6d9e5558cba6eed933562ca3b" diff --git a/api/pyproject.toml b/api/pyproject.toml index 674abda8..990bb6f2 100644 --- a/api/pyproject.toml +++ b/api/pyproject.toml @@ -9,7 +9,7 @@ license = "" cachetools = "^5.3.0" python = "^3.10" fastapi = "^0.101.0" -python-jose = {extras = ["cryptography"], version = "^3.3.0"} +pyjwt = "^2.8.0" uvicorn = {extras = ["standard"], version = "^0.21.1"} pymongo = "4.1.1" certifi = "^2023.7.22" diff --git a/api/src/authentication/authentication.py b/api/src/authentication/authentication.py index 4b0d3c89..ccc04216 100644 --- a/api/src/authentication/authentication.py +++ b/api/src/authentication/authentication.py @@ -1,8 +1,8 @@ import httpx +import jwt from cachetools import TTLCache, cached from fastapi import Security from fastapi.security import OAuth2AuthorizationCodeBearer -from jose import JWTError, jwt from authentication.mock_token_generator import mock_rsa_public_key from authentication.models import User @@ -18,18 +18,12 @@ @cached(cache=TTLCache(maxsize=32, ttl=86400)) -def fetch_openid_configuration() -> dict[str, str]: +def fetch_openid_configuration() -> jwt.PyJWKClient: try: oid_conf_response = httpx.get(config.OAUTH_WELL_KNOWN) oid_conf_response.raise_for_status() oid_conf = oid_conf_response.json() - json_web_key_set_response = httpx.get(oid_conf["jwks_uri"]) - json_web_key_set_response.raise_for_status() - return { - "authorization_endpoint": oid_conf["authorization_endpoint"], - "token_endpoint": oid_conf["token_endpoint"], - "jwks": json_web_key_set_response.json()["keys"], - } + return jwt.PyJWKClient(oid_conf["jwks_uri"]) except Exception as error: logger.error(f"Failed to fetch OpenId Connect configuration for '{config.OAUTH_WELL_KNOWN}': {error}") raise credentials_exception @@ -41,7 +35,11 @@ def auth_with_jwt(jwt_token: str = Security(oauth2_scheme)) -> User: if not jwt_token: raise credentials_exception # If TEST_TOKEN is true, we are running tests. Use the self-signed keys. If not, get keys from auth server. - key = mock_rsa_public_key if config.TEST_TOKEN else {"keys": fetch_openid_configuration()["jwks"]} + key = ( + mock_rsa_public_key + if config.TEST_TOKEN + else fetch_openid_configuration().get_signing_key_from_jwt(jwt_token).key + ) try: payload = jwt.decode(jwt_token, key, algorithms=["RS256"], audience=config.OAUTH_AUDIENCE) @@ -50,7 +48,7 @@ def auth_with_jwt(jwt_token: str = Security(oauth2_scheme)) -> User: user = User(user_id=payload["oid"], **payload) else: user = User(user_id=payload["sub"], **payload) - except JWTError as error: + except jwt.exceptions.InvalidTokenError as error: logger.warning(f"Failed to decode JWT: {error}") raise credentials_exception diff --git a/api/src/authentication/mock_token_generator.py b/api/src/authentication/mock_token_generator.py index 48db897c..8600c67c 100644 --- a/api/src/authentication/mock_token_generator.py +++ b/api/src/authentication/mock_token_generator.py @@ -1,4 +1,4 @@ -from jose import jwt +import jwt from authentication.models import User from config import default_user @@ -63,5 +63,6 @@ def generate_mock_token(user: User = default_user) -> str: "sub": user.user_id, "roles": user.roles, "iss": "mock-auth-server", + "aud": "TEST", } return jwt.encode(payload, mock_rsa_private_key, algorithm="RS256") From 618258ca76fb00b8f2dabe2d68cd9d3519b403c2 Mon Sep 17 00:00:00 2001 From: Marius Lie Winger Date: Tue, 11 Jun 2024 11:13:20 +0200 Subject: [PATCH 2/2] test: mock authorization --- .pre-commit-config.yaml | 2 +- api/src/authentication/authentication.py | 15 ++----- api/src/config.py | 9 ++-- api/src/tests/conftest.py | 3 ++ .../features/whoami/test_whoami_feature.py | 4 +- .../integration/mock_authentication.py} | 45 +++++++++++++++---- 6 files changed, 49 insertions(+), 29 deletions(-) rename api/src/{authentication/mock_token_generator.py => tests/integration/mock_authentication.py} (66%) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f7fe4412..3a19d3a3 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -26,7 +26,7 @@ repos: - id: mixed-line-ending exclude: ^.*\.(lock)$ - id: detect-private-key - exclude: api/src/authentication/mock_token_generator.py + exclude: api/src/tests/integration/mock_authentication.py - id: no-commit-to-branch args: [--branch, main, --branch, master] stages: [commit-msg] diff --git a/api/src/authentication/authentication.py b/api/src/authentication/authentication.py index ccc04216..8037817d 100644 --- a/api/src/authentication/authentication.py +++ b/api/src/authentication/authentication.py @@ -4,11 +4,10 @@ from fastapi import Security from fastapi.security import OAuth2AuthorizationCodeBearer -from authentication.mock_token_generator import mock_rsa_public_key from authentication.models import User from common.exceptions import credentials_exception from common.logger import logger -from config import config, default_user +from config import config oauth2_scheme = OAuth2AuthorizationCodeBearer( authorizationUrl=config.OAUTH_AUTH_ENDPOINT, @@ -18,7 +17,7 @@ @cached(cache=TTLCache(maxsize=32, ttl=86400)) -def fetch_openid_configuration() -> jwt.PyJWKClient: +def get_JWK_client() -> jwt.PyJWKClient: try: oid_conf_response = httpx.get(config.OAUTH_WELL_KNOWN) oid_conf_response.raise_for_status() @@ -30,17 +29,9 @@ def fetch_openid_configuration() -> jwt.PyJWKClient: def auth_with_jwt(jwt_token: str = Security(oauth2_scheme)) -> User: - if not config.AUTH_ENABLED: - return default_user if not jwt_token: raise credentials_exception - # If TEST_TOKEN is true, we are running tests. Use the self-signed keys. If not, get keys from auth server. - key = ( - mock_rsa_public_key - if config.TEST_TOKEN - else fetch_openid_configuration().get_signing_key_from_jwt(jwt_token).key - ) - + key = get_JWK_client().get_signing_key_from_jwt(jwt_token).key try: payload = jwt.decode(jwt_token, key, algorithms=["RS256"], audience=config.OAUTH_AUDIENCE) if config.MICROSOFT_AUTH_PROVIDER in payload["iss"]: diff --git a/api/src/config.py b/api/src/config.py index 859dea7a..27382efa 100644 --- a/api/src/config.py +++ b/api/src/config.py @@ -43,14 +43,13 @@ class Config(BaseSettings): raise ValueError("Authentication was enabled, but some auth configuration parameters are missing") if not config.AUTH_ENABLED: + print("\n") print("################ WARNING ################") print("# Authentication is disabled #") print("################ WARNING ################\n") default_user: User = User( - **{ - "user_id": "nologin", - "full_name": "Not Authenticated", - "email": "nologin@example.com", - } + user_id="nologin", + full_name="Not Authenticated", + email="nologin@example.com", ) diff --git a/api/src/tests/conftest.py b/api/src/tests/conftest.py index ea592eb6..e6e90b40 100644 --- a/api/src/tests/conftest.py +++ b/api/src/tests/conftest.py @@ -9,9 +9,11 @@ from starlette.testclient import TestClient from app import create_app +from authentication.authentication import auth_with_jwt from config import config from data_providers.clients.mongodb.mongo_database_client import MongoDatabaseClient from features.todo.repository.todo_repository import TodoRepository, get_todo_repository +from tests.integration.mock_authentication import mock_auth_with_jwt @pytest.fixture(scope="function") @@ -39,6 +41,7 @@ def use_todo_repository_mock(): return TodoRepository(client=test_client) app.dependency_overrides[get_todo_repository] = use_todo_repository_mock + app.dependency_overrides[auth_with_jwt] = mock_auth_with_jwt yield client diff --git a/api/src/tests/integration/features/whoami/test_whoami_feature.py b/api/src/tests/integration/features/whoami/test_whoami_feature.py index 95147ed7..9a8b5123 100644 --- a/api/src/tests/integration/features/whoami/test_whoami_feature.py +++ b/api/src/tests/integration/features/whoami/test_whoami_feature.py @@ -2,9 +2,9 @@ from starlette.status import HTTP_200_OK from starlette.testclient import TestClient -from authentication.mock_token_generator import generate_mock_token from authentication.models import User from config import config +from tests.integration.mock_authentication import get_mock_jwt_token pytestmark = pytest.mark.integration @@ -14,7 +14,7 @@ def test_whoami(self, test_app: TestClient): config.AUTH_ENABLED = True config.TEST_TOKEN = True user = User(user_id="1", email="foo@bar.baz", roles=["a"]) - headers = {"Authorization": f"Bearer {generate_mock_token(user)}"} + headers = {"Authorization": f"Bearer {get_mock_jwt_token(user)}"} response = test_app.get("/whoami", headers=headers) data = response.json() assert response.status_code == HTTP_200_OK diff --git a/api/src/authentication/mock_token_generator.py b/api/src/tests/integration/mock_authentication.py similarity index 66% rename from api/src/authentication/mock_token_generator.py rename to api/src/tests/integration/mock_authentication.py index 8600c67c..be91f535 100644 --- a/api/src/authentication/mock_token_generator.py +++ b/api/src/tests/integration/mock_authentication.py @@ -1,10 +1,18 @@ import jwt +from fastapi import Security +from authentication.authentication import oauth2_scheme from authentication.models import User -from config import default_user +from common.exceptions import credentials_exception +from config import config, default_user -# Generated with: 'openssl req -nodes -new -x509 -keyout server.key -out server.cert' -mock_rsa_private_key = """ + +def get_mock_rsa_private_key() -> str: + """ + Used for testing. + Generated with: 'openssl req -nodes -new -x509 -keyout server.key -out server.cert'. + """ + return """ -----BEGIN PRIVATE KEY----- MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDfsOW9ih/oBUwl LEH4t2C2GZeq3/dEXCkK54CNPZv979rir0nQQ5pLVcoohoVFe+QwC746xg8t7/YP @@ -35,9 +43,13 @@ -----END PRIVATE KEY----- """ -# Python-jose require public keys instead of x509 certs. -# Convert cert to pub key with: 'openssl x509 -pubkey -noout < server.cert' -mock_rsa_public_key = """ + +def get_mock_rsa_public_key() -> str: + """ + Used for testing. + Convert cert to pub key with: 'openssl x509 -pubkey -noout < server.cert' + """ + return """ -----BEGIN PUBLIC KEY----- MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA37DlvYof6AVMJSxB+Ldg thmXqt/3RFwpCueAjT2b/e/a4q9J0EOaS1XKKIaFRXvkMAu+OsYPLe/2D3Fh8HB1 @@ -50,7 +62,7 @@ """ -def generate_mock_token(user: User = default_user) -> str: +def get_mock_jwt_token(user: User = default_user) -> str: """ This function is for testing purposes only Used for behave testing @@ -59,10 +71,25 @@ def generate_mock_token(user: User = default_user) -> str: payload = { "name": user.full_name, "preferred_username": user.email, - "scp": "FoR_test_scope", + "scp": "testing", "sub": user.user_id, "roles": user.roles, "iss": "mock-auth-server", "aud": "TEST", } - return jwt.encode(payload, mock_rsa_private_key, algorithm="RS256") + # This absolutely returns a str, so this is possibly a mypy bug + return jwt.encode(payload, get_mock_rsa_private_key(), algorithm="RS256") # type: ignore[no-any-return] + + +def mock_auth_with_jwt(jwt_token: str = Security(oauth2_scheme)) -> User: + if not config.AUTH_ENABLED: + return default_user + try: + payload = jwt.decode(jwt_token, get_mock_rsa_public_key(), algorithms=["RS256"], audience="TEST") + print(payload) + user = User(user_id=payload["sub"], **payload) + except jwt.exceptions.InvalidTokenError as error: + raise credentials_exception from error + if user is None: + raise credentials_exception + return user