Apigw fix openapi identity sources#11939
Conversation
cloutierMat
added
aws:apigateway
| if authorizer := get_authorizer(method_schema) or default_authorizer: | ||
| method_authorizer = authorizer or default_authorizer |
There was a problem hiding this comment.
@bentsku This seemed to be a redundant operation... Do you see any reason to keep this? Was this a issue with the minifier/obfuscation tool?
There was a problem hiding this comment.
Seems like this was just being too safe probably? Don't think it's because of the minifier, as this is community and this code isn't minified. Probably just a mistake not caught in a review 😄
bentsku
left a comment
bentsku
left a comment
There was a problem hiding this comment.
LGTM! Nice find! I can't wait for us to rework the Import/Export functionality in the future, as this helper should at least be extracted in its own file, and reworked to be a bit clearer... and more tested 😄 nice and clean fix! 💯
| if authorizer := get_authorizer(method_schema) or default_authorizer: | ||
| method_authorizer = authorizer or default_authorizer |
There was a problem hiding this comment.
Seems like this was just being too safe probably? Don't think it's because of the minifier, as this is community and this code isn't minified. Probably just a mistake not caught in a review 😄
| # assert that are no multiple authorizers | ||
| authorizers = aws_client.apigateway.get_authorizers(restApiId=rest_api_id) | ||
| snapshot.match("get-authorizers", authorizers) | ||
| snapshot.match("get-authorizers", sorted(authorizers["items"], key=lambda x: x["name"])) |
There was a problem hiding this comment.
very minor nit: could also use itemgetter("name") here like the sorted call below.
There was a problem hiding this comment.
Honestly, don't have to change it, the CI is green, let's keep it!
2 participants
Motivation
Our current implementation of the OpenApi import did not add authorization scopes when creating a method with an authorizer. This was not an issue with Legacy apigw, as we did not handle the scopes properly.
Authorization scopes can be added to Cognito authorizers. They directly impact whether the api accepts an Identity token or an Access Token see docs. As Next Gen provider does enforce this, providing a greater compatibility with aws, user's configs would fail to authorize a request when an Access token is used to authenticate.
Changes