Add inst scientist auth (#92)

* Add AD checks for instrument scientists

* Formatting and linting commit

* Trial fixes for e2e

* Add is_instrument_scientist patchs to e2e tests

* Add docstring

* Add docstring checks to ruff

* DO a whole bunch of docs fixes.

* DO a whole bunch of docs fixes.

* DO a whole bunch of docs fixes.

* Remove support for D100 ruff linter

* Formatting and linting commit

---------

Co-authored-by: github-actions <[email protected]>

fia_auth/auth.py

@@ -1,6 +1,4 @@
-"""
-Module containing code to authenticate with the UOWS
-"""
+"""Module containing code to authenticate with the UOWS"""
 
 import logging
 import os

fia_auth/db.py

@@ -1,6 +1,4 @@
-"""
-DB Access moculde
-"""
+"""DB Access moculde"""
 
 import logging
 import os
@@ -13,17 +11,15 @@
 
 
 class Base(DeclarativeBase):
-    """
-    SQLAlchemy Base Model
-    """
+
+    """SQLAlchemy Base Model"""
 
     id: Mapped[int] = mapped_column(primary_key=True)
 
 
 class Staff(Base):
-    """
-    Staff user
-    """
+
+    """Staff user"""
 
     __tablename__ = "staff"
     user_number: Mapped[int] = mapped_column(Integer())
@@ -47,7 +43,6 @@ def is_staff_user(user_number: int) -> bool:
     :param user_number: The user number to check
     :return: boolean indicating if it is a staff
     """
-
     try:
         with SESSION() as session:
             session.execute(select(Staff.user_number).where(Staff.user_number == user_number)).one()

fia_auth/exception_handlers.py

@@ -1,3 +1,5 @@
+"""Error handlers"""
+
 import logging
 from http import HTTPStatus
 

fia_auth/exceptions.py

@@ -1,23 +1,26 @@
-"""
-FIA Auth custom exceptions
-"""
+"""FIA Auth custom exceptions"""
 
 
 class UOWSError(Exception):
+
     """Problem authenticating with the user office web service"""
 
 
 class ProposalAllocationsError(Exception):
+
     """Problem connecting with the proposal allocations api"""
 
 
 class AuthenticationError(Exception):
+
     """Problem with authentication mechanism"""
 
 
 class BadCredentialsError(AuthenticationError):
-    """ "Bad Credentials Provided"""
+
+    """Bad Credentials Provided"""
 
 
 class BadJWTError(AuthenticationError):
+
     """Raised when a bad jwt has been given to the service"""

fia_auth/experiments.py

@@ -1,6 +1,4 @@
-"""
-Module for dealing with experiment user data via proposal allocations API
-"""
+"""Module for dealing with experiment user data via proposal allocations API"""
 
 import logging
 import os

fia_auth/fia_auth.py

@@ -1,6 +1,4 @@
-"""
-Module containing the fast api app. Uvicorn loads this to start the api
-"""
+"""Module containing the fast api app. Uvicorn loads this to start the api"""
 
 from fastapi import FastAPI
 from starlette.middleware.cors import CORSMiddleware

fia_auth/model.py

@@ -1,38 +1,34 @@
-"""
-Internal Models to help abstract and encapsulate the authentication process
-"""
+"""Internal Models to help abstract and encapsulate the authentication process"""
 
 import enum
 from dataclasses import dataclass
 
 from pydantic import BaseModel
 
 from fia_auth.db import is_staff_user
+from fia_auth.roles import is_instrument_scientist
 
 
 class UserCredentials(BaseModel):
-    """
-    Pydantic model for user credentials. Allows FastAPI to validate the object recieved in the login endpoint
-    """
+
+    """Pydantic model for user credentials. Allows FastAPI to validate the object recieved in the login endpoint"""
 
     username: str
     password: str
 
 
 class Role(enum.Enum):
-    """
-    Role Enum to differentiate between user and staff. It is assumed staff will see all data
-    """
+
+    """Role Enum to differentiate between user and staff. It is assumed staff will see all data"""
 
     STAFF = "staff"
     USER = "user"
 
 
 @dataclass
 class User:
-    """
-    Internal User Model for packing JWTs
-    """
+
+    """Internal User Model for packing JWTs"""
 
     user_number: int
 
@@ -42,6 +38,6 @@ def role(self) -> Role:
         Determine and determine the role of the user based on their usernumber
         :return:
         """
-        if is_staff_user(self.user_number):
+        if is_staff_user(self.user_number) or is_instrument_scientist(self.user_number):
             return Role.STAFF
         return Role.USER

fia_auth/roles.py

@@ -0,0 +1,28 @@
+"""Functions for handling role checks"""
+
+import os
+from http import HTTPStatus
+
+import requests
+
+
+def is_instrument_scientist(user_number: int) -> bool:
+    """
+    Check if the user number is an instrument scientist according to UOWs (User Office Web Service).
+    :param user_number: The user number assigned to each user from UOWs
+    :return: True if the user number is an instrument scientist, false if not or failed connection.
+    """
+    uows_url = os.environ.get("UOWS_URL", "https://devapi.facilities.rl.ac.uk/users-service")
+    uows_api_key = os.environ.get("UOWS_API_KEY", "")
+    response = requests.get(
+        url=f"{uows_url}/v1/role/{user_number}",
+        headers={"Authorization": f"Api-key {uows_api_key}", "accept": "application/json"},
+        timeout=1,
+    )
+    if response.status_code != HTTPStatus.OK:
+        from fia_auth.auth import logger
+
+        logger.info("User number %s is not an instrument scientist or UOWS API is down", user_number)
+        return False
+    roles = response.json()
+    return {"name": "ISIS Instrument Scientist"} in roles

fia_auth/routers.py

@@ -1,6 +1,4 @@
-"""
-Module containing the fastapi routers
-"""
+"""Module containing the fastapi routers"""
 
 from __future__ import annotations
 
@@ -31,7 +29,7 @@
 async def get_experiments(
     user_number: int, credentials: Annotated[HTTPAuthorizationCredentials, Depends(security)]
 ) -> list[int]:
-    """
+    r"""
     Get the experiment (RB) numbers for the given user number provided by query string parameter
 
     \f
@@ -46,7 +44,7 @@ async def get_experiments(
 
 @ROUTER.post("/api/jwt/authenticate", tags=["auth"])
 async def login(credentials: UserCredentials) -> JSONResponse:
-    """
+    r"""
     Login with facilities account
     \f
     :param credentials: username and password
@@ -74,7 +72,7 @@ async def login(credentials: UserCredentials) -> JSONResponse:
 
 @ROUTER.post("/api/jwt/checkToken")
 def verify(token: dict[str, Any]) -> Literal["ok"]:
-    """
+    r"""
     Verify an access token was generated by this auth server and has not expired
     \f
     :param token: The JWT
@@ -90,7 +88,7 @@ def verify(token: dict[str, Any]) -> Literal["ok"]:
 def refresh(
     body: dict[str, Any], refresh_token: Annotated[str | None, Cookie(alias="refresh_token")] = None
 ) -> JSONResponse:
-    """
+    r"""
     Refresh an access token based on a refresh token
     \f
     :param refresh_token:

fia_auth/tokens.py

@@ -1,6 +1,4 @@
-"""
-Module containing token classes, creation, and loading functions
-"""
+"""Module containing token classes, creation, and loading functions"""
 
 from __future__ import annotations
 
@@ -25,15 +23,14 @@
 
 
 class Token(ABC):
-    """
-    Abstract token class defines verify method
-    """
+
+    """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
+        Verify the token, ensuring that it has a valid format, signature, and has not expired. Will raise a
         BadJWTError if verification fails
         :return: None
         """
@@ -65,11 +62,15 @@ def _encode(self) -> None:
 
 
 class AccessToken(Token):
-    """
-    Access Token is a short-lived (5 minute) token that stores user information
-    """
+
+    """Access Token is a short-lived (5 minute) token that stores user information"""
 
     def __init__(self, jwt_token: str | None = None, payload: dict[str, Any] | None = None) -> None:
+        """
+        Create AccessToken, requires jwt_token XOR a payload
+        :param jwt_token: JWT to populate the payload if JWT provided and no payload.
+        :param payload: Payload to encode if no JWT provided
+        """
         if payload and not jwt_token:
             self._payload = payload
             self._payload["exp"] = datetime.now(UTC) + timedelta(minutes=float(ACCESS_TOKEN_LIFETIME_MINUTES))
@@ -99,11 +100,15 @@ def refresh(self) -> None:
 
 
 class RefreshToken(Token):
-    """
-    Refresh token is a long-lived (12 hour) token that is required to refresh an access token
-    """
+
+    """Refresh token is a long-lived (12 hour) token that is required to refresh an access token"""
 
     def __init__(self, jwt_token: str | None = None) -> None:
+        """
+        Create the RefreshToken
+        :param jwt_token: Optionally provide the jwt_token to create the payload else construct payload using default
+        method.
+        """
         if jwt_token is None:
             self._payload = {"exp": datetime.now(UTC) + timedelta(hours=12)}
             self._encode()

pyproject.toml

@@ -11,7 +11,8 @@ dependencies = [
     "psycopg2==2.9.9",
     "PyJWT==2.8.0",
     "SQLAlchemy==2.0.30",
-    "uvicorn==0.30.1"
+    "uvicorn==0.30.1",
+    "requests==2.32.3"
 ]
 
 [project.urls]
@@ -45,6 +46,7 @@ select = [
     "F",     # flake8 - Basic initial rules
     "E",     # pycodestyle (Error) - pep8 compliance
     "W",     # pycodestyle (Warning) - pep8 compliance
+    "D",     # pydocstyle - Enforce docstrings present
     "C90",   # mccabe - flags extremely complex functions
     "I",     # isort - Sort imports and flag missing imports
     "N",     # pep8-naming - Ensures pep8 compliance for naming
@@ -77,6 +79,8 @@ ignore = [
     "S101",    # flake8-bandit - Use of assert (all over pytest tests)
     "ISC001",  # Conflicts with the formatter
     "COM812",  # Conflicts with the formatter
+    "D211",    # Conflicts with it's own rules: D203
+    "D104", "D205", "D212", "D400", "D415" # Overzealous docstring checker
 ]
 
 [tool.ruff.lint.pylint]

test/e2e/conftest.py

@@ -1,6 +1,4 @@
-"""
-e2e session scoped fixtures
-"""
+"""e2e session scoped fixtures"""
 
 import pytest
 
@@ -9,9 +7,6 @@
 
 @pytest.fixture(scope="session", autouse=True)
 def _setup():
-    """
-    Setup database pre-testing
-    :return:
-    """
+    """Set up database pre-testing"""
     Base.metadata.drop_all(ENGINE)
     Base.metadata.create_all(ENGINE)