diff --git a/cmd/server/main.go b/cmd/server/main.go index ac16a2c..0114702 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -2761,6 +2761,14 @@ func (a oidcProvidersListAdapter) List(ctx context.Context, tenantID string) ([] } out := make([]*handler.OIDCProviderInfo, 0, len(provs)) for _, p := range provs { + // Audit 2026-05-10 MED-9 closure — filter disabled providers + // at the adapter so the LoginPage's "Sign in with X" buttons + // don't render for offline IdPs. The HandleAuthRequest + // service-layer ErrProviderDisabled check is the + // defense-in-depth guard for direct API / MCP / CLI callers. + if !p.Enabled { + continue + } out = append(out, &handler.OIDCProviderInfo{ ID: p.ID, DisplayName: p.Name, diff --git a/internal/api/handler/health.go b/internal/api/handler/health.go index f5dac22..fe3166b 100644 --- a/internal/api/handler/health.go +++ b/internal/api/handler/health.go @@ -208,6 +208,13 @@ func (h HealthHandler) AuthInfo(w http.ResponseWriter, r *http.Request) { "required": h.AuthType != "none", } if h.OIDCProvidersResolver != nil { + // Audit 2026-05-10 MED-9 closure — the adapter + // (cmd/server/main.go::oidcProvidersListAdapter.List) filters + // disabled providers before constructing OIDCProviderInfo, so + // the LoginPage never sees a button for an offline IdP. The + // HandleAuthRequest service-layer ErrProviderDisabled check + // is the defense-in-depth guard for direct API / MCP / CLI + // callers that bypass the GUI. if provs, err := h.OIDCProvidersResolver.List(r.Context(), authdomain.DefaultTenantID); err == nil { response["oidc_providers"] = provs } diff --git a/internal/auth/oidc/domain/types.go b/internal/auth/oidc/domain/types.go index a959ff4..c05a53b 100644 --- a/internal/auth/oidc/domain/types.go +++ b/internal/auth/oidc/domain/types.go @@ -47,8 +47,16 @@ type OIDCProvider struct { AllowedEmailDomains []string `json:"allowed_email_domains"` IATWindowSeconds int `json:"iat_window_seconds"` JWKSCacheTTLSeconds int `json:"jwks_cache_ttl_seconds"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` + // Enabled gates whether the provider is offered on the LoginPage and + // accepted at HandleAuthRequest. Audit 2026-05-10 MED-9 closure: + // pre-fix the only way to take a provider offline was DELETE (which + // breaks active user_oidc_provider FK references); now operators can + // flip Enabled=false to keep the row + group mappings around while + // suppressing new logins. Default true (existing rows are enabled + // post-migration). + Enabled bool `json:"enabled"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` } // GroupRoleMapping maps a group name (string from the IdP's group diff --git a/internal/auth/oidc/provider_enabled_test.go b/internal/auth/oidc/provider_enabled_test.go new file mode 100644 index 0000000..613091b --- /dev/null +++ b/internal/auth/oidc/provider_enabled_test.go @@ -0,0 +1,39 @@ +package oidc + +import ( + "context" + "errors" + "testing" +) + +// Audit 2026-05-10 MED-9 closure — pin the disabled-provider behavior. +// HandleAuthRequest must reject pre-login creation with +// ErrProviderDisabled when the operator has flipped Enabled=false. The +// LoginPage's AuthInfo provider list filters disabled providers at the +// adapter (cmd/server/main.go::oidcProvidersListAdapter.List) so the +// button doesn't render in the first place; ErrProviderDisabled is the +// defense-in-depth guard for direct API / MCP / CLI callers. + +func TestService_HandleAuthRequest_DisabledProvider_RejectsWithErrProviderDisabled(t *testing.T) { + mockIdP := newMockIdP(t) + svc, _ := newServiceWithProvider(t, mockIdP.URL(), "op-disabled") + + // Warm the entry cache via a successful HandleAuthRequest (this runs + // real discovery against mockIdP), then flip cfgRow.Enabled to false + // to simulate the operator toggling the provider offline. The next + // HandleAuthRequest hits the disabled-check before the cached entry + // is reused. + if _, _, _, err := svc.HandleAuthRequest(context.Background(), "op-disabled"); err != nil { + t.Fatalf("warm HandleAuthRequest: %v", err) + } + if entry, ok := svc.cache["op-disabled"]; ok && entry.cfgRow != nil { + entry.cfgRow.Enabled = false + } else { + t.Fatal("expected cache entry for op-disabled after warmup") + } + + _, _, _, err := svc.HandleAuthRequest(context.Background(), "op-disabled") + if !errors.Is(err, ErrProviderDisabled) { + t.Errorf("HandleAuthRequest(disabled provider) err = %v; want ErrProviderDisabled", err) + } +} diff --git a/internal/auth/oidc/service.go b/internal/auth/oidc/service.go index b12d81c..d0387e7 100644 --- a/internal/auth/oidc/service.go +++ b/internal/auth/oidc/service.go @@ -231,6 +231,14 @@ var ( // includes `email` and the IdP releases the claim. ErrEmailMissingButRequired = errors.New("oidc: provider requires email but token has none") + // ErrProviderDisabled signals the operator has flipped + // OIDCProvider.Enabled=false on the matched provider. HandleAuthRequest + // rejects with this sentinel so the LoginPage doesn't initiate a + // handshake; AuthInfo's provider list filters disabled providers + // out so the LoginPage button doesn't appear in the first place. + // Audit 2026-05-10 MED-9 closure. + ErrProviderDisabled = errors.New("oidc: provider is disabled") + // ErrGroupsUnmapped: the user's groups don't match any of the // operator's group_role_mappings for this provider. No session // minted; audit row records auth.oidc_login_unmapped_groups. @@ -312,6 +320,13 @@ func (s *Service) HandleAuthRequest(ctx context.Context, providerID string) (aut if err != nil { return "", "", "", err } + // Audit 2026-05-10 MED-9 closure — refuse to mint a pre-login row + // for a disabled provider. The LoginPage's AuthInfo filter should + // already prevent the button from rendering, but defense-in-depth + // catches the direct-API/MCP/CLI invocation path too. + if entry.cfgRow != nil && !entry.cfgRow.Enabled { + return "", "", "", ErrProviderDisabled + } state, err := randomB64URL(32) if err != nil { diff --git a/internal/auth/oidc/service_test.go b/internal/auth/oidc/service_test.go index a69ce93..dee01ce 100644 --- a/internal/auth/oidc/service_test.go +++ b/internal/auth/oidc/service_test.go @@ -753,6 +753,7 @@ func makeProvider(idpURL, providerID string) *oidcdomain.OIDCProvider { Scopes: []string{"openid", "profile", "email"}, IATWindowSeconds: 300, JWKSCacheTTLSeconds: 3600, + Enabled: true, // MED-9: default-on for test fixtures } } diff --git a/internal/repository/postgres/oidc.go b/internal/repository/postgres/oidc.go index 9cf08a4..a0ec971 100644 --- a/internal/repository/postgres/oidc.go +++ b/internal/repository/postgres/oidc.go @@ -27,11 +27,14 @@ func NewOIDCProviderRepository(db *sql.DB) *OIDCProviderRepository { return &OIDCProviderRepository{db: db} } +// Audit 2026-05-10 MED-9: `enabled` column added to the SELECT/INSERT/ +// UPDATE column list. Migration 000042 added the column with default +// TRUE; existing rows are all enabled post-migration. const oidcProviderColumns = `id, tenant_id, name, issuer_url, client_id, client_secret_encrypted, redirect_uri, groups_claim_path, groups_claim_format, fetch_userinfo, scopes, allowed_email_domains, iat_window_seconds, - jwks_cache_ttl_seconds, created_at, updated_at` + jwks_cache_ttl_seconds, enabled, created_at, updated_at` func scanOIDCProvider(row interface{ Scan(...interface{}) error }) (*oidcdomain.OIDCProvider, error) { var p oidcdomain.OIDCProvider @@ -41,7 +44,7 @@ func scanOIDCProvider(row interface{ Scan(...interface{}) error }) (*oidcdomain. &p.ClientSecretEncrypted, &p.RedirectURI, &p.GroupsClaimPath, &p.GroupsClaimFormat, &p.FetchUserinfo, &scopes, &domains, &p.IATWindowSeconds, - &p.JWKSCacheTTLSeconds, &p.CreatedAt, &p.UpdatedAt, + &p.JWKSCacheTTLSeconds, &p.Enabled, &p.CreatedAt, &p.UpdatedAt, ); err != nil { return nil, err } @@ -104,19 +107,24 @@ func (r *OIDCProviderRepository) GetByName(ctx context.Context, tenantID, name s // Translates SQLSTATE 23505 (unique_violation) to // ErrOIDCProviderDuplicateName. func (r *OIDCProviderRepository) Create(ctx context.Context, p *oidcdomain.OIDCProvider) error { + // MED-9: persist `enabled` on Create. New providers default to + // enabled=true; the schema column also has DEFAULT TRUE, so an + // older client sending the pre-MED-9 row shape without the column + // would still get enabled=true. We pass the field explicitly to + // honor a `Enabled=false` create. _, err := r.db.ExecContext(ctx, ` INSERT INTO oidc_providers ( id, tenant_id, name, issuer_url, client_id, client_secret_encrypted, redirect_uri, groups_claim_path, groups_claim_format, fetch_userinfo, scopes, allowed_email_domains, iat_window_seconds, - jwks_cache_ttl_seconds - ) VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14)`, + jwks_cache_ttl_seconds, enabled + ) VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15)`, p.ID, p.TenantID, p.Name, p.IssuerURL, p.ClientID, p.ClientSecretEncrypted, p.RedirectURI, p.GroupsClaimPath, p.GroupsClaimFormat, p.FetchUserinfo, pq.StringArray(p.Scopes), pq.StringArray(p.AllowedEmailDomains), p.IATWindowSeconds, - p.JWKSCacheTTLSeconds, + p.JWKSCacheTTLSeconds, p.Enabled, ) if err != nil { var pqErr *pq.Error @@ -131,6 +139,8 @@ func (r *OIDCProviderRepository) Create(ctx context.Context, p *oidcdomain.OIDCP // Update writes the mutable fields back. Immutable: id, tenant_id, // created_at. updated_at = NOW(). func (r *OIDCProviderRepository) Update(ctx context.Context, p *oidcdomain.OIDCProvider) error { + // MED-9: persist `enabled` on Update so the toggle endpoint and + // the regular update path share the same write surface. res, err := r.db.ExecContext(ctx, ` UPDATE oidc_providers SET name = $2, @@ -145,13 +155,14 @@ func (r *OIDCProviderRepository) Update(ctx context.Context, p *oidcdomain.OIDCP allowed_email_domains = $11, iat_window_seconds = $12, jwks_cache_ttl_seconds = $13, + enabled = $14, updated_at = NOW() WHERE id = $1`, p.ID, p.Name, p.IssuerURL, p.ClientID, p.ClientSecretEncrypted, p.RedirectURI, p.GroupsClaimPath, p.GroupsClaimFormat, p.FetchUserinfo, pq.StringArray(p.Scopes), pq.StringArray(p.AllowedEmailDomains), p.IATWindowSeconds, - p.JWKSCacheTTLSeconds, + p.JWKSCacheTTLSeconds, p.Enabled, ) if err != nil { var pqErr *pq.Error diff --git a/internal/repository/postgres/oidc_test.go b/internal/repository/postgres/oidc_test.go index 427d176..e59f4ad 100644 --- a/internal/repository/postgres/oidc_test.go +++ b/internal/repository/postgres/oidc_test.go @@ -36,6 +36,7 @@ func newValidProvider(suffix string) *oidcdomain.OIDCProvider { AllowedEmailDomains: []string{}, IATWindowSeconds: 300, JWKSCacheTTLSeconds: 3600, + Enabled: true, // MED-9: default-on for test fixtures } } diff --git a/migrations/000042_oidc_provider_enabled.down.sql b/migrations/000042_oidc_provider_enabled.down.sql new file mode 100644 index 0000000..c2d2219 --- /dev/null +++ b/migrations/000042_oidc_provider_enabled.down.sql @@ -0,0 +1,3 @@ +-- Rollback for 000042_oidc_provider_enabled.up.sql +ALTER TABLE oidc_providers + DROP COLUMN IF EXISTS enabled; diff --git a/migrations/000042_oidc_provider_enabled.up.sql b/migrations/000042_oidc_provider_enabled.up.sql new file mode 100644 index 0000000..3fa9cc3 --- /dev/null +++ b/migrations/000042_oidc_provider_enabled.up.sql @@ -0,0 +1,18 @@ +-- ============================================================================= +-- 2026-05-10 Audit / MED-9 closure +-- ============================================================================= +-- +-- OIDCProvider.enabled toggle. Pre-fix, the only way to take a provider +-- offline was to DELETE the row, which breaks active users that reference +-- it via user_oidc_provider FKs (and any session that minted under the +-- provider stays orphaned). Post-fix, operators flip enabled=false to +-- keep the row + group mappings + user records intact while suppressing +-- the provider from the LoginPage and rejecting new HandleAuthRequest +-- attempts with ErrProviderDisabled. +-- +-- Default true — existing rows pre-migration are all considered enabled +-- so this migration is a no-op for the active set. +-- ============================================================================= + +ALTER TABLE oidc_providers + ADD COLUMN IF NOT EXISTS enabled BOOLEAN NOT NULL DEFAULT TRUE;