tiled icon indicating copy to clipboard operation
tiled copied to clipboard

Epic: Refactor authentication and authorization

Open DiamondJoseph opened this issue 11 months ago • 1 comments

From discussion earlier in the week, I think our ideal state would be one where we have two protocols or abstract base classes that handle all of the logic currently expressed in various Dependencies on the FastAPI app. One Authenticator and one Authorizer, which are then configured in the Settings object on the server configuration, are passed into router construction as dependencies that will discover who a user is (if required) and what a user can do (if required).

class Authenticator(ABC):
    @abstractmethod
    def authenticate(request: Request) -> UserSessionState:
        ...

class APIAuthenticator(Authenticator):
    def authenticate(request: Request) -> UserSessionState:
        api_key = get_api_key(request)
        if not api_key:
            raise NotAuthenticatedError("No API key in request")
        if api_Key not in known_users:
            raise NotAuthenticatedError("API key not recognised")
        return UserSessionState(known_users[api_key])

class OIDCAuthenticator(Authenticator):
    def authenticate(request: Request) -> UserSessionState:
        jwt = self.jwt_client.decode(request.headers["Authorization"])
        return UserSessionState(jwt["sub"])
class Authorizer(ABC):
    @abstractmethod
    def authorize(<unknown signature>) -> bool:
        ...

class ExternalAuthorizer(Authorizer):
    def authorize(<unknown signature>) -> bool:
        return self.external_service.authorize(foo, bar)

DiamondJoseph avatar Feb 14 '25 13:02 DiamondJoseph

I like this. Two thoughts:

  1. We currently support multiple IdPs. Everyone who I've discussed it with seems to agree that reducing it to one (which may in turn support N if you like) is a good idea. But it's a sweeping change that will have to be done carefully.
  2. We can "Authorizer" AccesPolicy and it's in tiled.access_policy. Work is in progress (by @nmaytan) to make these a bit nicer.

danielballan avatar Feb 14 '25 20:02 danielballan