Skip to content

Conversation

michaelbrewer
Copy link
Contributor

@michaelbrewer michaelbrewer commented Mar 1, 2022

Issue #, if available:

Following runtime code:

mock_event = {
    "type": "TOKEN",
    "methodArn": "arn:aws:execute-api:us-east-2:1234567890:abcd1234/latest/GET/path/part/part/1",
    "authorizationToken": "Bearer TOKEN",
}
event = APIGatewayAuthorizerTokenEvent(mock_event)
event_arn = event.parsed_arn
authorizer_policy = APIGatewayAuthorizerResponse(...)
authorizer_policy.allow_route(http_method=event_arn.method, resource= event_arn.resource)

will raise a ValueError

[ERROR] ValueError: Invalid resource path: . Path should match ^[/.a-zA-Z0-9-_\*]+$

Description of changes:

Correctly parse method arn to include the full resource path
ie: arn:aws:execute-api:us-west-2:123456789012:ymy8tbxw7b/*/GET/foo/bar

Checklist

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 1, 2022
@github-actions github-actions bot added the bug Something isn't working label Mar 1, 2022
@codecov-commenter
Copy link

codecov-commenter commented Mar 1, 2022

Codecov Report

Merging #1051 (e4c5821) into develop (6e4cbd7) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1051   +/-   ##
========================================
  Coverage    99.96%   99.96%           
========================================
  Files          119      119           
  Lines         5365     5365           
  Branches       612      612           
========================================
  Hits          5363     5363           
  Partials         2        2           
Impacted Files Coverage Δ
...ities/data_classes/api_gateway_authorizer_event.py 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e4cbd7...e4c5821. Read the comment docs.

Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding a comment to easily reason about this change later. Then merging.

Thanks a lot for the fix @michaelbrewer. It does make me think whether we need a hard check, but am struggling with bandwidth to make a more informed decision.

@heitorlessa heitorlessa changed the title fix(lambda-authorizer): Properly parse resource path in arn fix(lambda-authorizer): allow proxy resources path in arn Mar 2, 2022
@heitorlessa heitorlessa merged commit 51ff350 into aws-powertools:develop Mar 2, 2022
@heitorlessa heitorlessa deleted the fix-1047 branch March 2, 2022 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working size/S Denotes a PR that changes 10-29 lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants