mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 14:21:37 +00:00
feat(oidc): Enabled toggle on OIDCProvider (MED-9)
Audit 2026-05-10 Fix 13 Phase B — close MED-9. MED-4/5/6/7 deferred to v3.
MED-9: ship the OIDCProvider.Enabled boolean. Pre-fix, the only way
to take a provider offline during an incident was DELETE, which
breaks active user_oidc_provider FK references and orphans any
session that minted under the provider. Post-fix:
- Migration 000042 adds enabled BOOLEAN NOT NULL DEFAULT TRUE.
Default-true means existing pre-migration rows are all enabled
post-deploy; no breaking-change window.
- internal/auth/oidc/domain/types.go::OIDCProvider.Enabled ships
the domain field with JSON tag 'enabled'.
- Repository read/write paths (List, Get, GetByName, Create, Update)
all carry the column.
- internal/auth/oidc/service.go::HandleAuthRequest rejects with
the new ErrProviderDisabled sentinel when cfgRow.Enabled=false.
- cmd/server/main.go::oidcProvidersListAdapter.List filters
disabled providers before constructing OIDCProviderInfo so the
LoginPage's 'Sign in with X' buttons never render for offline
IdPs.
- Defense-in-depth: the ErrProviderDisabled service-layer check
is the guard for direct API / MCP / CLI callers that bypass the
GUI.
Regression test: internal/auth/oidc/provider_enabled_test.go warms
the entry cache via a successful HandleAuthRequest, flips
cfgRow.Enabled=false on the cached entry, then asserts the next call
returns ErrProviderDisabled (errors.Is). Test fixtures (newValidProvider,
makeProvider) updated to set Enabled: true so existing tests stay
green.
Operators can toggle Enabled today via the existing PUT
/api/v1/auth/oidc/providers/{id} body field. A dedicated GUI
toggle on OIDCProviderDetailPage and a single-purpose PUT-just-enabled
endpoint are deferred to the v3 GUI-polish bundle — the load-bearing
wire is in place now.
MED-4 (GUI advanced fields on edit), MED-5 (POST .../test endpoint
+ button), MED-6 (JWKS auto-refresh on cache-miss), MED-7 (JWKS
health endpoint + GUI panel): DEFERRED to v3 with explicit
annotations in the audit doc. Workarounds: MED-4 fields are
PUT-editable via curl/MCP; MED-5 → call refresh post-create;
MED-6 → call refresh manually on key rotation.
Refs: cowork/auth-bundles-audit-2026-05-10.md MED-4, MED-5, MED-6,
MED-7, MED-9
Spec: cowork/auth-bundles-fixes-2026-05-10/13-med-bundle.md Phase B
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,3 @@
|
||||
-- Rollback for 000042_oidc_provider_enabled.up.sql
|
||||
ALTER TABLE oidc_providers
|
||||
DROP COLUMN IF EXISTS enabled;
|
||||
@@ -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;
|
||||
Reference in New Issue
Block a user