stac-fastapi-elasticsearch-opensearch icon indicating copy to clipboard operation
stac-fastapi-elasticsearch-opensearch copied to clipboard

Adding route dependencies module.

Open rhysrevans3 opened this issue 1 year ago • 7 comments

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:

  • [x] Code is formatted and linted (run pre-commit run --all-files)
  • [ ] Tests pass (run make test)
  • [ ] Documentation has been updated to reflect changes, if applicable
  • [ ] Changes are added to the changelog

rhysrevans3 avatar May 10 '24 07:05 rhysrevans3

@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?

rhysrevans3 avatar May 13 '24 13:05 rhysrevans3

@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!

jonhealy1 avatar May 13 '24 16:05 jonhealy1

@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.

pedro-cf avatar May 13 '24 16:05 pedro-cf

@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

rhysrevans3 avatar May 14 '24 14:05 rhysrevans3

I definitely like the idea of supporting OAuth2.

jonhealy1 avatar May 15 '24 03:05 jonhealy1

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.

jonhealy1 avatar May 17 '24 10:05 jonhealy1

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 avatar May 17 '24 14:05 rhysrevans3

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 avatar Jun 10 '24 10:06 rhysrevans3

@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 avatar Jun 13 '24 11:06 rhysrevans3

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.

rhysrevans3 avatar Jun 18 '24 09:06 rhysrevans3

Nice changelog updates thanks!

jonhealy1 avatar Jun 21 '24 16:06 jonhealy1

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

jonhealy1 avatar Jun 22 '24 03:06 jonhealy1

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

jonhealy1 avatar Jun 22 '24 04:06 jonhealy1

@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.

pedro-cf avatar Jun 22 '24 20:06 pedro-cf

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

jonhealy1 avatar Jun 23 '24 03:06 jonhealy1

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

@jonhealy1

  • Would it make sense to simply remove the section? or Add a description for every file? @rhysrevans3 could perhaps help with this?
  • Also would it make sense to move the docker-compose files to the examples directory?

pedro-cf avatar Jun 23 '24 13:06 pedro-cf

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

jonhealy1 avatar Jun 23 '24 14:06 jonhealy1