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

Authorizers: Support AuthorizationScopes #652

Closed
keetonian opened this issue Nov 6, 2018 · 25 comments
Closed

Authorizers: Support AuthorizationScopes #652

keetonian opened this issue Nov 6, 2018 · 25 comments

Comments

@keetonian
Copy link
Contributor

Description:

This was brought up in Slack:

Starting to work on a PoC using the new authorizer features in SAM and already notice one huge shortcoming I didn't think about.
AuthorizationScopes - this would have been really powerful to include so I could define required scopes for authenticating tokens at the API level instead of inside the code.

This is also necessary for using access_tokens.

Docs: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-apigateway-method.html#cfn-apigateway-method-authorizationscopes

SAM should support AuthorizationScopes.

@brysontyrrell
Copy link
Contributor

brysontyrrell commented Nov 7, 2018

This was my thread in the channel. My proposal would look something like this:

  MyApi:
    Type: AWS::Serverless::Api
    Properties:
      StageName: Prod
      Auth:
        DefaultAuthorizer: MyCognitoAuthorizer
        Authorizers:
          MyCognitoAuthorizer:
            UserPoolArn: !Ref CognitoUserPoolArn
            OAuthScopes:
              - scope1
              - scope2

  MyFunction:
    Type: AWS::Serverless::Function
    Properties:
      CodeUri: ./src
      Handler: lambda.handler
      MemorySize: 1024
      Runtime: nodejs8.10
      Events:
        Root:
          Type: Api
          Properties:
            RestApiId: !Ref MyApi
            Path: /
            Method: GET
            Auth:
              Authorizer: MyCognitoAuthorizer
              OAuthScopes:
                - scope3

This is a swagger doc from one of my templates to show what it looks like with scopes defined:

ApiGateway:
    Type: AWS::Serverless::Api
    Properties:
      StageName: Prod
      EndpointConfiguration: EDGE

      DefinitionBody:
        swagger: 2.0
        info:
          title: !Ref AWS::StackName
        securityDefinitions:
          CognitoAuth:
            type: apiKey
            name: Authorization
            in: header
            x-amazon-apigateway-authtype: cognito_user_pools
            x-amazon-apigateway-authorizer:
              type: cognito_user_pools
              providerARNs:
                - !Ref CognitoUserPoolArn

        paths:
          "/messages":
            post:
              x-amazon-apigateway-integration:
                httpMethod: post
                type: aws_proxy
                uri: !Sub arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${SendMessage.Arn}/invocations
              responses: {}
              security:
                - CognitoAuth:
                  - "rds-worker/send_message"

          "/errors":
            get:
              x-amazon-apigateway-integration:
                httpMethod: post
                type: aws_proxy
                uri: !Sub arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${Errors.Arn}/invocations
              responses: {}
              security:
                - CognitoAuth:
                  - "rds-worker/read_errors

Edit: Noticed a bad indent.

@keetonian
Copy link
Contributor Author

Leaving this issue open for +1's and further comments. Happy to work with anyone that wants to implement this feature.

@mau-fyayc
Copy link

mau-fyayc commented Jan 31, 2019

+1
I find it quite an important feature to use the full power of OAuth and Cognito.

@uokitomer
Copy link

Is there any plan to include this feature in the near future?

@klmz
Copy link
Contributor

klmz commented May 3, 2019

I can look into implementing this. The transformation to swagger is simple enough, although I'm not sure if there are other steps that need to be implemented.
In any case, what would be the expected behavior when only a scope is specified? Without an authorizer like this:

MyApi:
    Type: AWS::Serverless::Api
    Properties:
      StageName: Prod
      Auth:
        DefaultAuthorizer: MyCognitoAuthorizer
        Authorizers:
          MyCognitoAuthorizer:
            UserPoolArn: !Ref CognitoUserPoolArn
              OAuthScopes:
                - scope1
                - scope2

  MyFunction:
    Type: AWS::Serverless::Function
    Properties:
      CodeUri: ./src
      Handler: lambda.handler
      MemorySize: 1024
      Runtime: nodejs8.10
      Events:
        Root:
          Type: Api
          Properties:
            RestApiId: !Ref MyApi
            Path: /
            Method: GET
            Auth:
              OAuthScopes:
                - scope3

I would propose choosing the default authorizer in this case.

@klmz
Copy link
Contributor

klmz commented May 3, 2019

I also want to propose calling it AuthorizationScopes instead of OAuthScopes since it is more consistent with the naming of the AWS::ApiGateway::Method in normal CloudFormation.
Or is it better to make the naming different to ensure there is no mix up for inexperienced users?

