Skip to content

fix: preserve authorizer config for overlapping API routes #9166

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.

Sign up for GitHub

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

Open
Zahoor-ishfaq wants to merge 9 commits into
base: develop
Choose a base branch
from
Open

fix: preserve authorizer config for overlapping API routes #9166

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
bfb7b0e
fix: preserve authorizer config for overlapping API routes
Zahoor-ishfaq Aug 4, 2026
c5a226e
fix: address route deduplication review feedback
Zahoor-ishfaq Aug 4, 2026
df9f9b1
fix: preserve route ownership during CORS normalization
Zahoor-ishfaq Aug 4, 2026
6b5599b
fix: preserve specific routes during SAM route merge
Zahoor-ishfaq Aug 4, 2026
6f849fb
fix: narrow SAM route preservation
Zahoor-ishfaq Aug 4, 2026
4e8413c
fix: require explicit authorizer intent for route preservation
Zahoor-ishfaq Aug 4, 2026
2535804
fix: prefer unprotected route for synthesized OPTIONS
Zahoor-ishfaq Aug 4, 2026
3a8614f
fix: avoid payload inheritance from preserved routes
Zahoor-ishfaq Aug 4, 2026
a227931
fix: reconcile routes across operation names
Zahoor-ishfaq Aug 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 85 additions & 27 deletions samcli/lib/providers/api_collector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ def get_api(self) -> Api:
@staticmethod
def normalize_cors_methods(routes: List[Route], cors: Optional[Cors]) -> List[Route]:
"""
Adds OPTIONS method to all the route methods if cors exists
Adds OPTIONS method to route methods if cors exists while preserving
explicit OPTIONS ownership within a route group.

Parameters
-----------
Expand All @@ -229,15 +230,29 @@ def normalize_cors_methods(routes: List[Route], cors: Optional[Cors]) -> List[Ro

Return
-------
A list of routes without duplicate routes with the same function_name and method
A list of routes with at most one OPTIONS owner per route group
"""
if not cors:
return routes

def add_options_to_route(route: Route) -> Route:
if "OPTIONS" not in route.methods:
route.methods.append("OPTIONS")
return route
grouped_routes: Dict[str, List[Route]] = {}

return routes if not cors else [add_options_to_route(route) for route in routes]
for route in routes:
key = "{}-{}-{}-{}".format(route.stack_path, route.function_name, route.path, route.operation_name or "")
grouped_routes.setdefault(key, []).append(route)

result: List[Route] = []

for route_group in grouped_routes.values():
options_claimed = any("OPTIONS" in route.methods for route in route_group)

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[GENERAL] options_claimed short-circuits the new unauthenticated-owner preference whenever the group contains an ANY-derived route, because Route.normalize_method expands ANY to ANY_HTTP_METHODS, which always includes OPTIONS. So the docstring's claim that "synthesized OPTIONS prefers a route without a linked local authorizer" does not hold for the most common CORS shape.

Concretely, an AWS::Serverless::Api with Cors set and a group of ANY /x (authorizer MyAuth) plus POST /x (security: []): dedupe_function_routes strips POST from the ANY route but leaves OPTIONS on it, so options_claimed is True and OPTIONS stays owned by the authorizer-protected route. _request_handler dispatches on route.authorizer_object, so the local preflight is challenged. On AWS the Cors property causes the transform to emit an unauthenticated OPTIONS mock integration that overrides ANY, so preflight succeeds there — a browser-visible local/deployed divergence.

The check that matters is not "does any route in the group list OPTIONS" but "does any route explicitly declare OPTIONS". An OPTIONS entry that only exists because ANY was expanded is not an ownership claim, and when cors is configured it should be re-assignable to an unauthorized sibling the same way an explicitly declared OPTIONS route already takes precedence in dedupe_function_routes. As written, the preference only ever fires for groups made up entirely of single-method routes — which is what the new tests cover.


