Conversation
|
Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need. |
boring-cyborg
Bot
added
dependencies
powertools-for-aws-oss-automation
Bot
added
the
size/XXL
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if not isinstance(token, str) or not token: | ||
| raise InvalidTokenError() | ||
| try: | ||
| header = jwt.get_unverified_header(token) |
| try: | ||
| # This payload selects a configured verifier. No unverified claim | ||
| # is returned to callers or used to discover another provider. | ||
| payload = jwt.decode(token, options={"verify_signature": False}) |
|
I reviewed the two SonarCloud findings in
I reran the verifier and profile tests: 63 passed, including rejection of tampered signatures, tokens signed with another issuer’s key, and unknown issuers. Could you review these as potential false positives in SonarCloud? The issuer-routing code already documents this behavior; I can add a similar explanation beside the header parsing. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #8469 +/- ##
===========================================
- Coverage 96.66% 96.46% -0.20%
===========================================
Files 296 310 +14
Lines 14911 15631 +720
Branches 1268 1372 +104
===========================================
+ Hits 14413 15079 +666
- Misses 363 394 +31
- Partials 135 158 +23 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
leandrodamascena
left a comment
leandrodamascena
left a comment
There was a problem hiding this comment.
Thanks for the work here and for updating the RFC. I went through the new version again, and the changes around JWKS freshness, Cognito access tokens, expiration, API Gateway caching, async usage, and PyJWT address my earlier concerns.
I would like to keep this PR focused on inbound JWT verification. The OAuth client makes the change much larger and I still want to review that API separately. Please move OAuth2Client, its tests, documentation, and outbound example to a follow-up PR.
The JWT part is close. The inline comments cover the remaining points around error visibility, the generic token profile, the Layer dependency, and the Lambda examples.
| from aws_lambda_powertools.utilities.auth.oauth2 import OAuth2Client as OAuth2Client | ||
| from aws_lambda_powertools.utilities.auth.verifier import JWTVerifier as JWTVerifier | ||
|
|
||
| __all__ = ["JWTVerifier", "OAuth2Client"] |
There was a problem hiding this comment.
Following the scope above, please remove OAuth2Client from this PR together with its implementation, tests, documentation, and outbound example. We can review the complete OAuth experience in a separate PR.
| enforce_scopes(claims, self._scopes) | ||
| if self._authorize is not None and self._authorize(claims) is not True: | ||
| raise ForbiddenError() | ||
| except AuthError as error: |
There was a problem hiding this comment.
The fail-closed behavior is correct: the protected handler does not run. The missing part is visibility for the Lambda owner.
Please map the safe reason into AuthErrorContext so customers can log or emit metrics from on_error. I would not add automatic logs for invalid tokens because that can create noise and log flooding.
| if require_principal: | ||
| _validate_principal(candidate) | ||
| return candidate | ||
| except (InvalidTokenError, ForbiddenError): |
There was a problem hiding this comment.
This behavior is correct: invalid credentials return Deny, while JWKSFetchError escapes and fails the authorizer invocation.
The authorizer should have the same safe failure visibility as the middleware. Could we provide a callback for recording the reason, or document a supported wrapper pattern? Otherwise rejected tokens disappear into Deny, and JWKS failures are only visible as uncaught Lambda errors.
| Expired keys are never used after a failed refresh. Unknown keys during a cooldown are rejected, so a newly published key may take time to become usable. | ||
| Choose freshness and cooldown settings together with your provider's key rotation policy. | ||
|
|
||
| `prefetch()` fetches absent or expired keys during initialization. Later rotation, expiration, and outages can still cause network I/O. |
There was a problem hiding this comment.
Please explain the Lambda tradeoff here. Calling prefetch() at module level moves the first JWKS fetch into INIT, but an IdP outage can then fail the cold start. Without prefetch, construction performs no network I/O and the first invocation pays the fetch latency.
This should remain an explicit option, not the default recommendation.
| IAM allows require a nonempty string `sub` as principal and cover only the supplied request ARN. | ||
| Wildcard, missing, or malformed ARNs raise `ValueError`; the helper cannot construct a request-specific IAM policy without a valid ARN. | ||
| Other routes need their own decision. | ||
| Invalid tokens and insufficient scopes produce a Deny or `isAuthorized=False`; unavailable signing keys raise `JWKSFetchError`. |
There was a problem hiding this comment.
Can we describe what the API caller actually sees? Middleware converts unavailable keys to 503. A Lambda authorizer instead fails its invocation, and API Gateway normally returns a 5xx.
That difference matters when customers configure retries, alarms, and dashboards.
| async def verify_token(self, token: str) -> AccessToken | None: | ||
| try: | ||
| claims = await asyncio.to_thread(verifier.verify, token) | ||
| except (InvalidTokenError, JWKSFetchError): |
There was a problem hiding this comment.
This converts unavailable JWKS infrastructure into the same result as an invalid token. It is fail-closed, but during an IdP outage the client may incorrectly try to authenticate again.
If the MCP SDK cannot represent availability separately here, please show how the Lambda owner can record JWKSFetchError before returning None.
| Globals: | ||
| Function: | ||
| Runtime: python3.12 | ||
| CodeUri: src/ |
There was a problem hiding this comment.
The authorizers and backend functions share the same source and requirements. This packages PyJWT and cryptography into backend functions that do not use Auth.
Can we separate the authorizer and backend artifacts so the additional package size stays with the functions that need it?
bfreiberg
changed the title
|
Not all issues are linked correctly. Please link each issue to the PR either manually or using a closing keyword in the format If mentioning more than one issue, separate them with commas: i.e. |
Add JWT verification, coordinated JWKS caching, API Gateway authorization, and OAuth client credentials with optional dependencies, documentation, examples, and tests. Include exception-safe claims cleanup, sanitized provider errors, and lazy imports for OAuth-only clients and static-key verification.
Exercise malformed inputs, shared failures, waiter deadlines, and persistent HTTPS connections. Collect fresh-process import coverage through coverage.py's subprocess patch for pytest-cov 7.
Defer OAuth client credentials to a follow-up. Add fixed failure reasons, authorizer diagnostics, and signed claim/header profile constraints. Retain urllib3 in Layers and separate the SAM authorizer and backend artifacts, with expanded tests and documentation.
c251f11 to
ec3225c
Compare
|
3 participants
Applications that need a custom JWT verifier currently assemble signing-key refresh, token-profile validation, and Lambda authorization behavior themselves. This adds an optional Auth utility with a shared verifier for direct use, Event Handler middleware, and API Gateway authorizers.
This PR covers inbound JWT verification. OAuth client credentials, its documentation, tests, and outbound example have been removed for a separate follow-up; that implementation is preserved on
bfreiberg:follow-up/oauth-client-8466.Issue number: #8466 — inbound JWT portion of the RFC.
Summary
Changes
expected_claims/expected_headersvalues to enforce provider-specific token purpose after baseline verification.AuthFailureReasonstring-enum values and retryability. Middleware callbacks receive them throughAuthErrorContext; authorizers have an observation callback for rejected tokens and unavailable keys. Default responses remain generic, and the utility performs no automatic logging.examples/auth/templates/sam.yamland build authorizers with the Auth extra separately from base-only backend artifacts. Disable authorizer-result caching in both Gateway examples.User experience
Applications can also call
verify()directly or returnauthorize()from a Lambda authorizer. Error callbacks let the Lambda owner record fixed reasons and retryability without logging credentials. Invalid credentials still deny access; unavailable JWKS still fails an authorizer invocation even when a callback is configured.Validation
The Layer matrix used urllib3 2.8.0 from the Layer with runtime boto3/botocore 1.42.97. Checks confirmed dependency constraints, module locations, a real SDK HTTPS request, remote discovery/JWKS verification, invalid-token rejection, and authorizer error visibility. The SAM checks used separate authorizer/backend artifacts on Python 3.12 x86_64; both APIs returned HTTP 500 during the controlled JWKS outage. Test infrastructure and signing material were removed afterward.
Functional tests use real signatures and in-memory providers; TLS tests use a loopback server with both closing and persistent connections. Deployment checks use a controlled HTTPS provider. They complement the repository tests and do not claim live Cognito/Keycloak interoperability coverage. Coverage thresholds and exclusions are unchanged.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.