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

Add support for Cognito User Pool authorizer #24

Closed
sapessi opened this issue Apr 17, 2017 · 15 comments
Closed

Add support for Cognito User Pool authorizer #24

sapessi opened this issue Apr 17, 2017 · 15 comments
Assignees
Milestone

Comments

@sapessi
Copy link
Collaborator

sapessi commented Apr 17, 2017

Update internal methods to support the claims object in the request authorization context. This is relevant when an API is configured with a User Pool authorizer from Amazon Cognito.

@sapessi sapessi self-assigned this Apr 17, 2017
@sapessi sapessi added this to the Release 0.5 milestone Apr 17, 2017
@yyolk
Copy link
Contributor

yyolk commented Apr 19, 2017

This is currently my most-wanted :)

@sapessi
Copy link
Collaborator Author

sapessi commented Apr 19, 2017

Thanks. Working on this now. It may result in an API change. I will probably have to change the shape of the AuthorizerContext object

@sapessi
Copy link
Collaborator Author

sapessi commented Apr 19, 2017

Just committed a first implementation of this. You can try it by cloning this branch and running mvn install on your local machine: https://github.com/awslabs/aws-serverless-java-container/tree/cognito-user-pool

The branch is for version 0.5-SNAPSHOT. This adds the CognitoAuthorizerClaims object in the authorizer context:

request.getRequestContext().getAuthorizer().getClaims();

@yyolk
Copy link
Contributor

yyolk commented Apr 20, 2017

Excited :)

Checking out and using that branch, I'm now able to authenticate via cognito-user-pools (like before) but now my Lambda isn't throwing back Internal Server Error when correctly authenticating!! 👍 👍

I am getting this following error when trying to do request.getRequestContext().getAuthorizer().getClaims();, but I'm almost positive it's something on my side currently

