refactor(express)!: remove the dead issuer option#93
Merged
Conversation
`issuer` was required on `SeamlessAuthServerOptions` but write-only: it was assigned to `resolvedOpts` and never read. Once the silent-refresh path moved to the fixed M2M contract constants, the adopter-supplied value stopped reaching anything, so adopters were required to configure a string that had no effect. Remove it from the public and resolved option types and from the test fixtures that passed it. Core-level `issuer` plumbing is left alone: it carries the "seamless-portal-api" contract constant and is still in use. Also document that `authServerUrl` must match the auth server's `ISSUER`, since signed responses are verified with `iss === authServerUrl` and the check fails closed with only a generic error. Closes #77
This was referenced Jul 20, 2026
Closed
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.
Closes #77.
What
issuerwas required on the publicSeamlessAuthServerOptionsbut write-only. It was assigned toresolvedOptsincreateSeamlessAuthServerand never read anywhere. Once the silent-refresh path moved to the fixed M2M contract constants (iss: seamless-portal-api,aud: seamless-auth), the adopter-supplied value stopped reaching anything, so adopters were required to configure a string with no effect.This removes it from
SeamlessAuthServerOptionsandResolvedSeamlessAuthServerOptions, drops the assignment, and updates the test fixtures and README.Scope of the issue as filed
Issue #77 originally proposed removing both
issuerandaudience. The correcting comment on the issue narrows that: #80 (audience binding, closing #63) landed in parallel and madeaudienceload-bearing. It is now read by seven handlers and threaded intoverifySignedAuthResponseas the audience check, so removing it would reopen the cross-relying-party token acceptance gap.audienceis untouched here.Repurposing
issuerwas also considered and rejected. Every consumer passes its own app origin, so reinterpreting that value as the auth server's expectedisswould never match, andverifySignedAuthResponsefails closed, so every login would break with an opaque error.What is deliberately kept
The core-level
issuerplumbing carries the"seamless-portal-api"contract constant and is still reachable. Verified before touching anything:packages/express/src/middleware/ensureCookies.tsreceives the constant fromcreateServer.tspackages/core/src/ensureCookies.ts,createServiceToken.ts,refreshAccessToken.tspackages/core/src/verifySignedAuthResponse.ts(issuer: authServerUrl), behavior unchangedTest fixtures were split accordingly:
issuerwas removed only fromcreateSeamlessAuthServer(...)call sites, and left in place forcreateEnsureCookiesMiddleware(...)and the core-level tests.Docs
Added a note to the adapter README, near the configuration section, documenting a constraint that currently fails opaquely:
verifySignedAuthResponserequiresiss === authServerUrl, while the auth API stampsissfrom its ownISSUERenv var. If the adapter dials the API on an internal address while the API'sISSUERis a public URL, every login fails with only a generic[SeamlessAuth] Failed to verify signed auth response.log line. Docs only, no change to the verification code.The README configuration table and the options block no longer list
issueras required.Versioning
Changeset is
minor, notmajor. Both packages are pre-1.0 (core 0.7.0, express 0.8.0), andmajorwould bump to 1.0.0 and declare API stability, which is not intended. This matches the precedent of #89 and #92.Downstream impact
Not changed in this PR, but these each pass
issuertoday and will need to drop that line when they upgrade:seamless-portal-api(src/app.ts)seamless-review-api(src/app.ts)seamless-templates(templates/api/express/src/index.ts)The migration is deleting one line; no replacement option is needed.
Verification
pnpm buildat the workspace root: both packages build clean.pnpm testat the workspace root: