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
24 stars 14 forks source link

Adding route dependencies module. #251

Closed rhysrevans3 closed 1 week ago

rhysrevans3 commented 1 month ago

Description:

Generate a list of route dependencies from the environment variable STAC_FASTAPI_ROUTE_DEPENDENCIES or the file that this environment variable points to. Feed these route dependencies into the STACApi on initialisation.

PR Checklist:

rhysrevans3 commented 1 month ago

@jonhealy1 sorry I've been at a conference so have only just caught up on the authentication work. The basic auth looks great. Am I right in saying that all endpoints are now closed unless they're included in the public_endpoints list in the BASIC_AUTH environment variable?

jonhealy1 commented 1 month ago

@pedro-cf Hi Pedro. Are you able to look at this pr and respond to @rhysrevans3? I think you would do a better job with this. Thanks!

pedro-cf commented 1 month ago

@jonhealy1 sorry I've been at a conference so have only just caught up on the authentication work. The basic auth looks great. Am I right in saying that all endpoints are now closed unless they're included in the public_endpoints list in the BASIC_AUTH environment variable?

Greetings,

if BASIC_AUTH is enabled the endpoints outside of public_endpoints will be protected. BASIC_AUTH is optional and off by default. You enable it by setting the BASIC_AUTH env variable with your desired configuration.

rhysrevans3 commented 1 month ago

@jonhealy1 @pedro-cf thanks both I think I understand now. Could I get your opinion on this pull request as it also deals with auth. Are you happy that this and basic auth can co-exist? Or would you like me to try and merge the two? I currently don't have a way to protect all endpoints without explicitly writing them all out. You could add wildcards but this would probably need changes to https://github.com/stac-utils/stac-fastapi

jonhealy1 commented 1 month ago

I definitely like the idea of supporting OAuth2.

jonhealy1 commented 1 month ago

Wondering about the best way to integrate this with basic auth? This will need instructions added to the readme as well and a docker-compose demo file I think.

rhysrevans3 commented 1 month ago

I've been thinking the same thing I will add instructions to the readme and a docker-compose demo file. They do currently work together but it might be confusing to have both in their current implementation.

I could update basic auth to use this pull requests methods but it would change the config and would remove the * option. I could add the star option to this pull request but that would require changes to stac-fastapi.

another thought I had was this would fit better in the stac-fastapi so it can be used in all of the backends?

rhysrevans3 commented 3 weeks ago

I've made a pull request in stac-fastapi that would allow for default route dependencies to be added. This would make the merging of basic auth and this pull request much simpler.

rhysrevans3 commented 3 weeks ago

@jonhealy1 @pedro-cf I've updated basic auth to use the route dependencies method. The configuration for basic auth has changed.

Basic auth can now be used as a dependency within route dependencies.

The example in the readme now looks like:

[
  {
    "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"
            }
          ]
        }
      }
    ]
  }
]

I've also removed public_endpoints. The default is for all endpoints to be public and * can be used for a route's path and/or method to add a default dependency.

rhysrevans3 commented 2 weeks ago

Good work, I would suggest:

  • add jsonschema validation for the route_dependencies.json
  • add real complete working examples route_dependencies.json

I've add some jsonschema validation for route_dependencies. And added a second docker compose file that use example/route_dependencies/route_dependencies.json and gives an example of an Oauth2 route dependency.

jonhealy1 commented 1 week ago

Nice changelog updates thanks!

jonhealy1 commented 1 week ago

@rhysrevans3 I moved the changelog entries to the Unreleased section in the changelog.

jonhealy1 commented 1 week ago

Once we release a new version here and on PyPI then we move the changelog entries to the appropriate section which will hopefully be v3.0.0. Waiting for stac-fastapi v3.0.0

pedro-cf commented 1 week ago

@rhysrevans3 didn't notice but the README needs some correcting:

Docker Compose Configurations See docker-compose.basic_auth_protected.yml and docker-compose.basic_auth_public.yml for basic authentication configurations.

jonhealy1 commented 1 week ago

@pedro-cf Can you open up a new pr to fix this? Good catch.

pedro-cf commented 1 week ago

@pedro-cf Can you open up a new pr to fix this? Good catch.

@jonhealy1

jonhealy1 commented 1 week ago

It does make some sense to move them to the examples folder I think.