diff --git a/README.md b/README.md index 580d383f4..870fd86e6 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ For production builds, tests, and Docker, see [Getting Started](#getting-started - ✅ Secure session management - ✅ Email verification - ✅ OAuth2 and OpenID Connect compatible APIs (IdP, Relying Party/broker, and both simultaneously for multi-tenant SSO) -- ✅ Machine-to-machine (service-to-service) authentication with `client_credentials` grant and secretless workload identity (RFC 7523 client_assertion, SPIFFE JWT-SVID, Kubernetes TokenReview) +- ✅ Machine-to-machine (service-to-service) authentication with `client_credentials` grant and secretless workload identity (RFC 7523 client_assertion, Kubernetes TokenReview) — SPIFFE JWT-SVID is **preview**: its draft (`draft-schwenkschuster-oauth-spiffe-client-auth-00`) expired 2026-01-02, is not WG-adopted, and its assertion-type URN is not IANA-registered, so the value may change - ✅ Agent-to-agent (A2A) delegation via RFC 8693 token-exchange with nested `act` chains and scope attenuation - ✅ APIs to update profile securely - ✅ Forgot password flow using email diff --git a/internal/constants/grant_types.go b/internal/constants/grant_types.go index 261fea4ca..30db37443 100644 --- a/internal/constants/grant_types.go +++ b/internal/constants/grant_types.go @@ -59,8 +59,19 @@ const ( // Preferred for private clusters — avoids exposing K8s discovery endpoints. KeySourceStaticJWKSURL = "static_jwks_url" - // KeySourceSPIFFEBundleEndpoint fetches keys from a SPIFFE bundle endpoint. - // Requires SpiffeRefreshHintSeconds to be honoured at runtime (Phase 5). + // KeySourceSPIFFEBundleEndpoint names a SPIFFE bundle endpoint as the key + // source. + // + // NOT IMPLEMENTED. fetchJWKSBytes has no case for it and returns + // "unsupported key_source_type", and SpiffeRefreshHintSeconds — which a + // bundle endpoint's refresh cadence depends on — is stored and returned by + // the admin API but never read at runtime. + // + // The constant is kept, rather than deleted, because + // service.validateKeySourceType rejects it BY NAME with "not implemented + // yet". Before that existed the value was accepted and stored verbatim, so an + // issuer configured with it looked healthy and failed only when the first + // workload tried to authenticate. KeySourceSPIFFEBundleEndpoint = "spiffe_bundle_endpoint" ) @@ -82,9 +93,13 @@ const ( // TrustedIssuer authentication method identifiers. const ( - // AuthMethodJWTAssertion uses a JWT as the client_assertion (Phases 3–5, default). + // AuthMethodJWTAssertion uses a JWT as the client_assertion. It is the only + // value AddTrustedIssuer writes, and the only one anything reads. AuthMethodJWTAssertion = "jwt_assertion" - - // AuthMethodX509MTLS uses an X.509-SVID via mTLS (Phase 6). - AuthMethodX509MTLS = "x509_mtls" ) + +// An x509_mtls auth method (X.509-SVID over mTLS) was declared here and never +// implemented: nothing read it, AddTrustedIssuer hardcoded jwt_assertion, and no +// request field could set it, so the constant was unreachable in every direction. +// Removed rather than left as a claim the code does not honour. Reintroduce it +// with the implementation, not ahead of it. diff --git a/internal/integration_tests/trusted_issuer_test.go b/internal/integration_tests/trusted_issuer_test.go index f443fb776..a53b814eb 100644 --- a/internal/integration_tests/trusted_issuer_test.go +++ b/internal/integration_tests/trusted_issuer_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/authorizerdev/authorizer/internal/constants" "github.com/authorizerdev/authorizer/internal/graph/model" "github.com/authorizerdev/authorizer/internal/refs" ) @@ -239,4 +240,66 @@ func TestTrustedIssuerAdmin(t *testing.T) { require.NoError(t, err) assert.Equal(t, 0, len(listRes)) }) + + // key_source_type and issuer_type were checked for non-emptiness only, so any + // string was stored verbatim. The value that mattered was + // "spiffe_bundle_endpoint": a declared constant with no implementation, which + // fetchJWKSBytes rejects at authentication time with "unsupported + // key_source_type". An operator got a 200 and a row that looked configured, + // and found out it was dead only when the first workload tried to + // authenticate. A plain typo failed identically, at the same unhelpful moment. + t.Run("key_source_type is allow-listed against what is implemented", func(t *testing.T) { + saID := createSA(t) + + t.Run("spiffe_bundle_endpoint is refused as not implemented", func(t *testing.T) { + req := newIssuerReq(saID) + req.KeySourceType = constants.KeySourceSPIFFEBundleEndpoint + _, err := ts.GraphQLProvider.AddTrustedIssuer(ctx, req) + require.Error(t, err, "a key source with no implementation must not be storable") + assert.Contains(t, err.Error(), "not implemented", + "the error must say the value is unimplemented, not merely invalid — "+ + "they call for different actions") + }) + + t.Run("a typo is refused", func(t *testing.T) { + req := newIssuerReq(saID) + req.KeySourceType = "static_jwks_urls" // trailing s + _, err := ts.GraphQLProvider.AddTrustedIssuer(ctx, req) + require.Error(t, err) + assert.Contains(t, err.Error(), "unsupported key_source_type") + }) + + for _, good := range []string{constants.KeySourceOIDCDiscovery, constants.KeySourceStaticJWKSURL} { + t.Run("implemented source "+good+" is still accepted", func(t *testing.T) { + req := newIssuerReq(saID) + req.KeySourceType = good + _, err := ts.GraphQLProvider.AddTrustedIssuer(ctx, req) + require.NoError(t, err, "the allow-list must not break a working configuration") + }) + } + }) + + // issuer_type drives no behaviour today — nothing switches on it — but it is + // stored, returned by the admin API and rendered in the dashboard. An + // unconstrained free-text column shaped like an enum diverges across + // deployments and cannot later be given meaning without a migration. + t.Run("issuer_type is allow-listed", func(t *testing.T) { + saID := createSA(t) + + req := newIssuerReq(saID) + req.IssuerType = "kubernetes_service_account" // plausible, and wrong + _, err := ts.GraphQLProvider.AddTrustedIssuer(ctx, req) + require.Error(t, err) + assert.Contains(t, err.Error(), "unsupported issuer_type") + + for _, good := range []string{ + constants.IssuerTypeKubernetesSA, constants.IssuerTypeSPIFFEJWT, + constants.IssuerTypeOIDC, constants.IssuerTypeCloudOIDC, + } { + ok := newIssuerReq(saID) + ok.IssuerType = good + _, err := ts.GraphQLProvider.AddTrustedIssuer(ctx, ok) + require.NoError(t, err, "declared issuer type %q must be accepted", good) + } + }) } diff --git a/internal/service/admin_trusted_issuers.go b/internal/service/admin_trusted_issuers.go index 79af329dd..6197c51ea 100644 --- a/internal/service/admin_trusted_issuers.go +++ b/internal/service/admin_trusted_issuers.go @@ -38,6 +38,62 @@ func normalizeAPIServerURL(raw *string) *string { // https URL and require it whenever online TokenReview is enabled (fail-closed: // enabling review without a reachable apiserver would silently reject every // token at runtime). apiServerURL is expected to be already normalized. +// validateKeySourceType allow-lists key_source_type against the sources +// fetchJWKSBytes can actually resolve. +// +// Both fields used to be checked for non-emptiness only, so any string was +// stored verbatim. That mattered most for "spiffe_bundle_endpoint": it is a +// declared constant with no implementation — fetchJWKSBytes has no case for it +// and falls through to `unsupported key_source_type` — so an operator could +// create an issuer, receive a 200 and a row that looked configured, and discover +// it was dead only when the first workload tried to authenticate. A plain typo +// ("static_jwks_urls") failed exactly the same way, at exactly the same +// unhelpful moment. +// +// The set here is deliberately the set fetchJWKSBytes IMPLEMENTS, not the set of +// declared constants. When spiffe_bundle_endpoint is implemented, this is the +// second place to change, and the error message below is the reminder. +func validateKeySourceType(v string) error { + switch strings.TrimSpace(v) { + case "": + return InvalidArgument("key_source_type is required") + case constants.KeySourceOIDCDiscovery, constants.KeySourceStaticJWKSURL: + return nil + case constants.KeySourceSPIFFEBundleEndpoint: + return InvalidArgument(fmt.Sprintf( + "key_source_type %q is not implemented yet; use %q or %q", + constants.KeySourceSPIFFEBundleEndpoint, + constants.KeySourceOIDCDiscovery, constants.KeySourceStaticJWKSURL)) + default: + return InvalidArgument(fmt.Sprintf( + "unsupported key_source_type %q; supported values are %q and %q", + v, constants.KeySourceOIDCDiscovery, constants.KeySourceStaticJWKSURL)) + } +} + +// validateIssuerType allow-lists issuer_type. +// +// Unlike key_source_type this field drives no behaviour today — nothing switches +// on it, which is why every IssuerType* constant is otherwise unreferenced. It is +// still validated, because it is stored, returned by the admin API and shown in +// the dashboard: an unconstrained free-text column that looks like an enum will +// diverge across deployments and cannot later be given meaning without a +// migration. Rejecting a typo now is cheaper than reconciling one later. +func validateIssuerType(v string) error { + switch strings.TrimSpace(v) { + case "": + return InvalidArgument("issuer_type is required") + case constants.IssuerTypeKubernetesSA, constants.IssuerTypeSPIFFEJWT, + constants.IssuerTypeOIDC, constants.IssuerTypeCloudOIDC: + return nil + default: + return InvalidArgument(fmt.Sprintf( + "unsupported issuer_type %q; supported values are %q, %q, %q and %q", + v, constants.IssuerTypeKubernetesSA, constants.IssuerTypeSPIFFEJWT, + constants.IssuerTypeOIDC, constants.IssuerTypeCloudOIDC)) + } +} + func validateTokenReviewConfig(enableTokenReview bool, apiServerURL *string) error { raw := refs.StringValue(apiServerURL) if raw == "" { @@ -76,14 +132,14 @@ func (p *provider) AddTrustedIssuer(ctx context.Context, meta RequestMetadata, p if strings.TrimSpace(params.IssuerURL) == "" { return nil, nil, InvalidArgument("issuer_url is required") } - if strings.TrimSpace(params.KeySourceType) == "" { - return nil, nil, InvalidArgument("key_source_type is required") + if err := validateKeySourceType(params.KeySourceType); err != nil { + return nil, nil, err } if strings.TrimSpace(params.ExpectedAud) == "" { return nil, nil, InvalidArgument("expected_aud is required") } - if strings.TrimSpace(params.IssuerType) == "" { - return nil, nil, InvalidArgument("issuer_type is required") + if err := validateIssuerType(params.IssuerType); err != nil { + return nil, nil, err } // Reject issuers bound to a non-existent service account — otherwise a typo @@ -139,7 +195,7 @@ func (p *provider) AddTrustedIssuer(ctx context.Context, meta RequestMetadata, p // only ever creates client_assertion_trust rows; org-scoped SSO connections // are created through the dedicated OIDC-connection admin API. Kind: constants.TrustKindClientAssertion, - AuthMethod: "jwt_assertion", + AuthMethod: constants.AuthMethodJWTAssertion, IsActive: true, SpiffeRefreshHintSeconds: refs.Int64Value(params.SpiffeRefreshHintSeconds), EnableTokenReview: enableTokenReview, diff --git a/web/dashboard/src/components/UpdateTrustedIssuerModal.tsx b/web/dashboard/src/components/UpdateTrustedIssuerModal.tsx index 459c0dff6..761cac2b8 100644 --- a/web/dashboard/src/components/UpdateTrustedIssuerModal.tsx +++ b/web/dashboard/src/components/UpdateTrustedIssuerModal.tsx @@ -19,12 +19,17 @@ import { SheetFooter, } from './ui/sheet'; -const keySourceTypes = [ - 'oidc_discovery', - 'static_jwks_url', - 'spiffe_bundle_endpoint', -]; +// Only the sources the server can actually resolve. 'spiffe_bundle_endpoint' was +// offered here but has no implementation behind it — fetchJWKSBytes rejects it +// with "unsupported key_source_type" — so picking it produced an issuer that +// looked saved and failed at the first authentication. The API now refuses it at +// write time (service.validateKeySourceType), which would have made this dropdown +// an option guaranteed to error. Re-add it together with the implementation. +const keySourceTypes = ['oidc_discovery', 'static_jwks_url']; +// Kept in step with service.validateIssuerType. 'spiffe_jwt' is preview: its +// draft expired 2026-01-02, is not WG-adopted, and its assertion-type URN is not +// IANA-registered. const issuerTypes = ['kubernetes_sa', 'spiffe_jwt', 'oidc', 'cloud_oidc']; interface UpdateTrustedIssuerModalProps {