Skip to content

Commit 890178e

Browse files
committed
Fix bugs, adding breaking changes
1 parent 25bd329 commit 890178e

6 files changed

Lines changed: 176 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ All notable changes to this project will be documented in this file. See [standa
1414
- A VPC is no longer auto-enabled. If a VPC-requiring feature (ALB, OpenSearch Provisioned, or any container-based pipeline) is enabled while `app.useGlobalVpc.enabled` is `false`, configuration validation now fails with an explicit error listing the offending features instead of silently turning the VPC on. **Existing config files may need updating:** if you hit this error on upgrade, set `app.useGlobalVpc.enabled` to `true` (the value the deployment was implicitly using before) or disable the listed features. See the [v2.5 to v2.6 migration guide](https://awslabs.github.io/visual-asset-management-system/deployment/update-the-solution#v25-to-v26).
1515
- Provisioned OpenSearch `availabilityZoneCount` now defaults to `2`, and the VPC is now built with exactly that many Availability Zones. Previously the VPC builder always provisioned **3** Availability Zones for provisioned OpenSearch even though the OpenSearch domain only ever used 2 of them, so the third AZ's subnet was created but unused. VAMS now deploys 2 AZs by default (or 3 only when `availabilityZoneCount` is set to `3`) and uses them consistently. On upgrade this is a VPC downgrade: the previously-unused third Availability Zone's subnet is deleted. Because that subnet can still hold elastic network interfaces (the shared interface VPC endpoints, and VPC-attached Lambda ENIs when `useForAllLambdas` is set), AWS CloudFormation may fail to delete it. See the [v2.5 to v2.6 migration guide](https://awslabs.github.io/visual-asset-management-system/deployment/update-the-solution#v25-to-v26) and the [networking troubleshooting procedure](https://awslabs.github.io/visual-asset-management-system/troubleshooting/common-issues).
1616
- API Gateway HTTP API → REST API migration changes the API endpoint. The backend API is now an API Gateway REST API (v1) served under a stage path (default `/api`) instead of the previous HTTP API (v2). The API Gateway identifier and invoke URL change on deployment. Any client registered directly against the old API Gateway endpoint URL must be re-setup against the new endpoint — this includes the VAMS CLI (re-run `vamscli setup`) and any external integrations or scripts that stored the API base URL. Clients that reach the API through the CloudFront or ALB front (the web application, and CLIs configured with the front's `/api` URL) continue to work without change. See the v2.6.0 entry in [Update the solution](deployment/update-the-solution.md).
17+
- **The authorizer claims context shape changed, which can break customized MFA, claims, and login-profile logic.** Deployments that have edited the customization hooks under `backend/backend/customConfigCommon/` must review them against the new shape before upgrading. Stock (unedited) deployments need no action — VAMS ships working defaults for all three hooks. Three related changes drive this:
18+
- **The authorizer context is now a flat string map.** Under the HTTP API (v2), a Lambda authorizer's claims arrived nested at `requestContext.authorizer.jwt.claims` or `requestContext.authorizer.lambda`. The REST API (v1) REQUEST authorizer delivers them as a **flat map of string values** directly under `requestContext.authorizer` (alongside a `principalId` key). Custom logic that branches on `'jwt' in ...` / `'lambda' in ...` and falls through to an empty dict now silently reads **no claims** rather than raising — the failure is a quiet behavior change, not an error. Read claims through `request_to_claims(event)` (which handles all three shapes and normalizes the event) instead of indexing `requestContext.authorizer` directly. Note that every context value is a **string**, so JSON-valued claims such as `vams:tokens` and `vams:roles` must be `json.loads`-ed, and `vams:mfaEnabled` is the string `"true"`/`"false"` rather than a boolean.
19+
- The shipped `customAuthProfileLoginWriteOverride` default in `customAuthLoginProfile.py` still carries the old nested-shape branches, so under the REST API its email-from-claims override is inert. It is harmless as shipped (the handler already persists the correct `userId`, and a stored profile email set at creation is unaffected), but a deployment that relies on populating profile fields from token claims in that hook must update the extraction to the flat shape.
20+
- **`customMFATokenScopeCheckOverride` takes a new argument and no longer extracts claims itself.** The signature changed from `(user, lambdaRequest)` to `(user, authorizerJwtClaims, lambdaRequest)` — the verified claims are now passed in directly, so the hook no longer digs them out of the event. It is also called from the **authorizer** rather than from each handler, and the Cognito default now resolves MFA with `admin_get_user` (`UserPoolId` + `Username`, requiring the new `USER_POOL_ID` environment variable) instead of `get_user` with the caller's access token. A customized hook written against the two-argument signature will fail to be called correctly and its result is caught and defaulted to `false`, silently disabling MFA-gated roles. The external OAuth IDP branch remains a customization slot that returns `false` until implemented.
21+
- **MFA state is resolved once at authorization time.** The result is passed to handlers as the `vams:mfaEnabled` authorizer context value and is consumed by `request_to_claims` before `customAuthClaimsCheckOverride` runs, so that hook should read `claims_and_roles["mfaEnabled"]` rather than re-deriving MFA. Handler Lambda functions no longer call an identity provider themselves. Because the authorizer must reach Amazon Cognito for this check, it is disabled when Lambda functions run inside the VPC (`app.useGlobalVpc.useForAllLambdas`), in which case `mfaRequired` on a role has no effect.
1722

1823
**Recommended Upgrade Path:** Run the upgrade script to redindex opensearch data if using OpenSearch serverless or provisioned: `infra\deploymentDataMigration\v2.5_to_v2.6\upgrade`
1924

backend/backend/customConfigCommon/customAuthLoginProfile.py

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,23 @@
1717

1818
def customAuthProfileLoginWriteOverride(userProfile, lambdaRequestEvent):
1919

20-
#Handle both claims from APIGateway standard authorizer format, lambda authorizers, or lambda cross-calls
21-
if 'jwt' in lambdaRequestEvent['requestContext']['authorizer'] and 'claims' in lambdaRequestEvent['requestContext']['authorizer']['jwt']:
22-
claims = lambdaRequestEvent['requestContext']['authorizer']['jwt']['claims']
23-
elif 'lambda' in lambdaRequestEvent['requestContext']['authorizer']:
24-
claims = lambdaRequestEvent['requestContext']['authorizer']['lambda']
25-
elif 'lambdaCrossCall' in lambdaRequestEvent: #currently this case wouldn't apply for now due to check above
20+
#Handle claims from: lambda cross-calls, HTTP API JWT authorizer, HTTP API lambda
21+
#authorizer (v2), or REST API REQUEST lambda authorizer (flat string map under
22+
#'authorizer'). The REST case is the shape VAMS deploys today; the nested forms are
23+
#retained so a customized copy of this hook keeps working against either.
24+
if 'lambdaCrossCall' in lambdaRequestEvent:
2625
claims = lambdaRequestEvent['lambdaCrossCall']
2726
else:
28-
claims = {}
27+
authorizer_ctx = (lambdaRequestEvent.get('requestContext', {}) or {}).get('authorizer') or {}
28+
if 'jwt' in authorizer_ctx and 'claims' in authorizer_ctx['jwt']:
29+
claims = authorizer_ctx['jwt']['claims']
30+
elif 'lambda' in authorizer_ctx:
31+
claims = authorizer_ctx['lambda']
32+
elif isinstance(authorizer_ctx, dict):
33+
#REST REQUEST authorizer: context is a flat map of string values.
34+
claims = {k: v for k, v in authorizer_ctx.items() if k != 'principalId'}
35+
else:
36+
claims = {}
2937

3038
###################ADD CUSTOM LOGIC TO GET USER PROFILE DATA AT LOGIN FOR USER PROFILE###################
3139

backend/tests/customConfigCommon/__init__.py

Whitespace-only changes.
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
# Copyright 2026 Amazon.com, Inc. or its affiliates. All Rights Reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
"""Claims extraction in the login-profile customization hook.
4+
5+
``customAuthProfileLoginWriteOverride`` reads the caller's claims to populate the stored
6+
user profile. The REST API (v1) REQUEST authorizer delivers claims as a flat map of string
7+
values under ``requestContext.authorizer``, not nested under ``authorizer.jwt.claims`` or
8+
``authorizer.lambda`` as the HTTP API (v2) did. These tests pin that the hook reads the
9+
deployed REST shape (so the default email override is not silently inert), still reads the
10+
nested shapes for a customized copy carried across the migration, and does not raise on
11+
event shapes that carry no authorizer context.
12+
13+
The module is loaded directly from its file path because it imports VAMS handler packages
14+
that the shared ``conftest.py`` replaces with mocks.
15+
"""
16+
import importlib.util
17+
import os
18+
import sys
19+
from unittest.mock import MagicMock
20+
21+
import pytest
22+
23+
_HOOK_PATH = os.path.join(
24+
os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))),
25+
"backend", "customConfigCommon", "customAuthLoginProfile.py",
26+
)
27+
28+
29+
_STUBS = {
30+
"customLogging": {},
31+
"customLogging.logger": {"safeLogger": MagicMock(return_value=MagicMock())},
32+
"handlers": {},
33+
"handlers.auth": {"request_to_claims": MagicMock(return_value={})},
34+
"handlers.authz": {"CasbinEnforcer": MagicMock()},
35+
"common": {},
36+
"common.constants": {"STANDARD_JSON_RESPONSE": {}},
37+
"requests": {"get": MagicMock()},
38+
}
39+
40+
41+
def _load_hook():
42+
"""Load the hook module with its VAMS imports stubbed out.
43+
44+
Other test modules (and the shared conftest) may already have registered partial mocks
45+
for these package names, so the required attributes are ensured on whatever object is
46+
present rather than only when the name is absent from sys.modules. Prior module state is
47+
restored afterwards so this does not perturb tests that run later in the session.
48+
"""
49+
saved = {}
50+
for name, attrs in _STUBS.items():
51+
existing = sys.modules.get(name)
52+
if existing is None:
53+
sys.modules[name] = MagicMock()
54+
saved[name] = (False, None)
55+
else:
56+
saved[name] = (True, {a: getattr(existing, a, None) for a in attrs})
57+
for attr, value in attrs.items():
58+
if not hasattr(sys.modules[name], attr) or getattr(sys.modules[name], attr) is None:
59+
setattr(sys.modules[name], attr, value)
60+
61+
try:
62+
spec = importlib.util.spec_from_file_location("_real_customAuthLoginProfile", _HOOK_PATH)
63+
module = importlib.util.module_from_spec(spec)
64+
spec.loader.exec_module(module)
65+
return module
66+
finally:
67+
for name, (existed, attrs) in saved.items():
68+
if not existed:
69+
sys.modules.pop(name, None)
70+
elif attrs:
71+
for attr, value in attrs.items():
72+
if value is None:
73+
try:
74+
delattr(sys.modules[name], attr)
75+
except AttributeError:
76+
pass
77+
else:
78+
setattr(sys.modules[name], attr, value)
79+
80+
81+
@pytest.fixture(scope="module")
82+
def hook():
83+
return _load_hook().customAuthProfileLoginWriteOverride
84+
85+
86+
def _profile():
87+
return {"userId": "u1", "email": "stored@example.com"}
88+
89+
90+
@pytest.mark.unit
91+
class TestClaimsEmailOverride:
92+
def test_rest_flat_authorizer_context_overrides_email(self, hook):
93+
"""The deployed REST shape: claims are a flat string map under 'authorizer'."""
94+
event = {
95+
"requestContext": {
96+
"authorizer": {
97+
"principalId": "u1",
98+
"sub": "u1",
99+
"email": "rest@example.com",
100+
"vams:tokens": '["u1"]',
101+
}
102+
}
103+
}
104+
assert hook(_profile(), event)["email"] == "rest@example.com"
105+
106+
def test_v2_nested_jwt_claims_overrides_email(self, hook):
107+
event = {"requestContext": {"authorizer": {"jwt": {"claims": {"email": "jwt@example.com"}}}}}
108+
assert hook(_profile(), event)["email"] == "jwt@example.com"
109+
110+
def test_v2_nested_lambda_claims_overrides_email(self, hook):
111+
event = {"requestContext": {"authorizer": {"lambda": {"email": "lam@example.com"}}}}
112+
assert hook(_profile(), event)["email"] == "lam@example.com"
113+
114+
def test_user_id_is_never_altered(self, hook):
115+
"""userId is the profile lookup key and must survive the override untouched."""
116+
event = {"requestContext": {"authorizer": {"sub": "someone-else", "email": "x@example.com"}}}
117+
assert hook(_profile(), event)["userId"] == "u1"
118+
119+
120+
@pytest.mark.unit
121+
class TestNoClaimsAvailable:
122+
def test_cross_call_event_keeps_stored_email(self, hook):
123+
"""A cross-call carries no email claim, so the stored value must be preserved."""
124+
event = {"lambdaCrossCall": {"userName": "SYSTEM_USER"}}
125+
assert hook(_profile(), event)["email"] == "stored@example.com"
126+
127+
def test_missing_request_context_does_not_raise(self, hook):
128+
assert hook(_profile(), {})["email"] == "stored@example.com"
129+
130+
def test_null_authorizer_does_not_raise(self, hook):
131+
assert hook(_profile(), {"requestContext": {"authorizer": None}})["email"] == "stored@example.com"
132+
133+
def test_empty_authorizer_keeps_stored_email(self, hook):
134+
assert hook(_profile(), {"requestContext": {"authorizer": {}}})["email"] == "stored@example.com"
135+
136+
def test_blank_email_claim_does_not_overwrite_stored_email(self, hook):
137+
event = {"requestContext": {"authorizer": {"sub": "u1", "email": ""}}}
138+
assert hook(_profile(), event)["email"] == "stored@example.com"
139+
140+
def test_principal_id_alone_is_not_treated_as_an_email_claim(self, hook):
141+
"""principalId is authorizer metadata, not a claim; it must be stripped."""
142+
event = {"requestContext": {"authorizer": {"principalId": "u1"}}}
143+
assert hook(_profile(), event)["email"] == "stored@example.com"

documentation/docusaurus-site/docs/additional/revisions.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ This page tracks the version history of the Visual Asset Management System (VAMS
5858
- OpenSearch index names rolled forward to `vams-assets-v3` and `vams-files-v3`. The schema-deploy custom resource creates the empty v3 indexes; the previous v2 indexes are abandoned and left in place until you delete them manually. A reindex is required to populate v3.
5959
- `OPENSEARCH_VERSION` switched from `OPENSEARCH_2_7` to `OPENSEARCH_3_5`. Provisioned OpenSearch domains will perform a major-version engine upgrade. Serverless collections are unaffected.
6060
- OpenSearch Serverless now deploys the collection into a collection group and adds `nextGen`, `allowPublic`, `enableStandbyReplicas`, and OCU capacity options. Because the collection is placed in a collection group, the public-access network-policy change is applied, and the group generation (`CLASSIC` or `NEXTGEN`) is set, enabling or changing Serverless requires a re-deployment: disable Serverless and deploy, then re-enable with the new settings and deploy, and reindex. A minimum OCU of `0` requires `nextGen=true`, and `nextGen=true` requires `enableStandbyReplicas=true` (NEXTGEN collection groups do not support disabled standby replicas); a private collection (`allowPublic=false`) requires `app.useGlobalVpc.enabled` (only the OpenSearch-facing Lambda functions are placed in the VPC, so `app.useGlobalVpc.useForAllLambdas` is not required). See [v2.5 to v2.6 update guide](../deployment/update-the-solution.md#v25-to-v26).
61+
- **The authorizer claims context shape changed, which can break customized MFA, claims, and login-profile logic.** Deployments that have edited the customization hooks under `backend/backend/customConfigCommon/` must review them before upgrading; stock deployments need no action. The REST API (v1) REQUEST authorizer delivers claims as a **flat map of string values** under `requestContext.authorizer`, rather than nested at `requestContext.authorizer.jwt.claims` or `requestContext.authorizer.lambda` as the HTTP API (v2) did — so custom logic that branches on those nested keys and falls through to an empty dict now silently reads no claims instead of raising. Every value is a string, so JSON-valued claims (`vams:tokens`, `vams:roles`) need `json.loads` and `vams:mfaEnabled` is `"true"`/`"false"`. Read claims through `request_to_claims(event)` instead of indexing the authorizer context directly. Additionally, `customMFATokenScopeCheckOverride` changed signature from `(user, lambdaRequest)` to `(user, authorizerJwtClaims, lambdaRequest)`, is now called from the authorizer instead of each handler, and its Cognito default uses `admin_get_user` with the new `USER_POOL_ID` environment variable; a hook still written against the old signature is caught and defaulted to `false`, silently disabling MFA-gated roles. MFA is resolved once at authorization time and delivered as `vams:mfaEnabled`, so `customAuthClaimsCheckOverride` should read `claims_and_roles["mfaEnabled"]` rather than re-deriving it. See [Authentication and Authorization Flow](../developer/security.md#authentication-and-authorization-flow) and [Authentication Override Hooks](../developer/security.md#authentication-override-hooks).
6162
- Enabling a VPC-requiring feature (ALB, OpenSearch Provisioned, or any container-based pipeline) while `app.useGlobalVpc.enabled` is `false` now fails configuration validation with an explicit error instead of silently auto-enabling the VPC. Configurations that previously relied on the implicit auto-enable must set `app.useGlobalVpc.enabled` to `true` explicitly. See [Configuration reference](../deployment/configuration-reference.md) and [Plan your deployment](../deployment/plan-your-deployment.md).
6263
- Provisioned OpenSearch `availabilityZoneCount` defaults to `2` and the VPC is built with that many Availability Zones. Earlier releases built the VPC across 3 Availability Zones for provisioned OpenSearch while the domain used only 2, so on upgrade the unused third AZ subnet is removed — a VPC downgrade that can fail subnet deletion when elastic network interfaces are still attached. Set `availabilityZoneCount` to `3` to preserve the existing VPC, or follow the drain-and-redeploy teardown to move to 2 AZs. See [v2.5 to v2.6 update guide](../deployment/update-the-solution.md#v25-to-v26).
6364

0 commit comments

Comments
 (0)