for route in route_group:
if not options_claimed:

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[BUG] The synthesized OPTIONS owner is chosen by group iteration order, so it can land on an authorizer-protected route even when an unauthenticated sibling exists in the same group.

After the dedupe_function_routes change, a group can legitimately contain several routes with different authorizers. Consider an AWS::Serverless::Api with Cors set and a DefinitionBody where /x get has security: [{MyAuth: []}] and /x post has security: []. The Swagger parser gives both the same backing function, no operationId, and the same path, so they share the dedupe group key and now survive as two separate routes ([GET] with MyAuth, [POST] with none). Neither contains OPTIONS, so options_claimed is False and the first route in the group — Swagger paths declaration order — gets OPTIONS appended.

If that first route is get, preflight is served by a route carrying authorizer_object, and _request_handler invokes the Lambda authorizer for it. Browser preflight requests do not carry credentials, so the preflight fails with 401/403 and the real request is never sent. Before this PR these routes were collapsed into one, so this selection did not exist.

Since keeping preflight out of the authorizer path is the point of the PR, pick the owner deliberately rather than by position:

for route_group in grouped_routes.values():
   options_claimed = any("OPTIONS" in route.methods for route in route_group)

   if not options_claimed:
       # Prefer a route without an authorizer so CORS preflight is not authorized
       owner = next(
           (route for route in route_group if route.authorizer_object is None),
           route_group[0],
       )
       owner.methods.append("OPTIONS")

   result.extend(route_group)

route.methods.append("OPTIONS")
options_claimed = True
result.append(route)

return result

@staticmethod
def dedupe_function_routes(routes: List[Route]) -> List[Route]:
Expand All @@ -251,30 +266,73 @@ def dedupe_function_routes(routes: List[Route]) -> List[Route]:
-------
A list of routes without duplicate routes with the same stack_path, function_name and method
"""
grouped_routes: Dict[str, Route] = {}
grouped_routes: Dict[str, List[Route]] = {}

for route in routes:
key = "{}-{}-{}-{}".format(route.stack_path, route.function_name, route.path, route.operation_name or "")
config = grouped_routes.get(key, None)
methods = route.methods
if config:
methods += config.methods
sorted_methods = sorted(methods)
# Prefer route-specific CORS over None
cors = route.cors if route.cors is not None else (config.cors if config else None)
grouped_routes[key] = Route(
function_name=route.function_name,
path=route.path,
methods=sorted_methods,
event_type=route.event_type,
payload_format_version=route.payload_format_version,
operation_name=route.operation_name,
stack_path=route.stack_path,
authorizer_name=route.authorizer_name,
authorizer_object=route.authorizer_object,
cors=cors,
grouped_routes.setdefault(key, []).append(route)

result: List[Route] = []

def has_same_authorizer(first: Route, second: Route) -> bool:
return (
first.authorizer_name == second.authorizer_name and first.authorizer_object == second.authorizer_object
)
return list(grouped_routes.values())

for route_group in grouped_routes.values():

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[GENERAL] The split logic here is correct, but for SAM templates it is only reachable when the explicit OPTIONS event is declared after the ANY event, so the order-dependence the PR aims to remove still exists one layer up.

SamApiProvider.merge_routes() runs before get_api() (see sam_api_provider.py:100) and de-dupes by path + method, keeping the last writer per key:

for config in all_configs:
   # Normalize the methods before de-duping to allow an ANY method in implicit API to override a regular HTTP
   # method on explicit route.
   for normalized_method in config.methods:
       key = config.path + normalized_method
       ...
       all_routes[key] = config

result = set(all_routes.values())  # Assign to a set() to de-dupe

Because Route.normalize_method already expands ANY into all seven verbs, the ANY route claims the /{proxy+}OPTIONS key too. The explicit OPTIONS route only has that one key, so if it is written first and then overwritten, it is no longer a value in all_routes and is dropped by set(all_routes.values()) — dedupe_function_routes never sees it, and the OPTIONS method keeps the ANY route's authorizer.

Concretely, for the template in #9165:

  • ANY event declared first, OPTIONS event second → both routes survive merge_routes → this fix applies.
  • OPTIONS event declared first, ANY second → the OPTIONS route is discarded → preflight is still authorized.
  • ANY event implicit (no RestApiId) and OPTIONS explicit → implicit routes are iterated last by design, so the OPTIONS route is always discarded.

The new tests all call dedupe_function_routes/get_api directly, so they cannot catch this. Please either give the more specific method precedence in merge_routes (an explicit single-method route should not be clobbered by an expanded ANY route) or add a test that drives the scenario through SamApiProvider.extract_resources with the OPTIONS event declared first, so the end-to-end behavior is pinned.

merged_routes: List[Route] = []
group_cors = next((route.cors for route in route_group if route.cors is not None), None)

# Process broader routes first so a more specific route can own
# overlapping methods, e.g. explicit OPTIONS overriding ANY.
for route in sorted(route_group, key=lambda item: len(item.methods), reverse=True):
methods = list(dict.fromkeys(route.methods))

for existing_route in merged_routes:
if not has_same_authorizer(existing_route, route):
existing_route.methods = [method for method in existing_route.methods if method not in methods]

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[BUG] Stripping the overlapping methods makes the split disjoint here, but get_api() runs the CORS normalization immediately after dedupe (lines 195-196):

routes = self.dedupe_function_routes(self.routes)
routes = self.normalize_cors_methods(routes, self.cors)

and normalize_cors_methods appends OPTIONS back onto every route that does not already have it:

def add_options_to_route(route: Route) -> Route:
   if "OPTIONS" not in route.methods:
       route.methods.append("OPTIONS")
   return route

So whenever the API has CORS configured, the authorizer-protected route regains OPTIONS and both split routes claim <path>:OPTIONS. LocalApigwService.create() keys _dict_of_routes by path:method only (local_apigw_service.py:145-146), so one entry silently overwrites the other and the winner is whichever route comes later in api.routes. That means the "explicit route owns the overlapping method" rule this PR establishes is re-decided by list position: it currently produces the right result only because the broadest route is appended first by the sort on line 273.

Consider making the released methods explicit instead of relying on ordering — e.g. track the methods a route gave up during dedupe and have normalize_cors_methods skip injecting OPTIONS into a route that deliberately released it (or skip injection when another route in the same group already serves OPTIONS). Otherwise the order-dependence the PR removes from dedupe_function_routes reappears one step later in the pipeline.


matching_route = next(
(existing_route for existing_route in merged_routes if has_same_authorizer(existing_route, route)),
None,
)

if matching_route:
matching_route.methods = sorted(set(matching_route.methods + methods))

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[GENERAL] When two routes in a group share an authorizer, the merge only updates methods and cors on the surviving route — which is the first one created, i.e. the route with the most methods (usually the ANY route). Every other attribute of the narrower route is discarded, including payload_format_version.

That one has real consequences for HTTP APIs, because a missing payload format version is treated as 2.0 (local_apigw_service.py:460):

if route.event_type == Route.HTTP and route.payload_format_version in [None, "2.0"]:

So an ANY route with no PayloadFormatVersion now deterministically shadows a sibling method route (same function/path/operation and same authorizer) that declares "1.0", and that method's Lambda receives a v2 event instead of a v1 event. SamApiProvider.merge_routes already guards against exactly this loss:

if route and route.payload_format_version and config.payload_format_version is None:
   config.payload_format_version = route.payload_format_version

Mirroring it in the merge branch keeps the behavior consistent:

if matching_route:
   matching_route.methods = sorted(set(matching_route.methods + methods))
   if matching_route.payload_format_version is None:
       matching_route.payload_format_version = route.payload_format_version
   if route.cors is not None:
       matching_route.cors = route.cors
   continue

The same one-sided loss applies to use_default_authorizer, which the PR description says is preserved: it is preserved on the copy (line 302) but on a merge only the first route's value survives, and test_merges_routes_with_same_resolved_authorizer locks that in (the False from the POST route is dropped). That is currently inert because _link_authorizers() has already run, but it is worth a comment in the code so the next reader does not assume the flag is still meaningful.


if matching_route.payload_format_version is None:
matching_route.payload_format_version = route.payload_format_version

if route.cors is not None:
matching_route.cors = route.cors

# Authorizers are already resolved by _link_authorizers() before
# deduplication, so use_default_authorizer does not affect this merge.
continue

merged_routes.append(
Route(
function_name=route.function_name,
path=route.path,
methods=sorted(methods),
event_type=route.event_type,
payload_format_version=route.payload_format_version,
operation_name=route.operation_name,
stack_path=route.stack_path,
authorizer_name=route.authorizer_name,
authorizer_object=route.authorizer_object,
use_default_authorizer=route.use_default_authorizer,
cors=route.cors,

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[BUG] Route-level CORS is no longer propagated across the routes in a group. The removed code deliberately merged it:

# Prefer route-specific CORS over None
cors = route.cors if route.cors is not None else (config.cors if config else None)

The new code only carries cors forward inside the if matching_route: branch, i.e. only between routes that share an authorizer config. When a group now splits because the authorizers differ, the non-OPTIONS route is constructed with cors=route.cors, which is None for every non-OPTIONS method — cfn_api_provider.py only ever attaches route-level CORS to OPTIONS methods:

cors=cors if method == "OPTIONS" else None,

There is no fallback to recover it. In local_apigw_service._request_handler:

cors = route.cors if route.cors is not None else self.api.cors
...
headers.update(cors_headers)

self.api.cors is not set in this path either, because cfn_api_provider assigns collector.cors only in the elif cors: branch for non-OPTIONS methods. So for a REST API whose OPTIONS method carries the CORS integration responses while its other methods carry an authorizer, the preflight still succeeds but the actual GET/POST response is returned with no Access-Control-Allow-* headers, and the browser rejects it. Previously the single merged route held cors and all methods got the headers.

Compute the group's effective CORS once and apply it to every resulting route that has none of its own:

group_cors = next((route.cors for route in route_group if route.cors is not None), None)
...
for merged_route in merged_routes:
   if merged_route.cors is None:
       merged_route.cors = group_cors

result.extend(route for route in merged_routes if route.methods)

)
)

for merged_route in merged_routes:
if merged_route.cors is None:
merged_route.cors = group_cors

result.extend(route for route in merged_routes if route.methods)

return result

def add_binary_media_types(self, logical_id: str, binary_media_types: Optional[List[str]]) -> None:
"""
Expand Down
15 changes: 15 additions & 0 deletions samcli/lib/providers/sam_api_provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,21 @@ def merge_routes(collector: ApiCollector) -> List[Route]:
route = all_routes.get(key)
if route and route.payload_format_version and config.payload_format_version is None:
config.payload_format_version = route.payload_format_version

