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

Principal.getName with federated cognito user not returning identityId #43

Closed
Fefbug opened this issue Jul 3, 2017 · 4 comments
Closed
Assignees
Milestone

Comments

@Fefbug
Copy link

Fefbug commented Jul 3, 2017

Hi,
I am using spring with Api gateway and lamba proxy.
When i Inject the principal on my controller it does not return the cognito identity with federated user.
The principale getName method now return the temporary user generated by the assume role call.

before the 0.5 my getAuthentificationScheme was AUTH_SCHEME_COGNITO, but now it fall into the AUTH_SCHEME_AWS_IAM

4f1e773#diff-dd23022884ed5861d345f0b5f36b2cc7

seen with @sapessi in gittter

@sapessi
Copy link
Collaborator

sapessi commented Jul 3, 2017

There was indeed a breaking change when we started handling user pool auth properly. You are now going to the AWS_IAM branch of the code - which is the correct behavior. However, you expect (rightly) the Cognito identity id as a principal since the call is authorized with credentials produced by Cognito Federated Identities.

This is caused by the fact that the library returns the userArn when the call is authorized by AWS credentials, regardless of the origin of the credentials: Code here.

We can make a simple change to the method to handle Cognito credentials as a special case:

if (event.getRequestContext().getIdentity().getCognitoIdentityId() != null) {
    return event.getRequestContext().getIdentity().getCognitoIdentityId();
} else {
    return event.getRequestContext().getIdentity().getUserArn();
}

The behavior in this case would be to return the Cognito identity id whenever the call was signed with credentials from Amazon Cognito or the user ARN whenever the request was signed with non-Cognito credentials. This is potentially another breaking change to the behavior of the library so I'm going to leave it here for a bit to gather some feedback before I implement it.

Alternatively, we could introduce a new auth scheme called AUTH_SCHEME_COGNITO_IAM which returns the Cognito identity id when it is populated.

Let us know which option you'd prefer

@mkjois
Copy link

mkjois commented Jul 3, 2017

I'm not very familiar with this use case, but if it's going to be a breaking change, perhaps it should be in 0.6? The proposed fix looks fine to me

@sapessi
Copy link
Collaborator

sapessi commented Jul 5, 2017

Following the conversation on Gitter looks like we are settled on option 1. Committing a fix for this now.

sapessi added a commit that referenced this issue Jul 5, 2017
sapessi added a commit that referenced this issue Jul 5, 2017
@sapessi
Copy link
Collaborator

sapessi commented Jul 6, 2017

Resolving now that the fix is merged into master

@sapessi sapessi closed this as completed Jul 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants