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

SAM throws error if DefaultAuthorizer is a dict #1245

Closed
jlhood opened this issue Nov 13, 2019 · 2 comments
Closed

SAM throws error if DefaultAuthorizer is a dict #1245

jlhood opened this issue Nov 13, 2019 · 2 comments
Labels
contributors/good-first-issue Good first issue for a contributor contributors/welcome Contributors are welcome to work on this maintainer/need-response type/bug

Comments

@jlhood
Copy link
Contributor

jlhood commented Nov 13, 2019

Description:

We saw a case where a customer tried to use !Ref in the DefaultAuthorizer field of the Auth object on a AWS::Serverless::Api, rather than just a string. In this case, SAM throws a 5xx error here because dict objects are not hashable. We should add a validation check to ensure the value of the DefaultAuthorizer field is a string so we can throw a useful error message back to the customer.

Steps to reproduce the issue:

  1. Either run sam-translate.py or try to deploy this template:
AWSTemplateFormatVersion: "2010-09-09"
Transform: "AWS::Serverless-2016-10-31"

Resources:
  MyApi:
    Type: "AWS::Serverless::Api"
    Properties:
      StageName: Prod
      Auth:
        DefaultAuthorizer: !Ref MyCognitoAuth
        Authorizers:
          MyCognitoAuth:
            UserPoolArn: arn:aws:1
            Identity:
              Header: MyAuthorizationHeader
              ValidationExpression: myauthvalidationexpression

  MyFunction:
    Type: AWS::Serverless::Function
    Properties:
      CodeUri: s3://sam-demo-bucket/thumbnails.zip
      Handler: index.handler
      Runtime: nodejs8.10
      Events:
        Api:
          Type: Api
          Properties:
            RestApiId: !Ref MyApi
            Path: /
            Method: get

Observed result:

sam-translate.py returns this:

Traceback (most recent call last):
  File "bin/sam-translate.py", line 141, in <module>
    transform_template(input_file_path, output_file_path)
  File "bin/sam-translate.py", line 98, in transform_template
    sam_template, {}, ManagedPolicyLoader(iam_client))
  File "/Users/jahood/Documents/workspace/serverless-application-model/bin/../samtranslator/translator/transform.py", line 16, in transform
    return translator.translate(input_fragment, parameter_values=parameter_values)
  File "/Users/jahood/Documents/workspace/serverless-application-model/bin/../samtranslator/translator/translator.py", line 83, in translate
    translated = macro.to_cloudformation(**kwargs)
  File "/Users/jahood/Documents/workspace/serverless-application-model/bin/../samtranslator/model/sam_resources.py", line 535, in to_cloudformation
    rest_api, deployment, stage, permissions, domain, basepath_mapping, route53 = api_generator.to_cloudformation()
  File "/Users/jahood/Documents/workspace/serverless-application-model/bin/../samtranslator/model/api/api_generator.py", line 342, in to_cloudformation
    rest_api = self._construct_rest_api()
  File "/Users/jahood/Documents/workspace/serverless-application-model/bin/../samtranslator/model/api/api_generator.py", line 118, in _construct_rest_api
    self._add_auth()
  File "/Users/jahood/Documents/workspace/serverless-application-model/bin/../samtranslator/model/api/api_generator.py", line 449, in _add_auth
    auth_properties.AddDefaultAuthorizerToCorsPreflight)
  File "/Users/jahood/Documents/workspace/serverless-application-model/bin/../samtranslator/model/api/api_generator.py", line 645, in _set_default_authorizer
    if not authorizers.get(default_authorizer) and default_authorizer != 'AWS_IAM':
TypeError: unhashable type: 'dict'

CloudFormation returns this:

Failed to create the changeset: Waiter ChangeSetCreateComplete failed: Waiter encountered a terminal failure state Status: FAILED. Reason: Transform AWS::Serverless-2016-10-31 failed with: Internal transform failure.

Expected result:

SAM returns helpful error message saying DefaultAuthorizer must be a string.

@jlhood jlhood added contributors/good-first-issue Good first issue for a contributor contributors/welcome Contributors are welcome to work on this type/bug labels Nov 13, 2019
@tbutler4
Copy link

tbutler4 commented Nov 29, 2020

@jlhood @praneetap @Shivakishore14
...
Is this issue still open? I see it has been some time since it was first opened and there has been some work with issue #1286.. I want to try to tackle this for my first open source contribution.

@jfuss
Copy link
Contributor

jfuss commented Feb 5, 2022

See #1286 (comment) but this was patched in #1774

@jfuss jfuss closed this as completed Feb 5, 2022
jfuss added a commit to jfuss/serverless-application-model that referenced this issue Feb 5, 2022
In issue aws#1245, we had a cases were SAM would fail due to improper validation.
In updating aws#1286, I noticed this was patched in aws#1774 but we only added tests
for AWS::Serverless::StateMachine. This commit adds an additional test to cover
AWS::Serverless::Api.
jfuss added a commit that referenced this issue Feb 7, 2022
In issue #1245, we had a cases were SAM would fail due to improper validation.
In updating #1286, I noticed this was patched in #1774 but we only added tests
for AWS::Serverless::StateMachine. This commit adds an additional test to cover
AWS::Serverless::Api.

Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributors/good-first-issue Good first issue for a contributor contributors/welcome Contributors are welcome to work on this maintainer/need-response type/bug
Projects
None yet
Development

No branches or pull requests

6 participants