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

Separating route securities from other dependencies. #766

Closed

Conversation

rhysrevans3
Copy link
Contributor

Related Issue(s):

Description:
Currently if you have more than one security dependency on a single route, if either fail then the route will be unauthorised. This is a known issue in FastAPI and the suggested solution is to use a single dependency to merge the sub dependencies. This makes configuration more difficult and removes the ability to use default security dependencies.

Suggested solution is to separate route securities from other dependencies. This then allows them to be merged when dynamically.

There are two options for merging in this pull request.

merge_dependencies1 uses the traditional merging method but this requires dependencies return a value or None rather than raise an error. This means it would need to be used for all routes even those without multiple dependencies.

merge_dependencies2 calls each dependency itself which allows them to raise their own errors so can be used only on routes that have multiple dependencies.

PR Checklist:

  • pre-commit hooks pass locally
  • Tests pass (run make test)
  • Documentation has been updated to reflect changes, if applicable, and docs build successfully (run make docs)
  • Changes are added to the CHANGELOG.

Allow for multiple securities per route through dependency merging.
@rhysrevans3
Copy link
Contributor Author

Moving to stac-fastapi-elasticsearch-opensearch see issue #765 for details.

@rhysrevans3 rhysrevans3 closed this Dec 2, 2024
@rhysrevans3 rhysrevans3 deleted the seperate_sercurity_dependencies branch December 2, 2024 16:08
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.

1 participant