at com.gopangea.customer.lambda.LambdaHandler.handleRequest(LambdaHandler.java:36)
at com.gopangea.customer.lambda.LambdaHandler.handleRequest(LambdaHandler.java:13)
at lambdainternal.EventHandlerLoader$PojoHandlerAsStreamHandler.handleRequest(EventHandlerLoader.java:375)
at lambdainternal.EventHandlerLoader$2.call(EventHandlerLoader.java:1139)
at lambdainternal.AWSLambda.startRuntime(AWSLambda.java:298)
at lambdainternal.AWSLambda.<clinit>(AWSLambda.java:62)
at java.lang.Class.forName0(Native Method)
at java.lang.Class.forName(Class.java:348)
at lambdainternal.LambdaRTEntry.main(LambdaRTEntry.java:94)```

@sapessi
Copy link
Collaborator Author

sapessi commented Apr 20, 2017

Could you share the code at (and around) LambdaHandler.java:36?

@yyolk
Copy link
Contributor

yyolk commented Apr 20, 2017

Here's the full class, without imports that was built off of the samples. Our container is based off of the Spring Container, but we're not currently dispatching() to the servlet - line36 is the System.out.println in the try block - again, I'm almost positive it's on my side.

//line 13
public class LambdaHandler implements RequestHandler<AwsProxyRequest, AwsProxyResponse> {                                     
          
  private PangeaLambdaContainerHandler<AwsProxyRequest, AwsProxyResponse> handler;                                            
          
  public AwsProxyResponse handleRequest(AwsProxyRequest awsProxyRequest, Context context) {                                   
    if (handler == null) {
      try {
        handler = PangeaLambdaContainerHandler.getAwsProxyHandler();                                                          
      } catch (ContainerInitializationException e) {                                                                          
        e.printStackTrace();                                                                                                  
        return null;                                                                                                          
      }   
    }     

    try {
      //line36
      System.out.println("identity?: " + awsProxyRequest.getRequestContext().getAuthorizer().getClaims());                    
    } catch (Exception e) {                                                                                                   
      e.printStackTrace();                                                                                                    
    }     
          
    return handler.proxy(awsProxyRequest, context);
  }         
}           

@sapessi
Copy link
Collaborator Author

sapessi commented Apr 20, 2017

What's the exception? Was that a NullPointer?

@yyolk
Copy link
Contributor

yyolk commented Apr 20, 2017

Yes it was java.lang.NullPointerException

However, on further investigation (I was looking at old logs)... I get this only if I invoke from the lambda console, I get what I assume is expected if I route through API gateway endpoint with COGNITO_USERPOOL as the authorizer

When invoking through API-gateway I get this from line 36:

identity?: com.amazonaws.serverless.proxy.internal.model.CognitoAuthorizerClaims@27a5f880

@sapessi
Copy link
Collaborator Author

sapessi commented Apr 20, 2017

That's the expected behavior. When testing from the Lambda console, make sure that your test event contains the user pools context. Take a look at the sample event in the tests

When invoking from API Gateway, you are getting the expected behavior. The Claims object contains all the info from the user pools (getSubject, getIssuer, etc). I've made a small fix that I'm about to push, please pull again in a minute.

sapessi added a commit that referenced this issue Apr 20, 2017
Fixed the principal id in the JaxRs security context to read the
subject property from the user pools authorizer claims. Fixed a bug in
the Claims object (private getSubject method). Added some comments to
the `ZonedDateTime` methods in the claims object. This should
completely address #24.
sapessi added a commit that referenced this issue Apr 20, 2017
@yyolk
Copy link
Contributor

yyolk commented Apr 20, 2017

Thank you for the link to the sample event.

I'm currently trying to grab a cognitoIdentityId from an authorized request with Authorization: idToken but getting null for both getCognitoAuthenticationType() and getCognitioIdentityID() off of awsProxyRequest.getRequestContext().getIdentity(), I'm not sure this is correct?

I used this as a reference.

System.out.println("auth scheme?: " + awsProxyRequest.getRequestContext().getIdentity().getCognitoAuthenticationType());
System.out.println("identity?: " + awsProxyRequest.getRequestContext().getIdentity().getCognitoIdentityId());

Perhaps I'm missing permissions somewhere?

@sapessi
Copy link
Collaborator Author

sapessi commented Apr 20, 2017

The Cognito identity is populated only if you are using AWS_IAM as the authorization strategy and the temporary credentials you use to make the call were generated by the Cognito Identity service. If you are looking for the User Pool identity id then you need to you the getSubject() method of the claims object.

AuthScheme is a field that exists in the JSON model for the event, but doesn't seem to be used/populated by the service. It's there just for completeness at the moment.

@sapessi
Copy link
Collaborator Author

sapessi commented Apr 20, 2017

By the way, are you building support for a new container? What is Pangea?

@yyolk
Copy link
Contributor

yyolk commented Apr 20, 2017

Thank you for the insight. That makes much more sense now x-ref'ing the https://github.com/awslabs/aws-serverless-auth-reference-app

Pangea is whom I work for, our container is a WIP and may be dropped ultimately but it's useful for me right now to understand the library.

I also started the container because I wasn't sure why I was getting Internal Server Error (exception on lambda) when successful authentications from cognito-user-pool (vs unauthorized) - which only after implementing our own container was I sure (and getting the same error) vs something like

public class Task implements RequestHandler<AwsProxyRequest, AwsProxyResponse> {
    public handler(AwsProxyRequest event, Context context) {
       //do stuff
       return new AwsProxyResponse(200, null, "{\"message\": \"Hello, from Lambda!\"}");
    }
}

@sapessi
Copy link
Collaborator Author

sapessi commented Apr 20, 2017

Great. I'll merge this into master soon. By the way, I've just created a gitter room for this repo: https://gitter.im/awslabs/aws-serverless-java-container

@sapessi
Copy link
Collaborator Author

sapessi commented Apr 21, 2017

This is merged into master. Closing issue.

@sapessi sapessi closed this as completed Apr 21, 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

2 participants