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

Feature request - configurable Principal field #55

Closed
eranation opened this issue Aug 12, 2017 · 6 comments
Closed

Feature request - configurable Principal field #55

eranation opened this issue Aug 12, 2017 · 6 comments

Comments

@eranation
Copy link
Contributor

At the moment in a Spring scenario, if you inject the Principal and use Cognito auth, you get the cognito sub (Subject). If you need the username, you must either do a lookup request to Cognito, or fish the ApiGatewayRequestContext and grab it via .getAuthorizer().getClaims().getUsername()

It would be nice if the field that is mapped to the Principal name can be configurable so that one can point it to the username instead of the sub.

@eranation eranation changed the title Feature request - configurable Principle field Feature request - configurable Principal field Aug 12, 2017
@sapessi sapessi self-assigned this Aug 13, 2017
@sapessi sapessi added this to the Release 0.7 milestone Aug 13, 2017
@sapessi
Copy link
Collaborator

sapessi commented Aug 13, 2017

This is possible today by creating a custom SecurityContextWriter and passing it to the library.

We could solve this easily by giving you an easy way to inject the security context writer without having to manually create the full object constructor, perhaps just add it as a parameter to the static getAwsProxyHandler?

@sapessi
Copy link
Collaborator

sapessi commented Aug 16, 2017

We discussed this offline. We see two option:

  1. Create a new HandlerMethodArgumentResolver for Spring and a ContextResolver in Jersey. All you'd have to do then is:
@RequestMapping(path = "/pets", method = RequestMethod.POST)
public Pet createPet(@RequestBody Pet newPet, ApiGatewayAuthorizerContext context) {
...
}
  1. Extend the Principal interface with a few methods to access the additional properties of the authorization context from API Gateway:
public Pet createPet(@RequestBody Pet newPet, Principal principal) {
((CognitoUserPoolPrincipal)principal).getClaims();
}

Would love to hear from the community what the preferred option is.

@sapessi
Copy link
Collaborator

sapessi commented Aug 21, 2017

Since there is no feedback and this is not a high priority fix I'm pulling it out of 0.7. The data is still accessible through the request attributes.

@sapessi sapessi removed this from the Release 0.7 milestone Aug 21, 2017
@yyolk
Copy link
Contributor

yyolk commented Aug 22, 2017

I'm not a user of Spring or Jersey, so I see a lot of benefit in option 2

@eranation
Copy link
Contributor Author

I think option 2, as much as I hate casting or doing instanceof, is the most consistent with Spring Security. this also seems to be backward compatible.

sapessi added a commit that referenced this issue Nov 21, 2017
…hat exposes claims from the token. Also added support for custom claims.
@sapessi
Copy link
Collaborator

sapessi commented Nov 21, 2017

I've implemented these changes. You can now cast the Principal field to CognitoUserPoolPrincipal and call the getClaims() method to extract all of the claims in the token. I have also added support for custom claims in the object. You can get custom claims using the getClaim(String key) method of the CognitoAuthorizerClaims object.

Resolving the issue.

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