From 53ad9162f256bfa4c795b720655e8ccbc2b084ad Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Mon, 10 Jun 2024 13:24:31 +0100 Subject: [PATCH 01/47] Add tokens --- src/tokens.py | 110 +++++++++++++++++++++++++++ test/test_tokens.py | 178 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 288 insertions(+) create mode 100644 src/tokens.py create mode 100644 test/test_tokens.py diff --git a/src/tokens.py b/src/tokens.py new file mode 100644 index 0000000..630d4a1 --- /dev/null +++ b/src/tokens.py @@ -0,0 +1,110 @@ +from __future__ import annotations + +import logging +import os +from abc import ABC +from datetime import datetime, timezone, timedelta +from typing import Any, Dict, Optional + +import jwt + +from src.exceptions import BadJWTError +from src.model import User + +PRIVATE_KEY = os.environ.get("PRIVATE_KEY", "shh") +logger = logging.getLogger(__name__) + + +class Token(ABC): + + def verify(self): + try: + self._payload = jwt.decode( + self.jwt, + PRIVATE_KEY, + algorithms=["HS256"], + options={"verify_signature": True, "require": ["exp"], "verify_exp": True}, + ) + return + except jwt.InvalidSignatureError: + logger.warning("token has bad signature - %s", self.jwt) + except jwt.ExpiredSignatureError: + logger.warning("token signature is expired - %s", self.jwt) + except jwt.InvalidTokenError: + logger.warning("Issue decoding token - %s", self.jwt) + except Exception as e: + logger.exception("Oh Dear", e) + raise BadJWTError("oh dear") + + def _encode(self) -> None: + bytes_key = bytes(PRIVATE_KEY, encoding="utf8") + + self.jwt = jwt.encode(self._payload, bytes_key, algorithm="HS256") + + +class AccessToken(Token): + def __init__(self, jwt_token: Optional[str] = None, payload: Optional[Dict[str, Any]] = None) -> None: + if payload and not jwt_token: + self._payload = payload + self._payload["exp"] = datetime.now(timezone.utc) + timedelta(minutes=1) + self._encode() + elif jwt_token and not payload: + try: + self._payload = jwt.decode( + jwt_token, + PRIVATE_KEY, + algorithms=["HS256"], + options={"verify_signature": True, "require": ["exp"], "verify_exp": True}, + ) + self.jwt = jwt_token + except jwt.DecodeError as e: + raise BadJWTError("Token could not be decoded") from e + else: + raise BadJWTError("Access token creation requires jwt_token string XOR a payload") + + def refresh(self): + self.verify() + self._payload["exp"] = datetime.now(timezone.utc) + timedelta(minutes=1) + self._encode() + + +class RefreshToken(Token): + def __init__(self, jwt_token: Optional[str] = None) -> None: + + if jwt_token is None: + self._payload = {"exp": datetime.now(timezone.utc) + timedelta(hours=12)} + self._encode() + else: + self.jwt = jwt_token + try: + self._payload = jwt.decode( + self.jwt, + PRIVATE_KEY, + algorithms=["HS256"], + options={"verify_signature": True, "require": ["exp"], "verify_exp": True}, + ) + except jwt.DecodeError as e: + raise BadJWTError("Badly formed JWT given") from e + except jwt.ExpiredSignatureError as e: + raise BadJWTError("Token signature has expired") from e + except Exception: + raise BadJWTError("Problem decoding JWT") + + +def generate_access_token(user: User) -> AccessToken: + payload = {"usernumber": user.user_number, "role": user.role.value, "username": "foo"} + return AccessToken(payload=payload) + + +def load_access_token(token: str) -> AccessToken: + return AccessToken(jwt_token=token) + + +def load_refresh_token(token: str) -> RefreshToken: + if token is None: + raise BadJWTError("Token is None") + return RefreshToken(jwt_token=token) + + +def generate_refresh_token() -> RefreshToken: + return RefreshToken() diff --git a/test/test_tokens.py b/test/test_tokens.py new file mode 100644 index 0000000..1ddc9f7 --- /dev/null +++ b/test/test_tokens.py @@ -0,0 +1,178 @@ +from datetime import datetime, timezone, timedelta +from unittest.mock import patch + +import jwt +import pytest + +from src.exceptions import BadJWTError +from src.model import User +from src.tokens import Token, AccessToken, RefreshToken, generate_access_token + + +@patch("jwt.decode") +@patch("src.tokens.logger") +def test_verify_success(mock_logger, mock_decode): + token_instance = Token() + token_instance.jwt = "valid_jwt_token" + mock_decode.return_value = {"some": "payload"} + + token_instance.verify() + + mock_decode.assert_called_once_with( + "valid_jwt_token", + "shh", + algorithms=["HS256"], + options={"verify_signature": True, "require": ["exp"], "verify_exp": True}, + ) + mock_logger.warning.assert_not_called() + + +@patch("jwt.decode") +@patch("src.tokens.logger") +def test_verify_invalid_signature(mock_logger, mock_decode): + token_instance = Token() + token_instance.jwt = "bad_signature_jwt" + mock_decode.side_effect = jwt.InvalidSignatureError() + + with pytest.raises(BadJWTError): + token_instance.verify() + mock_logger.warning.assert_called_once_with("token has bad signature - %s", "bad_signature_jwt") + + +@patch("jwt.decode") +@patch("src.tokens.logger") +def test_verify_expired_signature(mock_logger, mock_decode): + token_instance = Token() + token_instance.jwt = "expired_jwt_token" + mock_decode.side_effect = jwt.ExpiredSignatureError() + + with pytest.raises(BadJWTError): + token_instance.verify() + mock_logger.warning.assert_called_once_with("token signature is expired - %s", "expired_jwt_token") + + +@patch("jwt.decode") +@patch("src.tokens.logger") +def test_verify_invalid_token(mock_logger, mock_decode): + token_instance = Token() + token_instance.jwt = "invalid_jwt_token" + mock_decode.side_effect = jwt.InvalidTokenError() + + with pytest.raises(BadJWTError): + token_instance.verify() + mock_logger.warning.assert_called_once_with("Issue decoding token - %s", "invalid_jwt_token") + + +@patch("jwt.decode") +@patch("src.tokens.logger") +def test_verify_general_exception(mock_logger, mock_decode): + token_instance = Token() + token_instance.jwt = "jwt_with_general_issue" + exception = Exception("Unexpected error") + mock_decode.side_effect = exception + + with pytest.raises(BadJWTError): + token_instance.verify() + mock_logger.exception.assert_called_once_with("Oh Dear", exception) + + +@patch("src.tokens.datetime") +@patch("jwt.encode") +@patch("jwt.decode") +def test_access_token_with_payload(_, mock_encode, mock_datetime): + fixed_time = datetime(2021, 1, 1, 12, 0, 0, tzinfo=timezone.utc) + mock_datetime.now.return_value = fixed_time + payload = {"user": "test_user"} + + AccessToken(payload=payload) + + mock_encode.assert_called_once_with( + {"user": "test_user", "exp": fixed_time + timedelta(minutes=1)}, + b"shh", + algorithm="HS256", + ) + + +@patch("jwt.decode") +def test_access_token_with_jwt_token(mock_decode): + jwt_token = "encoded.jwt.token" + expected_payload = {"exp": datetime.now(timezone.utc) + timedelta(minutes=1)} + mock_decode.return_value = expected_payload + + token = AccessToken(jwt_token=jwt_token) + + assert token.jwt == jwt_token + assert token._payload == expected_payload + mock_decode.assert_called_once_with( + jwt_token, + "shh", + algorithms=["HS256"], + options={"verify_signature": True, "require": ["exp"], "verify_exp": True}, + ) + + +def test_access_token_with_both_none(): + with pytest.raises(BadJWTError): + AccessToken() + + +@patch("jwt.encode") +@patch("jwt.decode") +def test_access_token_refresh(mock_decode, mock_encode): + jwt_token = "valid.jwt.token" + mock_decode.return_value = {"user": "test_user", "exp": datetime.now(timezone.utc)} + token = AccessToken(jwt_token=jwt_token) + + token.refresh() + + args, kwargs = mock_encode.call_args + assert args[0]["exp"] > datetime.now(timezone.utc) # checks if the expiration time is extended + + +@patch("src.tokens.datetime") +@patch("jwt.encode") +@patch("jwt.decode") +def test_refresh_token_creation_no_jwt(_, mock_encode, mock_datetime): + fixed_time = datetime(2021, 1, 1, 12, 0, 0, tzinfo=timezone.utc) + mock_datetime.now.return_value = fixed_time + + RefreshToken() + + mock_encode.assert_called_once_with( + {"exp": fixed_time + timedelta(hours=12)}, + b"shh", + algorithm="HS256", + ) + + +@patch("jwt.decode") +def test_refresh_token_creation_with_jwt(mock_decode): + jwt_token = "encoded.jwt.token" + expected_payload = {"exp": datetime.now(timezone.utc) + timedelta(hours=12)} + mock_decode.return_value = expected_payload + + token = RefreshToken(jwt_token=jwt_token) + + assert token._payload == expected_payload + mock_decode.assert_called_once_with( + jwt_token, + "shh", + algorithms=["HS256"], + options={"verify_signature": True, "require": ["exp"], "verify_exp": True}, + ) + + +@patch("jwt.decode", side_effect=BadJWTError) +def test_refresh_token_with_invalid_jwt(_): + with pytest.raises(BadJWTError): + RefreshToken(jwt_token="invalid.jwt.token") + + +def test_generate_access_token(): + user = User(user_number=12345) + + access_token = generate_access_token(user) + + expected_payload = {"usernumber": 12345, "role": "user", "username": "foo"} + + assert access_token._payload == expected_payload From 8eefe30f6317f65dca666ec9959b55b92bb1dea8 Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Mon, 10 Jun 2024 13:24:43 +0100 Subject: [PATCH 02/47] Add codecov badge --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index bd5f18b..26f0272 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,5 @@ # fia-auth - +[![codecov](https://codecov.io/gh/fiaisis/fia-auth/graph/badge.svg?token=lzm0DS1jhG)](https://codecov.io/gh/fiaisis/fia-auth) ## Testing Locally You must be connected to the vpn to make use of the uows and allocations api. From 3c5e27124393e01be5325cde25862af3fdaa776d Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Mon, 10 Jun 2024 13:25:08 +0100 Subject: [PATCH 03/47] Add routers --- src/routers.py | 59 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/src/routers.py b/src/routers.py index 9d13000..256ae7b 100644 --- a/src/routers.py +++ b/src/routers.py @@ -3,12 +3,17 @@ """ import os -from typing import List, Annotated +from typing import List, Annotated, Dict, Any -from fastapi import APIRouter, Depends, HTTPException +from fastapi import APIRouter, Depends, HTTPException, Cookie from fastapi.security import HTTPBearer, HTTPAuthorizationCredentials +from starlette.responses import JSONResponse +from src.auth import authenticate +from src.exceptions import BadJWTError, UOWSError, BadCredentialsError from src.experiments import get_experiments_for_user_number +from src.model import UserCredentials +from src.tokens import generate_refresh_token, generate_access_token, load_access_token, load_refresh_token ROUTER = APIRouter() @@ -32,3 +37,53 @@ async def get_experiments( if credentials.credentials != API_KEY: raise HTTPException(status_code=403, detail="Forbidden") return await get_experiments_for_user_number(user_number) + + +@ROUTER.post("/api/jwt/authenticate", tags=["auth"]) +async def login(credentials: UserCredentials): + try: + user_number = authenticate(credentials) + refresh_token = generate_refresh_token().jwt + access_token = generate_access_token(user_number).jwt + response = JSONResponse(content={"token": access_token}, status_code=200) + response.set_cookie( + "refresh-token", value=refresh_token, max_age=60 * 60 * 12, secure=True, httponly=True, samesite="lax" + ) # 12 hours + return response + except UOWSError: + raise HTTPException(status_code=403, detail="Forbidden") + except BadCredentialsError: + raise HTTPException(status_code=403, detail="Invalid") + + +@ROUTER.post("/api/jwt/checkToken") +def verify(token: Dict[str, Any]): + """ + Verify an access token was generated by this auth server and has not expired + \f + :param token: The JWT + :return: "OK" + """ + try: + load_access_token(token["token"]).verify() + return "ok" + except BadJWTError: + raise HTTPException(status_code=403, detail="") + + +@ROUTER.post("/api/jwt/refresh") +def refresh(token: Dict[str, Any], refresh_token: Annotated[str | None, Cookie()] = None): + """ + Refresh an access token based on a refresh token + \f + :param token: The access token to be refreshed + :return: The new access token + """ + try: + access_token = load_access_token(token["token"]) + refresh_token = load_refresh_token(refresh_token) + refresh_token.verify() + access_token.refresh() + return {"token": access_token.jwt} + except BadJWTError: + raise HTTPException(status_code=403) From fe7c5adb382175d638ebdb5de2168db1ea1c83e0 Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Mon, 10 Jun 2024 13:27:54 +0100 Subject: [PATCH 04/47] Add uows connectivity --- src/auth.py | 27 +++++++++++++ test/e2e/test_auth.py | 91 +++++++++++++++++++++++++++++++++++++++++++ test/test_auth.py | 51 ++++++++++++++++++++++++ 3 files changed, 169 insertions(+) create mode 100644 src/auth.py create mode 100644 test/e2e/test_auth.py create mode 100644 test/test_auth.py diff --git a/src/auth.py b/src/auth.py new file mode 100644 index 0000000..9208c1d --- /dev/null +++ b/src/auth.py @@ -0,0 +1,27 @@ +import requests + +from src.exceptions import BadCredentialsError, UOWSError +from src.model import User +from src.model import UserCredentials + + +def authenticate(credentials: UserCredentials) -> User: + """ + Authenticate the user based on the given credentials + :param credentials: The user credentials + :return: UserNumber + """ + data = ({"username": credentials.username, "password": credentials.password},) + response = requests.post( + "https://devapi.facilities.rl.ac.uk/users-service/v0/sessions", + json=data, + headers={"Content-Type": "application/json"}, + ) + if response.status_code == 201: + return User(user_number=response.json()["userId"]) + if response.status_code == 401: + raise BadCredentialsError("Invalid user credentials provided to authenticate with the user office web service.") + else: + raise UOWSError( + "An unexpected error occurred when authenticating with the user office web service %s", response.json() + ) diff --git a/test/e2e/test_auth.py b/test/e2e/test_auth.py new file mode 100644 index 0000000..5e44cb4 --- /dev/null +++ b/test/e2e/test_auth.py @@ -0,0 +1,91 @@ +from unittest.mock import patch, Mock + +from starlette.testclient import TestClient + +from src.app import app +from src.model import User +from src.tokens import generate_access_token, generate_refresh_token + +client = TestClient(app) + + +@patch("src.auth.requests.post") +def test_successful_login(mock_post): + mock_response = Mock() + mock_post.return_value = mock_response + + mock_response.status_code = 201 + mock_response.json.return_value = {"userId": 1234} + response = client.post("/api/jwt/authenticate", json={"username": "foo", "password": "foo"}) + assert response.json()["token"].startswith("ey") + assert response.cookies["refresh-token"].startswith("ey") + + +@patch("src.auth.requests.post") +def test_unsuccessful_login(mock_post): + mock_response = Mock() + mock_post.return_value = mock_response + + mock_response.status_code = 401 + + response = client.post("/api/jwt/authenticate", json={"username": "foo", "password": "foo"}) + assert response.status_code == 403 + + +@patch("src.auth.requests.post") +def test_unsuccessful_login_uows_failure(mock_post): + mock_response = Mock() + mock_post.return_value = mock_response + + mock_response.status_code = 500 + + response = client.post("/api/jwt/authenticate", json={"username": "foo", "password": "foo"}) + assert response.status_code == 403 + + +def test_verify_success(): + user = User(123) + access_token = generate_access_token(user) + response = client.post("/api/jwt/checkToken", json={"token": access_token.jwt}) + + assert response.status_code == 200 + + +def test_verify_fail(): + response = client.post("/api/jwt/checkToken", json={"token": "foo"}) + assert response.status_code == 403 + + +def test_token_refresh_success(): + user = User(123) + access_token = generate_access_token(user) + refresh_token = generate_refresh_token() + response = client.post( + "/api/jwt/refresh", json={"token": access_token.jwt}, cookies={"refresh_token": refresh_token.jwt} + ) + + assert response.json()["token"].startswith("ey") + + +def test_token_refresh_no_refresh_token_given(): + user = User(123) + access_token = generate_access_token(user) + response = client.post( + "/api/jwt/refresh", + json={"token": access_token.jwt}, + ) + + assert response.status_code == 403 + + +def test_token_refresh_expired_refresh_token(): + user = User(123) + access_token = generate_access_token(user) + refresh_token = ( + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE3MTgwMjEyNDB9.iHITGf2RyX_49pY7Xy8xdutYE4Pc6k9mfKWQjxCKgOk" + ) + response = client.post( + "/api/jwt/refresh", json={"token": access_token.jwt}, cookies={"refresh_token": refresh_token} + ) + + assert response.status_code == 403 diff --git a/test/test_auth.py b/test/test_auth.py new file mode 100644 index 0000000..a3ce1fd --- /dev/null +++ b/test/test_auth.py @@ -0,0 +1,51 @@ +from unittest.mock import patch, Mock + +import pytest + +from src.auth import authenticate +from src.exceptions import BadCredentialsError, UOWSError +from src.model import UserCredentials + + +@patch("requests.post") +def test_authenticate_success(mock_post): + mock_response = Mock(status_code=201, json=lambda: {"userId": "12345"}) + mock_post.return_value = mock_response + + credentials = UserCredentials(username="valid_user", password="valid_password") + + user = authenticate(credentials) + + assert user.user_number == "12345" + + mock_post.assert_called_once_with( + "https://devapi.facilities.rl.ac.uk/users-service/v0/sessions", + json=({"username": "valid_user", "password": "valid_password"},), + headers={"Content-Type": "application/json"}, + ) + + +@patch("requests.post") +def test_authenticate_bad_credentials(mock_post): + mock_response = Mock(status_code=401, json=lambda: {}) + mock_post.return_value = mock_response + + credentials = UserCredentials(username="invalid_user", password="invalid_password") + + with pytest.raises(BadCredentialsError) as exc_info: + authenticate(credentials) + + assert str(exc_info.value) == "Invalid user credentials provided to authenticate with the user office web service." + + +@patch("requests.post") +def test_authenticate_unexpected_error(mock_post): + mock_response = Mock(status_code=500, json=lambda: {"error": "Server error"}) + mock_post.return_value = mock_response + + credentials = UserCredentials(username="user", password="password") + + with pytest.raises(UOWSError) as exc_info: + authenticate(credentials) + + assert "An unexpected error occurred when authenticating with the user office web service" in str(exc_info.value) From b984b536ad2e111c7931315eb33e09d129f93d48 Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Mon, 10 Jun 2024 13:29:32 +0100 Subject: [PATCH 05/47] Add unit test action step --- .github/workflows/tests.yml | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index c739ac7..aa27c01 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -6,6 +6,42 @@ permissions: jobs: + unit: + runs-on: ubuntu-latest + + steps: + - name: Check out repository + uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 + + - name: Set up Python + uses: actions/setup-python@82c7e631bb3cdc910f68e0081d67478d79c6982d # v5.1.0 + with: + python-version: '3.12' + + - name: Set up cache for Python dependencies + uses: actions/cache@v4 + with: + path: ~/.cache/pip + key: ${{ runner.os }}-pip-${{ hashFiles('**/pyproject.toml') }} + restore-keys: | + ${{ runner.os }}-pip- + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install .[test] + + - name: Run tests + env: + DATABASE_URL: postgresql://postgres:password@localhost:5432/test_db + run: | + pytest test --ignore=test/e2e --random-order --random-order-bucket=global --cov --cov-report=xml + + - name: Upload coverage + uses: codecov/codecov-action@125fc84a9a348dbcf27191600683ec096ec9021c # v4.4.1 + with: + token: ${{ secrets.CODECOV_TOKEN }} + e2e: runs-on: ubuntu-latest From 58d636a461f19bf6167c538be6c06414252bde3e Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Mon, 10 Jun 2024 13:42:17 +0100 Subject: [PATCH 06/47] Add type hints --- src/routers.py | 14 +++++++------- src/tokens.py | 7 ++++--- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/routers.py b/src/routers.py index 256ae7b..75ac779 100644 --- a/src/routers.py +++ b/src/routers.py @@ -3,7 +3,7 @@ """ import os -from typing import List, Annotated, Dict, Any +from typing import List, Annotated, Dict, Any, Literal from fastapi import APIRouter, Depends, HTTPException, Cookie from fastapi.security import HTTPBearer, HTTPAuthorizationCredentials @@ -40,7 +40,7 @@ async def get_experiments( @ROUTER.post("/api/jwt/authenticate", tags=["auth"]) -async def login(credentials: UserCredentials): +async def login(credentials: UserCredentials) -> JSONResponse: try: user_number = authenticate(credentials) refresh_token = generate_refresh_token().jwt @@ -57,7 +57,7 @@ async def login(credentials: UserCredentials): @ROUTER.post("/api/jwt/checkToken") -def verify(token: Dict[str, Any]): +def verify(token: Dict[str, Any]) -> Literal["ok"]: """ Verify an access token was generated by this auth server and has not expired \f @@ -72,7 +72,7 @@ def verify(token: Dict[str, Any]): @ROUTER.post("/api/jwt/refresh") -def refresh(token: Dict[str, Any], refresh_token: Annotated[str | None, Cookie()] = None): +def refresh(token: Dict[str, Any], refresh_token: Annotated[str | None, Cookie()] = None) -> JSONResponse: """ Refresh an access token based on a refresh token \f @@ -81,9 +81,9 @@ def refresh(token: Dict[str, Any], refresh_token: Annotated[str | None, Cookie() """ try: access_token = load_access_token(token["token"]) - refresh_token = load_refresh_token(refresh_token) - refresh_token.verify() + loaded_refresh_token = load_refresh_token(refresh_token) + loaded_refresh_token.verify() access_token.refresh() - return {"token": access_token.jwt} + return JSONResponse({"token": access_token.jwt}) except BadJWTError: raise HTTPException(status_code=403) diff --git a/src/tokens.py b/src/tokens.py index 630d4a1..de1dd69 100644 --- a/src/tokens.py +++ b/src/tokens.py @@ -16,8 +16,9 @@ class Token(ABC): + jwt: str - def verify(self): + def verify(self) -> None: try: self._payload = jwt.decode( self.jwt, @@ -62,7 +63,7 @@ def __init__(self, jwt_token: Optional[str] = None, payload: Optional[Dict[str, else: raise BadJWTError("Access token creation requires jwt_token string XOR a payload") - def refresh(self): + def refresh(self) -> None: self.verify() self._payload["exp"] = datetime.now(timezone.utc) + timedelta(minutes=1) self._encode() @@ -100,7 +101,7 @@ def load_access_token(token: str) -> AccessToken: return AccessToken(jwt_token=token) -def load_refresh_token(token: str) -> RefreshToken: +def load_refresh_token(token: Optional[str]) -> RefreshToken: if token is None: raise BadJWTError("Token is None") return RefreshToken(jwt_token=token) From c553f2d81d3a5deb2d365527451618945962051f Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Mon, 10 Jun 2024 13:42:32 +0100 Subject: [PATCH 07/47] Add dependency --- pyproject.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index f5314d1..f3ab9d6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,7 +18,8 @@ dependencies = [ [project.optional-dependencies] code-inspection = [ "pylint==3.2.2", - "mypy==1.10.0" + "mypy==1.10.0", + "types-requests==2.32.0.20240602" ] formatting = [ From 9cc804e8a30607f571df16ba2c7225b6393ccef0 Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Mon, 10 Jun 2024 14:13:13 +0100 Subject: [PATCH 08/47] Add docstrings --- src/auth.py | 11 ++++++---- src/routers.py | 24 +++++++++++++------- src/tokens.py | 59 +++++++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 77 insertions(+), 17 deletions(-) diff --git a/src/auth.py b/src/auth.py index 9208c1d..8de0bae 100644 --- a/src/auth.py +++ b/src/auth.py @@ -1,3 +1,7 @@ +""" +Module containing code to authenticate with the UOWS +""" + import requests from src.exceptions import BadCredentialsError, UOWSError @@ -16,12 +20,11 @@ def authenticate(credentials: UserCredentials) -> User: "https://devapi.facilities.rl.ac.uk/users-service/v0/sessions", json=data, headers={"Content-Type": "application/json"}, + timeout=30, ) if response.status_code == 201: return User(user_number=response.json()["userId"]) if response.status_code == 401: raise BadCredentialsError("Invalid user credentials provided to authenticate with the user office web service.") - else: - raise UOWSError( - "An unexpected error occurred when authenticating with the user office web service %s", response.json() - ) + + raise UOWSError("An unexpected error occurred when authenticating with the user office web service") diff --git a/src/routers.py b/src/routers.py index 75ac779..8dfb811 100644 --- a/src/routers.py +++ b/src/routers.py @@ -2,6 +2,8 @@ Module containing the fastapi routers """ +from __future__ import annotations + import os from typing import List, Annotated, Dict, Any, Literal @@ -41,6 +43,12 @@ async def get_experiments( @ROUTER.post("/api/jwt/authenticate", tags=["auth"]) async def login(credentials: UserCredentials) -> JSONResponse: + """ + Login with facilities account + \f + :param credentials: username and password + :return: JSON Response for access token and cookie set for refresh token + """ try: user_number = authenticate(credentials) refresh_token = generate_refresh_token().jwt @@ -50,10 +58,10 @@ async def login(credentials: UserCredentials) -> JSONResponse: "refresh-token", value=refresh_token, max_age=60 * 60 * 12, secure=True, httponly=True, samesite="lax" ) # 12 hours return response - except UOWSError: - raise HTTPException(status_code=403, detail="Forbidden") - except BadCredentialsError: - raise HTTPException(status_code=403, detail="Invalid") + except UOWSError as exc: + raise HTTPException(status_code=403, detail="Forbidden") from exc + except BadCredentialsError as exc: + raise HTTPException(status_code=403, detail="Invalid") from exc @ROUTER.post("/api/jwt/checkToken") @@ -67,8 +75,8 @@ def verify(token: Dict[str, Any]) -> Literal["ok"]: try: load_access_token(token["token"]).verify() return "ok" - except BadJWTError: - raise HTTPException(status_code=403, detail="") + except BadJWTError as exc: + raise HTTPException(status_code=403, detail="") from exc @ROUTER.post("/api/jwt/refresh") @@ -85,5 +93,5 @@ def refresh(token: Dict[str, Any], refresh_token: Annotated[str | None, Cookie() loaded_refresh_token.verify() access_token.refresh() return JSONResponse({"token": access_token.jwt}) - except BadJWTError: - raise HTTPException(status_code=403) + except BadJWTError as exc: + raise HTTPException(status_code=403) from exc diff --git a/src/tokens.py b/src/tokens.py index de1dd69..3a4fd58 100644 --- a/src/tokens.py +++ b/src/tokens.py @@ -1,3 +1,7 @@ +""" +Module containing token classes, creation, and loading functions +""" + from __future__ import annotations import logging @@ -16,16 +20,28 @@ class Token(ABC): + """ + Abstract token class defines verify method + """ + jwt: str def verify(self) -> None: + """ + Verifies the token, ensuring that it has a valid format, signature, and has not expired. Will raise a + BadJWTError if verification fails + :return: None + """ try: + # pylint: disable=attribute-defined-outside-init + # class is abstract and all subclasses define payload within the init self._payload = jwt.decode( self.jwt, PRIVATE_KEY, algorithms=["HS256"], options={"verify_signature": True, "require": ["exp"], "verify_exp": True}, ) + # pylint: enable=attribute-defined-outside-init return except jwt.InvalidSignatureError: logger.warning("token has bad signature - %s", self.jwt) @@ -33,8 +49,10 @@ def verify(self) -> None: logger.warning("token signature is expired - %s", self.jwt) except jwt.InvalidTokenError: logger.warning("Issue decoding token - %s", self.jwt) - except Exception as e: - logger.exception("Oh Dear", e) + # pylint: disable=broad-exception-caught + except Exception: + logger.exception("Oh Dear") + # pylint: enable=broad-exception-caught raise BadJWTError("oh dear") def _encode(self) -> None: @@ -44,6 +62,10 @@ def _encode(self) -> None: class AccessToken(Token): + """ + Access Token is a short-lived (5 minute) token that stores user information + """ + def __init__(self, jwt_token: Optional[str] = None, payload: Optional[Dict[str, Any]] = None) -> None: if payload and not jwt_token: self._payload = payload @@ -64,12 +86,20 @@ def __init__(self, jwt_token: Optional[str] = None, payload: Optional[Dict[str, raise BadJWTError("Access token creation requires jwt_token string XOR a payload") def refresh(self) -> None: + """ + Refresh the access token by extending the expiry time by 5 minutes and resigning + :return: None + """ self.verify() - self._payload["exp"] = datetime.now(timezone.utc) + timedelta(minutes=1) + self._payload["exp"] = datetime.now(timezone.utc) + timedelta(minutes=5) self._encode() class RefreshToken(Token): + """ + Refresh token is a long-lived (12 hour) token that is required to refresh an access token + """ + def __init__(self, jwt_token: Optional[str] = None) -> None: if jwt_token is None: @@ -88,24 +118,43 @@ def __init__(self, jwt_token: Optional[str] = None) -> None: raise BadJWTError("Badly formed JWT given") from e except jwt.ExpiredSignatureError as e: raise BadJWTError("Token signature has expired") from e - except Exception: - raise BadJWTError("Problem decoding JWT") + except Exception as e: + raise BadJWTError("Problem decoding JWT") from e def generate_access_token(user: User) -> AccessToken: + """ + Given a user, generate an AccessToken for them + :param user: The user + :return: The generated Access Token + """ payload = {"usernumber": user.user_number, "role": user.role.value, "username": "foo"} return AccessToken(payload=payload) def load_access_token(token: str) -> AccessToken: + """ + Given a jwt string, return an access token object for it + :param token: the jwt string + :return: The access token object + """ return AccessToken(jwt_token=token) def load_refresh_token(token: Optional[str]) -> RefreshToken: + """ + Given a jwt string, return a refresh token object for it + :param token: the jwt string + :return: The refresh token object + """ if token is None: raise BadJWTError("Token is None") return RefreshToken(jwt_token=token) def generate_refresh_token() -> RefreshToken: + """ + Generate a new Refresh Token + :return: The refresh token object + """ return RefreshToken() From 1ce3ce0ce1dc67567ce8f1c9f9f21e8289022d97 Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Tue, 11 Jun 2024 09:10:44 +0100 Subject: [PATCH 09/47] Add test --- test/e2e/test_auth.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/e2e/test_auth.py b/test/e2e/test_auth.py index 5e44cb4..241bea5 100644 --- a/test/e2e/test_auth.py +++ b/test/e2e/test_auth.py @@ -1,5 +1,6 @@ from unittest.mock import patch, Mock +import jwt from starlette.testclient import TestClient from src.app import app @@ -51,11 +52,16 @@ def test_verify_success(): assert response.status_code == 200 -def test_verify_fail(): +def test_verify_fail_badly_formed_token(): response = client.post("/api/jwt/checkToken", json={"token": "foo"}) assert response.status_code == 403 +def test_verify_fail_bad_signature(): + response = client.post("/api/jwt/checkToken", json={"token": jwt.encode({"foo": "bar"}, key="foo")}) + assert response.status_code == 403 + + def test_token_refresh_success(): user = User(123) access_token = generate_access_token(user) From 11322f0aab1dc46eac0d4cb061947328cfe6b36e Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Wed, 12 Jun 2024 08:18:54 +0100 Subject: [PATCH 10/47] Add super exception --- src/exceptions.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/exceptions.py b/src/exceptions.py index 6be347b..15bc44c 100644 --- a/src/exceptions.py +++ b/src/exceptions.py @@ -11,9 +11,13 @@ class ProposalAllocationsError(Exception): """Problem connecting with the proposal allocations api""" -class BadCredentialsError(Exception): +class AuthenticationError(Exception): + """Problem with authentication mechanism""" + + +class BadCredentialsError(AuthenticationError): """ "Bad Credentials Provided""" -class BadJWTError(Exception): +class BadJWTError(AuthenticationError): """Raised when a bad jwt has been given to the service""" From b8b1e50221808a0671868b8fd9dd781ad0935c5c Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Wed, 12 Jun 2024 08:19:10 +0100 Subject: [PATCH 11/47] Add Exception Handler --- src/exception_handlers.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 src/exception_handlers.py diff --git a/src/exception_handlers.py b/src/exception_handlers.py new file mode 100644 index 0000000..1f415e7 --- /dev/null +++ b/src/exception_handlers.py @@ -0,0 +1,15 @@ +from starlette.requests import Request +from starlette.responses import JSONResponse + + +async def auth_error_handler(_: Request, __: Exception) -> JSONResponse: + """ + Automatically return a 403 when an authentication error is raised + :param _: + :param __: + :return: JSONResponse with 404 + """ + return JSONResponse( + status_code=403, + content={"message": "Forbidden"}, + ) From 80973049f928d9f4f33a79f4c45b7689a5dffb0d Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Wed, 12 Jun 2024 08:19:32 +0100 Subject: [PATCH 12/47] Include exception handler --- src/app.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/app.py b/src/app.py index 3e6b4a8..863a2ca 100644 --- a/src/app.py +++ b/src/app.py @@ -5,6 +5,8 @@ from fastapi import FastAPI from starlette.middleware.cors import CORSMiddleware +from src.exception_handlers import auth_error_handler +from src.exceptions import AuthenticationError from src.routers import ROUTER app = FastAPI() @@ -19,3 +21,5 @@ ) app.include_router(ROUTER) + +app.add_exception_handler(AuthenticationError, auth_error_handler) From 1e76629ad01f56cf70e298bf1ace09c2c5a2e529 Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Wed, 12 Jun 2024 08:19:56 +0100 Subject: [PATCH 13/47] Remove explicit try excepts --- src/routers.py | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/routers.py b/src/routers.py index 8dfb811..b490742 100644 --- a/src/routers.py +++ b/src/routers.py @@ -12,7 +12,7 @@ from starlette.responses import JSONResponse from src.auth import authenticate -from src.exceptions import BadJWTError, UOWSError, BadCredentialsError +from src.exceptions import UOWSError from src.experiments import get_experiments_for_user_number from src.model import UserCredentials from src.tokens import generate_refresh_token, generate_access_token, load_access_token, load_refresh_token @@ -60,8 +60,6 @@ async def login(credentials: UserCredentials) -> JSONResponse: return response except UOWSError as exc: raise HTTPException(status_code=403, detail="Forbidden") from exc - except BadCredentialsError as exc: - raise HTTPException(status_code=403, detail="Invalid") from exc @ROUTER.post("/api/jwt/checkToken") @@ -72,11 +70,8 @@ def verify(token: Dict[str, Any]) -> Literal["ok"]: :param token: The JWT :return: "OK" """ - try: - load_access_token(token["token"]).verify() - return "ok" - except BadJWTError as exc: - raise HTTPException(status_code=403, detail="") from exc + load_access_token(token["token"]).verify() + return "ok" @ROUTER.post("/api/jwt/refresh") @@ -87,11 +82,8 @@ def refresh(token: Dict[str, Any], refresh_token: Annotated[str | None, Cookie() :param token: The access token to be refreshed :return: The new access token """ - try: - access_token = load_access_token(token["token"]) - loaded_refresh_token = load_refresh_token(refresh_token) - loaded_refresh_token.verify() - access_token.refresh() - return JSONResponse({"token": access_token.jwt}) - except BadJWTError as exc: - raise HTTPException(status_code=403) from exc + access_token = load_access_token(token["token"]) + loaded_refresh_token = load_refresh_token(refresh_token) + loaded_refresh_token.verify() + access_token.refresh() + return JSONResponse({"token": access_token.jwt}) From cff276bc7a1edadbf07c88c4a57046a60af8b7da Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Wed, 12 Jun 2024 08:31:17 +0100 Subject: [PATCH 14/47] Fix tests --- src/tokens.py | 2 +- test/test_tokens.py | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/tokens.py b/src/tokens.py index 3a4fd58..a8f1f8b 100644 --- a/src/tokens.py +++ b/src/tokens.py @@ -69,7 +69,7 @@ class AccessToken(Token): def __init__(self, jwt_token: Optional[str] = None, payload: Optional[Dict[str, Any]] = None) -> None: if payload and not jwt_token: self._payload = payload - self._payload["exp"] = datetime.now(timezone.utc) + timedelta(minutes=1) + self._payload["exp"] = datetime.now(timezone.utc) + timedelta(minutes=5) self._encode() elif jwt_token and not payload: try: diff --git a/test/test_tokens.py b/test/test_tokens.py index 1ddc9f7..4aeb37e 100644 --- a/test/test_tokens.py +++ b/test/test_tokens.py @@ -73,7 +73,7 @@ def test_verify_general_exception(mock_logger, mock_decode): with pytest.raises(BadJWTError): token_instance.verify() - mock_logger.exception.assert_called_once_with("Oh Dear", exception) + mock_logger.exception.assert_called_once_with("Oh Dear") @patch("src.tokens.datetime") @@ -87,7 +87,7 @@ def test_access_token_with_payload(_, mock_encode, mock_datetime): AccessToken(payload=payload) mock_encode.assert_called_once_with( - {"user": "test_user", "exp": fixed_time + timedelta(minutes=1)}, + {"user": "test_user", "exp": fixed_time + timedelta(minutes=5)}, b"shh", algorithm="HS256", ) @@ -168,11 +168,18 @@ def test_refresh_token_with_invalid_jwt(_): RefreshToken(jwt_token="invalid.jwt.token") -def test_generate_access_token(): +@patch("src.tokens.datetime") +def test_generate_access_token(mock_datetime): user = User(user_number=12345) - + fixed_time = datetime(2000, 12, 12, 12, 0, tzinfo=timezone.utc) + mock_datetime.now.return_value = fixed_time access_token = generate_access_token(user) - expected_payload = {"usernumber": 12345, "role": "user", "username": "foo"} + expected_payload = { + "usernumber": 12345, + "role": "user", + "username": "foo", + "exp": fixed_time + timedelta(minutes=5), + } assert access_token._payload == expected_payload From 4185cc5b49f904e174e2992c7e5df41b03f46658 Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Wed, 12 Jun 2024 09:10:49 +0100 Subject: [PATCH 15/47] Add ruff, remove pylint --- .github/workflows/formatting.yml | 44 --- .github/workflows/linting.yml | 39 +- .pylintrc | 624 ------------------------------- pyproject.toml | 52 ++- src/auth.py | 7 +- src/model.py | 2 +- src/routers.py | 18 +- src/tokens.py | 22 +- test/e2e/test_auth.py | 18 +- test/e2e/test_experiments.py | 18 +- test/test_auth.py | 8 +- test/test_tokens.py | 23 +- 12 files changed, 138 insertions(+), 737 deletions(-) delete mode 100644 .github/workflows/formatting.yml delete mode 100644 .pylintrc diff --git a/.github/workflows/formatting.yml b/.github/workflows/formatting.yml deleted file mode 100644 index 92458f3..0000000 --- a/.github/workflows/formatting.yml +++ /dev/null @@ -1,44 +0,0 @@ ---- -on: - push: - branches-ignore: - - main -permissions: - contents: write - -jobs: - black: - permissions: - contents: write # for Git to git push - runs-on: ubuntu-latest - steps: - - name: Checkout project - uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v3.5.0 - - - name: Set up Python - uses: actions/setup-python@82c7e631bb3cdc910f68e0081d67478d79c6982d # v4.5.0 - with: - python-version: '3.12' - - - name: Set up cache for Python dependencies. - uses: actions/cache@v4 - with: - path: ~/.cache/pip - key: ${{ runner.os }}-pip-${{ hashFiles('**/pyproject.toml') }} - restore-keys: | - ${{ runner.os }}-pip- - - - name: Install dependencies - run: | - python -m pip install --upgrade pip - python -m pip install .[formatting] - - name: Run black - run: | - black --line-length 120 . - - name: Commit changes - run: | - git config user.name github-actions - git config user.email github-actions@github.com - git add . - git commit -m "black auto commit" || true - git push \ No newline at end of file diff --git a/.github/workflows/linting.yml b/.github/workflows/linting.yml index a03defd..9f138c6 100644 --- a/.github/workflows/linting.yml +++ b/.github/workflows/linting.yml @@ -4,15 +4,27 @@ on: push permissions: contents: read + +--- +on: + push: + branches-ignore: + - main +permissions: + contents: read + + jobs: - pylint: + formatting_and_linting: + permissions: + contents: write # for Git to git push runs-on: ubuntu-latest steps: - name: Checkout project - uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v3.5.0 + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 - - name: Set up python - uses: actions/setup-python@82c7e631bb3cdc910f68e0081d67478d79c6982d # v4.5.0 + - name: Set up Python + uses: actions/setup-python@82c7e631bb3cdc910f68e0081d67478d79c6982d # v5.1.0 with: python-version: '3.12' @@ -27,10 +39,23 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - python -m pip install .[code-inspection] - - name: Run pylint - run: pylint src + python -m pip install .[formatting] + + - name: Run ruff formatting + run: | + ruff format . + - name: Run ruff linting + run: | + ruff check --fix + + - name: Commit changes + run: | + git config user.name github-actions + git config user.email github-actions@github.com + git add . + git commit -m "Formatting and linting commit" || true + git push mypy: runs-on: ubuntu-latest steps: diff --git a/.pylintrc b/.pylintrc deleted file mode 100644 index 5dface4..0000000 --- a/.pylintrc +++ /dev/null @@ -1,624 +0,0 @@ -[MAIN] - -# Analyse import fallback blocks. This can be used to support both Python 2 and -# 3 compatible code, which means that the block might have code that exists -# only in one or another interpreter, leading to false positives when analysed. -analyse-fallback-blocks=no - -# Load and enable all available extensions. Use --list-extensions to see a list -# all available extensions. -#enable-all-extensions= - -# In error mode, messages with a category besides ERROR or FATAL are -# suppressed, and no reports are done by default. Error mode is compatible with -# disabling specific errors. -#errors-only= - -# Always return a 0 (non-error) status code, even if lint errors are found. -# This is primarily useful in continuous integration scripts. -#exit-zero= - -# A comma-separated list of package or module names from where C extensions may -# be loaded. Extensions are loading into the active Python interpreter and may -# run arbitrary code. -extension-pkg-allow-list= - -# A comma-separated list of package or module names from where C extensions may -# be loaded. Extensions are loading into the active Python interpreter and may -# run arbitrary code. (This is an alternative name to extension-pkg-allow-list -# for backward compatibility.) -extension-pkg-whitelist=pydantic - -# Return non-zero exit code if any of these messages/categories are detected, -# even if score is above --fail-under value. Syntax same as enable. Messages -# specified are enabled, while categories only check already-enabled messages. -fail-on= - -# Specify a score threshold under which the program will exit with error. -fail-under=10 - -# Interpret the stdin as a python script, whose filename needs to be passed as -# the module_or_package argument. -#from-stdin= - -# Files or directories to be skipped. They should be base names, not paths. -ignore=CVS,venv - -# Add files or directories matching the regular expressions patterns to the -# ignore-list. The regex matches against paths and can be in Posix or Windows -# format. Because '\' represents the directory delimiter on Windows systems, it -# can't be used as an escape character. -ignore-paths= - -# Files or directories matching the regular expression patterns are skipped. -# The regex matches against base names, not paths. The default value ignores -# Emacs file locks -ignore-patterns=^\.# - -# List of module names for which member attributes should not be checked -# (useful for modules/projects where namespaces are manipulated during runtime -# and thus existing member attributes cannot be deduced by static analysis). It -# supports qualified module names, as well as Unix pattern matching. -ignored-modules= - -# Python code to execute, usually for sys.path manipulation such as -# pygtk.require(). -#init-hook= - -# Use multiple processes to speed up Pylint. Specifying 0 will auto-detect the -# number of processors available to use, and will cap the count on Windows to -# avoid hangs. -jobs=0 - -# Control the amount of potential inferred values when inferring a single -# object. This can help the performance when dealing with large functions or -# complex, nested conditions. -limit-inference-results=100 - -# List of plugins (as comma separated values of python module names) to load, -# usually to register additional checkers. -load-plugins= - -# Pickle collected data for later comparisons. -persistent=yes - -# Minimum Python version to use for version dependent checks. Will default to -# the version used to run pylint. -py-version=3.9 - -# Discover python modules and packages in the file system subtree. -recursive=no - -# When enabled, pylint would attempt to guess common misconfiguration and emit -# user-friendly hints instead of false-positive error messages. -suggestion-mode=yes - -# Allow loading of arbitrary C extensions. Extensions are imported into the -# active Python interpreter and may run arbitrary code. -unsafe-load-any-extension=no - -# In verbose mode, extra non-checker-related info will be displayed. -#verbose= - - -[BASIC] - -# Naming style matching correct argument names. -argument-naming-style=snake_case - -# Regular expression matching correct argument names. Overrides argument- -# naming-style. If left empty, argument names will be checked with the set -# naming style. -#argument-rgx= - -# Naming style matching correct attribute names. -attr-naming-style=snake_case - -# Regular expression matching correct attribute names. Overrides attr-naming- -# style. If left empty, attribute names will be checked with the set naming -# style. -#attr-rgx= - -# Bad variable names which should always be refused, separated by a comma. -bad-names=foo, - bar, - baz, - toto, - tutu, - tata - -# Bad variable names regexes, separated by a comma. If names match any regex, -# they will always be refused -bad-names-rgxs= - -# Naming style matching correct class attribute names. -class-attribute-naming-style=any - -# Regular expression matching correct class attribute names. Overrides class- -# attribute-naming-style. If left empty, class attribute names will be checked -# with the set naming style. -#class-attribute-rgx= - -# Naming style matching correct class constant names. -class-const-naming-style=UPPER_CASE - -# Regular expression matching correct class constant names. Overrides class- -# const-naming-style. If left empty, class constant names will be checked with -# the set naming style. -#class-const-rgx= - -# Naming style matching correct class names. -class-naming-style=PascalCase - -# Regular expression matching correct class names. Overrides class-naming- -# style. If left empty, class names will be checked with the set naming style. -#class-rgx= - -# Naming style matching correct constant names. -const-naming-style=UPPER_CASE - -# Regular expression matching correct constant names. Overrides const-naming- -# style. If left empty, constant names will be checked with the set naming -# style. -#const-rgx= - -# Minimum line length for functions/classes that require docstrings, shorter -# ones are exempt. -docstring-min-length=-1 - -# Naming style matching correct function names. -function-naming-style=snake_case - -# Regular expression matching correct function names. Overrides function- -# naming-style. If left empty, function names will be checked with the set -# naming style. -#function-rgx= - -# Good variable names which should always be accepted, separated by a comma. -good-names=i, - j, - k, - ex, - Run, - _, - __, - ___, - id, - ip - -# Good variable names regexes, separated by a comma. If names match any regex, -# they will always be accepted -good-names-rgxs= - -# Include a hint for the correct naming format with invalid-name. -include-naming-hint=yes - -# Naming style matching correct inline iteration names. -inlinevar-naming-style=any - -# Regular expression matching correct inline iteration names. Overrides -# inlinevar-naming-style. If left empty, inline iteration names will be checked -# with the set naming style. -#inlinevar-rgx= - -# Naming style matching correct method names. -method-naming-style=snake_case - -# Regular expression matching correct method names. Overrides method-naming- -# style. If left empty, method names will be checked with the set naming style. -#method-rgx= - -# Naming style matching correct module names. -module-naming-style=snake_case - -# Regular expression matching correct module names. Overrides module-naming- -# style. If left empty, module names will be checked with the set naming style. -#module-rgx= - -# Colon-delimited sets of names that determine each other's naming style when -# the name regexes allow several styles. -name-group= - -# Regular expression which should only match function or class names that do -# not require a docstring. -no-docstring-rgx=^_ - -# List of decorators that produce properties, such as abc.abstractproperty. Add -# to this list to register other decorators that produce valid properties. -# These decorators are taken in consideration only for invalid-name. -property-classes=abc.abstractproperty - -# Regular expression matching correct type variable names. If left empty, type -# variable names will be checked with the set naming style. -#typevar-rgx= - -# Naming style matching correct variable names. -variable-naming-style=snake_case - -# Regular expression matching correct variable names. Overrides variable- -# naming-style. If left empty, variable names will be checked with the set -# naming style. -#variable-rgx= - - -[CLASSES] - -# Warn about protected attribute access inside special methods -check-protected-access-in-special-methods=no - -# List of method names used to declare (i.e. assign) instance attributes. -defining-attr-methods=__init__, - __new__, - setUp, - __post_init__ - -# List of member names, which should be excluded from the protected access -# warning. -exclude-protected=_asdict, - _fields, - _replace, - _source, - _make - -# List of valid names for the first argument in a class method. -valid-classmethod-first-arg=cls - -# List of valid names for the first argument in a metaclass class method. -valid-metaclass-classmethod-first-arg=cls - - -[DESIGN] - -# List of regular expressions of class ancestor names to ignore when counting -# public methods (see R0903) -exclude-too-few-public-methods= - -# List of qualified class names to ignore when counting class parents (see -# R0901) -ignored-parents= - -# Maximum number of arguments for function / method. -max-args=6 - -# Maximum number of attributes for a class (see R0902). -max-attributes=7 - -# Maximum number of boolean expressions in an if statement (see R0916). -max-bool-expr=5 - -# Maximum number of branch for function / method body. -max-branches=12 - -# Maximum number of locals for function / method body. -max-locals=15 - -# Maximum number of parents for a class (see R0901). -max-parents=7 - -# Maximum number of public methods for a class (see R0904). -max-public-methods=20 - -# Maximum number of return / yield for function / method body. -max-returns=6 - -# Maximum number of statements in function / method body. -max-statements=50 - -# Minimum number of public methods for a class (see R0903). -min-public-methods=0 - - -[EXCEPTIONS] - -# Exceptions that will emit a warning when caught. -overgeneral-exceptions=builtins.BaseException, - builtins.Exception - - -[FORMAT] - -# Expected format of line ending, e.g. empty (any line ending), LF or CRLF. -expected-line-ending-format= - -# Regexp for a line that is allowed to be longer than the limit. -ignore-long-lines=^\s*(# )??$ - -# Number of spaces of indent required inside a hanging or continued line. -indent-after-paren=4 - -# String used as indentation unit. This is usually " " (4 spaces) or "\t" (1 -# tab). -indent-string=' ' - -# Maximum number of characters on a single line. -max-line-length=120 - -# Maximum number of lines in a module. -max-module-lines=1000 - -# Allow the body of a class to be on the same line as the declaration if body -# contains single statement. -single-line-class-stmt=no - -# Allow the body of an if to be on the same line as the test if there is no -# else. -single-line-if-stmt=no - - -[IMPORTS] - -# List of modules that can be imported at any level, not just the top level -# one. -allow-any-import-level= - -# Allow wildcard imports from modules that define __all__. -allow-wildcard-with-all=no - -# Deprecated modules which should not be used, separated by a comma. -deprecated-modules= - -# Output a graph (.gv or any supported image format) of external dependencies -# to the given file (report RP0402 must not be disabled). -ext-import-graph= - -# Output a graph (.gv or any supported image format) of all (i.e. internal and -# external) dependencies to the given file (report RP0402 must not be -# disabled). -import-graph= - -# Output a graph (.gv or any supported image format) of internal dependencies -# to the given file (report RP0402 must not be disabled). -int-import-graph= - -# Force import order to recognize a module as part of the standard -# compatibility libraries. -known-standard-library= - -# Force import order to recognize a module as part of a third party library. -known-third-party=enchant - -# Couples of modules and preferred modules, separated by a comma. -preferred-modules= - - -[LOGGING] - -# The type of string formatting that logging methods do. `old` means using % -# formatting, `new` is for `{}` formatting. -logging-format-style=old - -# Logging modules to check that the string format arguments are in logging -# function parameter format. -logging-modules=logging - - -[MESSAGES CONTROL] - -# Only show warnings with the listed confidence levels. Leave empty to show -# all. Valid levels: HIGH, CONTROL_FLOW, INFERENCE, INFERENCE_FAILURE, -# UNDEFINED. -confidence=HIGH, - CONTROL_FLOW, - INFERENCE, - INFERENCE_FAILURE, - UNDEFINED - -# Disable the message, report, category or checker with the given id(s). You -# can either give multiple identifiers separated by comma (,) or put this -# option multiple times (only on the command line, not in the configuration -# file where it should appear only once). You can also use "--disable=all" to -# disable everything first and then re-enable specific checks. For example, if -# you want to run only the similarities checker, you can use "--disable=all -# --enable=similarities". If you want to run only the classes checker, but have -# no Warning level messages displayed, use "--disable=all --enable=classes -# --disable=W". -disable=raw-checker-failed, - bad-inline-option, - locally-disabled, - file-ignored, - suppressed-message, - useless-suppression, - deprecated-pragma, - use-symbolic-message-instead, - fixme, - duplicate-code - -# Enable the message, report, category or checker with the given id(s). You can -# either give multiple identifier separated by comma (,) or put this option -# multiple time (only on the command line, not in the configuration file where -# it should appear only once). See also the "--disable" option for examples. -enable=c-extension-no-member - - -[METHOD_ARGS] - -# List of qualified names (i.e., library.method) which require a timeout -# parameter e.g. 'requests.api.get,requests.api.post' -timeout-methods=requests.api.delete,requests.api.get,requests.api.head,requests.api.options,requests.api.patch,requests.api.post,requests.api.put,requests.api.request - - -[MISCELLANEOUS] - -# List of note tags to take in consideration, separated by a comma. -notes=FIXME, - XXX, - TODO - -# Regular expression of note tags to take in consideration. -notes-rgx= - - -[REFACTORING] - -# Maximum number of nested blocks for function / method body -max-nested-blocks=5 - -# Complete name of functions that never returns. When checking for -# inconsistent-return-statements if a never returning function is called then -# it will be considered as an explicit return statement and no message will be -# printed. -never-returning-functions=sys.exit,argparse.parse_error - - -[REPORTS] - -# Python expression which should return a score less than or equal to 10. You -# have access to the variables 'fatal', 'error', 'warning', 'refactor', -# 'convention', and 'info' which contain the number of messages in each -# category, as well as 'statement' which is the total number of statements -# analyzed. This score is used by the global evaluation report (RP0004). -evaluation=max(0, 0 if fatal else 10.0 - ((float(5 * error + warning + refactor + convention) / statement) * 10)) - -# Template used to display messages. This is a python new-style format string -# used to format the message information. See doc for all details. -msg-template= - -# Set the output format. Available formats are text, parseable, colorized, json -# and msvs (visual studio). You can also give a reporter class, e.g. -# mypackage.mymodule.MyReporterClass. -#output-format= - -# Tells whether to display a full report or only the messages. -reports=no - -# Activate the evaluation score. -score=yes - - -[SIMILARITIES] - -# Comments are removed from the similarity computation -ignore-comments=yes - -# Docstrings are removed from the similarity computation -ignore-docstrings=yes - -# Imports are removed from the similarity computation -ignore-imports=yes - -# Signatures are removed from the similarity computation -ignore-signatures=yes - -# Minimum lines number of a similarity. -min-similarity-lines=4 - - -[SPELLING] - -# Limits count of emitted suggestions for spelling mistakes. -max-spelling-suggestions=4 - -# Spelling dictionary name. Available dictionaries: none. To make it work, -# install the 'python-enchant' package. -spelling-dict= - -# List of comma separated words that should be considered directives if they -# appear at the beginning of a comment and should not be checked. -spelling-ignore-comment-directives=fmt: on,fmt: off,noqa:,noqa,nosec,isort:skip,mypy: - -# List of comma separated words that should not be checked. -spelling-ignore-words= - -# A path to a file that contains the private dictionary; one word per line. -spelling-private-dict-file= - -# Tells whether to store unknown words to the private dictionary (see the -# --spelling-private-dict-file option) instead of raising a message. -spelling-store-unknown-words=no - - -[STRING] - -# This flag controls whether inconsistent-quotes generates a warning when the -# character used as a quote delimiter is used inconsistently within a module. -check-quote-consistency=no - -# This flag controls whether the implicit-str-concat should generate a warning -# on implicit string concatenation in sequences defined over several lines. -check-str-concat-over-line-jumps=no - - -[TYPECHECK] - -# List of decorators that produce context managers, such as -# contextlib.contextmanager. Add to this list to register other decorators that -# produce valid context managers. -contextmanager-decorators=contextlib.contextmanager - -# List of members which are set dynamically and missed by pylint inference -# system, and so shouldn't trigger E1101 when accessed. Python regular -# expressions are accepted. -generated-members= - -# Tells whether to warn about missing members when the owner of the attribute -# is inferred to be None. -ignore-none=yes - -# This flag controls whether pylint should warn about no-member and similar -# checks whenever an opaque object is returned when inferring. The inference -# can return multiple potential results while evaluating a Python object, but -# some branches might not be evaluated, which results in partial inference. In -# that case, it might be useful to still emit no-member and other checks for -# the rest of the inferred objects. -ignore-on-opaque-inference=yes - -# List of symbolic message names to ignore for Mixin members. -ignored-checks-for-mixins=no-member, - not-async-context-manager, - not-context-manager, - attribute-defined-outside-init - -# List of class names for which member attributes should not be checked (useful -# for classes with dynamically set attributes). This supports the use of -# qualified names. -ignored-classes=optparse.Values,thread._local,_thread._local,argparse.Namespace - -# Show a hint with possible names when a member name was not found. The aspect -# of finding the hint is based on edit distance. -missing-member-hint=yes - -# The minimum edit distance a name should have in order to be considered a -# similar match for a missing member name. -missing-member-hint-distance=1 - -# The total number of similar names that should be taken in consideration when -# showing a hint for a missing member. -missing-member-max-choices=1 - -# Regex pattern to define which classes are considered mixins. -mixin-class-rgx=.*[Mm]ixin - -# List of decorators that change the signature of a decorated function. -signature-mutators= - - -[VARIABLES] - -# List of additional names supposed to be defined in builtins. Remember that -# you should avoid defining new builtins when possible. -additional-builtins= - -# Tells whether unused global variables should be treated as a violation. -allow-global-unused-variables=yes - -# List of names allowed to shadow builtins -allowed-redefined-builtins= - -# List of strings which can identify a callback function by name. A callback -# name must start or end with one of those strings. -callbacks=cb_, - _cb - -# A regular expression matching the name of dummy variables (i.e. expected to -# not be used). -dummy-variables-rgx=_+$|(_[a-zA-Z0-9_]*[a-zA-Z0-9]+?$)|dummy|^ignored_|^unused_ - -# Argument names that match this expression will be ignored. -ignored-argument-names=_.*|^ignored_|^unused_ - -# Tells whether we should check for unused import in __init__ files. -init-import=no - -# List of qualified module names which can have objects that can redefine -# builtins. -redefining-builtins-modules=six.moves,past.builtins,future.builtins,builtins,io diff --git a/pyproject.toml b/pyproject.toml index f3ab9d6..63e2943 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -17,14 +17,11 @@ dependencies = [ [project.optional-dependencies] code-inspection = [ - "pylint==3.2.2", + "ruff==0.4.8", "mypy==1.10.0", "types-requests==2.32.0.20240602" ] -formatting = [ - "black==24.4.2" -] test = [ "pytest==8.2.1", @@ -35,4 +32,49 @@ test = [ [tool.setuptools] -packages = ["src"] \ No newline at end of file +packages = ["src"] + +[tool.ruff] +line-length = 120 + +[tool.ruff.lint] +select = [ + "F", # flake8 - Basic initial rules + "E", # pycodestyle (Error) - pep8 compliance + "W", # pycodestyle (Warning) - pep8 compliance + "C90", # mccabe - flags extremely complex functions + "I", # isort - Sort imports and flag missing imports + "N", # pep8-naming - Ensures pep8 compliance for naming + "UP", # pyupgrade - Automatically upgrade syntax for newer versions + "S", # flake8-bandit - Flake8 security + "B", # flake8-bugbear - Finding likely bugs and design problems + "A", # flake8-builtins - Finds code shadowing builtins + "COM", # flake8-commas - Find and fixes issues with commas in lists/dicts + "C4", # flake8-comprehensions - Simplify list/dict comprehension + "DTZ", # flake8-datetimez - Ensure timezones are enforced for code + "EXE", # flake8-executable - Fix issues around shebangs and executable files + "ISC", # flake8-implicit-str-concat - Find implicitly concatenated strings + "LOG", # flake8-logging - Enforce basic rules with builtin logger + "T20", # flake8-print - Remove print statements + "PT", # flake8-pytest-style - Fix issues with pytest + "Q", # flake8-quotes - Bad quote handling + "RET", # flake8-return - Fix issues with return values + "SIM", # flake8-simplify - Simplify parts of the code + "TCH", # flake8-type-checking - Move imports only for typing behind TYPE_CHECKING + "PTH", # flake8-use-pathlib - Replace os with pathlib + "TD", # flake8-todos - Enforce basic TODOs + "FIX", # flake8-fix me - Resolve the issue instead of a fix me + "ERA", # eradicate - Remove commented out code. + "PL", "C", "E", "R", "W", # Pylint - does a lot + "FLY", # flynt - prefer f string over .format + "PERF", # Perflint - Flag performance antipatterns + "RUF", # Ruff specific rules +] +ignore = [ + "S101", # flake8-bandit - Use of assert (all over pytest tests) + "ISC001", # Conflicts with the formatter + "COM812", # Conflicts with the formatter +] + +[tool.ruff.lint.pylint] +max-args = 10 \ No newline at end of file diff --git a/src/auth.py b/src/auth.py index 8de0bae..4e50ec8 100644 --- a/src/auth.py +++ b/src/auth.py @@ -5,8 +5,7 @@ import requests from src.exceptions import BadCredentialsError, UOWSError -from src.model import User -from src.model import UserCredentials +from src.model import User, UserCredentials def authenticate(credentials: UserCredentials) -> User: @@ -22,9 +21,9 @@ def authenticate(credentials: UserCredentials) -> User: headers={"Content-Type": "application/json"}, timeout=30, ) - if response.status_code == 201: + if response.status_code == 201: # noqa: PLR2004 return User(user_number=response.json()["userId"]) - if response.status_code == 401: + if response.status_code == 401: # noqa: PLR2004 raise BadCredentialsError("Invalid user credentials provided to authenticate with the user office web service.") raise UOWSError("An unexpected error occurred when authenticating with the user office web service") diff --git a/src/model.py b/src/model.py index eaaeef6..044b52f 100644 --- a/src/model.py +++ b/src/model.py @@ -40,5 +40,5 @@ def role(self) -> Role: Determine and determine the role of the user based on their usernumber :return: """ - # TODO: Actually assign a role + # TODO: Actually assign a role # noqa: FIX002, TD002, TD003 return Role.USER diff --git a/src/routers.py b/src/routers.py index b490742..eeae67e 100644 --- a/src/routers.py +++ b/src/routers.py @@ -5,17 +5,19 @@ from __future__ import annotations import os -from typing import List, Annotated, Dict, Any, Literal +from typing import TYPE_CHECKING, Annotated, Any, Literal -from fastapi import APIRouter, Depends, HTTPException, Cookie -from fastapi.security import HTTPBearer, HTTPAuthorizationCredentials +from fastapi import APIRouter, Cookie, Depends, HTTPException +from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer from starlette.responses import JSONResponse from src.auth import authenticate from src.exceptions import UOWSError from src.experiments import get_experiments_for_user_number -from src.model import UserCredentials -from src.tokens import generate_refresh_token, generate_access_token, load_access_token, load_refresh_token +from src.tokens import generate_access_token, generate_refresh_token, load_access_token, load_refresh_token + +if TYPE_CHECKING: + from src.model import UserCredentials ROUTER = APIRouter() @@ -27,7 +29,7 @@ @ROUTER.get("/experiments", tags=["internal"]) async def get_experiments( user_number: int, credentials: Annotated[HTTPAuthorizationCredentials, Depends(security)] -) -> List[int]: +) -> list[int]: """ Get the experiment (RB) numbers for the given user number provided by query string parameter @@ -63,7 +65,7 @@ async def login(credentials: UserCredentials) -> JSONResponse: @ROUTER.post("/api/jwt/checkToken") -def verify(token: Dict[str, Any]) -> Literal["ok"]: +def verify(token: dict[str, Any]) -> Literal["ok"]: """ Verify an access token was generated by this auth server and has not expired \f @@ -75,7 +77,7 @@ def verify(token: Dict[str, Any]) -> Literal["ok"]: @ROUTER.post("/api/jwt/refresh") -def refresh(token: Dict[str, Any], refresh_token: Annotated[str | None, Cookie()] = None) -> JSONResponse: +def refresh(token: dict[str, Any], refresh_token: Annotated[str | None, Cookie()] = None) -> JSONResponse: """ Refresh an access token based on a refresh token \f diff --git a/src/tokens.py b/src/tokens.py index a8f1f8b..6d5dd2a 100644 --- a/src/tokens.py +++ b/src/tokens.py @@ -7,13 +7,15 @@ import logging import os from abc import ABC -from datetime import datetime, timezone, timedelta -from typing import Any, Dict, Optional +from datetime import datetime, timedelta, timezone +from typing import TYPE_CHECKING, Any import jwt from src.exceptions import BadJWTError -from src.model import User + +if TYPE_CHECKING: + from src.model import User PRIVATE_KEY = os.environ.get("PRIVATE_KEY", "shh") logger = logging.getLogger(__name__) @@ -33,7 +35,6 @@ def verify(self) -> None: :return: None """ try: - # pylint: disable=attribute-defined-outside-init # class is abstract and all subclasses define payload within the init self._payload = jwt.decode( self.jwt, @@ -41,7 +42,7 @@ def verify(self) -> None: algorithms=["HS256"], options={"verify_signature": True, "require": ["exp"], "verify_exp": True}, ) - # pylint: enable=attribute-defined-outside-init + return except jwt.InvalidSignatureError: logger.warning("token has bad signature - %s", self.jwt) @@ -49,10 +50,10 @@ def verify(self) -> None: logger.warning("token signature is expired - %s", self.jwt) except jwt.InvalidTokenError: logger.warning("Issue decoding token - %s", self.jwt) - # pylint: disable=broad-exception-caught + except Exception: logger.exception("Oh Dear") - # pylint: enable=broad-exception-caught + raise BadJWTError("oh dear") def _encode(self) -> None: @@ -66,7 +67,7 @@ class AccessToken(Token): Access Token is a short-lived (5 minute) token that stores user information """ - def __init__(self, jwt_token: Optional[str] = None, payload: Optional[Dict[str, Any]] = None) -> None: + def __init__(self, jwt_token: str | None = None, payload: dict[str, Any] | None = None) -> None: if payload and not jwt_token: self._payload = payload self._payload["exp"] = datetime.now(timezone.utc) + timedelta(minutes=5) @@ -100,8 +101,7 @@ class RefreshToken(Token): Refresh token is a long-lived (12 hour) token that is required to refresh an access token """ - def __init__(self, jwt_token: Optional[str] = None) -> None: - + def __init__(self, jwt_token: str | None = None) -> None: if jwt_token is None: self._payload = {"exp": datetime.now(timezone.utc) + timedelta(hours=12)} self._encode() @@ -141,7 +141,7 @@ def load_access_token(token: str) -> AccessToken: return AccessToken(jwt_token=token) -def load_refresh_token(token: Optional[str]) -> RefreshToken: +def load_refresh_token(token: str | None) -> RefreshToken: """ Given a jwt string, return a refresh token object for it :param token: the jwt string diff --git a/test/e2e/test_auth.py b/test/e2e/test_auth.py index 241bea5..25e6699 100644 --- a/test/e2e/test_auth.py +++ b/test/e2e/test_auth.py @@ -1,4 +1,4 @@ -from unittest.mock import patch, Mock +from unittest.mock import Mock, patch import jwt from starlette.testclient import TestClient @@ -30,7 +30,7 @@ def test_unsuccessful_login(mock_post): mock_response.status_code = 401 response = client.post("/api/jwt/authenticate", json={"username": "foo", "password": "foo"}) - assert response.status_code == 403 + assert response.status_code == 403 # noqa: PLR2004 @patch("src.auth.requests.post") @@ -41,7 +41,7 @@ def test_unsuccessful_login_uows_failure(mock_post): mock_response.status_code = 500 response = client.post("/api/jwt/authenticate", json={"username": "foo", "password": "foo"}) - assert response.status_code == 403 + assert response.status_code == 403 # noqa: PLR2004 def test_verify_success(): @@ -49,17 +49,17 @@ def test_verify_success(): access_token = generate_access_token(user) response = client.post("/api/jwt/checkToken", json={"token": access_token.jwt}) - assert response.status_code == 200 + assert response.status_code == 200 # noqa: PLR2004 def test_verify_fail_badly_formed_token(): response = client.post("/api/jwt/checkToken", json={"token": "foo"}) - assert response.status_code == 403 + assert response.status_code == 403 # noqa: PLR2004 def test_verify_fail_bad_signature(): response = client.post("/api/jwt/checkToken", json={"token": jwt.encode({"foo": "bar"}, key="foo")}) - assert response.status_code == 403 + assert response.status_code == 403 # noqa: PLR2004 def test_token_refresh_success(): @@ -81,17 +81,17 @@ def test_token_refresh_no_refresh_token_given(): json={"token": access_token.jwt}, ) - assert response.status_code == 403 + assert response.status_code == 403 # noqa: PLR2004 def test_token_refresh_expired_refresh_token(): user = User(123) access_token = generate_access_token(user) refresh_token = ( - "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE3MTgwMjEyNDB9.iHITGf2RyX_49pY7Xy8xdutYE4Pc6k9mfKWQjxCKgOk" + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE3MTgwMjEyNDB9.iHITGf2RyX_49pY7Xy8xdutYE4Pc6k9mfKWQjxCKgOk" # noqa: S105 ) response = client.post( "/api/jwt/refresh", json={"token": access_token.jwt}, cookies={"refresh_token": refresh_token} ) - assert response.status_code == 403 + assert response.status_code == 403 # noqa: PLR2004 diff --git a/test/e2e/test_experiments.py b/test/e2e/test_experiments.py index 1c298c6..90bf63a 100644 --- a/test/e2e/test_experiments.py +++ b/test/e2e/test_experiments.py @@ -28,23 +28,25 @@ def test_get_experiments_with_missing_api_key_returns_403(): response = client.get("/experiments?user_number=123") - assert response.status_code == 403 + assert response.status_code == 403 # noqa: PLR2004 def test_get_experiments_with_bad_api_key_returns_403(): response = client.get("/experiments?user_number=123", headers={"Authorization": "Bearer 123"}) - assert response.status_code == 403 + assert response.status_code == 403 # noqa: PLR2004 -@patch("src.experiments.Client.execute_async", return_value=ALLOCATIONS_EMPTY_RESPONSE) -def test_get_experiments_none_exist_for_user_returns_empty(_): +@patch("src.experiments.Client.execute_async") +def test_get_experiments_none_exist_for_user_returns_empty(mock_exec): + mock_exec.return_value = ALLOCATIONS_EMPTY_RESPONSE response = client.get("/experiments?user_number=123", headers={"Authorization": "Bearer shh"}) - assert response.status_code == 200 + assert response.status_code == 200 # noqa: PLR2004 assert response.json() == [] -@patch("src.experiments.Client.execute_async", return_value=ALLOCATIONS_RESPONSE) -def test_get_experiments_for_user(_): +@patch("src.experiments.Client.execute_async") +def test_get_experiments_for_user(mock_exec): + mock_exec.return_value = ALLOCATIONS_RESPONSE response = client.get("/experiments?user_number=123", headers={"Authorization": "Bearer shh"}) - assert response.status_code == 200 + assert response.status_code == 200 # noqa: PLR2004 assert response.json() == [9723, 2200087, 2200084, 2200081, 2200083, 2200085, 2200086, 2200082, 1620354] diff --git a/test/test_auth.py b/test/test_auth.py index a3ce1fd..e161033 100644 --- a/test/test_auth.py +++ b/test/test_auth.py @@ -1,4 +1,4 @@ -from unittest.mock import patch, Mock +from unittest.mock import Mock, patch import pytest @@ -12,7 +12,7 @@ def test_authenticate_success(mock_post): mock_response = Mock(status_code=201, json=lambda: {"userId": "12345"}) mock_post.return_value = mock_response - credentials = UserCredentials(username="valid_user", password="valid_password") + credentials = UserCredentials(username="valid_user", password="valid_password") # noqa: S106 user = authenticate(credentials) @@ -30,7 +30,7 @@ def test_authenticate_bad_credentials(mock_post): mock_response = Mock(status_code=401, json=lambda: {}) mock_post.return_value = mock_response - credentials = UserCredentials(username="invalid_user", password="invalid_password") + credentials = UserCredentials(username="invalid_user", password="invalid_password") # noqa: S106 with pytest.raises(BadCredentialsError) as exc_info: authenticate(credentials) @@ -43,7 +43,7 @@ def test_authenticate_unexpected_error(mock_post): mock_response = Mock(status_code=500, json=lambda: {"error": "Server error"}) mock_post.return_value = mock_response - credentials = UserCredentials(username="user", password="password") + credentials = UserCredentials(username="user", password="password") # noqa: S106 with pytest.raises(UOWSError) as exc_info: authenticate(credentials) diff --git a/test/test_tokens.py b/test/test_tokens.py index 4aeb37e..88155ab 100644 --- a/test/test_tokens.py +++ b/test/test_tokens.py @@ -1,4 +1,4 @@ -from datetime import datetime, timezone, timedelta +from datetime import datetime, timedelta, timezone from unittest.mock import patch import jwt @@ -6,7 +6,7 @@ from src.exceptions import BadJWTError from src.model import User -from src.tokens import Token, AccessToken, RefreshToken, generate_access_token +from src.tokens import AccessToken, RefreshToken, Token, generate_access_token @patch("jwt.decode") @@ -78,8 +78,7 @@ def test_verify_general_exception(mock_logger, mock_decode): @patch("src.tokens.datetime") @patch("jwt.encode") -@patch("jwt.decode") -def test_access_token_with_payload(_, mock_encode, mock_datetime): +def test_access_token_with_payload(mock_encode, mock_datetime): fixed_time = datetime(2021, 1, 1, 12, 0, 0, tzinfo=timezone.utc) mock_datetime.now.return_value = fixed_time payload = {"user": "test_user"} @@ -95,7 +94,7 @@ def test_access_token_with_payload(_, mock_encode, mock_datetime): @patch("jwt.decode") def test_access_token_with_jwt_token(mock_decode): - jwt_token = "encoded.jwt.token" + jwt_token = "encoded.jwt.token" # noqa: S105 expected_payload = {"exp": datetime.now(timezone.utc) + timedelta(minutes=1)} mock_decode.return_value = expected_payload @@ -119,7 +118,7 @@ def test_access_token_with_both_none(): @patch("jwt.encode") @patch("jwt.decode") def test_access_token_refresh(mock_decode, mock_encode): - jwt_token = "valid.jwt.token" + jwt_token = "valid.jwt.token" # noqa: S105 mock_decode.return_value = {"user": "test_user", "exp": datetime.now(timezone.utc)} token = AccessToken(jwt_token=jwt_token) @@ -131,8 +130,7 @@ def test_access_token_refresh(mock_decode, mock_encode): @patch("src.tokens.datetime") @patch("jwt.encode") -@patch("jwt.decode") -def test_refresh_token_creation_no_jwt(_, mock_encode, mock_datetime): +def test_refresh_token_creation_no_jwt(mock_encode, mock_datetime): fixed_time = datetime(2021, 1, 1, 12, 0, 0, tzinfo=timezone.utc) mock_datetime.now.return_value = fixed_time @@ -147,7 +145,7 @@ def test_refresh_token_creation_no_jwt(_, mock_encode, mock_datetime): @patch("jwt.decode") def test_refresh_token_creation_with_jwt(mock_decode): - jwt_token = "encoded.jwt.token" + jwt_token = "encoded.jwt.token" # noqa: S105 expected_payload = {"exp": datetime.now(timezone.utc) + timedelta(hours=12)} mock_decode.return_value = expected_payload @@ -162,10 +160,11 @@ def test_refresh_token_creation_with_jwt(mock_decode): ) -@patch("jwt.decode", side_effect=BadJWTError) -def test_refresh_token_with_invalid_jwt(_): +@patch("jwt.decode") +def test_refresh_token_with_invalid_jwt(mock_decode): + mock_decode.side_effect = BadJWTError with pytest.raises(BadJWTError): - RefreshToken(jwt_token="invalid.jwt.token") + RefreshToken(jwt_token="invalid.jwt.token") # noqa: S106 @patch("src.tokens.datetime") From 1dff2a1be02ed6992f5aeba21f751066e0229b69 Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Wed, 12 Jun 2024 09:13:36 +0100 Subject: [PATCH 16/47] Reinclude type import --- src/routers.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/routers.py b/src/routers.py index eeae67e..4ce3ee8 100644 --- a/src/routers.py +++ b/src/routers.py @@ -5,7 +5,7 @@ from __future__ import annotations import os -from typing import TYPE_CHECKING, Annotated, Any, Literal +from typing import Annotated, Any, Literal from fastapi import APIRouter, Cookie, Depends, HTTPException from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer @@ -14,11 +14,9 @@ from src.auth import authenticate from src.exceptions import UOWSError from src.experiments import get_experiments_for_user_number +from src.model import UserCredentials # noqa: TCH001 # Required for fastapi from src.tokens import generate_access_token, generate_refresh_token, load_access_token, load_refresh_token -if TYPE_CHECKING: - from src.model import UserCredentials - ROUTER = APIRouter() security = HTTPBearer(scheme_name="APIKey", description="API Key for internal routes") From d0a030f2acef50ba8bd3626be2fa9c6a843ac82c Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Wed, 12 Jun 2024 09:14:11 +0100 Subject: [PATCH 17/47] Fix workflow file --- .github/workflows/linting.yml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.github/workflows/linting.yml b/.github/workflows/linting.yml index 9f138c6..aad6f6c 100644 --- a/.github/workflows/linting.yml +++ b/.github/workflows/linting.yml @@ -1,10 +1,3 @@ ---- -on: push - -permissions: - contents: read - - --- on: push: From f352a8db3155d903a00cc0d7fc46b1443ffe2c0e Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Wed, 12 Jun 2024 09:15:35 +0100 Subject: [PATCH 18/47] Update install --- .github/workflows/linting.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/linting.yml b/.github/workflows/linting.yml index aad6f6c..e024131 100644 --- a/.github/workflows/linting.yml +++ b/.github/workflows/linting.yml @@ -32,7 +32,7 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - python -m pip install .[formatting] + python -m pip install .[code-inspection] - name: Run ruff formatting run: | From 51c5b00bb82933e275bfaddd9c84778e76d21851 Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Wed, 12 Jun 2024 09:17:12 +0100 Subject: [PATCH 19/47] Update test to include timeout --- test/test_auth.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_auth.py b/test/test_auth.py index e161033..9c183e5 100644 --- a/test/test_auth.py +++ b/test/test_auth.py @@ -22,6 +22,7 @@ def test_authenticate_success(mock_post): "https://devapi.facilities.rl.ac.uk/users-service/v0/sessions", json=({"username": "valid_user", "password": "valid_password"},), headers={"Content-Type": "application/json"}, + timeout=30, ) From 662d34dcc58f0d627ed0fae34a5e957a99fc5032 Mon Sep 17 00:00:00 2001 From: keiranjprice101 <44777678+keiranjprice101@users.noreply.github.com> Date: Thu, 13 Jun 2024 09:47:49 +0100 Subject: [PATCH 20/47] Update src/tokens.py Co-authored-by: Samuel Jones --- src/tokens.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tokens.py b/src/tokens.py index 6d5dd2a..926f3cb 100644 --- a/src/tokens.py +++ b/src/tokens.py @@ -54,7 +54,7 @@ def verify(self) -> None: except Exception: logger.exception("Oh Dear") - raise BadJWTError("oh dear") + raise BadJWTError("jwt token verification failed") def _encode(self) -> None: bytes_key = bytes(PRIVATE_KEY, encoding="utf8") From ab869f6d75d30d8d76ba5b513ddb61f2b974c8b4 Mon Sep 17 00:00:00 2001 From: keiranjprice101 <44777678+keiranjprice101@users.noreply.github.com> Date: Thu, 13 Jun 2024 09:53:59 +0100 Subject: [PATCH 21/47] Update test/e2e/test_experiments.py Co-authored-by: Samuel Jones --- test/e2e/test_experiments.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/test_experiments.py b/test/e2e/test_experiments.py index 90bf63a..3867016 100644 --- a/test/e2e/test_experiments.py +++ b/test/e2e/test_experiments.py @@ -28,7 +28,7 @@ def test_get_experiments_with_missing_api_key_returns_403(): response = client.get("/experiments?user_number=123") - assert response.status_code == 403 # noqa: PLR2004 + assert response.status_code == HTTPStatus.FORBIDDEN def test_get_experiments_with_bad_api_key_returns_403(): From e4507c3cdfae927220e3a7c1e989231c9c3642a3 Mon Sep 17 00:00:00 2001 From: keiranjprice101 <44777678+keiranjprice101@users.noreply.github.com> Date: Thu, 13 Jun 2024 09:54:14 +0100 Subject: [PATCH 22/47] Update test/e2e/test_experiments.py Co-authored-by: Samuel Jones --- test/e2e/test_experiments.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/test_experiments.py b/test/e2e/test_experiments.py index 3867016..b77d40f 100644 --- a/test/e2e/test_experiments.py +++ b/test/e2e/test_experiments.py @@ -48,5 +48,5 @@ def test_get_experiments_none_exist_for_user_returns_empty(mock_exec): def test_get_experiments_for_user(mock_exec): mock_exec.return_value = ALLOCATIONS_RESPONSE response = client.get("/experiments?user_number=123", headers={"Authorization": "Bearer shh"}) - assert response.status_code == 200 # noqa: PLR2004 + assert response.status_code == HTTPStatus.OK assert response.json() == [9723, 2200087, 2200084, 2200081, 2200083, 2200085, 2200086, 2200082, 1620354] From 53ef1c0f0ed96c0d48d504c4f57b357401a24fd7 Mon Sep 17 00:00:00 2001 From: keiranjprice101 <44777678+keiranjprice101@users.noreply.github.com> Date: Thu, 13 Jun 2024 09:54:23 +0100 Subject: [PATCH 23/47] Update test/e2e/test_experiments.py Co-authored-by: Samuel Jones --- test/e2e/test_experiments.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/test_experiments.py b/test/e2e/test_experiments.py index b77d40f..0ebedce 100644 --- a/test/e2e/test_experiments.py +++ b/test/e2e/test_experiments.py @@ -40,7 +40,7 @@ def test_get_experiments_with_bad_api_key_returns_403(): def test_get_experiments_none_exist_for_user_returns_empty(mock_exec): mock_exec.return_value = ALLOCATIONS_EMPTY_RESPONSE response = client.get("/experiments?user_number=123", headers={"Authorization": "Bearer shh"}) - assert response.status_code == 200 # noqa: PLR2004 + assert response.status_code == HTTPStatus.OK assert response.json() == [] From 3e743dcf511efd887899197be6ecd2318082bb6c Mon Sep 17 00:00:00 2001 From: keiranjprice101 <44777678+keiranjprice101@users.noreply.github.com> Date: Thu, 13 Jun 2024 09:54:32 +0100 Subject: [PATCH 24/47] Update test/e2e/test_experiments.py Co-authored-by: Samuel Jones --- test/e2e/test_experiments.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/test_experiments.py b/test/e2e/test_experiments.py index 0ebedce..dff1550 100644 --- a/test/e2e/test_experiments.py +++ b/test/e2e/test_experiments.py @@ -33,7 +33,7 @@ def test_get_experiments_with_missing_api_key_returns_403(): def test_get_experiments_with_bad_api_key_returns_403(): response = client.get("/experiments?user_number=123", headers={"Authorization": "Bearer 123"}) - assert response.status_code == 403 # noqa: PLR2004 + assert response.status_code == HTTPStatus.FORBIDDEN @patch("src.experiments.Client.execute_async") From e65927530897f07b35790d214a1d1abc69c7b073 Mon Sep 17 00:00:00 2001 From: keiranjprice101 <44777678+keiranjprice101@users.noreply.github.com> Date: Thu, 13 Jun 2024 09:54:44 +0100 Subject: [PATCH 25/47] Update src/auth.py Co-authored-by: Samuel Jones --- src/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/auth.py b/src/auth.py index 4e50ec8..5de5eaf 100644 --- a/src/auth.py +++ b/src/auth.py @@ -21,7 +21,7 @@ def authenticate(credentials: UserCredentials) -> User: headers={"Content-Type": "application/json"}, timeout=30, ) - if response.status_code == 201: # noqa: PLR2004 + if response.status_code == HTTPStatus.CREATED: return User(user_number=response.json()["userId"]) if response.status_code == 401: # noqa: PLR2004 raise BadCredentialsError("Invalid user credentials provided to authenticate with the user office web service.") From c0902bc85fc5a166ed15cfdaf347399c75926ab9 Mon Sep 17 00:00:00 2001 From: keiranjprice101 <44777678+keiranjprice101@users.noreply.github.com> Date: Thu, 13 Jun 2024 09:55:00 +0100 Subject: [PATCH 26/47] Update src/auth.py Co-authored-by: Samuel Jones --- src/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/auth.py b/src/auth.py index 5de5eaf..8381a12 100644 --- a/src/auth.py +++ b/src/auth.py @@ -23,7 +23,7 @@ def authenticate(credentials: UserCredentials) -> User: ) if response.status_code == HTTPStatus.CREATED: return User(user_number=response.json()["userId"]) - if response.status_code == 401: # noqa: PLR2004 + if response.status_code == HTTPStatus.UNAUTHORIZED: raise BadCredentialsError("Invalid user credentials provided to authenticate with the user office web service.") raise UOWSError("An unexpected error occurred when authenticating with the user office web service") From 90056d690bad7e2f23c634558e762c6fc1fd7a86 Mon Sep 17 00:00:00 2001 From: keiranjprice101 <44777678+keiranjprice101@users.noreply.github.com> Date: Thu, 13 Jun 2024 09:59:45 +0100 Subject: [PATCH 27/47] Update src/exception_handlers.py Co-authored-by: Samuel Jones --- src/exception_handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/exception_handlers.py b/src/exception_handlers.py index 1f415e7..cf109cd 100644 --- a/src/exception_handlers.py +++ b/src/exception_handlers.py @@ -10,6 +10,6 @@ async def auth_error_handler(_: Request, __: Exception) -> JSONResponse: :return: JSONResponse with 404 """ return JSONResponse( - status_code=403, + status_code=HTTPStatus.FORBIDDEN, content={"message": "Forbidden"}, ) From c63c6967a19130176c5b7b331e08b70bdf98db5e Mon Sep 17 00:00:00 2001 From: keiranjprice101 <44777678+keiranjprice101@users.noreply.github.com> Date: Thu, 13 Jun 2024 10:00:15 +0100 Subject: [PATCH 28/47] Update src/exception_handlers.py Co-authored-by: Samuel Jones --- src/exception_handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/exception_handlers.py b/src/exception_handlers.py index cf109cd..f5932b5 100644 --- a/src/exception_handlers.py +++ b/src/exception_handlers.py @@ -7,7 +7,7 @@ async def auth_error_handler(_: Request, __: Exception) -> JSONResponse: Automatically return a 403 when an authentication error is raised :param _: :param __: - :return: JSONResponse with 404 + :return: JSONResponse with 403 """ return JSONResponse( status_code=HTTPStatus.FORBIDDEN, From 4d22182dcae78b449f907b702f6799c3064779a8 Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Thu, 13 Jun 2024 10:50:11 +0100 Subject: [PATCH 29/47] Update README.md --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 26f0272..b4031f0 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,9 @@ # fia-auth [![codecov](https://codecov.io/gh/fiaisis/fia-auth/graph/badge.svg?token=lzm0DS1jhG)](https://codecov.io/gh/fiaisis/fia-auth) +![License: GPL-3.0](https://img.shields.io/github/license/fiaisis/run-detection) +![Build: passing](https://img.shields.io/github/actions/workflow/status/fiaisis/fia-auth/tests.yml?branch=main) +[![Code style: ruff](https://img.shields.io/badge/code%20style-ruff-30173d)](https://docs.astral.sh/ruff/) +[![linting: ruff](https://img.shields.io/badge/linting-ruff-30173d)](https://docs.astral.sh/ruff/) ## Testing Locally You must be connected to the vpn to make use of the uows and allocations api. From 8392a727ce4299da97b3955e26d413294d1d2cac Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Thu, 13 Jun 2024 10:50:24 +0100 Subject: [PATCH 30/47] Rename workflow --- ...linting.yml => formatting-and-linting.yml} | 24 +------------------ 1 file changed, 1 insertion(+), 23 deletions(-) rename .github/workflows/{linting.yml => formatting-and-linting.yml} (62%) diff --git a/.github/workflows/linting.yml b/.github/workflows/formatting-and-linting.yml similarity index 62% rename from .github/workflows/linting.yml rename to .github/workflows/formatting-and-linting.yml index e024131..96fc57b 100644 --- a/.github/workflows/linting.yml +++ b/.github/workflows/formatting-and-linting.yml @@ -32,7 +32,7 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - python -m pip install .[code-inspection] + python -m pip install .[formatting] - name: Run ruff formatting run: | @@ -49,28 +49,6 @@ jobs: git add . git commit -m "Formatting and linting commit" || true git push - mypy: - runs-on: ubuntu-latest - steps: - - name: Checkout project - uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v3.5.0 - - name: Set up python - uses: actions/setup-python@82c7e631bb3cdc910f68e0081d67478d79c6982d # v4.5.0 - with: - python-version: '3.12' - - - name: Set up cache for Python dependencies - uses: actions/cache@v4 - with: - path: ~/.cache/pip - key: ${{ runner.os }}-pip-${{ hashFiles('**/pyproject.toml') }} - restore-keys: | - ${{ runner.os }}-pip- - - - name: Install dependencies - run: | - python -m pip install --upgrade pip - python -m pip install .[code-inspection] - name: Run MyPy run: mypy --strict src \ No newline at end of file From 7408e86a757daebfbcd4771f0c0b85d51774fbb1 Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Thu, 13 Jun 2024 10:50:37 +0100 Subject: [PATCH 31/47] Update group name --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 63e2943..060ed10 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,7 +16,7 @@ dependencies = [ "Repository" = "https://github.com/fiaisis/fia-auth" [project.optional-dependencies] -code-inspection = [ +formatting = [ "ruff==0.4.8", "mypy==1.10.0", "types-requests==2.32.0.20240602" From 2dc106221e758995eeab3c5c43b1ab5a77f6d32b Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Thu, 13 Jun 2024 10:50:46 +0100 Subject: [PATCH 32/47] More explicit warning --- src/tokens.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tokens.py b/src/tokens.py index 926f3cb..7e14087 100644 --- a/src/tokens.py +++ b/src/tokens.py @@ -50,9 +50,8 @@ def verify(self) -> None: logger.warning("token signature is expired - %s", self.jwt) except jwt.InvalidTokenError: logger.warning("Issue decoding token - %s", self.jwt) - except Exception: - logger.exception("Oh Dear") + logger.exception("JWT verification Failed for unknown reason") raise BadJWTError("jwt token verification failed") From b81abf71af00a4e75f0d64495522a43bfd76c24f Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Thu, 13 Jun 2024 10:52:38 +0100 Subject: [PATCH 33/47] Add missing imports --- src/auth.py | 2 ++ test/e2e/test_experiments.py | 1 + 2 files changed, 3 insertions(+) diff --git a/src/auth.py b/src/auth.py index 8381a12..064a149 100644 --- a/src/auth.py +++ b/src/auth.py @@ -2,6 +2,8 @@ Module containing code to authenticate with the UOWS """ +from http import HTTPStatus + import requests from src.exceptions import BadCredentialsError, UOWSError diff --git a/test/e2e/test_experiments.py b/test/e2e/test_experiments.py index dff1550..d6859ac 100644 --- a/test/e2e/test_experiments.py +++ b/test/e2e/test_experiments.py @@ -2,6 +2,7 @@ e2e test cases """ +from http import HTTPStatus from unittest.mock import patch from starlette.testclient import TestClient From 7a167de2f26aba592d48d559291c0e4def21fd77 Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Thu, 13 Jun 2024 10:54:44 +0100 Subject: [PATCH 34/47] Fix tests --- src/exception_handlers.py | 2 ++ test/test_tokens.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/exception_handlers.py b/src/exception_handlers.py index f5932b5..3d3a54d 100644 --- a/src/exception_handlers.py +++ b/src/exception_handlers.py @@ -1,3 +1,5 @@ +from http import HTTPStatus + from starlette.requests import Request from starlette.responses import JSONResponse diff --git a/test/test_tokens.py b/test/test_tokens.py index 88155ab..505743b 100644 --- a/test/test_tokens.py +++ b/test/test_tokens.py @@ -73,7 +73,7 @@ def test_verify_general_exception(mock_logger, mock_decode): with pytest.raises(BadJWTError): token_instance.verify() - mock_logger.exception.assert_called_once_with("Oh Dear") + mock_logger.exception.assert_called_once_with("JWT verification Failed for unknown reason") @patch("src.tokens.datetime") From e137a6e273a23ab3f54ed07b041a28da13d107d2 Mon Sep 17 00:00:00 2001 From: keiranjprice101 <44777678+keiranjprice101@users.noreply.github.com> Date: Thu, 13 Jun 2024 10:55:20 +0100 Subject: [PATCH 35/47] Update README.md Co-authored-by: Samuel Jones --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b4031f0..55ff71b 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # fia-auth [![codecov](https://codecov.io/gh/fiaisis/fia-auth/graph/badge.svg?token=lzm0DS1jhG)](https://codecov.io/gh/fiaisis/fia-auth) -![License: GPL-3.0](https://img.shields.io/github/license/fiaisis/run-detection) +![License: GPL-3.0](https://img.shields.io/github/license/fiaisis/fia-auth) ![Build: passing](https://img.shields.io/github/actions/workflow/status/fiaisis/fia-auth/tests.yml?branch=main) [![Code style: ruff](https://img.shields.io/badge/code%20style-ruff-30173d)](https://docs.astral.sh/ruff/) [![linting: ruff](https://img.shields.io/badge/linting-ruff-30173d)](https://docs.astral.sh/ruff/) From e8a492c765cea12070c08ede1dbb10d3ea8558f4 Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Thu, 13 Jun 2024 10:58:32 +0100 Subject: [PATCH 36/47] test --- test/e2e/test_auth.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/test/e2e/test_auth.py b/test/e2e/test_auth.py index 25e6699..d6289d7 100644 --- a/test/e2e/test_auth.py +++ b/test/e2e/test_auth.py @@ -29,6 +29,12 @@ def test_unsuccessful_login(mock_post): mock_response.status_code = 401 + + + + + + response = client.post("/api/jwt/authenticate", json={"username": "foo", "password": "foo"}) assert response.status_code == 403 # noqa: PLR2004 @@ -87,9 +93,7 @@ def test_token_refresh_no_refresh_token_given(): def test_token_refresh_expired_refresh_token(): user = User(123) access_token = generate_access_token(user) - refresh_token = ( - "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE3MTgwMjEyNDB9.iHITGf2RyX_49pY7Xy8xdutYE4Pc6k9mfKWQjxCKgOk" # noqa: S105 - ) + refresh_token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE3MTgwMjEyNDB9.iHITGf2RyX_49pY7Xy8xdutYE4Pc6k9mfKWQjxCKgOk" # noqa: S105 response = client.post( "/api/jwt/refresh", json={"token": access_token.jwt}, cookies={"refresh_token": refresh_token} ) From 8336c92a501ea5ac52016c03b57cd20ca8e64434 Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Thu, 13 Jun 2024 10:58:56 +0100 Subject: [PATCH 37/47] test --- test/e2e/test_auth.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/e2e/test_auth.py b/test/e2e/test_auth.py index d6289d7..62a3534 100644 --- a/test/e2e/test_auth.py +++ b/test/e2e/test_auth.py @@ -29,12 +29,6 @@ def test_unsuccessful_login(mock_post): mock_response.status_code = 401 - - - - - - response = client.post("/api/jwt/authenticate", json={"username": "foo", "password": "foo"}) assert response.status_code == 403 # noqa: PLR2004 From f92b069521ffb8350231ebb4a87f732dc1a8cf8d Mon Sep 17 00:00:00 2001 From: github-actions Date: Thu, 13 Jun 2024 09:59:37 +0000 Subject: [PATCH 38/47] Formatting and linting commit --- test/e2e/test_auth.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/e2e/test_auth.py b/test/e2e/test_auth.py index 62a3534..25e6699 100644 --- a/test/e2e/test_auth.py +++ b/test/e2e/test_auth.py @@ -87,7 +87,9 @@ def test_token_refresh_no_refresh_token_given(): def test_token_refresh_expired_refresh_token(): user = User(123) access_token = generate_access_token(user) - refresh_token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE3MTgwMjEyNDB9.iHITGf2RyX_49pY7Xy8xdutYE4Pc6k9mfKWQjxCKgOk" # noqa: S105 + refresh_token = ( + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE3MTgwMjEyNDB9.iHITGf2RyX_49pY7Xy8xdutYE4Pc6k9mfKWQjxCKgOk" # noqa: S105 + ) response = client.post( "/api/jwt/refresh", json={"token": access_token.jwt}, cookies={"refresh_token": refresh_token} ) From 2c58df98a7ed03eb493cf635a7b24db1c7bfc4a1 Mon Sep 17 00:00:00 2001 From: keiranjprice101 <44777678+keiranjprice101@users.noreply.github.com> Date: Thu, 13 Jun 2024 11:04:02 +0100 Subject: [PATCH 39/47] Update pyproject.toml Co-authored-by: Samuel Jones --- pyproject.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 060ed10..4e0e600 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -19,7 +19,8 @@ dependencies = [ formatting = [ "ruff==0.4.8", "mypy==1.10.0", - "types-requests==2.32.0.20240602" + "types-requests==2.32.0.20240602", + "fia-api[test], ] From c3fc8461ce834f7aec3c58291b9cf6120026e5f0 Mon Sep 17 00:00:00 2001 From: Samuel Jones Date: Thu, 13 Jun 2024 11:05:23 +0100 Subject: [PATCH 40/47] Update pyproject.toml --- pyproject.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 4e0e600..cdbebfe 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,7 +20,7 @@ formatting = [ "ruff==0.4.8", "mypy==1.10.0", "types-requests==2.32.0.20240602", - "fia-api[test], + "fia-api[test]", ] @@ -78,4 +78,4 @@ ignore = [ ] [tool.ruff.lint.pylint] -max-args = 10 \ No newline at end of file +max-args = 10 From 141eabdb914aa96f33466cbfe5dc7a6928152a69 Mon Sep 17 00:00:00 2001 From: Samuel Jones Date: Thu, 13 Jun 2024 11:06:27 +0100 Subject: [PATCH 41/47] Update pyproject.toml --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index cdbebfe..6aeeff6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,7 +20,7 @@ formatting = [ "ruff==0.4.8", "mypy==1.10.0", "types-requests==2.32.0.20240602", - "fia-api[test]", + "fia-auth[test]", ] From db58018276e7d8f0936d1a76c0b016ac6a45c7f7 Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Thu, 13 Jun 2024 11:30:43 +0100 Subject: [PATCH 42/47] Use enum --- test/e2e/test_auth.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/test/e2e/test_auth.py b/test/e2e/test_auth.py index 25e6699..8c7ba55 100644 --- a/test/e2e/test_auth.py +++ b/test/e2e/test_auth.py @@ -1,3 +1,4 @@ +from http import HTTPStatus from unittest.mock import Mock, patch import jwt @@ -15,7 +16,7 @@ def test_successful_login(mock_post): mock_response = Mock() mock_post.return_value = mock_response - mock_response.status_code = 201 + mock_response.status_code = HTTPStatus.CREATED mock_response.json.return_value = {"userId": 1234} response = client.post("/api/jwt/authenticate", json={"username": "foo", "password": "foo"}) assert response.json()["token"].startswith("ey") @@ -27,10 +28,10 @@ def test_unsuccessful_login(mock_post): mock_response = Mock() mock_post.return_value = mock_response - mock_response.status_code = 401 + mock_response.status_code = HTTPStatus.UNAUTHORIZED response = client.post("/api/jwt/authenticate", json={"username": "foo", "password": "foo"}) - assert response.status_code == 403 # noqa: PLR2004 + assert response.status_code == HTTPStatus.FORBIDDEN @patch("src.auth.requests.post") @@ -38,10 +39,10 @@ def test_unsuccessful_login_uows_failure(mock_post): mock_response = Mock() mock_post.return_value = mock_response - mock_response.status_code = 500 + mock_response.status_code = HTTPStatus.UNAUTHORIZED.INTERNAL_SERVER_ERROR response = client.post("/api/jwt/authenticate", json={"username": "foo", "password": "foo"}) - assert response.status_code == 403 # noqa: PLR2004 + assert response.status_code == HTTPStatus.FORBIDDEN def test_verify_success(): @@ -49,17 +50,17 @@ def test_verify_success(): access_token = generate_access_token(user) response = client.post("/api/jwt/checkToken", json={"token": access_token.jwt}) - assert response.status_code == 200 # noqa: PLR2004 + assert response.status_code == HTTPStatus.OK def test_verify_fail_badly_formed_token(): response = client.post("/api/jwt/checkToken", json={"token": "foo"}) - assert response.status_code == 403 # noqa: PLR2004 + assert response.status_code == HTTPStatus.FORBIDDEN def test_verify_fail_bad_signature(): response = client.post("/api/jwt/checkToken", json={"token": jwt.encode({"foo": "bar"}, key="foo")}) - assert response.status_code == 403 # noqa: PLR2004 + assert response.status_code == HTTPStatus.FORBIDDEN def test_token_refresh_success(): @@ -81,17 +82,15 @@ def test_token_refresh_no_refresh_token_given(): json={"token": access_token.jwt}, ) - assert response.status_code == 403 # noqa: PLR2004 + assert response.status_code == HTTPStatus.FORBIDDEN def test_token_refresh_expired_refresh_token(): user = User(123) access_token = generate_access_token(user) - refresh_token = ( - "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE3MTgwMjEyNDB9.iHITGf2RyX_49pY7Xy8xdutYE4Pc6k9mfKWQjxCKgOk" # noqa: S105 - ) + refresh_token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE3MTgwMjEyNDB9.iHITGf2RyX_49pY7Xy8xdutYE4Pc6k9mfKWQjxCKgOk" # noqa: S105 response = client.post( "/api/jwt/refresh", json={"token": access_token.jwt}, cookies={"refresh_token": refresh_token} ) - assert response.status_code == 403 # noqa: PLR2004 + assert response.status_code == HTTPStatus.FORBIDDEN From 077f7f692bd1a74c70e390f47d11a4319ca5792c Mon Sep 17 00:00:00 2001 From: github-actions Date: Thu, 13 Jun 2024 10:31:28 +0000 Subject: [PATCH 43/47] Formatting and linting commit --- test/e2e/test_auth.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/e2e/test_auth.py b/test/e2e/test_auth.py index 8c7ba55..684b856 100644 --- a/test/e2e/test_auth.py +++ b/test/e2e/test_auth.py @@ -88,7 +88,9 @@ def test_token_refresh_no_refresh_token_given(): def test_token_refresh_expired_refresh_token(): user = User(123) access_token = generate_access_token(user) - refresh_token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE3MTgwMjEyNDB9.iHITGf2RyX_49pY7Xy8xdutYE4Pc6k9mfKWQjxCKgOk" # noqa: S105 + refresh_token = ( + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE3MTgwMjEyNDB9.iHITGf2RyX_49pY7Xy8xdutYE4Pc6k9mfKWQjxCKgOk" # noqa: S105 + ) response = client.post( "/api/jwt/refresh", json={"token": access_token.jwt}, cookies={"refresh_token": refresh_token} ) From 712b1138a3cef5673a4b7dae54f2298c24fd44ea Mon Sep 17 00:00:00 2001 From: keiranjprice101 <44777678+keiranjprice101@users.noreply.github.com> Date: Thu, 13 Jun 2024 11:32:35 +0100 Subject: [PATCH 44/47] Update test/test_auth.py Co-authored-by: Samuel Jones --- test/test_auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_auth.py b/test/test_auth.py index 9c183e5..45876a6 100644 --- a/test/test_auth.py +++ b/test/test_auth.py @@ -41,7 +41,7 @@ def test_authenticate_bad_credentials(mock_post): @patch("requests.post") def test_authenticate_unexpected_error(mock_post): - mock_response = Mock(status_code=500, json=lambda: {"error": "Server error"}) + mock_response = Mock(status_code=HTTPStatus.INTERNAL_SERVER_ERROR, json=lambda: {"error": "Server error"}) mock_post.return_value = mock_response credentials = UserCredentials(username="user", password="password") # noqa: S106 From 0ba391819b3be10527a2bc940b071ff4027e42f0 Mon Sep 17 00:00:00 2001 From: Keiran Price Date: Thu, 13 Jun 2024 11:35:38 +0100 Subject: [PATCH 45/47] Add missing import --- test/test_auth.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_auth.py b/test/test_auth.py index 45876a6..d543f16 100644 --- a/test/test_auth.py +++ b/test/test_auth.py @@ -1,3 +1,4 @@ +from http import HTTPStatus from unittest.mock import Mock, patch import pytest From 154964b77e21337b9bd606a990cf921bd5ac772c Mon Sep 17 00:00:00 2001 From: keiranjprice101 <44777678+keiranjprice101@users.noreply.github.com> Date: Thu, 13 Jun 2024 11:40:20 +0100 Subject: [PATCH 46/47] Update test/test_auth.py Co-authored-by: Samuel Jones --- test/test_auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_auth.py b/test/test_auth.py index d543f16..3cc6a38 100644 --- a/test/test_auth.py +++ b/test/test_auth.py @@ -29,7 +29,7 @@ def test_authenticate_success(mock_post): @patch("requests.post") def test_authenticate_bad_credentials(mock_post): - mock_response = Mock(status_code=401, json=lambda: {}) + mock_response = Mock(status_code=HTTPStatus.UNAUTHORIZED, json=lambda: {}) mock_post.return_value = mock_response credentials = UserCredentials(username="invalid_user", password="invalid_password") # noqa: S106 From 029ed5c86e1cf73b3c40ba3e651f9b207db9e92d Mon Sep 17 00:00:00 2001 From: keiranjprice101 <44777678+keiranjprice101@users.noreply.github.com> Date: Thu, 13 Jun 2024 11:40:28 +0100 Subject: [PATCH 47/47] Update test/test_auth.py Co-authored-by: Samuel Jones --- test/test_auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_auth.py b/test/test_auth.py index 3cc6a38..74e668d 100644 --- a/test/test_auth.py +++ b/test/test_auth.py @@ -10,7 +10,7 @@ @patch("requests.post") def test_authenticate_success(mock_post): - mock_response = Mock(status_code=201, json=lambda: {"userId": "12345"}) + mock_response = Mock(status_code=HTTPStatus.CREATED, json=lambda: {"userId": "12345"}) mock_post.return_value = mock_response credentials = UserCredentials(username="valid_user", password="valid_password") # noqa: S106