Apigw fix openapi identity sources #11939
Conversation
| 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 😄
| 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 😄
| 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!
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