fix(express): warn when external delivery returns no delivery payload#91
Merged
Conversation
The four auth-message routes each fell through to a plain success response when the upstream body had no delivery field. A serviceSecret that does not match the auth server's API_SERVICE_TOKEN therefore produced a successful-looking response with no message sent and nothing logged. The shared delivery branch is now a single applyExternalDelivery helper that logs a warning when messaging is configured but no payload came back. Response bodies and status codes are unchanged. Also documents serviceSecret as a prerequisite for auth-message delivery in the messaging section of the express README.
Bccorb
force-pushed
the
fix/warn-on-missing-external-delivery
branch
from
July 20, 2026 02:40
fe41a7d to
471d2d1
Compare
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.
Why
seamless-auth-apiis tightening its external delivery gate (fells-code/seamless-auth-api#92): a validated internal service token is required in every environment, not only production. This adapter already sends the right token with the right claims (buildAuthorization.ts:14-26mintsiss=seamless-portal-api,aud=seamless-auth, and a subject), so the supported path keeps working and no code change is required for correctness.What the change does expose is a configuration failure that this adapter currently swallows.
The problem
All four auth-message routes had the same shape:
If
deliveryis absent, the adapter falls through, returns 200 with the upstream's "If an account exists, a login link has been sent", sends no email or SMS, and logs nothing. Until now aserviceSecretthat did not match the auth server'sAPI_SERVICE_TOKENwas invisible outside production, because the API returned the payload anyway. After the API change it is a real, silent non-delivery.What changed
applyExternalDeliveryininternal/deliverAuthMessage.ts. It delivers and strips as before, and logs a warning whenmessagingis configured but no payload came back.requestOtp,requestMagicLink,register, andbootstrapAdminInvitecall the helper. Response bodies, status codes, and the.end()behavior inregisterare unchanged.messagingis not configured, since the adapter never asks for external delivery in that case. There is a test for that.serviceSecretas a prerequisite for auth-message delivery, and says what a mismatch looks like.The four endpoints routed through this helper always include
deliverywhen the API accepts the request as externally delivered, so the warning should not fire spuriously. (/registration/phoneon the API can legitimately return no delivery when the phone is unchanged and already verified, but this adapter does not route that endpoint.)Tests
Two added to
packages/express/tests/messagingDelivery.test.js: a missing payload warns and sends nothing, and a no-messaging config does not warn.pnpm buildpasses for both packagespnpm testpasses, 60 core tests and 78 express testsOne caveat: on one intermediate run,
tests/oauthRoutes.test.jsreported a single failure. It passed in isolation and on eight subsequent full runs, and five runs onmainwere clean. I did not capture the message and could not reproduce it. That suite is untouched by this change, and the express test script rebuildsdistbefore running jest, so a rebuild race is the likeliest explanation. Flagging it rather than leaving it out.Scope
@seamless-auth/expressonly.@seamless-auth/coreis unchanged.One thing I noticed but did not touch:
core/src/authFetch.ts:22falls back tooptions.authorizationwhenserviceAuthorizationis absent, so a direct core consumer settingexternalDelivery: truewithoutserviceAuthorizationsends the end-user's access token in the service-token slot. Those claims will not validate upstream. The express adapter never hits this, and it predates this change, so it seems worth its own PR rather than folding in here.