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

feat: httpApi with request authorizer #1600

Merged
merged 5 commits into from
Nov 8, 2022
Merged

feat: httpApi with request authorizer #1600

merged 5 commits into from
Nov 8, 2022

Conversation

rion18
Copy link
Contributor

@rion18 rion18 commented Oct 24, 2022

Description

I've added a simple check on the #configureJWTAuthorization function to take care of a case that wasn't handled before: when the endpoint declares an authorizer with type request, or when the authorizers inside of provider.httpApi does so. This will prompt #setAuthorizationStrategy to use #configureAuthorization, instead of #configureJWTAuthorization.

Motivation and Context

This solves the problem of using local authorizers with serverless-offline. Starting the server when using httpApi events would ALWAYS assume that the local authorizers are of type jwt. This adds support for the request type.

This fixes issues:
resolves #1311
resolves #1343

How Has This Been Tested?

This has been tested using Mocha, creating a new set of integration tests called request-authorizers. Tests a scenario where the provider's httpApi has defined an authorizers property.

This change should only affect the proposed scenario: when you want to run serverless-offline with a request type authorizer.

Updates

This also fixes issues:
resolves #1341
resolves #1475

After doing some more integration testing, I discovered that request authorizers were not correctly implemented for httpApi events. So this also adds support for payloadFormat 2.0 as well as simpleResponses.

This has been tested by adding upon the previous request-authorizers tests, where we define three configuration scenarios:

  • An httpApi event with payloadFormat 1.0
  • An httpApi event with payloadFormat 2.0
  • An httpApi event with payloadFormat 2.0 with simple responses enabled. (Bear in mind that in order to test the new identitySource field as an array, these tests use the AuthorizationSimple header instead of Authorization.
    It's also been tested by deploying the integration test on AWS, as well as personal project, with success in all cases.

@dnalborczyk
Copy link
Collaborator

thank you for the PR @rion18 !

@dnalborczyk
Copy link
Collaborator

I just deployed your serverless.yml from the test and it's throwing an error when deployed to aws and requested with your authorization token

{
    "errorType": "TypeError",
    "errorMessage": "Cannot read properties of undefined (reading 'split')",
    "stack": [
        "TypeError: Cannot read properties of undefined (reading 'split')",
        "    at Runtime.authorizerFunction [as handler] (file:///var/task/src/authorizer.js:27:62)",
        "    at Runtime.handleOnceNonStreaming (file:///var/runtime/index.mjs:1028:29)"
    ]
}

authorizationToken does not exist on the event object.

@rion18
Copy link
Contributor Author

rion18 commented Nov 5, 2022

I just deployed your serverless.yml from the test and it's throwing an error when deployed to aws and requested with your authorization token

{
    "errorType": "TypeError",
    "errorMessage": "Cannot read properties of undefined (reading 'split')",
    "stack": [
        "TypeError: Cannot read properties of undefined (reading 'split')",
        "    at Runtime.authorizerFunction [as handler] (file:///var/task/src/authorizer.js:27:62)",
        "    at Runtime.handleOnceNonStreaming (file:///var/runtime/index.mjs:1028:29)"
    ]
}

authorizationToken does not exist on the event object.

After deploying this myself, I found my project to be under the same circumstances as
#1341 and #1475

It seems that no authorizer was implemented for an httpApi event.

I've went on ahead and added support for payloadFormat 2.0, and payloadFormat 2.0 with simple responses enabled, for request type authorizers with httpApi events.

Integration tests have been added, this has been tested by deploying the small test app in AWS, as well as using this fork on a local project. I've also added support for $request.headers.XXX instead of only maintaining method.request.headers.XXX.

I've made some cleanup inside the configureAuthorization function which was only deploying authorizers for http events.

@rion18
Copy link
Contributor Author

rion18 commented Nov 5, 2022

@dnalborczyk Sorry for the delay, but I have some good news detailed above :) Let me know if something else is up.

@rion18 rion18 requested a review from dnalborczyk November 5, 2022 08:09
@rion18 rion18 mentioned this pull request Nov 5, 2022
@rion18
Copy link
Contributor Author

rion18 commented Nov 6, 2022

Tests are failing on node.js / [macos-latest] node.js v14 (pull_request). Somehow one of the tests failed to close and then most tests failed due to EADDRINUSE. Is there a way to rerun that check? @dnalborczyk

@dnalborczyk
Copy link
Collaborator

dnalborczyk commented Nov 6, 2022

@rion18 the tests can be a bit flaky sometimes. I restarted that one particular failure. I try to have a look at your PR later today.

@dnalborczyk dnalborczyk changed the title Fixes httpApi with request authorizer feat: httpApi with request authorizer Nov 8, 2022
@dnalborczyk dnalborczyk merged commit ec0d31c into dherault:master Nov 8, 2022
@dnalborczyk
Copy link
Collaborator

thank you @rion18 ! great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants