tokusumi / fastapi-cloudauth

Simple integration between FastAPI and cloud authentication services (AWS Cognito, Auth0, Firebase Authentication).
MIT License
336 stars 34 forks source link

CognitoCurrentUser() produces "Validation Error for Claims" #27

Open mjvdvlugt opened 3 years ago

mjvdvlugt commented 3 years ago

When using CognitoCurrentUser(region=settings.aws_region, userPoolId=settings.aws_cognito_user_pool_id) in my endpoint depencies, it consistently errors a 403: "Validation Error for Claims". Further investigation indicated this has to do with an issue mapping the Cognito reply to the CognitoClaims Pydantic model here: https://github.com/tokusumi/fastapi-cloudauth/blob/a8db880b841149dc5cf4b76762a98d74daff21a2/fastapi_cloudauth/base.py#L232

The ValidationError is:

ValidationError(model='CognitoClaims', errors=[{'loc': ('cognito:username',), 'msg': 'field required', 'type': 'value_error.missing'}])

and indeed, the 'cognito:username' field is not available in the claims:

{'sub': '5a8b7ac6-5cd3-4279-b89b-5f59ca6c4144', 'cognito:groups': ['Users'], 'iss': '[Redacted]', 'version': 2, 'client_id': '[Redacted]', 'event_id': 'b0fcb01a-971b-40fe-942c-550dbf0915d2', 'token_use': 'access', 'scope': 'openid email', 'auth_time': 1614092323, 'exp': 1614095923, 'iat': 1614092323, 'jti': '48884493-8fad-47e5-8d8b-a4f44aa41900', 'username': '5a8b7ac6-5cd3-4279-b89b-5f59ca6c4144'}

but 'username' is!

I wasn't able to find why Cognito returns the claims in this different way.

Proposed compatibility fix

class CognitoClaims(BaseModel):
    username: str = Field(alias="cognito:username")
    email: str = Field(None, alias="email")

    class Config:
        allow_population_by_field_name = True

Adding the allow_population_by_field_name = True in the CognitoClaims model config makes it compatible with this other Cognito output.

mjvdvlugt commented 3 years ago

An alternative solution could also parse the "sub" uuid and scopes('cognito:groups') from the Access Token Payload, and make username/email optional. I found some claims contents documentation: https://docs.aws.amazon.com/cognito/latest/developerguide/amazon-cognito-user-pools-using-tokens-with-identity-providers.html#amazon-cognito-user-pools-using-the-access-token

mjvdvlugt commented 3 years ago

By the way: I've managed to work around this using a custom CognitoClaim object. 💪

mjvdvlugt commented 3 years ago

I thought of another possibility that might help:

class CognitoClaims(BaseModel):
    username: str = Field(alias="cognito:username")
    email: str = Field(None, alias="email")

    class Config:
        extra = Extra.allow

The extra = Extra.allow will make Pydantic accept all fields from the auth provider. With this also any added custom fields are automatically accessible through the user_info object.

A combination of this with allow_population_by_field_name = True is also possible of course.

tokusumi commented 3 years ago

@mjvdvlugt Always thank you for your feedback and proposals. Very helpful !!!

I would like to investigate a little further whether to make it the default configuration.

DarioPanada commented 3 years ago

I thought of another possibility that might help:

class CognitoClaims(BaseModel):
    username: str = Field(alias="cognito:username")
    email: str = Field(None, alias="email")

    class Config:
        extra = Extra.allow

The extra = Extra.allow will make Pydantic accept all fields from the auth provider. With this also any added custom fields are automatically accessible through the user_info object.

A combination of this with allow_population_by_field_name = True is also possible of course.

A small note - if you want to avoid modifying the package code you'll also need to create a custom CognitoCurrentUserclass, something like:

class CognitoClaimsCustom(BaseModel):
    username: str = Field(alias="cognito:username")
    email: str = Field(None, alias="email")
    sub: str = Field(None, alias="cognito:sub")

    class Config:
        extra = Extra.allow

class CognitoCurrentUserCustom(CognitoCurrentUser):

    user_info = CognitoClaimsCustom

    def __init__(
            self, *args: Any, **kwargs: Any,
    ):
        super().__init__(*args, **kwargs)

Otherwise the CognitoCurrentUser instance will by default use the original version of CognitoClaims which only provide the username and string. With this approach instead you get all claims.