Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Epic: Refactor authentication and authorization #887

Open
DiamondJoseph opened this issue Feb 14, 2025 · 1 comment
Open

Epic: Refactor authentication and authorization #887

DiamondJoseph opened this issue Feb 14, 2025 · 1 comment

Comments

@DiamondJoseph
Copy link
Contributor

DiamondJoseph commented Feb 14, 2025

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)
@danielballan
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants