Skip to content

Consider migrating the SPA's hand-rolled login to OpenIddict once it is proven for MCP #793

Open

Description

@mforce

Follow-up agreed while designing MCP authentication (#788). It is deferred on purpose, not rejected.

Context

The MCP work adds OpenIddict as an OAuth 2.1 authorization server, limited to third-party clients (AI assistants). The SPA keeps its existing hand-rolled login unchanged: IdentityProvider.LoginAsync, RSA-signed JWTs, rotating refresh tokens in an HttpOnly cookie, and farm-code sign-in.

So the app will run two authentication paths side by side. That is the right call for the MCP slice. It is still a duplication, and duplication in auth deserves a deliberate second look before it becomes permanent.

Why this was deferred rather than done

The existing login is the most security-sensitive code in the repo. Much of its behaviour is earned, and most of that came from defects that actually shipped:

Rewriting that onto OpenIddict means re-proving every one of those invariants. Doing it at the same time as adding OpenIddict would put the MCP feature and an auth rewrite in the same change, and a defect in either would be hard to attribute.

What would make this worth doing later

  • OpenIddict has been running in production for the MCP path long enough to trust.
  • Someone wants a capability the hand-rolled login cannot give cheaply, such as third-party sign-in, SSO/SAML, or a second first-party client (a mobile app) that would otherwise duplicate the login logic a third time.
  • The maintenance cost of two auth paths becomes visible, for example a change that has to be made twice or a guard that covers only one path.

What would make this a bad idea

  • If the MCP path stays small and the SPA login keeps working, this is a rewrite with no user-visible benefit and considerable risk. "Two auth paths" is not by itself a reason to act.
  • OpenIddict has a single primary maintainer. Putting all authentication behind it raises the cost if that ever changes. Apache-2.0 makes forking possible, but forking an OAuth server is a large job.

Not in scope

This issue is not a commitment. It exists so the duplication is a recorded decision with stated revisit conditions, rather than something a future reader discovers and assumes was an oversight.

Related: #788 (the MCP auth decision), #789 (EPIC: MCP server support).

Activity

  1. added
    enhancementNew feature or request
    area:apiAPI/endpoint layer
    epic-789MCP server support (#789)
    priority:tier3Real product weight, real cost
    on Sep 12, 2026
  2. mforce commented on Sep 13, 2026

    @mforce
    OwnerAuthor

    This issue now carries #268's obligation. The maintainer intends to ship this migration, so #268 (JWT signing-key rotation) is being closed on that basis rather than implemented.

    What transfers here

    #268 recorded that the hand-rolled JWT path cannot rotate signing keys without downtime. Validation accepts exactly one public key via IssuerSigningKey (singular), and issued tokens carry no kid header, so swapping a key instantly 401s every already-issued access token. The impact is bounded. Access tokens live 15 minutes, and refresh tokens are DB-hash-based, so the refresh tokens survive. But a zero-downtime or emergency rotation is impossible as written. That is the wrong position to be in during an actual key compromise.

    This migration closes that as a side effect. OpenIddict supports additive rollover natively. You register several certificates, and it selects by validity window and prefers the furthest expiry, while it keeps validating against the others. The hand-rolled single-key path goes away.

    Why this note exists

    The body below is written as a consideration on purpose. It lists revisit conditions and also, deliberately, the conditions under which this migration would be a bad idea. That framing was right when nothing depended on it.

    It is no longer only a consideration. If this migration is declined, deferred indefinitely, or descoped, #268's gap returns and nothing tracks it. #268 will be closed, and the defect will be live and invisible.

    So, if this issue is ever closed without the migration happening, reopen #268.

    The heading says this as plainly as possible because that is the failure mode. A gap gets closed against a plan, then the plan changes, and nobody connects the two. #268's own thread keeps the original analysis.

  3. mforce commented on Sep 13, 2026

    @mforce
    OwnerAuthor

    This issue now carries #268's requirement: a "no" here reopens it

    Added 2026-09-13 during the issue cleanup. #268 (JWT signing-key rotation needs a multi-key
    validation ring + kid) was closed and deferred to this issue.
    The reasoning is that migrating
    the SPA to OpenIddict brings JWKS, kid stamping and native key rollover with it, so building a
    hand-rolled rotation ring first would mean building it twice.

    That means other work depends on this issue in a way its title does not suggest. It says
    "Consider migrating…", and considering has two outcomes.

    The condition

    If this issue resolves as "yes, migrate the SPA to OpenIddict", key rotation arrives with it.
    Add an acceptance criterion here that a signing key can be rotated with an overlap window and no
    forced re-authentication, and verify it. That criterion is #268's "Verify" section, and it should
    not get lost in the migration's own scope.

    If this issue resolves as "no, keep the hand-rolled login", then reopen #268. Do not close
    this one as won't-fix without reopening it. In that case the gap #268 described is still live.
    Validation binds a single IssuerSigningKey, issued tokens carry no kid, and there is no overlap
    window, so a key swap 401s every live access token.

    What is accepted while this stays undecided

    The production app cannot rotate its JWT signing key without about 15 minutes of degradation.
    Access tokens live 15 minutes, and DB-hash refresh tokens survive rotation, so clients recover on
    their next refresh. A planned rotation is therefore tolerable. An emergency rotation on a
    compromised key is the case that hurts, and it is the reason #268 was tier1.

    The full accepted-risk record, including the operational answer if a key is compromised before this
    lands, is in the #268 close-out.

  4. added this to the MCP server support milestone on Sep 13, 2026
  5. added
    epic-788OAuth 2.1 authorization server for MCP (#788)
    and removed
    epic-789MCP server support (#789)
    on Oct 6, 2026
  6. mforce commented on Oct 6, 2026

    @mforce
    OwnerAuthor

    Moved from milestone "MCP server support" to "OAuth 2.1 authorization server" in the 2026-10-06 re-evaluation of #789. This is OAuth-side work. #794 is #795's prerequisite, and #793 concerns the SPA login. No MCP slice depends on it directly.

  7. added
    deferredPostponed on purpose until its stated revisit condition is met
    on Oct 6, 2026
  8. mforce commented on Oct 6, 2026

    @mforce
    OwnerAuthor

    Marked deferred on 2026-10-06. The issue stays in this milestone. Work starts only after the revisit conditions in the body are met, the first being that OpenIddict (#795 onward) has run in production long enough to trust.

    The #268 condition still applies. If this issue closes without the migration, reopen #268 first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:apiAPI/endpoint layerdeferredPostponed on purpose until its stated revisit condition is metenhancementNew feature or requestepic-788OAuth 2.1 authorization server for MCP (#788)priority:tier3Real product weight, real cost

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions