-
Notifications
You must be signed in to change notification settings - Fork 18
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
Basic Authentication not working as intended #310
Comments
any chance you could take a look at this @rhysrevans3 ? |
Happy to have a look! |
@rhysrevans3 |
@pedro-cf I've figure out the cause of the issue. As the dependencies (one for admin and one for reader) are separate they both get run and if either fail then the user is denied access. Because they don't know about each other they will fail on any endpoint where both are active. One solution is to not use I think a better solution for this is to handle security dependencies separately to other dependencies and have a main security dependency that can check that at least one has a passed. |
If possible can you share a working example for this solution? for the standard admin + reader user setup |
Here's an example where only
Hope that helps. |
Greetings @rhysrevans3 I was testing this setup. I noticed you set the credentials for reader twice: but even after removing the second one I am still getting some issues Reader Request: curl --request GET \
--url http://localhost:8084/ \
--header 'Authorization: Basic cmVhZGVyOnJlYWRlcg==' Reader Response: {
....
} Admin Request: curl --request GET \
--url http://localhost:8084/ \
--header 'Authorization: Basic YWRtaW46YWRtaW4=' Admin Response: {
"detail": "Incorrect username or password"
} |
Additionally if I add:
to the admin's routes, then both users cannot access "/" and both get the response: {
"detail": "Incorrect username or password"
} |
Hi @pedro-cf sorry I probably wasn't very clear with my example. The two route dependencies in my example weren't one for admin and one for reader. It was one for admin only:
and one for admin and reader:
That's why there are two sets of credentials in the second route dependency one for reader and one for admin. So any routes added to the first route dependency will be admin only and any added to the second will be accessible by both admin and reader. The reason for this is FastAPI runs all of the dependencies for an endpoint and will raise an error if any fail. So you can only have one Auth dependency per endpoint else they will clash. Hope this clarifies things 😄 |
Describe the bug
The Basic Authentication implementation in stac-fastapi-elasticsearch is not working as expected. When configuring multiple user credentials through STAC_FASTAPI_ROUTE_DEPENDENCIES, the authentication fails even with valid credentials.
To Reproduce
Steps to reproduce the behavior:
- STAC_FASTAPI_ROUTE_DEPENDENCIES=[{"routes":[{"method":"*","path":"*"}],"dependencies":[{"method":"stac_fastapi.core.basic_auth.BasicAuth","kwargs":{"credentials":[{"username":"admin","password":"admin"}]}}]},{"routes":[{"path":"/","method":["GET"]},{"path":"/conformance","method":["GET"]},{"path":"/collections/{collection_id}/items/{item_id}","method":["GET"]},{"path":"/search","method":["GET","POST"]},{"path":"/collections","method":["GET"]},{"path":"/collections/{collection_id}","method":["GET"]},{"path":"/collections/{collection_id}/items","method":["GET"]},{"path":"/queryables","method":["GET"]},{"path":"/queryables/collections/{collection_id}/queryables","method":["GET"]},{"path":"/_mgmt/ping","method":["GET"]}],"dependencies":[{"method":"stac_fastapi.core.basic_auth.BasicAuth","kwargs":{"credentials":[{"username":"reader","password":"reader"}]}}]}]
curl --request GET \ --url http://localhost:8080/ \ --header 'Authorization: Basic YWRtaW46YWRtaW4='
curl --request GET \ --url http://localhost:8080/ \ --header 'Authorization: Basic cmVhZGVyOnJlYWRlcg=='
Expected behavior
Environment:
The text was updated successfully, but these errors were encountered: