Skip to content

fix: HttpApi explicit AuthorizationScopes: [] is not overridden by authorizer default - #3980

Open
Adityaj0 wants to merge 1 commit into
aws:developfrom
Adityaj0:fix/httpapi-empty-authorization-scopes-override
Open

fix: HttpApi explicit AuthorizationScopes: [] is not overridden by authorizer default#3980
Adityaj0 wants to merge 1 commit into
aws:developfrom
Adityaj0:fix/httpapi-empty-authorization-scopes-override

Conversation

@Adityaj0

Copy link
Copy Markdown

Summary

Fixes #3979.

For AWS::Serverless::HttpApi, an event/function-level Auth.AuthorizationScopes: [] is meant to override the named authorizer's default AuthorizationScopes, requiring no scopes for that method — this already works correctly for AWS::Serverless::Api (REST APIs).

OpenApiEditor.add_auth_to_method (samtranslator/open_api/open_api.py) defaulted an unset AuthorizationScopes to []:

authorization_scopes = auth.get("AuthorizationScopes", [])

_set_method_authorizer then checked it with if authorization_scopes: — since [] is falsy, this is indistinguishable from "not set," so the explicit override was silently dropped and the authorizer's default scopes were enforced instead. This means requests the template author intended to allow without those scopes get rejected by API Gateway.

SwaggerEditor's equivalent REST API code path (samtranslator/swagger/swagger.py) already handles this correctly — it uses None as the "not set" sentinel (auth.get("AuthorizationScopes"), no default) and checks is not None. This PR applies the identical fix to OpenApiEditor, and also aligns the authorizer-presence check (authorizers.get(authorizer_name) is not None) with swagger.py's pattern, replacing an unguarded authorizers[authorizer_name] dict-subscript.

Test plan

  • Added tests/translator/input/http_api_with_auth_with_default_scopes.yaml, a direct HttpApi port of the existing REST API regression test tests/translator/input/api_with_auth_with_default_scopes.yaml (default authorizer, explicit authorizer, scope overwrite, and both AuthorizationScopes: [] cases), with expected output for all three partitions (aws, aws-cn, aws-us-gov).
  • Verified the new test fails against the pre-fix code — both AuthorizationScopes: [] cases produced the authorizer's default scopes instead of [] — and passes with the fix, with output matching the semantics already proven correct for REST APIs.
  • python -m pytest tests/translator tests/plugins tests/parser tests/openapi tests/swagger tests/model — 3241 passed (same 5 pre-existing, unrelated SAR-timing/region failures as on develop).
  • ruff check / black --check clean on the changed file.

…thorizer default

