Conversation
References: https://apollographql.atlassian.net/browse/AS-184 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Solid tech note; the Cognito access-vs-id-token guidance, the IRSA/ECS/EC2 credential-chain breakdown, and the strip-Authorization-before-SigV4 header rule are all accurate and genuinely useful. A few blocking issues against the AS-184 acceptance criteria before this is ready.
-
Invalid JWKS config key. In the Pattern 1 snippet the JWKS entry uses
issuer:(singular scalar). The Router JWT plugin option isissuers:(a list); see https://apollographql.com/docs/graphos/routing/security/jwt#configuration-options. As written this YAML will fail config validation, which conflicts with the AC "working YAML snippets" and the PR's own "validated against a current Router version" test-plan item. Useissuers: ["https://cognito-idp.${AWS_REGION}.amazonaws.com/${USER_POOL_ID}"]. -
Documentation links point at the archived IA. Every
https://www.apollographql.com/docs/router/configuration/...link (and its anchors:#token-claims,#renewing-tokens,#requiresscopes,#policy) targets the pre-migration Router docs path. The current IA places these under/docs/graphos/routing/...: JWT atgraphos/routing/security/jwt, SigV4 atgraphos/routing/security/subgraph-authentication, authorization atgraphos/routing/security/authorization. Since the ticket is explicitly motivated by the new docs IA and the test plan's "all links resolve" box is unchecked, please repoint all internal links and re-verify the anchors exist on the new pages. -
Missing architecture diagram. AC requires a diagram illustrating the client -> Router -> AWS-backed subgraph auth flow. The doc currently has none. A simple Mermaid sequence/flow diagram inline would satisfy this.
-
SE/SA technical review. AC requires sign-off from at least one SE/SA. No review is recorded on the PR yet; please capture that before merge.
Non-blocking nits:
- Pattern 1 mixes the
authorization.directives.enabledblock withrequire_authentication; confirm both keys are intended together and match the current authorization config schema on the new page. - "GA in 2.x" / "Router 1.43+" version claim for the SigV4 plugin should be spot-checked against release notes, since version strings drift.
- CI is green (CLA signed, secrets scan passed); the failures above are content, not pipeline.
| audiences: ["graphos-router"] | ||
| issuer: https://cognito-idp.${AWS_REGION}.amazonaws.com/${USER_POOL_ID} | ||
|
|
||
| authorization: |
There was a problem hiding this comment.
issuer: is not a valid JWKS key. The Router option is issuers: and takes a list. As written this fails config validation. Use issuers: ["https://cognito-idp.${AWS_REGION}.amazonaws.com/${USER_POOL_ID}"]. Ref: https://apollographql.com/docs/graphos/routing/security/jwt#configuration-options
There was a problem hiding this comment.
Confirmed at head 7adb688: Pattern 1 now uses issuers: as a list with the Cognito issuer URL, so the snippet passes config validation. Resolving.
| ## Pattern 1 — Validate inbound JWTs at the Router | ||
|
|
||
| The Router's [JWT authentication plugin](https://www.apollographql.com/docs/router/configuration/authn-jwt) validates a `Bearer` token against one or more JWKS endpoints and rejects unauthenticated traffic before composition. With AWS-hosted IdPs the JWKS URL is fully managed for you. | ||
|
|
There was a problem hiding this comment.
This and the other /docs/router/configuration/... links use the archived docs path. The current IA serves JWT auth at graphos/routing/security/jwt. Please repoint every internal link (and verify the deep-link anchors) to the new /docs/graphos/routing/... structure; the ticket is specifically about the new IA.
There was a problem hiding this comment.
Confirmed at head 7adb688: all internal links now point at /docs/graphos/routing/... (jwt, subgraph-authentication, authorization) with valid anchors; no archived /docs/router/configuration/ paths remain. Resolving.
…-184) Address docs-reviewer feedback: - The JWT plugin option is `issuers:` (list), not `issuer:` (scalar); as written, config validation fails. - Repoint every internal docs link from the archived `/docs/router/...` paths to the current `/docs/graphos/routing/security/...` IA. SigV4 is now at `/security/subgraph-authentication`; JWT at `/security/jwt`; authorization directives at `/security/authorization`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…208) Address docs-reviewer feedback: - Health-check endpoint binds 127.0.0.1 by default; show the explicit `listen: 0.0.0.0:8088` snippet so ALB/EKS probes can reach it. - Prometheus exporter is disabled by default and binds 127.0.0.1; show the enable + `listen: 0.0.0.0:9090` snippet for AMP scraping. - Repoint health-check and self-hosted-runtime links from the archived /docs/router/... paths to the current /docs/graphos/routing/self-hosted/ IA. The two cross-references to the sibling PRs (#35 examples/router-helm-non-oci and #37 router-aws-jwt-sigv4.md) are intentionally kept relative; they resolve once those sibling PRs merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Re-reviewed at head 7adb688. Two of the four blocking findings are fixed and their threads are resolved:
- JWKS config: Pattern 1 now uses issuers: as a list; the snippet validates.
- Doc links: all internal links repointed to /docs/graphos/routing/... (jwt, subgraph-authentication, authorization) with correct anchors; no archived /docs/router/configuration/ paths remain.
Two blocking AC items are still outstanding:
- Architecture diagram. The AC requires a diagram of the client -> Router -> AWS-backed subgraph auth flow. The current file has none; a Mermaid sequence or flow diagram inline would satisfy this.
- SE/SA technical review. The AC requires sign-off from at least one SE/SA. The only review on the PR is this bot's; no human SE/SA review is recorded yet.
Add the diagram and capture an SE/SA review, then re-request and I will approve.
Adds a Mermaid sequence diagram showing the client→Router→IdP→Subgraph authentication flow, satisfying the AC requirement for an architecture diagram. Still needs SE/SA technical review before final approval. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@andywgarcia Assigning to you — architecture diagram has been added (satisfying AC item 3), but this still needs SE/SA technical review (AC item 4) before it can be approved. Could you take a look or find an SE/SA to sign off? |
Summary
Adds
docs/router-aws-jwt-sigv4.mdcovering the two auth patterns Apollo Router needs on AWS:Bearertokens against Cognito / Auth0 / OIDC JWKS endpoints, with Cognito-specific gotchas (access vs id tokens, regional JWKS URLs, token TTL).Includes a combined
router.yamlexample with a header rule that strips the inboundAuthorizationheader before SigV4 signing, plus smoke-test guidance.Note
Originally drafted as a TN for
apollographql/docs. That repo is now archived; the newapollographql/platform-docsrequires SAML SSO grant on the contributor's OAuth token. Landing inapollosolutions/reference-architecture/docs/as an interim home; can be migrated upstream once the SSO grant is in place.Tracks AS-184.
Test plan
https://www.apollographql.com/docs/...links resolve🤖 Generated with Claude Code