@keetonian
Copy link
Contributor Author

@klmz Keeping the naming the same should be fine. I think the transformation into swagger + some supporting tests are all that are needed for this feature, correct me if I'm wrong.

@klmz
Copy link
Contributor

klmz commented May 4, 2019

Yes, that is what I figured. I got most of it working. Is there any documentation on what is expected in terms of tests? I saw some end-to-end test that will be needed, anything else?

@keetonian
Copy link
Contributor Author

@klmz we don't have any docs on what is expected for tests, but I would recommend some end-to-end tests like you suggested, as well as some tests for the new swagger that is generated in https://github.com/awslabs/serverless-application-model/blob/develop/tests/swagger/test_swagger.py.

@brettstack
Copy link
Contributor

I would propose choosing the default authorizer in this case.

This is correct. Looking forward to the PR!

@MohammedAlSafwanOld
Copy link

How long would it be until this get pushed in with the next release !? +1

@praneetap
Copy link
Contributor

@MohammedAlSafwan We are still working on reviewing the PR, once that's reviewed and merged in we will assign a release tag on it.

@MohammedAlSafwanOld
Copy link

@praneetap ... thank you, I just noticed that it had conflict... I really hope that could be done soon

@ghost
Copy link

ghost commented Sep 3, 2019

Looking forward to having this functionality!

@ross-humphrey
Copy link

+1 on looking forward to this functionality - having to work round this was a pain today. Any ETA on the release?

@keetonian
Copy link
Contributor Author

I've been looking into this (both fixing merge conflicts in the code and the actual spec).

Using scopes in method authorization for Cognito is possible as per APIGW Cognito Docs, and is also consistent with the example shared by @brysontyrrell

This issue is for scopes on methods using Cognito auth, and does not address adding generic OAuth 2.0 support yet.

Some existing questions:

  1. With the current proposal (included below for reference), would scopes scope1 and scope2 be applied to all methods using MyCognitoAuthorizer, meaning that the root event would have scope1, scope2, and scope3 as scopes?
  MyApi:
    Type: AWS::Serverless::Api
    Properties:
      StageName: Prod
      Auth:
        DefaultAuthorizer: MyCognitoAuthorizer
        Authorizers:
          MyCognitoAuthorizer:
            UserPoolArn: !Ref CognitoUserPoolArn
              OAuthScopes:
                - scope1
                - scope2

  MyFunction:
    Type: AWS::Serverless::Function
    Properties:
      CodeUri: ./src
      Handler: lambda.handler
      MemorySize: 1024
      Runtime: nodejs8.10
      Events:
        Root:
          Type: Api
          Properties:
            RestApiId: !Ref MyApi
            Path: /
            Method: GET
            Auth:
              Authorizer: MyCognitoAuthorizer
              OAuthScopes:
                - scope3

@DavidVergison
Copy link

@keetonian : logically, yes :)
I can't wait to use this !

@brysontyrrell
Copy link
Contributor

@keetonian @David-v2 That was the intent of my original example, but it might be more appropriate to have that be a part of Globals:

Globals:
  Function:
    Auth:
      OAuthScopes:
        - scope-1
        - scope-2

Note that Auth is not yet listed as a supported value in Globals.

@reshavk
Copy link

reshavk commented Oct 21, 2019

Any pointers on ETA of this feature ?

@MohammedAlSafwanOld
Copy link

I know that I'm asking again about this feature and I know that you guys are working hard. The feature is related to [WIP] Add authorizationscopes #917. The reason why I'm asking is because we are in need of defining a scope so that we are able to use Access Token instead of ID Token through the API Gateway. is there any solution around the problem other than adding the scope manually to every API method?

@klmz
Copy link
Contributor

klmz commented Nov 26, 2019

Unfortunately, I will not have time to work on this in the foreseeable future. It shouldn't be super hard to complete this PR though, so feel free to complete it.

@MohammedAlSafwanOld
Copy link

@klmz ... thank you so much for letting us know. It would be nice if [WIP] Add authorizationscopes #917 get moved to "To do" or removed so people don't get confused. Thank you so much for your effort.

@keetonian
Copy link
Contributor Author

Updated the PR, should get this merged soon!

@brysontyrrell
Copy link
Contributor

Is there any chance v1.20.0 is going to be released this week?

@praneetap
Copy link
Contributor

Is there any chance v1.20.0 is going to be released this week?

Just updated the tracker. We are deploying in multiple regions right now. We should be releasing early next week, if everything goes okay!

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