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

Add simple Basic auth #1203

Merged
merged 4 commits into from
Nov 12, 2023
Merged

Conversation

lopagela
Copy link
Contributor

@lopagela lopagela commented Nov 10, 2023

To enable the basic authentication, one must set server.auth.enabled to true.

The static string defined in server.auth.secret must be set in the header Authorization.

The health check endpoint will always be accessible, no matter the API auth configuration.


The authenticated "bean"/"method-in-the-chain" is declared as a dependency in the API router that requires authenticated connections. c.f. faspAPI documentation for details on dependencies: https://fastapi.tiangolo.com/tutorial/bigger-applications/

Security section of the fastAPI is also a good read: https://fastapi.tiangolo.com/tutorial/security/

@lopagela lopagela requested a review from imartinez November 10, 2023 17:50
imartinez
imartinez previously approved these changes Nov 11, 2023
Copy link
Collaborator

@imartinez imartinez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! We me be overusing the
if not settings.server.auth.enabled:
check in auth.py, but I think it makes it safer for future refactors.

As a general note, we should be documenting this (also the previous CORS support) in docs/description.md and then executing make api-docs to generate the udpated version of the api docs github pages, then commit those changes.

To enable the basic authentication, one must set `server.auth.enabled`
to true.

The static string defined in `server.auth.secret` must be set in the
header `Authorization`.

The health check endpoint will always be accessible, no matter the API
auth configuration.
Had to disable mypy in the `auth` as we are not using the same signature
for the authenticated method.

mypy was complaining that the signatures of `authenticated` must be
identical, no matter in which logical branch we are.
Given that fastapi is accomodating itself of method signatures (it will
inject the dependencies in the method call), this warning of mypy is
actually preventing us to do something legit.

mypy doc: https://mypy.readthedocs.io/en/stable/common_issues.html
@lopagela lopagela merged commit aa70d3d into zylon-ai:main Nov 12, 2023
6 checks passed
@lopagela lopagela deleted the add-simple-optional-auth branch November 12, 2023 18:05
simonbermudez pushed a commit to simonbermudez/saimon that referenced this pull request Feb 24, 2024
* Add simple Basic auth

To enable the basic authentication, one must set `server.auth.enabled`
to true.

The static string defined in `server.auth.secret` must be set in the
header `Authorization`.

The health check endpoint will always be accessible, no matter the API
auth configuration.

* Fix linting and type check

* Fighting with mypy being too restrictive

Had to disable mypy in the `auth` as we are not using the same signature
for the authenticated method.

mypy was complaining that the signatures of `authenticated` must be
identical, no matter in which logical branch we are.
Given that fastapi is accomodating itself of method signatures (it will
inject the dependencies in the method call), this warning of mypy is
actually preventing us to do something legit.

mypy doc: https://mypy.readthedocs.io/en/stable/common_issues.html

* Write tests to verify that the simple auth is working
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

Successfully merging this pull request may close these issues.

3 participants