OpenApiEditor.add_auth_to_method defaulted an unset AuthorizationScopes to
[], then _set_method_authorizer checked it with `if authorization_scopes:`.
Since [] is falsy, this couldn't distinguish "not set" from an explicit
empty-list override, so a method-level `AuthorizationScopes: []` (meant to
require no scopes, overriding the authorizer's default) was silently
ignored and the authorizer's default AuthorizationScopes were enforced
instead.

SwaggerEditor's equivalent REST API code path (swagger.py) already gets
this right by using None as the "not set" sentinel and an `is not None`
check. This applies the same fix to OpenApiEditor so HTTP APIs behave
consistently with REST APIs, and also aligns the authorizer-presence
check (`authorizers.get(authorizer_name) is not None`) with swagger.py's
pattern.

Fixes aws#3979
@Adityaj0
Adityaj0 requested a review from a team as a code owner August 14, 2026 07:07
@roger-zhangg

Copy link
Copy Markdown
Member

Maintainer review pass. I checked this out locally at f8b0c92 in a separate worktree and ran the tests; findings below.

Why this is BLOCKED (not a rebase, not a failing check)

  • mergeable is MERGEABLE. The branch is 5 commits behind develop (merge base bbc6a9ab) but conflict-free, so no rebase is needed.
  • The required workflows have never run on this head commit. GET /actions/runs?head_sha=f8b0c92... returns Tests, Check compatibility, and CodeQL all with conclusion: action_required — this is a fork PR, so a maintainer has to click "Approve and run workflows". The only check that has reported is lambda-tooling pr reviewer (pass), while develop requires ubuntu-latest / 3.10, ubuntu-latest / 3.11, check-compatibility, and Analyze (python).
  • Separately, develop requires 2 approving reviews including code-owner review; reviewDecision is REVIEW_REQUIRED and there are 0 reviews so far.

So the block is on our side (unblock CI, then review), not something the author needs to fix.

Diff size: real source vs generated fixtures

Of the +1194/-6, only samtranslator/open_api/open_api.py is source: +3/-6, net -3 lines. The other +1191 is transform-test fixtures — one hand-written 87-line input template plus three generated 368-line expected outputs (output/, output/aws-cn/, output/aws-us-gov/), which is exactly what bin/add_transform_test.py emits per DEVELOPMENT_GUIDE.md:132-147. Worth saying explicitly so the line count doesn't scare off reviewers.

The []-vs-None distinction is handled correctly — verified

This is the crux, and the patch uses None as the sentinel rather than relying on falsiness:

  • samtranslator/open_api/open_api.py:316auth.get("AuthorizationScopes") with no [] default, so "unset" stays None and an explicit [] stays [].
  • Deleting the if authorization_scopes is None: authorization_scopes = [] normalization that used to sit at the top of _set_method_authorizer is required, not incidental — it collapsed None into [] before anything could tell them apart.
  • samtranslator/open_api/open_api.py:345if authorization_scopes is not None: is what lets an explicit [] win over the authorizer's default.

This is line-for-line the same shape as the already-correct REST path at samtranslator/swagger/swagger.py:744-748; I compared them side by side and the description's parity claim holds.

One note for other reviewers, since it's the only part of the diff not strictly about scopes: the switch from authorizers[authorizer_name] to authorizers.get(authorizer_name) is not None at open_api.py:347 is defensive only on this path. The sole caller is samtranslator/model/eventsources/push.py:1560, and push.py:1539-1551 already raises InvalidEventException when the named authorizer is missing or falsy, so the old subscript could not KeyError and its truthiness could not be False here; NONE/AWS_IAM are short-circuited earlier at open_api.py:338. No behavior change, no regression risk — it just aligns with swagger.py.

Fixtures: both input and expected output cover the empty-list case

  • Input: tests/translator/input/http_api_with_auth_with_default_scopes.yaml:78 and :87 are the two AuthorizationScopes: [] cases (one against the default authorizer, one against an explicitly named one).
  • Expected output pins them to [] rather than to the authorizer default: tests/translator/output/http_api_with_auth_with_default_scopes.json:128-132 ("MyDefaultCognitoAuth": []) and :89-93 ("MyCognitoAuthWithDefaultScopes": []). The four non-empty cases retain their scopes.
  • All three partitions are present, and aws-cn/aws-us-gov differ from aws by exactly one line — the AWSLambdaBasicExecutionRole policy ARN partition. Consistent with the other partitioned fixtures.
  • It is a faithful port of the REST fixture: identical path names and identical expected scope values against tests/translator/output/api_with_auth_with_default_scopes.json, minus the NONE case, which isn't needed here because NONE never reaches the changed lines. {"AuthName": []} is an already-established emitted shape (cf. tests/translator/output/http_api_multiple_authorizers.json).
  • Wiring is correct: tests/translator/test_translator.py:36-40 auto-discovers input/, so the new fixture is collected as test_transform_success_0861/0862/0863. It does not need to be added to the explicit list at test_translator.py:315-361, since _read_input (test_translator.py:150-153) already performs the same JSON round-trip that test_transform_success_openapi3 adds.

Test results (local, Python 3.10.18, at f8b0c92)

  • pytest tests/translator/test_translator.py -k http_api_with_auth_with_default_scopes3 passed (one per partition).
  • pytest tests/translator/test_translator.py -k auth214 passed.
  • pytest tests/translator tests/openapi tests/swagger tests/model3086 passed, 0 failed.
  • ruff check samtranslator/open_api/open_api.py → clean.
  • I also confirmed the fixture is a genuine regression test rather than a snapshot of current behavior: reverting only open_api.py to develop while keeping the new fixtures fails all 3 partitions, with /cognitodefaultscopesnone emitting ["default.write", "default.read"] and /cognitodefaultauthdefaultscopesnone emitting ["default.delete", "default.update"] instead of []. That reproduces HttpApi: explicit AuthorizationScopes: [] is silently overridden by the authorizer's default scopes #3979 exactly, and the fix removes it.

Nit (non-blocking)

The new fixture uses Runtime: nodejs12.x, inherited from the REST template it was ported from. 242 of 856 existing input fixtures still use it and transform tests don't validate runtimes, so it's harmless — but newer fixtures tend to use python3.12.

Verdict

The source change is correct, minimal, and brings HttpApi in line with the REST semantics we already ship; test coverage is appropriate and demonstrably fails without the fix. Apologies for the 19-day silence. Next step is a maintainer approving the workflow runs so the required checks can report, after which this needs two approvals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HttpApi: explicit AuthorizationScopes: [] is silently overridden by the authorizer's default scopes

2 participants