Skip to content

feat(auth): surface debuggable reasons for rejected issuer/session tokens#272

Open
jeroenrinzema wants to merge 1 commit into
mainfrom
feat/trusted-issuer-debuggable-errors
Open

feat(auth): surface debuggable reasons for rejected issuer/session tokens#272
jeroenrinzema wants to merge 1 commit into
mainfrom
feat/trusted-issuer-debuggable-errors

Conversation

@jeroenrinzema

Copy link
Copy Markdown
Contributor

Problem

When authenticating with a trusted issuer (or a Lunogram session), any token failure collapsed into a single bare unauthorized. If a token was missing data such as the exp claim, the caller got the same 401 as a wrong signing key — with no way to tell which, making integrations painful to debug.

Change

Surface a precise, fixable reason in the 401 — but only once the token has proven to be authentically ours, so we never leak whether an issuer or project exists.

Trusted issuer (trusted_issuer.go)

  • A descriptive reason is returned only after the token's iss resolves to a configured trusted issuer. Failures before that (empty/unparseable token, unknown issuer) stay a generic ErrUnauthorized — they'd otherwise leak issuer/project existence.
  • verifyTrustedIssuerToken now preserves the underlying golang-jwt cause instead of flattening it, and describeTokenError maps it to a safe message (missing claim, expired, not-yet-valid, issuer/audience mismatch). Signature / malformed / key-resolution failures collapse to a generic reason so verification internals never leak.

Session (session.go)

  • A descriptive reason is returned only after the ES256 signature verifies. golang-jwt validates claims strictly after the signature, so a claim-level error (ErrTokenInvalidClaims) proves the token is one of ours.
  • Signature / signing-method / format failures stay ErrUnauthorized so the auth chain still falls through to the trusted-issuer handler (a real trusted-issuer JWT is RS256 and must not be mis-described as a session token).

Safety / compatibility

  • HTTP status is unchanged — still 401 via SecurityRequirementsError. Only the problem detail gains a specific message, e.g. token is missing the required "exp" claim or token has expired.
  • The descriptive errors deliberately do not satisfy errors.Is(err, ErrUnauthorized), so the middleware returns them instead of collapsing to a bare unauthorized. This only short-circuits the chain for tokens already proven to belong to that scheme, so no request that currently succeeds can regress.

Tests

Updated and extended trusted_issuer_middleware_test.go and session_middleware_test.go to cover the new reasons (missing exp, expired, wrong key, wrong/missing issuer, missing/invalid amid, missing sub, unknown session method) and to assert that signature/format failures still stay generic and fall through.

…kens

External JWT failures (trusted issuer and Lunogram sessions) all collapsed
into a single bare "unauthorized", so an integrator could not tell a
malformed token (e.g. a missing `exp` claim) from a wrong signing key.

Surface a precise reason — but only once the token has proven to be
authentically ours, so we never leak whether an issuer or project exists:

- Trusted issuer: a reason is returned only after the token's `iss`
  resolves to a configured issuer. Failures before that (empty/unparseable
  token, unknown issuer) stay a generic ErrUnauthorized.
- Session: a reason is returned only after the ES256 signature verifies
  (golang-jwt validates claims strictly after the signature). Signature,
  signing-method, and format failures stay ErrUnauthorized so the auth
  chain still falls through to the trusted-issuer handler.

The HTTP status is unchanged (still 401 via SecurityRequirementsError);
only the problem detail gains a specific, fixable message such as
`token is missing the required "exp" claim` or `token has expired`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant