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

Revisiting custom authentication. #3383

Open
tomchristie opened this issue Oct 30, 2024 · 1 comment
Open

Revisiting custom authentication. #3383

tomchristie opened this issue Oct 30, 2024 · 1 comment
Labels
api change PRs that contain breaking public API changes

Comments

@tomchristie
Copy link
Member

tomchristie commented Oct 30, 2024

Let's have a go at simplifying our custom authentication API.

We have an existing API using generators and an "auth_flow". (Fantastic at the time, tho now the codebase has matured, I think? can be simplified.)

I assume the following base API would be sufficient for almost all authentication use-cases...

class Auth:
    def authenticate_request(request: Request) -> Request:
        # Most authentication schemes only need to override this method.
        return request 

    def authenticate_response(response: Response) -> Request | None:
        # Challenge-response authentication schemes may override this method,
        # Allows a second request to optionally be made, once a server challenge is received.
        return None

It's feasible that there are might(???) be some exceptional cases where this might not be sufficient, but we have a "Transport API" that allows completely customising the entire request/response implementation. That'd be adequate for anyone needing to implement an oddball multi-stage authentication scheme.

Moderately involved, tho likely still suitable for a new contributor to deal with.

Checklist...

  • Update the base Auth class as above.
  • Update the BasicAuth, DigestAuth and NetRCAuth classes to use the new API.
  • Update the auth handling in _client.py.
  • Update the documentation.
  • Update tests as required.

Simplicity ftw.

@tomchristie tomchristie added the api change PRs that contain breaking public API changes label Oct 30, 2024
@tomchristie
Copy link
Member Author

(Some potentially subtleties around dealing with async cases, let's talk those through as we get there.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api change PRs that contain breaking public API changes
Projects
None yet
Development

No branches or pull requests

1 participant