fix: require a validated service token for external delivery in all environments#92
Merged
Merged
Conversation
…nvironments External delivery and sensitive bootstrap details were gated on NODE_ENV not being production, plus a client-supplied header. Both now fall through to the existing internal service token check (issuer, audience, and subject) in every environment. Local development that cannot present a service token can set ALLOW_UNCREDENTIALED_DELIVERY_SECRETS=true. The flag must be set deliberately and is ignored under a production NODE_ENV, so it cannot become the default. Specs that exercised external delivery relied on the NODE_ENV shortcut. They now mint a trusted service token through tests/factories/serviceTokenFactory.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
External delivery returns OTP and magic-link secrets in the response body instead of sending them. It now requires a validated internal service token in every environment.
canReturnExternalDeliveryno longer treats a non-productionNODE_ENVas sufficient. Thex-seamless-auth-delivery-mode: externalheader must be accompanied by anx-seamless-service-tokenthat passes the existingvalidateInternalServiceTokencheck plus the issuer, audience, and subject checks.canReturnSensitiveDevelopmentDetailsfollowed the same environment-based pattern and is now gated the same way. It is reached fromcreateAdminBootstrapInviteHandler, which returns the bootstrap invite URL and token.src/controllers/otp.ts,src/controllers/magicLinks.ts,src/controllers/registration.ts,src/controllers/bootstrap.ts.Local development opt-in
Local development and some manual tooling relied on the environment check, so there is a replacement that cannot be reached by accident:
ALLOW_UNCREDENTIALED_DELIVERY_SECRETS=trueIt is off unless set explicitly, it is ignored (with an error logged) when
NODE_ENV=production, and it is documented in.env.examplealongside the docs updates. I chose an explicit env opt-in over reintroducing an environment-derived default so the permissive mode is always a deliberate operator action and can never apply in production, regardless of how a deployment is configured.Tests
Specs that exercised external delivery previously relied on the environment shortcut. They now mint a trusted service token through the new
tests/factories/serviceTokenFactory, and the shared mock intests/setup/mocks.tsrejects every token by default so any spec not opting in exercises the credentialed path.tests/unit/lib/externalDelivery.spec.tscovers the new matrix: no token, valid token, untrusted issuer or audience, the opt-in flag, and the opt-in flag being ignored in production.Local run:
npm run typecheckpassesnpm run lintpassesnpm run format:checkpassesnpm run test:run951 passed, 1 skipped, 87 filesnpm run coveragepasses thresholds,src/lib/externalDelivery.tsat 100%Contract note
This changes the conditions under which the external delivery response shape is returned, so callers that use the header without a service token outside production will now receive the standard response instead of a
deliverypayload.seamless-auth-serveris the likely affected repo. Happy to survey it and propose coordinated edits as a follow-up.Follow-up for the maintainer
Worth confirming separately that every reachable deployed environment, including staging and preview, sets
NODE_ENV=production.