dougmoscrop / serverless-http

Use your existing middleware framework (e.g. Express, Koa) in AWS Lambda 🎉
Other
1.74k stars 167 forks source link

Request fails when using aws api gateway and a cognito authorizer #31

Closed fredmarques closed 6 years ago

fredmarques commented 7 years ago

Seems to me that aws maps cognito's claims to request's body and turns it into a object. Therefore, this line is executed.

Error:

{
    "errorMessage": "Unexpected event.body type: object",
    "errorType": "Error",
    "stackTrace": [
        "getBody (/var/task/node_modules/serverless-http/lib/request.js:17:9)",
        "new ServerlessRequest (/var/task/node_modules/serverless-http/lib/request.js:29:18)",
        "Promise.resolve.then (/var/task/node_modules/serverless-http/serverless-http.js:30:25)"
    ]
}
dougmoscrop commented 7 years ago

Do you have a sample of what it could look like?

atishnazir commented 6 years ago

From my observations with custom Cognito authorisation and API Gateway, the user information is located within APIGatewayEvent object (and contrary to the TS AuthResponseContext definition and documention) something like this:

{
        path: path,
        httpMethod: verb,
        requestContext: {
...
            authorizer: {
                        claims: {
                            'cognito:username': '3F52A76E-EEB2-4336-8A2C-A8B3F0D8CD9B',
                            'cognito:groups': 'humans'
                        }
                    }
        },
        body: body,
...
    }

This suggests that the cognito claims object is simply not mapped through in ServerlessRequest https://github.com/dougmoscrop/serverless-http/blob/master/lib/request.js#L21

Only spent a couple of minutes tinkering with serverless-http adapting an aws-serverless-express project. Once I'm confident about my usage/understanding will spend a few minutes working up a PR, maybe making it optionally compatible with aws-serverless-express/middleware to smooth the adaptation for others.

atishnazir commented 6 years ago

Forgot to mention. It's an easy, cheeky bodge for compatibility with Express code written to work with aws-serverless-express (although of course this doesn't then break the dependency). Assuming the following:

import * as aws from 'aws-lambda';
import * as awsServerlessExpressMiddleware from 'aws-serverless-express/middleware';
...
// place this before your other routes etc and then the following will magically appear:
//   req.apiGateway.event: aws.APIGatewayEvent
//   req.apiGateway.context: aws.Context
app.use(awsServerlessExpressMiddleware.eventContext());

For the handler function place an adaptor around the serverless-http handler, like this:

const serverlessHandler: aws.ProxyHandler = serverless(app);

export function handler(event: aws.APIGatewayEvent, context: aws.Context, callback?: aws.ProxyCallback): void {
    const encodedEvent: string = encodeURIComponent(JSON.stringify(event));
    const encodedContext: string = encodeURIComponent(JSON.stringify(context));
    event.headers['x-apigateway-event'] = encodedEvent;
    event.headers['x-apigateway-context'] = encodedContext;

    return serverlessHandler(event, context, callback);
}
dougmoscrop commented 6 years ago

@fredmarques posted an error message that suggests the body is not a string/buffer; I don't think that using a Cognito authorizer would affect this since you'd still need to be able to send non-JSON to an API secured by Cognito, would you not?

Unless it's doing something like

body: {
  rawBody: '<xml>abc</xml>',
  somethingAddedByCognito: ...
}

@atishnazir your comment suggests you just need the authorizer information, which is a different problem than the body-is-an-object one. I've documented the transforms that are supported in the README.

Basically, you can do:

module.exports.handler = serverless(app, {
  request: (request, event, context) => {
    request._claims = event.requestContext.authorizer.claims;
  }
});

@fredmarques can you give me an example of the body that is coming through? Just to confirm you are not using serverless-local to test this, right? It's within real deploy?

bsdkurt commented 6 years ago

I use cognito auth and use the transform to make the event exposed on the request as described in the readme. This works just fine and suggest that this bug should be closed.

dougmoscrop commented 6 years ago

Thanks, yeah, I am going to close, but if there's something persisting, please reopen.