mirumee / saleor-app-framework-python

Python Saleor App/Extension boilerplate. Batteries included.
https://mirumee.github.io/saleor-app-framework-python/
BSD 3-Clause "New" or "Revised" License
50 stars 23 forks source link

[POC/Draft] Add FastAPI SecurityBase-based authentication switch to Saleor's offline JWT (JWK) verification #33

Open pkucmus opened 2 years ago

pkucmus commented 2 years ago

Why the change

Schema documentation

FastAPI being tied to OAS should have apps built with it describing the API appropriately. The previous usage of simple deps was not leveraging the securityScheme definition, now it does. Ref: https://swagger.io/docs/specification/authentication/api-keys/#multiple image

Precise authentication

More importantly, the proposed change reflects the truth more accurately. A Saleor app on every call does multiple authentication checks, at the very least it authenticates a Saleor instance (SaleorDomainSecurity) then it validates the requestor either with SaleorUserAuthSecurity for Dashboard users or with SaleorWebhookSecurity when validating the source of a webhook event. The two latter ones are always dependent on domain authentication hence the dependency - this means we have endpoints secured with:

The auto_error logic is important as it allows to wrap the security methods with an additional layer in case there will be an endpoint with a OR security like

(Saleor Domain Validation **AND** Saleor Webhook Signature Verification) 
OR 
(Saleor Domain Validation **AND** Shared Secret Authentication)

in that case one might want to create something like the following:

saleor_auth = SaleorUserAuthSecurity(auto_error=False)
shared_secret_auth = SharedSecretAuth(auto_error=False)

async def my_authentication(
    saleor_principal: SaleorPrincipal = Security(saleor_auth),
    principal: Principal = Security(shared_secret_auth),
):
    if saleor_principal:
        return saleor_principal
    elif principal:
        return principal
    raise HTTPException(
        status_code=HTTP_401_UNAUTHORIZED,
        detail=f"Provided domain {saleor} is invalid.",
    )

Caveat: FastAPI does not have the ability to fully support what is described here - https://github.com/tiangolo/fastapi/issues/2835 - a user will need to rely on the verbosity of the errors when there is AND authentication despite the schema describing it being an OR

To do:

pkucmus commented 2 years ago

@koradon I'll do all that once we approve and agree that this PoC is the way to go