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

Adding route dependencies module. #251

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

rhysrevans3
Copy link
Contributor

@rhysrevans3 rhysrevans3 commented May 10, 2024

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:

  • 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
Copy link
Contributor Author

@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
Copy link
Collaborator

@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
Copy link
Collaborator

pedro-cf commented May 13, 2024

@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
Copy link
Contributor Author

rhysrevans3 commented May 14, 2024

@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
Copy link
Collaborator

I definitely like the idea of supporting OAuth2.

@jonhealy1
Copy link
Collaborator

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
Copy link
Contributor Author

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?

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.

None yet

3 participants