# Preserve a single-method route from the same function when a later
# expanded ANY route overlaps it. This keeps explicit method intent
# independent of declaration order while retaining the existing
# precedence rules between different functions and stacks.
if (

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[GENERAL] The new rule is broader than the OPTIONS scenario in the PR description: it preserves any single-method route of the same function against an expanding ANY route, regardless of the authorizer configuration. That reverses the precedence this function documents and that the loop comment immediately above still asserts:

# Normalize the methods before de-duping to allow an ANY method in implicit API to override a regular HTTP
# method on explicit route.

Concretely, for a function with an explicit GET /x event on Api1 plus an implicit ANY /x event, GET previously resolved to the implicit route; it now resolves to the explicit one, so Api1's authorizer (and default authorizer) applies to GET instead of the implicit API's. Nothing about OPTIONS or CORS is involved, and no test in the diff covers it — both new tests assert only the OPTIONS case.

Since dedupe_function_routes now splits by authorizer anyway, the skip only needs to happen when the two routes would not merge. Gating on differing authorizer intent keeps the fix targeted and leaves the documented implicit-over-explicit precedence intact for everything else:

and (
   route.authorizer_name != config.authorizer_name
   or route.use_default_authorizer != config.use_default_authorizer
)

Note this comparison must use the raw authorizer_name/use_default_authorizer fields, since merge_routes runs before _link_authorizers() and authorizer_object is still unset at this point. If the broader behavior is intentional, the stale loop comment and the merge_routes docstring should be updated to describe the new precedence.

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[BUG] The preservation rule is asymmetric: it is only consulted when the expanded ANY route is the second route to reach a given path+method key. When the ANY route is processed first, len(route.methods) == 1 is false (it is 7), the block is skipped entirely, and the narrow route unconditionally overwrites the key. That means the explicit-intent guard on line 612 (route.authorizer_name is not None or not route.use_default_authorizer) is bypassed, and authorization for a method still flips based on declaration order.

Trace with two Api events on the same function/path in one explicit AWS::Serverless::Api (so both land in explicit_routes at the same stack depth, preserving template order):

Events:
 Any:
   Type: Api
   Properties: { Path: /x, Method: ANY, RestApiId: Api1, Auth: { Authorizer: MyAuthorizer } }
 Get:
   Type: Api
   Properties: { Path: /x, Method: GET, RestApiId: Api1 }   # no Auth
  • Any first: all 7 keys map to anyRoute. Then Get is processed — the preservation block is skipped because len(anyRoute.methods) == 7, so all_routes["/xGET"] = getRoute. Both routes survive. dedupe_function_routes then sees authorizer_name="MyAuthorizer" vs None, so can_merge is false, strips GET off the ANY route, and keeps getRoute — GET /x ends up unauthenticated.
  • Get first: all_routes["/xGET"] = getRoute, then Any evaluates the preservation block. The explicit-intent guard fails (authorizer_name is None and use_default_authorizer is True), so anyRoute overwrites the key and getRoute is dropped entirely — GET /x ends up authenticated with MyAuthorizer.

Same template semantics, opposite authorization outcome purely from YAML key order. This is the order-dependence the PR sets out to remove, and it is the security-relevant direction (authorization silently dropped). Note test_must_prefer_implicit_any_for_same_function_with_same_authorizer_intent only exercises the narrow-first ordering, so the gap is not covered.

The rule needs to be applied from both directions: when a narrow route overwrites a key currently held by an expanded ANY route of the same function/stack/event type and the narrow route declares no authorizer intent, it should end up with the ANY route's authorizer configuration rather than silently winning with authorizer_name=None. I'd avoid prescribing an exact snippet here since mutating config in place has knock-on effects, but the condition needs a mirrored branch keyed on set(route.methods) == set(Route.ANY_HTTP_METHODS) and len(config.methods) == 1.

route
and len(route.methods) == 1
and set(config.methods) == set(Route.ANY_HTTP_METHODS)
and route.function_name == config.function_name

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[BUG] The preservation condition pins function_name, stack_path and event_type to be equal, but not operation_name. That matters because it is the only field in this condition that is also part of the grouping key used later by dedupe_function_routes:

[REDACTED] route.function_name, route.path, route.operation_name or "")

When the preserved narrow route and the skipped ANY route have different operation_name values, they land in different groups, so the new method-stripping logic in dedupe_function_routes never runs between them. The ANY route keeps the overlapping method in config.methods and both routes survive.

This is reachable: swagger-derived routes get operation_name from operationId (swagger/parser.py), while SAM Api/HttpApi events leave it None. A function referenced from an explicit DefinitionBody options: entry with an operationId, plus an implicit ANY event on the same path, produces two routes both claiming OPTIONS for that path.

Downstream, LocalApigwService.create() registers a Flask rule per route and fills _dict_of_routes keyed by path:method, so the two entries collide and the authorizer actually applied to OPTIONS depends on route ordering — the order-dependence this PR sets out to remove.

Adding operation_name to the condition keeps the routes in the same dedupe group (or lets the pre-existing clobber happen), so the overlap is always resolved:

if (
   route
   and len(route.methods) == 1
   and set(config.methods) == set(Route.ANY_HTTP_METHODS)
   and route.function_name == config.function_name
   and route.stack_path == config.stack_path
   and route.event_type == config.event_type
   and route.operation_name == config.operation_name
):
   continue

and route.stack_path == config.stack_path

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[GENERAL] The operation_name equality clause means the bug this PR targets is still reproducible whenever the narrow route carries an operationId, and the new test suite locks that gap in rather than flagging it.

