stac-utils / stac-fastapi-elasticsearch-opensearch

Elasticsearch backend for stac-fastapi with Opensearch support.
https://stac-utils.github.io/stac-fastapi-elasticsearch-opensearch
MIT License
30 stars 15 forks source link

Basic Authentication not working as intended #310

Open pedro-cf opened 2 weeks ago

pedro-cf commented 2 weeks ago

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:

  1. Configure basic auth with multiple users in docker-compose.yml:

    - 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"}]}}]}]
  2. Start the stac-fastapi-elasticsearch service with basic auth configuration:

    docker-compose up -d
  3. Try to access the root endpoint with valid credentials:

curl --request GET \
  --url http://localhost:8080/ \
  --header 'Authorization: Basic YWRtaW46YWRtaW4='
curl --request GET \
  --url http://localhost:8080/ \
  --header 'Authorization: Basic cmVhZGVyOnJlYWRlcg=='
  1. Receive error response:
    {"detail":"Incorrect username or password"}

Expected behavior

Environment:

pedro-cf commented 2 weeks ago

any chance you could take a look at this @rhysrevans3 ?

rhysrevans3 commented 2 weeks ago

Happy to have a look!

pedro-cf commented 1 week ago

Happy to have a look!

@rhysrevans3 Additional context: It works fine with a single user.

rhysrevans3 commented 1 week ago

@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 "*" and add both credentials to any endpoints that need them. I'll add a note to the auth readme to explain this.

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.

pedro-cf commented 1 week ago

One solution is to not use "*" and add both credentials to any endpoints that need them. I'll add a note to the auth readme to explain this.

If possible can you share a working example for this solution? for the standard admin + reader user setup

rhysrevans3 commented 1 week ago

Here's an example where only admin has access to the transaction endpoints, both admin and reader have access to the second set of routes, and all unspecified routes are public.

- STAC_FASTAPI_ROUTE_DEPENDENCIES=[{"routes":[{"path":"/collections", "method":["POST"]},{"path":"/collections/{collection_id}", "method":["DELETE", "PUT"]},{"path":"/collections/{collection_id}/items", "method":[ "POST"]}, {"path":"/collections/{collection_id}/items/{item_id}", "method":["DELETE", "PUT"]}], "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"}, {"username":"admin","password":"admin"}]}}]}]

Hope that helps.

pedro-cf commented 3 days ago

Here's an example where only admin has access to the transaction endpoints, both admin and reader have access to the second set of routes, and all unspecified routes are public.

- STAC_FASTAPI_ROUTE_DEPENDENCIES=[{"routes":[{"path":"/collections", "method":["POST"]},{"path":"/collections/{collection_id}", "method":["DELETE", "PUT"]},{"path":"/collections/{collection_id}/items", "method":[ "POST"]}, {"path":"/collections/{collection_id}/items/{item_id}", "method":["DELETE", "PUT"]}], "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"}, {"username":"admin","password":"admin"}]}}]}]

Hope that helps.

Greetings @rhysrevans3 I was testing this setup.

I noticed you set the credentials for reader twice:

image

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: 200 OK

{
....
}

Admin Request:

curl --request GET \
  --url http://localhost:8084/ \
  --header 'Authorization: Basic YWRtaW46YWRtaW4='

Admin Response:

{
    "detail": "Incorrect username or password"
}
pedro-cf commented 3 days ago

Additionally if I add:

{
   "path":"/",
   "method":[
      "GET"
   ]
}

to the admin's routes, then both users cannot access "/" and both get the response:

{
    "detail": "Incorrect username or password"
}
rhysrevans3 commented 2 days ago

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:

{"routes":[{"path":"/collections", "method":["POST"]},{"path":"/collections/{collection_id}", "method":["DELETE", "PUT"]},{"path":"/collections/{collection_id}/items", "method":[ "POST"]}, {"path":"/collections/{collection_id}/items/{item_id}", "method":["DELETE", "PUT"]}], "dependencies":[{"method":"stac_fastapi.core.basic_auth.BasicAuth","kwargs":{"credentials":[{"username":"admin","password":"admin"}]}}]}

and one for admin and reader:

{"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"}, {"username":"admin","password":"admin"}]}}]}

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 😄