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

Allows specification of an AuthorizerCredentials(Arn) for both HTTP and REST APIs #11959

Open
1 task done
lholota opened this issue May 11, 2023 · 14 comments
Open
1 task done

Comments

@lholota
Copy link

lholota commented May 11, 2023

Is there an existing issue for this?

  • I have searched existing issues, it hasn't been reported yet

Use case description

Both types of API gateway allow specifying in IAM role which is used to execute the authorizer:

  • AWS::ApiGateway::Authorizer -> AuthorizerCredentials
  • AWS::ApiGatewayV2::Authorizer -> AuthorizerCredentialsArn

This is important when using an authorizer from another AWS account where you must create a role allowing the execution of the underlying lambda which is used for the authorizer.

If I'm missing something and there is a way around I would be really glad to find out, we have several teams currently keen on using an authorizer shared across the whole company from one AWS account and they unfortunately can't because of this issue.

Proposed solution (optional)

Add property under:
...../events//httpApi/authorizer/authorizer_credentials
...../events/
/http/authorizer/authorizer_credentials
provider/httpApi/authorizers/*/authorizer_credentials

which would map to the corresponding CloudFormation properties based on the API type.

@medikoo
Copy link
Contributor

medikoo commented May 12, 2023

@lholota thanks for openning that issue. PR is welcome, but let's first agree how we will include it in the configuration (please post a proposal)

@lholota
Copy link
Author

lholota commented May 15, 2023

Hi @medikoo, as I hinted above, I would suggest adding it as follows:

HTTP API (Gateway V2)

provider:
  httpApi:
    authorizers:
      customAuthorizer:
        # ...
        authorizerCredentials: # Role ARN
           GetAtt: [MyRole, Arn] # Referencing a role created in the resources section

REST API (Gateway V1)

functions:
  create:
    handler: posts.create
    events:
      - http:
          path: posts/create
          # ...
          authorizer:
            name: authorizerFunc
             # ...
            authorizerCredentials: # Role ARN
               GetAtt: [MyRole, Arn] # Referencing a role created in the resources section
  authorizerFunc:
    handler: handler.authorizerFunc

Both V1 and V2 gateways have the same property (although it has a slightly different name in each it takes the same value).

@medikoo
Copy link
Contributor

medikoo commented May 16, 2023

@lholota that looks very good to me. PR's welcome!

@JSee98
Copy link

JSee98 commented May 20, 2023

Hi, I'd like to pick this issue.
Wanted a bit of clarifications @lholota
In summary the issue is:

  • The authorizer function exists in a different AWS account (lets say account#1) and the deployed resource that requires this authorizer is in a different AWS account (account#2).
  • We want that the Gateway is able to assume a IAM role (authorizerCredentials via its name or arn) which is defined in account#2.
  • For this functionality we want to allow specifying the said role in the .yaml file

P.S. I'm new here so please excuse me if anything sounds too obvious to answer.

@JSee98
Copy link

JSee98 commented May 20, 2023

/assign

@lholota
Copy link
Author

lholota commented May 22, 2023

Hi @JSee98,
Yes, your description is correct. For the cross account authorizer to work you need to allow the access from both sides. As you described the gateway is created as a part of the serverless app in account#2 and the gateway needs to get an explicit permission through an IAM role to access lambda functions in another account. This requirements have been tested and verified with AWS support.

@JSee98
Copy link

JSee98 commented May 22, 2023

Great!!
Then I guess all we ideally need is to make sure the parser is able to use the GetAtt function to fetch the said role.
Moreover, from what I've seen I guess the parser which generates the cloudformation file already should be able to do this.

@lholota just wanted to confirm have you tried the GetAtt function as you've defined above? If yes, what was the exact error over there?

@JSee98
Copy link

JSee98 commented May 22, 2023

@lholota I'm suggesting something of the sorts defined here https://github.com/serverless/serverless/blob/88099ad98c33ed97b1cf0471de03247c33928af0/docs/providers/aws/guide/iam.md#custom-iam-roles

Here as I can see we can define the role with an arn and use the same in any function. This should work right?

@JSee98
Copy link

JSee98 commented May 22, 2023

Please ignore the previous message. I believe I've identified the required changes.
First major change is in the following file: https://github.com/serverless/serverless/blob/88099ad98c33ed97b1cf0471de03247c33928af0/lib/plugins/aws/package/compile/events/api-gateway/lib/authorizers.js

This is where the authorizer is "compiled". Basically, the yaml details are used to create the required authorizer for cloudformation

More changes might be required under validation file and the methods/authorization file. Will check.

P.S. Please ignore the previous messages 😅

@lholota
Copy link
Author

lholota commented May 22, 2023

@JSee98 np, let me know if you need anything more. I'm just going to add that the role will always be defined in resources section because the role will need to use custom policy which will contain the number of the referenced AWS account so there's no chance the role could be somehow generated by the framework. This is why I used the GetAtt function in the example above.

@lholota
Copy link
Author

lholota commented Jun 2, 2023

Hi @JSee98, how is it going? We are eager to test a beta version of this feature :)

@JSee98
Copy link

JSee98 commented Jun 2, 2023

Hi @lholota sorry have been stuck with a prod issue. Will try raise something this week. Sorry again for the delay.

@ciradu2204
Copy link

ciradu2204 commented Aug 2, 2023

Hi @JSee98 and @medikoo , can I work on this issue?

@aikid
Copy link

aikid commented Sep 3, 2023

Hello can we be contributing with the adjustment of the problem in question? Can you give us the path of the file that contains the error?

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

5 participants