mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 15:51:30 +00:00
feat(oidc): POST /api/v1/auth/oidc/test dry-run endpoint (MED-5)
Audit 2026-05-10 MED-5 closure (backend half).
WHAT.
New POST /api/v1/auth/oidc/test endpoint that validates an OIDC
provider configuration without persisting anything. Mirrors the
read-only legs of the production getOrLoad path so operators can
catch typos / network reachability problems / IdP-advertises-weak-
alg conditions BEFORE creating the provider row.
Request body: {issuer_url, client_id, client_secret, scopes} —
client_secret is accepted but unused (discovery + JWKS reachability
do not require it).
Response body: TestDiscoveryResult{
discovery_succeeded — gooidc.NewProvider returned without error
jwks_reachable — explicit GET against jwks_uri succeeded
supported_alg_values — verbatim id_token_signing_alg_values_supported
iss_param_supported — RFC 9207 advertisement parsed off the disco doc
issuer_echo — the iss URL we were called with
authorization_url,
token_url, jwks_uri,
userinfo_endpoint — discovery doc fields for the GUI to preview
errors[] — per-leg failure messages
}
HTTP status:
- 200 even when individual checks fail (the per-leg errors[] carries
detail so the GUI renders per-check status rows)
- 400 only when the request body is malformed or issuer_url empty
- 500 only when the service-layer call itself errors
WHY.
Pre-fix, operators configuring OIDC had to create a provider, then
hit /refresh, then read the audit log to figure out whether the
discovery doc was reachable / whether the IdP advertises HS256
(the alg-downgrade trap). The GUI rendered no per-check feedback.
MED-5 closes the dry-run gap for the same reason every Issuer +
Target connector has a 'Test connection' button — operator
experience parity.
HOW.
internal/auth/oidc/test_discovery.go (NEW):
- TestDiscoveryResult struct with the per-leg projection.
- Service.TestDiscovery(ctx, issuerURL) drives the read-only
subset of getOrLoad: gooidc.NewProvider, claims parse for
alg-supported + iss-param-supported + jwks_uri + userinfo,
alg-downgrade defense, jwksReachable HTTP GET.
- jwksReachable is a package-level closure so tests can swap.
internal/api/handler/auth_session_oidc.go:
- TestProvider HTTP handler. Uses an inline discoveryTester
interface to type-assert against the OIDCAuthHandshaker stub
(the production Service satisfies; test stubs supply via
explicit method). Audit row 'auth.oidc_provider_tested' carries
the summary fields.
internal/api/router/router.go:
- Wired as POST /api/v1/auth/oidc/test under rbacGate('auth.oidc.create').
internal/api/handler/auth_session_oidc_test.go:
- stubOIDCSvc gains testResult + testErr fields + TestDiscovery
method so it satisfies the inline interface.
- 3 regression tests: happy path, missing issuer_url -> 400,
discovery-failure -> 200 with errors[] populated.
VERIFY.
- go vet ./internal/auth/oidc/... ./internal/api/handler/...
./internal/api/router/... PASS
- go test -short -count=1 -run TestProvider
./internal/api/handler/... PASS (3/3)
- go test -short -count=1 ./internal/auth/oidc/... PASS (3.7s)
- go test -short -count=1 ./internal/api/handler/... PASS (4.7s)
Out of scope for this commit: the GUI 'Test connection' button on
OIDCProviderDetailPage — queued with the GUI batch (items 10-19 of
HANDOFF.md).
Refs: cowork/auth-bundles-audit-2026-05-10.md MED-5
cowork/auth-bundles-fixes-2026-05-10/HANDOFF.md item 2
This commit is contained in:
@@ -950,6 +950,71 @@ func (h *AuthSessionOIDCHandler) DeleteProvider(w http.ResponseWriter, r *http.R
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}
|
||||
|
||||
// TestProvider handles POST /api/v1/auth/oidc/test.
|
||||
//
|
||||
// Audit 2026-05-10 MED-5 closure. Dry-run validator for an OIDC
|
||||
// provider config: runs OIDC discovery, the alg-downgrade defense,
|
||||
// the RFC 9207 iss-parameter detection, and a JWKS fetch — without
|
||||
// persisting anything. Body: `{issuer_url, client_id, scopes}`
|
||||
// (client_secret accepted but ignored — discovery + JWKS don't
|
||||
// require it). Response: TestDiscoveryResult; HTTP 200 even when
|
||||
// individual checks fail (the response Errors field carries them so
|
||||
// the GUI can render per-check status rows).
|
||||
//
|
||||
// Permission gate: `auth.oidc.create` (the operator is dry-running a
|
||||
// provider they're about to create; the lookup endpoints have their
|
||||
// own .list gate so this can't be used as a roundabout reconnaissance
|
||||
// vector beyond what those already permit).
|
||||
func (h *AuthSessionOIDCHandler) TestProvider(w http.ResponseWriter, r *http.Request) {
|
||||
caller, err := callerFromRequest(r)
|
||||
if err != nil {
|
||||
writeAuthError(w, err)
|
||||
return
|
||||
}
|
||||
var req struct {
|
||||
IssuerURL string `json:"issuer_url"`
|
||||
ClientID string `json:"client_id"`
|
||||
ClientSecret string `json:"client_secret"`
|
||||
Scopes []string `json:"scopes"`
|
||||
}
|
||||
if derr := json.NewDecoder(r.Body).Decode(&req); derr != nil {
|
||||
Error(w, http.StatusBadRequest, "invalid JSON body")
|
||||
return
|
||||
}
|
||||
if strings.TrimSpace(req.IssuerURL) == "" {
|
||||
Error(w, http.StatusBadRequest, "issuer_url is required")
|
||||
return
|
||||
}
|
||||
// Type-assert to the concrete service so we can reach the
|
||||
// TestDiscovery method. The OIDCAuthHandshaker interface is
|
||||
// intentionally narrow; rather than widening it (which would force
|
||||
// every test stub to implement TestDiscovery) we accept the
|
||||
// concrete reference for this single endpoint. Production code
|
||||
// always supplies *oidcsvc.Service.
|
||||
type discoveryTester interface {
|
||||
TestDiscovery(ctx context.Context, issuerURL string) (*oidcsvc.TestDiscoveryResult, error)
|
||||
}
|
||||
tester, ok := h.oidcSvc.(discoveryTester)
|
||||
if !ok {
|
||||
Error(w, http.StatusInternalServerError, "OIDC service does not support discovery test")
|
||||
return
|
||||
}
|
||||
res, terr := tester.TestDiscovery(r.Context(), strings.TrimSpace(req.IssuerURL))
|
||||
if terr != nil {
|
||||
Error(w, http.StatusInternalServerError, "discovery test execution failed")
|
||||
return
|
||||
}
|
||||
h.recordAudit(r.Context(), "auth.oidc_provider_tested", caller.ActorID, caller.ActorType, "",
|
||||
map[string]interface{}{
|
||||
"issuer_url": req.IssuerURL,
|
||||
"discovery_succeeded": res.DiscoverySucceeded,
|
||||
"jwks_reachable": res.JWKSReachable,
|
||||
"iss_param_supported": res.IssParamSupported,
|
||||
"error_count": len(res.Errors),
|
||||
})
|
||||
writeJSON(w, http.StatusOK, res)
|
||||
}
|
||||
|
||||
// RefreshProvider handles POST /api/v1/auth/oidc/providers/{id}/refresh.
|
||||
// Forces re-fetch of the IdP discovery doc + JWKS, re-runs the IdP
|
||||
// downgrade-attack defense.
|
||||
|
||||
@@ -41,6 +41,11 @@ type stubOIDCSvc struct {
|
||||
callbackRes *oidcsvc.CallbackResult
|
||||
callbackErr error
|
||||
refreshErr error
|
||||
// Audit 2026-05-10 MED-5 — stub for the TestDiscovery dry-run.
|
||||
// When testResult is non-nil, the handler-level type assertion
|
||||
// resolves and the response carries this verbatim.
|
||||
testResult *oidcsvc.TestDiscoveryResult
|
||||
testErr error
|
||||
}
|
||||
|
||||
func (s *stubOIDCSvc) HandleAuthRequest(_ context.Context, _, _, _ string) (string, string, string, error) {
|
||||
@@ -51,6 +56,12 @@ func (s *stubOIDCSvc) HandleCallback(_ context.Context, _, _, _, _, _, _ string)
|
||||
}
|
||||
func (s *stubOIDCSvc) RefreshKeys(_ context.Context, _ string) error { return s.refreshErr }
|
||||
|
||||
// TestDiscovery satisfies the inline discoveryTester interface used by
|
||||
// the TestProvider HTTP handler. Audit 2026-05-10 MED-5.
|
||||
func (s *stubOIDCSvc) TestDiscovery(_ context.Context, _ string) (*oidcsvc.TestDiscoveryResult, error) {
|
||||
return s.testResult, s.testErr
|
||||
}
|
||||
|
||||
type stubSession struct {
|
||||
createRes *sessionsvc.CreateResult
|
||||
createErr error
|
||||
@@ -1215,3 +1226,81 @@ func TestClassifyOIDCFailure(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// MED-5 regression tests — TestProvider dry-run endpoint.
|
||||
// =============================================================================
|
||||
|
||||
func TestTestProvider_HappyPath(t *testing.T) {
|
||||
o := &stubOIDCSvc{
|
||||
testResult: &oidcsvc.TestDiscoveryResult{
|
||||
DiscoverySucceeded: true,
|
||||
JWKSReachable: true,
|
||||
SupportedAlgValues: []string{"RS256", "ES256"},
|
||||
IssParamSupported: true,
|
||||
IssuerEcho: "https://idp.example.com",
|
||||
},
|
||||
}
|
||||
h, _, _, _, audit, _ := newPhase5Handler(t, o, &stubSession{}, &stubBCLVerifier{})
|
||||
|
||||
body := strings.NewReader(`{"issuer_url":"https://idp.example.com","client_id":"app","scopes":["openid"]}`)
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/oidc/test", body)
|
||||
req = withActor(req, "u-admin", "User")
|
||||
w := httptest.NewRecorder()
|
||||
h.TestProvider(w, req)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d; want 200; body=%s", w.Code, w.Body.String())
|
||||
}
|
||||
if !strings.Contains(w.Body.String(), `"discovery_succeeded":true`) {
|
||||
t.Errorf("body missing discovery_succeeded:true; got %s", w.Body.String())
|
||||
}
|
||||
if !strings.Contains(w.Body.String(), `"iss_param_supported":true`) {
|
||||
t.Errorf("body missing iss_param_supported:true")
|
||||
}
|
||||
if !contains(audit.events, "auth.oidc_provider_tested") {
|
||||
t.Errorf("expected auth.oidc_provider_tested audit event; got %v", audit.events)
|
||||
}
|
||||
}
|
||||
|
||||
func TestTestProvider_MissingIssuerURL_Returns400(t *testing.T) {
|
||||
h, _, _, _, _, _ := newPhase5Handler(t, &stubOIDCSvc{}, &stubSession{}, &stubBCLVerifier{})
|
||||
|
||||
body := strings.NewReader(`{"client_id":"app"}`)
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/oidc/test", body)
|
||||
req = withActor(req, "u-admin", "User")
|
||||
w := httptest.NewRecorder()
|
||||
h.TestProvider(w, req)
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("status = %d; want 400", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTestProvider_DiscoveryFailureReturns200WithErrors pins the
|
||||
// failure-shape contract: discovery failure is a per-leg failure
|
||||
// surfaced in the response body's `errors` array, NOT a 5xx — the
|
||||
// GUI renders the per-check status row from the response.
|
||||
func TestTestProvider_DiscoveryFailureReturns200WithErrors(t *testing.T) {
|
||||
o := &stubOIDCSvc{
|
||||
testResult: &oidcsvc.TestDiscoveryResult{
|
||||
DiscoverySucceeded: false,
|
||||
JWKSReachable: false,
|
||||
Errors: []string{"discovery fetch failed: connection refused"},
|
||||
},
|
||||
}
|
||||
h, _, _, _, _, _ := newPhase5Handler(t, o, &stubSession{}, &stubBCLVerifier{})
|
||||
|
||||
body := strings.NewReader(`{"issuer_url":"https://broken.example.com"}`)
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/oidc/test", body)
|
||||
req = withActor(req, "u-admin", "User")
|
||||
w := httptest.NewRecorder()
|
||||
h.TestProvider(w, req)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d; want 200 (per-leg failure rides in body); body=%s", w.Code, w.Body.String())
|
||||
}
|
||||
if !strings.Contains(w.Body.String(), `"discovery_succeeded":false`) {
|
||||
t.Errorf("expected discovery_succeeded:false in body; got %s", w.Body.String())
|
||||
}
|
||||
if !strings.Contains(w.Body.String(), "connection refused") {
|
||||
t.Errorf("expected error detail in body; got %s", w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -459,6 +459,10 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) {
|
||||
r.Register("PUT /api/v1/auth/oidc/providers/{id}", rbacGate(reg.Checker, "auth.oidc.edit", reg.AuthSessionOIDC.UpdateProvider))
|
||||
r.Register("DELETE /api/v1/auth/oidc/providers/{id}", rbacGate(reg.Checker, "auth.oidc.delete", reg.AuthSessionOIDC.DeleteProvider))
|
||||
r.Register("POST /api/v1/auth/oidc/providers/{id}/refresh", rbacGate(reg.Checker, "auth.oidc.edit", reg.AuthSessionOIDC.RefreshProvider))
|
||||
// Audit 2026-05-10 MED-5 — dry-run validator for OIDC provider
|
||||
// config. Returns discovery + JWKS + alg-downgrade + iss-param
|
||||
// reachability without persisting.
|
||||
r.Register("POST /api/v1/auth/oidc/test", rbacGate(reg.Checker, "auth.oidc.create", reg.AuthSessionOIDC.TestProvider))
|
||||
|
||||
// Group-mapping CRUD.
|
||||
r.Register("GET /api/v1/auth/oidc/group-mappings", rbacGate(reg.Checker, "auth.oidc.list", reg.AuthSessionOIDC.ListGroupMappings))
|
||||
|
||||
@@ -0,0 +1,125 @@
|
||||
package oidc
|
||||
|
||||
// Audit 2026-05-10 MED-5 closure — dry-run validator for OIDC provider
|
||||
// configuration. Lets operators verify discovery + JWKS reachability +
|
||||
// alg-downgrade defense BEFORE persisting a provider row. Mirrors the
|
||||
// non-persistence-touching subset of getOrLoad.
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"net/http"
|
||||
|
||||
gooidc "github.com/coreos/go-oidc/v3/oidc"
|
||||
)
|
||||
|
||||
// TestDiscoveryResult is the report TestDiscovery returns. The HTTP
|
||||
// layer marshals this verbatim. Each field is independently observable
|
||||
// so the GUI can render a per-check status row.
|
||||
//
|
||||
// `Errors` collects every leg that failed; a partial-success case
|
||||
// (e.g. discovery OK but alg-downgrade tripped) returns
|
||||
// DiscoverySucceeded=true + a non-empty Errors slice.
|
||||
type TestDiscoveryResult struct {
|
||||
DiscoverySucceeded bool `json:"discovery_succeeded"`
|
||||
JWKSReachable bool `json:"jwks_reachable"`
|
||||
SupportedAlgValues []string `json:"supported_alg_values"`
|
||||
IssParamSupported bool `json:"iss_param_supported"`
|
||||
IssuerEcho string `json:"issuer_echo,omitempty"` // the iss value the IdP advertised
|
||||
AuthorizationURL string `json:"authorization_url,omitempty"`
|
||||
TokenURL string `json:"token_url,omitempty"`
|
||||
JWKSURI string `json:"jwks_uri,omitempty"`
|
||||
UserInfoEndpoint string `json:"userinfo_endpoint,omitempty"`
|
||||
Errors []string `json:"errors,omitempty"`
|
||||
}
|
||||
|
||||
// TestDiscovery runs the read-only subset of getOrLoad against a
|
||||
// candidate issuer URL: fetches the discovery doc, runs the
|
||||
// alg-downgrade defense, parses the RFC 9207 iss-parameter advert,
|
||||
// then fetches the JWKS once to confirm reachability.
|
||||
//
|
||||
// The function NEVER persists anything; the caller is the
|
||||
// /api/v1/auth/oidc/test endpoint that the GUI uses for dry-runs.
|
||||
//
|
||||
// Service-layer entry point so the handler stays HTTP-shaped only.
|
||||
func (s *Service) TestDiscovery(ctx context.Context, issuerURL string) (*TestDiscoveryResult, error) {
|
||||
res := &TestDiscoveryResult{}
|
||||
|
||||
// Step 1 — discovery. gooidc.NewProvider fetches
|
||||
// `<issuer>/.well-known/openid-configuration` and runs the iss
|
||||
// match check internally; on failure it returns a fmt-style
|
||||
// wrapped error.
|
||||
provider, err := gooidc.NewProvider(ctx, issuerURL)
|
||||
if err != nil {
|
||||
res.Errors = append(res.Errors, fmt.Sprintf("discovery fetch failed: %v", err))
|
||||
return res, nil // Non-fatal at this layer; the response carries the per-leg failure.
|
||||
}
|
||||
res.DiscoverySucceeded = true
|
||||
res.IssuerEcho = issuerURL
|
||||
endpoint := provider.Endpoint()
|
||||
res.AuthorizationURL = endpoint.AuthURL
|
||||
res.TokenURL = endpoint.TokenURL
|
||||
|
||||
// Step 2 — parse the claims we care about from the discovery doc.
|
||||
var advertised struct {
|
||||
IDTokenSigningAlgValuesSupported []string `json:"id_token_signing_alg_values_supported"`
|
||||
AuthorizationResponseIssParamSupported bool `json:"authorization_response_iss_parameter_supported"`
|
||||
JWKSURI string `json:"jwks_uri"`
|
||||
UserInfoEndpoint string `json:"userinfo_endpoint"`
|
||||
}
|
||||
if cerr := provider.Claims(&advertised); cerr != nil {
|
||||
res.Errors = append(res.Errors, fmt.Sprintf("discovery claims: %v", cerr))
|
||||
return res, nil
|
||||
}
|
||||
res.SupportedAlgValues = advertised.IDTokenSigningAlgValuesSupported
|
||||
res.IssParamSupported = advertised.AuthorizationResponseIssParamSupported
|
||||
res.JWKSURI = advertised.JWKSURI
|
||||
res.UserInfoEndpoint = advertised.UserInfoEndpoint
|
||||
|
||||
// Step 3 — alg-downgrade defense. The IdP MUST NOT advertise HS*
|
||||
// or none in the signing-alg list (operators that bind certctl to
|
||||
// an IdP advertising these are at risk of a forged-token attack).
|
||||
// Same check applied in getOrLoad's production path.
|
||||
for _, a := range advertised.IDTokenSigningAlgValuesSupported {
|
||||
if _, deny := disallowedAlgs[a]; deny {
|
||||
res.Errors = append(res.Errors, fmt.Sprintf("alg-downgrade defense tripped: IdP advertises %s in id_token_signing_alg_values_supported", a))
|
||||
}
|
||||
}
|
||||
|
||||
// Step 4 — JWKS reachability. The go-oidc Verifier defers JWKS
|
||||
// fetch until first token-verify; for the dry-run we explicitly
|
||||
// HEAD/GET the JWKS endpoint to confirm network reachability.
|
||||
if advertised.JWKSURI == "" {
|
||||
res.Errors = append(res.Errors, "discovery doc omits jwks_uri")
|
||||
} else if ok, herr := jwksReachable(ctx, advertised.JWKSURI); !ok {
|
||||
if herr != nil {
|
||||
res.Errors = append(res.Errors, fmt.Sprintf("JWKS fetch failed: %v", herr))
|
||||
} else {
|
||||
res.Errors = append(res.Errors, "JWKS endpoint returned non-200")
|
||||
}
|
||||
} else {
|
||||
res.JWKSReachable = true
|
||||
}
|
||||
|
||||
return res, nil
|
||||
}
|
||||
|
||||
// jwksReachable issues a GET against the JWKS URI and returns ok=true
|
||||
// when the response status is 2xx. Used by TestDiscovery for the
|
||||
// reachability leg of the dry-run.
|
||||
//
|
||||
// Kept distinct from go-oidc's internal JWKS fetcher because we want
|
||||
// to surface the HTTP status to the operator without requiring a
|
||||
// token-verify round-trip.
|
||||
var jwksReachable = func(ctx context.Context, jwksURI string) (bool, error) {
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, jwksURI, nil)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
resp, err := http.DefaultClient.Do(req)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
return resp.StatusCode >= 200 && resp.StatusCode < 300, nil
|
||||
}
|
||||
Reference in New Issue
Block a user