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

Allow multiple security dependency per route #765

Closed
rhysrevans3 opened this issue Dec 2, 2024 · 4 comments
Closed

Allow multiple security dependency per route #765

rhysrevans3 opened this issue Dec 2, 2024 · 4 comments

Comments

@rhysrevans3
Copy link
Contributor

@pedro-cf raised an issue on the stac-fastapi-elasticsearch-opensearch stac-utils/stac-fastapi-elasticsearch-opensearch#310 that you can't use multiple security route dependency.

Use case for this would be if someone want to use multiple OAuth flows at the same time.

This is a known issue with FastAPI the suggested solution is to use another dependency to "merge" the different dependencies.

@vincentsarago
Copy link
Member

vincentsarago commented Dec 2, 2024

@rhysrevans3 I'm not sure to fully get what the issue with stac-fastapi and how to solve it 🤷

EDIT: stac-fastapi-elasticsearch has its own code for the auth and dependency injection https://github.com/stac-utils/stac-fastapi-elasticsearch-opensearch/blob/main/stac_fastapi/core/stac_fastapi/core/route_dependencies.py

@rhysrevans3
Copy link
Contributor Author

rhysrevans3 commented Dec 2, 2024

@vincentsarago I made this issue on stac-fastapi as this is where the dependency are injected and I thought this might be a common issue for all of the backends.

I'll close this here and move to the elasticsearch repo thanks for the feedback.

@alukach
Copy link
Contributor

alukach commented Dec 2, 2024

This is a known issue with FastAPI the suggested solution is to use another dependency to "merge" the different dependencies.

@rhysrevans3 Can you share links regarding this known issue from the FastAPI community?

My understanding is that if you want to use multiple auth strategies, you need to handle that on your own by merging them into a single dependency. Take a look at this example:

from fastapi import Depends, FastAPI, Security, HTTPException
from fastapi.security import APIKeyQuery, APIKeyHeader

app = FastAPI()

header_scheme = APIKeyHeader(name="X-API-Key", auto_error=False)
query_scheme = APIKeyQuery(name="key", auto_error=False)

def get_token(
    token_from_header: Optional[str] = Security(header_scheme),
    token_from_query: Optional[str] = Security(query_scheme),
) -> str:
    """
    Extract token from either header or query string.
    Priority is given to the query string.
    """
    if token_from_query:
        return token_from_query
    if token_from_header:
        return token_from_header
    raise HTTPException(status_code=401, detail="Token is required")

@app.get("/items/")
async def read_items(token: str = Security(get_token)):
    return {"token": token}

The critical part is to set auto_error=False in those security strategies, so they don't throw an exception if they don't find a token (you're responsible for throwing that exception in your dependency, as we are doing in the above example).

@rhysrevans3
Copy link
Contributor Author

@alukach thanks for the feedback. Yes sorry "issue" was probably the wrong terminology and merging dependencies was what I meant by

the suggested solution is to use another dependency to "merge" the different dependencies.

I did have a pull request #766 where I suggested separating the auth route dependencies and automatically merging them if there are multiple for a single route. But I'm in the process of moving this to the stac-fastapi-elasticsearch-opensearch repo.

The main reason for merging dynamically was to allow the * default route to still be used with multiple auth dependencies.

For example I want to use Basic Auth for my admin user to be able to access all endpoints but my reader client, that uses a different OAuth flow, to only access the search endpoint.

Without the default route I would have to write a route for every endpoint and write my own merge dependency for search to include both flows. Which works but I would say, it makes the auth configuration more difficult to work with. And could increase the chance of human errors, like missing an endpoint.

Maybe there is a way to merge them and still use the default route through more complex authorisation.

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

No branches or pull requests

3 participants