diff --git a/internal/http/auth/session.go b/internal/http/auth/session.go index 97a85524..a3e26aff 100644 --- a/internal/http/auth/session.go +++ b/internal/http/auth/session.go @@ -113,21 +113,25 @@ func WithSession(mgmt *management.State, signer *SessionSigner) Handler { jwt.WithExpirationRequired(), ) if err != nil || !token.Valid { - return ctx, ErrUnauthorized + return ctx, sessionTokenError(err) } + // The signature verified, so the token is authentically one of ours and + // the remaining failures are safe to describe: they mean a valid session + // whose claims we can't use (e.g. an expired token, or one minted under a + // session policy that has since been removed). methodID, err := uuid.Parse(claimString(claims, sessionMethodClaim)) if err != nil { - return ctx, ErrUnauthorized + return ctx, rejectSession(`token is missing or has an invalid auth-method ("amid") claim`) } subject := claimString(claims, "sub") if subject == "" { - return ctx, ErrUnauthorized + return ctx, rejectSession(`token is missing the "sub" subject claim`) } method, err := mgmt.GetSessionAuthMethod(methodID) if err != nil { - return ctx, ErrUnauthorized + return ctx, rejectSession("the session's auth method no longer exists") } // The session is only valid on its own project's URL; a token minted for @@ -147,3 +151,22 @@ func WithSession(mgmt *management.State, signer *SessionSigner) Handler { return rbac.WithActor(ctx, actor), nil } } + +// sessionTokenError decides what a parse/verify failure surfaces. A signature, +// signing-method, or format failure stays a generic ErrUnauthorized: it usually +// means the token simply wasn't minted by us (e.g. it's a trusted-issuer JWT), so +// the auth chain must fall through to the next handler rather than short-circuit. +// Only a claim-level failure (ErrTokenInvalidClaims) — which the parser reaches +// solely after the signature has verified — proves the token is authentically +// ours, and is surfaced with a precise, debuggable reason (expired, missing exp, +// ...). +func sessionTokenError(err error) error { + if err == nil || !errors.Is(err, jwt.ErrTokenInvalidClaims) { + return ErrUnauthorized + } + return rejectSession(describeTokenError(err)) +} + +func rejectSession(reason string) error { + return &rejectedTokenError{msg: "session token rejected: " + reason} +} diff --git a/internal/http/auth/session_middleware_test.go b/internal/http/auth/session_middleware_test.go index 7486d4a4..82d0e153 100644 --- a/internal/http/auth/session_middleware_test.go +++ b/internal/http/auth/session_middleware_test.go @@ -78,41 +78,64 @@ func TestWithSession(t *testing.T) { assert.ErrorIs(t, err, ErrUnauthorized) }) - t.Run("an expired token is rejected through the handler", func(t *testing.T) { + // A token whose signature does not verify (wrong key, wrong alg, malformed) + // stays a generic ErrUnauthorized so the auth chain falls through to the next + // handler. Only once the signature proves the token is authentically ours does + // the handler surface a precise, debuggable reason for the remaining failures. + + t.Run("an expired token surfaces a debuggable reason", func(t *testing.T) { t.Parallel() signer := testSigner(t, "") token, _, err := signer.Mint(uuid.New(), "user_1", -time.Hour) require.NoError(t, err) _, err = WithSession(nil, signer)(context.Background(), token) - assert.ErrorIs(t, err, ErrUnauthorized) + assert.NotErrorIs(t, err, ErrUnauthorized) + assert.ErrorContains(t, err, "expired") }) - t.Run("a wrong issuer is rejected", func(t *testing.T) { + t.Run("a missing exp claim surfaces a debuggable reason", func(t *testing.T) { t.Parallel() - // Signer expects the default issuer; mint with a different one. + signer := testSigner(t, "") + token := signES256(t, signer, jwt.MapClaims{ + "iss": signer.issuer, + "sub": "user_1", + sessionMethodClaim: uuid.New().String(), + // no "exp": WithExpirationRequired rejects it + }) + _, err := WithSession(nil, signer)(context.Background(), token) + assert.NotErrorIs(t, err, ErrUnauthorized) + assert.ErrorContains(t, err, `"exp"`) + }) + + t.Run("a wrong issuer surfaces a debuggable reason", func(t *testing.T) { + t.Parallel() + // Signer expects the default issuer; mint with a different one. The + // signature still verifies (same key), so this is one of our tokens. minter := testSigner(t, "https://evil.example") token, _, err := minter.Mint(uuid.New(), "user_1", time.Hour) require.NoError(t, err) verifier := &SessionSigner{key: minter.key, issuer: defaultSessionIssuer} _, err = WithSession(nil, verifier)(context.Background(), token) - assert.ErrorIs(t, err, ErrUnauthorized) + assert.NotErrorIs(t, err, ErrUnauthorized) + assert.ErrorContains(t, err, "issuer") }) - t.Run("a missing issuer is rejected", func(t *testing.T) { + t.Run("a token signed by a different key stays generic (chain falls through)", func(t *testing.T) { t.Parallel() - signer := testSigner(t, "") - token := signES256(t, signer, jwt.MapClaims{ - "sub": "user_1", - sessionMethodClaim: uuid.New().String(), - "exp": time.Now().Add(time.Hour).Unix(), - }) - _, err := WithSession(nil, signer)(context.Background(), token) + // A token minted by a foreign key (e.g. another scheme's) must not be + // described as a session token; it stays ErrUnauthorized so the next + // handler gets a turn. + minter := testSigner(t, "") + token, _, err := minter.Mint(uuid.New(), "user_1", time.Hour) + require.NoError(t, err) + + _, err = WithSession(nil, testSigner(t, ""))(context.Background(), token) assert.ErrorIs(t, err, ErrUnauthorized) }) - t.Run("an unparseable method id claim is rejected", func(t *testing.T) { + t.Run("an unparseable method id claim surfaces a debuggable reason", func(t *testing.T) { t.Parallel() signer := testSigner(t, "") token := signES256(t, signer, jwt.MapClaims{ @@ -122,10 +145,11 @@ func TestWithSession(t *testing.T) { "exp": time.Now().Add(time.Hour).Unix(), }) _, err := WithSession(nil, signer)(context.Background(), token) - assert.ErrorIs(t, err, ErrUnauthorized) + assert.NotErrorIs(t, err, ErrUnauthorized) + assert.ErrorContains(t, err, "amid") }) - t.Run("a missing method id claim is rejected", func(t *testing.T) { + t.Run("a missing method id claim surfaces a debuggable reason", func(t *testing.T) { t.Parallel() signer := testSigner(t, "") token := signES256(t, signer, jwt.MapClaims{ @@ -134,10 +158,11 @@ func TestWithSession(t *testing.T) { "exp": time.Now().Add(time.Hour).Unix(), }) _, err := WithSession(nil, signer)(context.Background(), token) - assert.ErrorIs(t, err, ErrUnauthorized) + assert.NotErrorIs(t, err, ErrUnauthorized) + assert.ErrorContains(t, err, "amid") }) - t.Run("an empty subject is rejected", func(t *testing.T) { + t.Run("an empty subject surfaces a debuggable reason", func(t *testing.T) { t.Parallel() signer := testSigner(t, "") token := signES256(t, signer, jwt.MapClaims{ @@ -147,7 +172,8 @@ func TestWithSession(t *testing.T) { "exp": time.Now().Add(time.Hour).Unix(), }) _, err := WithSession(nil, signer)(context.Background(), token) - assert.ErrorIs(t, err, ErrUnauthorized) + assert.NotErrorIs(t, err, ErrUnauthorized) + assert.ErrorContains(t, err, `"sub"`) }) } @@ -161,7 +187,8 @@ func TestWithSessionUnknownMethod(t *testing.T) { require.NoError(t, err) _, err = WithSession(mgmt, signer)(context.Background(), token) - assert.ErrorIs(t, err, ErrUnauthorized) + assert.NotErrorIs(t, err, ErrUnauthorized) + assert.ErrorContains(t, err, "auth method") } func TestWithSessionBuildsActor(t *testing.T) { diff --git a/internal/http/auth/trusted_issuer.go b/internal/http/auth/trusted_issuer.go index 3fde53fa..45d7b253 100644 --- a/internal/http/auth/trusted_issuer.go +++ b/internal/http/auth/trusted_issuer.go @@ -5,6 +5,8 @@ import ( "crypto/x509" "encoding/pem" "errors" + "fmt" + "strings" "github.com/golang-jwt/jwt/v5" "github.com/lunogram/platform/internal/jwks" @@ -48,14 +50,21 @@ func WithTrustedIssuer(mgmt *management.State, cache *jwks.Cache) Handler { return ctx, ErrUnauthorized } + // Past this point the token has resolved to a configured trusted issuer, + // so a failure is almost always a malformed or misconfigured integration + // token rather than a probe. Surface a precise reason (which claim is + // missing, expired, etc.) instead of a bare "unauthorized": the failures + // above stay generic because revealing them would leak whether an issuer + // or project exists, but here the caller already proved knowledge of a + // real issuer, so a debuggable message is safe and far more useful. claims, err := verifyTrustedIssuerToken(ctx, cache, method, tokenString) if err != nil { - return ctx, ErrUnauthorized + return ctx, rejectTrustedIssuer(describeTokenError(err)) } subject := claimString(claims, method.SubjectClaim) if subject == "" { - return ctx, ErrUnauthorized + return ctx, rejectTrustedIssuer(fmt.Sprintf("token is missing the %q subject claim", subjectClaimName(method.SubjectClaim))) } actor := rbac.NewActor( @@ -106,8 +115,15 @@ func verifyTrustedIssuerToken(ctx context.Context, cache *jwks.Cache, method *ma token, err = jwt.ParseWithClaims(tokenString, claims, refreshed, opts...) } } - if err != nil || !token.Valid { - return nil, errors.New("auth: invalid trusted-issuer token") + // Preserve the underlying cause (a golang-jwt sentinel such as + // ErrTokenRequiredClaimMissing or ErrTokenExpired) so the caller can turn it + // into a precise, debuggable message; describeTokenError keeps the surfaced + // text safe. + if err != nil { + return nil, err + } + if !token.Valid { + return nil, jwt.ErrTokenUnverifiable } return claims, nil } @@ -147,11 +163,75 @@ func parsePublicKeyPEM(pemData string) (any, error) { // claimString returns the string value of the named claim (defaulting to "sub"). func claimString(claims jwt.MapClaims, name string) string { - if name == "" { - name = "sub" - } - if v, ok := claims[name].(string); ok { + if v, ok := claims[subjectClaimName(name)].(string); ok { return v } return "" } + +// subjectClaimName resolves the configured subject claim, defaulting to "sub". +func subjectClaimName(name string) string { + if name == "" { + return "sub" + } + return name +} + +// rejectedTokenError carries a human-readable reason a token was rejected. +// Unlike ErrUnauthorized it is deliberately NOT part of the generic unauthorized +// set, so the authentication middleware returns it (and its reason) to the caller +// instead of collapsing it into a bare "unauthorized". It is only produced once a +// token has proven to be authentically ours (a resolved trusted issuer, or a +// verified session signature), so the detail aids an integrator debugging their +// token without leaking anything they don't already know. +type rejectedTokenError struct{ msg string } + +func (e *rejectedTokenError) Error() string { return e.msg } + +func rejectTrustedIssuer(reason string) error { + return &rejectedTokenError{msg: "trusted-issuer token rejected: " + reason} +} + +// describeTokenError maps a JWT verification failure to a concise reason that is +// safe to return to the integrator. It names the standard-claim problems an +// integrator can actually fix (missing/expired/not-yet-valid/issuer/audience); +// anything else — signature, malformed token, key resolution — collapses to a +// generic reason so verification internals are never leaked. +func describeTokenError(err error) string { + switch { + case errors.Is(err, jwt.ErrTokenRequiredClaimMissing): + // Both the mandatory "exp" and an enforced "iss"/"aud" land here; name the + // claim golang-jwt actually reported rather than assuming "exp". + return requiredClaimReason(err) + case errors.Is(err, jwt.ErrTokenExpired): + return "token has expired" + case errors.Is(err, jwt.ErrTokenNotValidYet): + return `token is not valid yet (its "nbf" not-before claim is in the future)` + case errors.Is(err, jwt.ErrTokenUsedBeforeIssued): + return `token was used before its "iat" issued-at time` + case errors.Is(err, jwt.ErrTokenInvalidIssuer): + return "token issuer does not match the expected issuer" + case errors.Is(err, jwt.ErrTokenInvalidAudience): + return "token audience does not match the expected audience" + case errors.Is(err, jwt.ErrTokenSignatureInvalid), errors.Is(err, jwt.ErrTokenUnverifiable): + return "token signature could not be verified against the issuer's keys" + default: + return "token could not be verified" + } +} + +// requiredClaimReason names the standard claim golang-jwt reported as missing. +// golang-jwt phrases the leaf as ` claim is required`, so an integrator +// sees exactly what to add — most often the mandatory "exp" expiry claim. It +// falls back to a generic (but still exp-centric) message if the library ever +// changes that phrasing. +func requiredClaimReason(err error) string { + const marker = "missing required claim: " + msg := err.Error() + if i := strings.LastIndex(msg, marker); i >= 0 { + if claim := strings.TrimSuffix(msg[i+len(marker):], " claim is required"); claim != "" && !strings.ContainsRune(claim, ' ') { + return fmt.Sprintf("token is missing the required %q claim", claim) + } + } + return `token is missing a required claim (the "exp" expiry claim is mandatory)` +} diff --git a/internal/http/auth/trusted_issuer_middleware_test.go b/internal/http/auth/trusted_issuer_middleware_test.go index 7a65775c..34e69edf 100644 --- a/internal/http/auth/trusted_issuer_middleware_test.go +++ b/internal/http/auth/trusted_issuer_middleware_test.go @@ -112,7 +112,11 @@ func TestWithTrustedIssuer(t *testing.T) { assert.ErrorIs(t, err, ErrUnauthorized) }) - t.Run("rejects when verification fails (wrong key)", func(t *testing.T) { + // Failures before the issuer resolves stay a generic ErrUnauthorized (above); + // once the token resolves to a configured issuer, the middleware surfaces a + // precise, debuggable reason instead so an integrator can fix their token. + + t.Run("rejects when verification fails (wrong key) with a debuggable reason", func(t *testing.T) { _, method := issuerMethod(t) other, err := rsa.GenerateKey(rand.Reader, 2048) require.NoError(t, err) @@ -122,10 +126,35 @@ func TestWithTrustedIssuer(t *testing.T) { "exp": time.Now().Add(time.Hour).Unix(), }) _, err = WithTrustedIssuer(newIssuerState(method), cache)(clientRequestCtx(method.ProjectID.String()), token) - assert.ErrorIs(t, err, ErrUnauthorized) + assert.NotErrorIs(t, err, ErrUnauthorized, "a resolved issuer surfaces a reason, not a bare unauthorized") + assert.ErrorContains(t, err, "signature could not be verified") + }) + + t.Run("rejects a missing exp claim with a debuggable reason", func(t *testing.T) { + key, method := issuerMethod(t) + token := signRS256(t, key, jwt.MapClaims{ + "iss": method.Issuer, + "sub": "user_123", + // no "exp": WithExpirationRequired rejects it + }) + _, err := WithTrustedIssuer(newIssuerState(method), cache)(clientRequestCtx(method.ProjectID.String()), token) + assert.NotErrorIs(t, err, ErrUnauthorized) + assert.ErrorContains(t, err, `"exp"`, "the reason names the missing claim") + }) + + t.Run("rejects an expired token with a debuggable reason", func(t *testing.T) { + key, method := issuerMethod(t) + token := signRS256(t, key, jwt.MapClaims{ + "iss": method.Issuer, + "sub": "user_123", + "exp": time.Now().Add(-time.Hour).Unix(), + }) + _, err := WithTrustedIssuer(newIssuerState(method), cache)(clientRequestCtx(method.ProjectID.String()), token) + assert.NotErrorIs(t, err, ErrUnauthorized) + assert.ErrorContains(t, err, "expired") }) - t.Run("rejects an empty subject claim", func(t *testing.T) { + t.Run("rejects an empty subject claim with a debuggable reason", func(t *testing.T) { key, method := issuerMethod(t) token := signRS256(t, key, jwt.MapClaims{ "iss": method.Issuer, @@ -133,7 +162,21 @@ func TestWithTrustedIssuer(t *testing.T) { "exp": time.Now().Add(time.Hour).Unix(), }) _, err := WithTrustedIssuer(newIssuerState(method), cache)(clientRequestCtx(method.ProjectID.String()), token) - assert.ErrorIs(t, err, ErrUnauthorized) + assert.NotErrorIs(t, err, ErrUnauthorized) + assert.ErrorContains(t, err, `"sub"`, "the reason names the missing subject claim") + }) + + t.Run("names a custom subject claim in the rejection reason", func(t *testing.T) { + key, method := issuerMethod(t) + method.SubjectClaim = "user_id" + token := signRS256(t, key, jwt.MapClaims{ + "iss": method.Issuer, + // no "user_id": the configured subject claim is absent + "exp": time.Now().Add(time.Hour).Unix(), + }) + _, err := WithTrustedIssuer(newIssuerState(method), cache)(clientRequestCtx(method.ProjectID.String()), token) + assert.NotErrorIs(t, err, ErrUnauthorized) + assert.ErrorContains(t, err, `"user_id"`) }) t.Run("builds an end-user actor on success", func(t *testing.T) {