A swagger-defined preflight normally has an operationId:

paths:
 /x:
   options:
     operationId: Preflight
     security: []          # explicitly unauthenticated
     x-amazon-apigateway-integration: {...}   # -> SamFunc1

plus an ANY /x event on the same function with Auth: {Authorizer: MyAuthorizer}. Here route.operation_name == "Preflight" while config.operation_name is None, so the guard short-circuits, all_routes["/xOPTIONS"] = config executes, and the swagger route is discarded entirely. The preflight then runs under MyAuthorizer — the exact behaviour #9165 reports. test_operation_name_mismatch_does_not_leave_duplicate_options_routes asserts this outcome (assertEqual(options_routes[0].authorizer_name, "MyAuthorizer")), so the case is knowingly excluded but reads as intended behaviour to a future reader.

I understand why the clause is there: dedupe_function_routes groups by "{stack_path}-{function_name}-{path}-{operation_name}", so two preserved routes with different operation_name land in different groups, the method-stripping in dedupe_function_routes never runs across them, and both would keep OPTIONS. local_apigw_service._dict_of_routes is keyed on path:method, so the last registration would silently win — worse than the current partial fix.

Two options that close the gap instead of excluding it:

  • Drop operation_name from the grouping key in dedupe_function_routes and normalize_cors_methods so overlapping routes are reconciled regardless of operationId. The merged route already discards the narrower route's operation_name today, so this mainly widens the grouping.
  • Keep the clause but record the limitation in the merge_routes docstring and rename the test to state that a differing operationId is an unsupported case, so the assertion is not mistaken for desired behaviour.

Either way the current test name reads as "no duplicates" when what it actually pins down is "authorization is applied to a route that declared security: []".

Scope of what I verified (workspace is the merge-base, so I traced the post-merge logic by hand rather than executing it): the previously raised findings are addressed in this revision — route-level CORS is restored via group_cors, use_default_authorizer is excluded from has_same_authorizer, normalize_cors_methods no longer re-appends OPTIONS onto a route whose sibling already owns it, payload_format_version inheritance moved after the continue, function_name/stack_path/event_type/operation_name are pinned in the preservation guard, and the guard now requires the narrow route to declare explicit authorizer intent. I also traced both event-declaration orders (OPTIONS before ANY and after) and confirmed dedupe_function_routes produces disjoint method sets in each, and checked that multiple Route objects sharing one path register cleanly in Flask/Werkzeug (same bound view_func, method-aware rule matching).

and route.event_type == config.event_type
):
continue

all_routes[key] = config

result = set(all_routes.values()) # Assign to a set() to de-dupe
Expand Down
180 changes: 180 additions & 0 deletions tests/unit/commands/local/lib/test_api_collector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,183 @@ def test_link_authorizers(self, routes, authorizers, default_authorizer, expecte
self.api_collector._link_authorizers()

self.assertEqual(self.api_collector._route_per_resource, {self.apigw_id: expected_routes})


class TestApiCollector_dedupe_function_routes(TestCase):
def test_preserves_options_route_with_different_authorizer(self):
routes = [
Route(
function_name="func",
path="/{proxy+}",
methods=["ANY"],
authorizer_name="MyAuthorizer",
),
Route(
function_name="func",
path="/{proxy+}",
methods=["OPTIONS"],
authorizer_name=None,
use_default_authorizer=False,
),
]

actual = ApiCollector.dedupe_function_routes(routes)

expected = [
Route(
function_name="func",
path="/{proxy+}",
methods=["GET", "DELETE", "PUT", "POST", "HEAD", "PATCH"],
authorizer_name="MyAuthorizer",
),
Route(
function_name="func",
path="/{proxy+}",
methods=["OPTIONS"],
authorizer_name=None,
use_default_authorizer=False,
),
]

self.assertCountEqual(expected, actual)

def test_preserves_cors_when_routes_split_by_authorizer(self):
cors = object()

routes = [
Route(
function_name="func",
path="/{proxy+}",
methods=["ANY"],
authorizer_name="MyAuthorizer",
),
Route(
function_name="func",
path="/{proxy+}",
methods=["OPTIONS"],
authorizer_name=None,
use_default_authorizer=False,
cors=cors,
),
]

actual = ApiCollector.dedupe_function_routes(routes)

self.assertEqual(len(actual), 2)
self.assertTrue(all(route.cors is cors for route in actual))

def test_merges_routes_with_same_resolved_authorizer(self):
routes = [
Route(
function_name="func",
path="/x",
methods=["GET"],
authorizer_name=None,
use_default_authorizer=True,
),
Route(
function_name="func",
path="/x",
methods=["POST"],
authorizer_name=None,
use_default_authorizer=False,
),
]

actual = ApiCollector.dedupe_function_routes(routes)

self.assertEqual(len(actual), 1)
self.assertEqual(sorted(actual[0].methods), ["GET", "POST"])

def test_cors_normalization_does_not_readd_options_to_protected_route(self):
routes = [
Route(
function_name="func",
path="/{proxy+}",
methods=["ANY"],
authorizer_name="MyAuthorizer",
),
Route(
function_name="func",
path="/{proxy+}",
methods=["OPTIONS"],
authorizer_name=None,
use_default_authorizer=False,
),
]

deduped_routes = ApiCollector.dedupe_function_routes(routes)
actual = ApiCollector.normalize_cors_methods(deduped_routes, object())

options_routes = [route for route in actual if "OPTIONS" in route.methods]

self.assertEqual(len(options_routes), 1)
self.assertIsNone(options_routes[0].authorizer_name)

def test_preserves_payload_format_version_when_merging_routes(self):
routes = [
Route(
function_name="func",
path="/x",
methods=["ANY"],
event_type=Route.HTTP,
authorizer_name=None,
),
Route(
function_name="func",
path="/x",
methods=["GET"],
event_type=Route.HTTP,
payload_format_version="1.0",
authorizer_name=None,
),
]

actual = ApiCollector.dedupe_function_routes(routes)

self.assertEqual(len(actual), 1)
self.assertEqual(actual[0].payload_format_version, "1.0")

def test_get_api_preserves_explicit_unauthorized_options_with_cors(self):
collector = ApiCollector()

authorizer = Authorizer(
authorizer_name="MyAuthorizer",
type="request",
payload_version="1.0",
)

collector.add_authorizers("api", {"MyAuthorizer": authorizer})
collector.set_default_authorizer("api", "MyAuthorizer")

collector.add_routes(
"api",
[
Route(
function_name="func",
path="/{proxy+}",
methods=["ANY"],
),
Route(
function_name="func",
path="/{proxy+}",
methods=["OPTIONS"],
authorizer_name=None,
use_default_authorizer=False,
),
],
)

collector.cors = object()

api = collector.get_api()

options_routes = [route for route in api.routes if "OPTIONS" in route.methods]
get_routes = [route for route in api.routes if "GET" in route.methods]

self.assertEqual(len(options_routes), 1)
self.assertIsNone(options_routes[0].authorizer_name)

self.assertEqual(len(get_routes), 1)
self.assertEqual(get_routes[0].authorizer_name, "MyAuthorizer")
self.assertIs(get_routes[0].authorizer_object, authorizer)
Loading