From af3b1b0dfbb34486efb1fc1edfa1b89a1370166b Mon Sep 17 00:00:00 2001 From: tegwick Date: Wed, 14 Oct 2026 23:45:11 +0200 Subject: [PATCH] Add opt-in sign-in diagnosis page and uid/assurance on auth log (NK-WP-0051-T02). A client registration flag, signInDiagnosis (default off), makes the callback render uid, sorted groups, assurance and client_id before the redirect, with one continue link to the unchanged code. assurance is aal2 only when this request verified a one-time code. The auth_success log line now carries uid and assurance beside client_id. Enabled for openbao-admin in the dev config; the live flag is documented in docs/operations.md. Co-Authored-By: Claude Opus 5.5 Assistant: claude-code Assistant-Model: opus Assistant-Process: 1041694@bnt-lap001 Assistant-Session: 7af0bb68-ac4a-4ce3-8639-24ddd84c0b52 --- config/dev-config.yaml | 2 + docs/operations.md | 12 + spec/canonical-model.yaml | 8 + src/cmd/keycape/main.go | 2 + src/internal/config/config.go | 4 + src/internal/domain/model.go | 4 + src/internal/server/oidc/authorize.go | 10 + src/internal/server/oidc/signin_diagnosis.go | 108 +++++++++ .../server/oidc/signin_diagnosis_test.go | 215 ++++++++++++++++++ src/internal/server/telemetry/emitter.go | 6 + src/internal/server/telemetry/events.go | 5 + 11 files changed, 376 insertions(+) create mode 100644 src/internal/server/oidc/signin_diagnosis.go create mode 100644 src/internal/server/oidc/signin_diagnosis_test.go diff --git a/config/dev-config.yaml b/config/dev-config.yaml index e09675b..bf8ea91 100644 --- a/config/dev-config.yaml +++ b/config/dev-config.yaml @@ -31,6 +31,8 @@ clients: allowedScopes: ["openid", "profile", "email", "groups"] grantTypes: ["authorization_code"] clientType: "public" + # NK-WP-0051-T02: show uid/groups/assurance/client_id before returning. + signInDiagnosis: true - clientId: "demo-app" displayName: "Demo Application" redirectUris: diff --git a/docs/operations.md b/docs/operations.md index b920fa2..68e12db 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -263,3 +263,15 @@ https://github.com/authelia/authelia/blob/v4.38.19/web/src/views/LoginPortal/Sig Existing RP sessions and issued JWTs are not revoked. The original `/logout` endpoint retains its local-only contract. Actual shared-session destruction requires browser execution; a redirect-only smoke is not acceptance evidence. + +## Sign-in diagnosis page (NK-WP-0051-T02) + +A client with `signInDiagnosis: true` in its registration shows a page before +the redirect back to it: `uid`, `groups`, `assurance` and `client_id` as defined +in `net-kingdom/docs/sign-in-diagnosis.md`, with one continue link carrying the +unchanged code. Default is off. The repo enables it only for `openbao-admin` in +`config/dev-config.yaml`; the live flag is set by the config custody owner on the +OpenBao browser client in `config.yaml` of the `sso/keycape-config` Secret, and +takes effect on the next issuer restart. A redirect URI that is not http(s) +skips the page. Every `auth_success` log line now carries `client_id`, `uid` and +`assurance`, and no token, code, email or one-time password. diff --git a/spec/canonical-model.yaml b/spec/canonical-model.yaml index a657bcd..fca9b9d 100644 --- a/spec/canonical-model.yaml +++ b/spec/canonical-model.yaml @@ -200,6 +200,14 @@ entities: format: uri nullable: true description: "Factor-enrollment handoff target for unenrolled subjects." + signInDiagnosis: + type: boolean + description: > + Opt-in (default false) pre-redirect page showing uid, groups, + assurance and client_id for this authorization request, with one + continue action (net-kingdom/docs/sign-in-diagnosis.md, + NK-WP-0051-T02). KeyCape runtime presentation only; not exported + to Keycloak. tokenProfile: type: string runtime: false diff --git a/src/cmd/keycape/main.go b/src/cmd/keycape/main.go index 483a613..16de34c 100644 --- a/src/cmd/keycape/main.go +++ b/src/cmd/keycape/main.go @@ -176,6 +176,7 @@ func main() { Handoffs: oidc.NewHandoffStore(), Issuer: issuer, Emitter: emitter, + Users: lldapAdapter, } if path := os.Getenv("KEYCAPE_POLICY_PATH"); path != "" { policies, err := policy.Open(path, clients) @@ -434,6 +435,7 @@ func buildClientRegistryWith(cfgClients []config.ClientConfig, getenv func(strin MFAOptional: c.MFAOptional, RegistrationURL: c.RegistrationURL, EnrollmentURL: c.EnrollmentURL, + SignInDiagnosis: c.SignInDiagnosis, } } return m, nil diff --git a/src/internal/config/config.go b/src/internal/config/config.go index 555f3dd..7895b73 100644 --- a/src/internal/config/config.go +++ b/src/internal/config/config.go @@ -65,6 +65,9 @@ type ClientConfig struct { MFARequired *bool `yaml:"mfaRequired,omitempty"` RegistrationURL string `yaml:"registrationUrl,omitempty"` EnrollmentURL string `yaml:"enrollmentUrl,omitempty"` + // SignInDiagnosis opts the client into the pre-redirect sign-in + // diagnosis page (NK-WP-0051-T02). Default off. + SignInDiagnosis bool `yaml:"signInDiagnosis,omitempty"` } // Load reads and parses the YAML config file at path. @@ -134,6 +137,7 @@ func (c *Config) Registrations() ([]domain.Client, error) { MFAOptional: cc.MFAOptional, RegistrationURL: cc.RegistrationURL, EnrollmentURL: cc.EnrollmentURL, + SignInDiagnosis: cc.SignInDiagnosis, }) } return clients, nil diff --git a/src/internal/domain/model.go b/src/internal/domain/model.go index 5a94da7..1cfda53 100644 --- a/src/internal/domain/model.go +++ b/src/internal/domain/model.go @@ -69,6 +69,10 @@ type Client struct { MFARequired *bool `yaml:"mfaRequired,omitempty" json:"mfaRequired,omitempty"` RegistrationURL string `yaml:"registrationUrl,omitempty" json:"registrationUrl,omitempty"` EnrollmentURL string `yaml:"enrollmentUrl,omitempty" json:"enrollmentUrl,omitempty"` + // SignInDiagnosis opts this client into the pre-redirect sign-in + // diagnosis page (uid, groups, assurance, client_id) defined by + // net-kingdom/docs/sign-in-diagnosis.md (NK-WP-0051-T02). Default off. + SignInDiagnosis bool `yaml:"signInDiagnosis,omitempty" json:"signInDiagnosis,omitempty"` } // Membership links a user to a group. diff --git a/src/internal/server/oidc/authorize.go b/src/internal/server/oidc/authorize.go index 7f976c5..60447bc 100644 --- a/src/internal/server/oidc/authorize.go +++ b/src/internal/server/oidc/authorize.go @@ -79,6 +79,9 @@ type AuthorizeHandler struct { Handoffs *HandoffStore Issuer string Emitter telemetry.Emitter + // Users resolves directory groups for the sign-in diagnosis page. Only + // consulted for clients with SignInDiagnosis set; may be nil. + Users domain.UserRepository pending *pendingStateStore once sync.Once @@ -522,6 +525,8 @@ func (h *AuthorizeHandler) completeAuthorization(w http.ResponseWriter, r *http. Endpoint: "/authorize/callback", Result: "success", Scopes: ps.Scopes, + UID: username, + Assurance: diagnosisAssurance(mfaVerified, freshMFA), }) // Redirect to client with code and state. @@ -534,6 +539,11 @@ func (h *AuthorizeHandler) completeAuthorization(w http.ResponseWriter, r *http. q.Set("code", authCode) q.Set("state", ps.State) redirectTo.RawQuery = q.Encode() + if client, ok := h.ClientConfig[ps.ClientID]; ok && client != nil && client.SignInDiagnosis { + if h.renderSignInDiagnosis(w, r, ps.ClientID, username, diagnosisAssurance(mfaVerified, freshMFA), redirectTo) { + return + } + } http.Redirect(w, r, redirectTo.String(), http.StatusFound) } diff --git a/src/internal/server/oidc/signin_diagnosis.go b/src/internal/server/oidc/signin_diagnosis.go new file mode 100644 index 0000000..ba82ee4 --- /dev/null +++ b/src/internal/server/oidc/signin_diagnosis.go @@ -0,0 +1,108 @@ +package oidc + +import ( + "html/template" + "net/http" + "net/url" + "sort" + "strings" +) + +// Sign-in diagnosis (NK-WP-0051-T02). +// +// The field names and the never-show list are defined only in +// net-kingdom/docs/sign-in-diagnosis.md. The page shows uid, groups, +// assurance and client_id for THIS authorization request. It never shows +// tokens, the authorization code (beyond the continue link target), email, +// one-time passwords, or raw memberOf DNs. + +// diagnosisAssurance is aal2 only when this request verified a one-time +// code. A reused session that was aal2 earlier is aal1 here: the contract's +// assurance is per request, so the session-satisfied MFAVerified from the +// assurance decision does not count unless the code was verified now. +func diagnosisAssurance(mfaVerified, freshMFA bool) string { + if mfaVerified && freshMFA { + return "aal2" + } + return "aal1" +} + +// renderSignInDiagnosis writes the pre-redirect page and reports true, or +// reports false (caller redirects as usual) when the redirect target is not +// an http(s) URL the template can safely place in a link. +func (h *AuthorizeHandler) renderSignInDiagnosis(w http.ResponseWriter, r *http.Request, clientID, uid, assurance string, continueTo *url.URL) bool { + if continueTo.Scheme != "http" && continueTo.Scheme != "https" { + return false + } + groups, groupsKnown := h.diagnosisGroups(r, uid) + + w.Header().Set("Cache-Control", "no-store") + w.Header().Set("Pragma", "no-cache") + // The continue URL carries the authorization code; keep it out of any + // Referer sent by this page. + w.Header().Set("Referrer-Policy", "no-referrer") + w.Header().Set("Content-Security-Policy", "default-src 'none'; style-src 'unsafe-inline'; frame-ancestors 'none'") + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.WriteHeader(http.StatusOK) + _ = signInDiagnosisTemplate.Execute(w, struct { + UID string + Groups []string + GroupsKnown bool + Assurance string + ClientID string + ContinueURL string + SignOutURL string + }{ + UID: uid, + Groups: groups, + GroupsKnown: groupsKnown, + Assurance: assurance, + ClientID: clientID, + ContinueURL: continueTo.String(), + SignOutURL: strings.TrimRight(h.Issuer, "/") + "/account/logout", + }) + return true +} + +// diagnosisGroups returns the uid's directory group names, sorted. The LLDAP +// adapter already reduces memberOf DNs to group names. +func (h *AuthorizeHandler) diagnosisGroups(r *http.Request, uid string) ([]string, bool) { + if h.Users == nil { + return nil, false + } + user, err := h.Users.LookupUser(r.Context(), uid) + if err != nil || user == nil { + return nil, false + } + groups := append([]string(nil), user.Groups...) + sort.Strings(groups) + return groups, true +} + +var signInDiagnosisTemplate = template.Must(template.New("signin-diagnosis").Parse(` + + + + + +Signed in + + + +

You are signed in

+
+
uid
{{.UID}}
+
groups
{{if .GroupsKnown}}{{if .Groups}}{{range $i, $g := .Groups}}{{if $i}}, {{end}}{{$g}}{{end}}{{else}}(none){{end}}{{else}}(unavailable){{end}}
+
assurance
{{.Assurance}}
+
client_id
{{.ClientID}}
+
+Continue +

Not this account? Sign out at {{.SignOutURL}} (or use a private browser window), then start the login again from the application.

+ + +`)) diff --git a/src/internal/server/oidc/signin_diagnosis_test.go b/src/internal/server/oidc/signin_diagnosis_test.go new file mode 100644 index 0000000..72a1b35 --- /dev/null +++ b/src/internal/server/oidc/signin_diagnosis_test.go @@ -0,0 +1,215 @@ +package oidc_test + +import ( + "bytes" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + "time" + + "github.com/rs/zerolog" + + "keycape/internal/domain" + "keycape/internal/server/oidc" + "keycape/internal/server/telemetry" +) + +// NK-WP-0051-T02: opt-in sign-in diagnosis page and auth log fields. +// Contract: net-kingdom/docs/sign-in-diagnosis.md. + +const diagEmail = "alice@example.org" + +func diagHandler(t *testing.T, flag bool, mfa *mockMFAProvider, emitter telemetry.Emitter) *oidc.AuthorizeHandler { + t.Helper() + clients := testClient() + clients["test-client"].SignInDiagnosis = flag + return &oidc.AuthorizeHandler{ + ClientConfig: clients, + Auth: &mockAuthProvider{callbackResult: &domain.AuthResult{Username: "alice"}}, + MFA: mfa, + Sessions: oidc.NewSessionStore(), + Logins: oidc.NewLoginSessionStore(), + Emitter: emitter, + Users: &mockUserRepo{users: map[string]*domain.User{ + "alice": {ID: "alice", Username: "alice", Email: diagEmail, + Groups: []string{"zeta-team", "Alpha ", "net-kingdom-users"}}, + }}, + } +} + +func seedDiagPending(h *oidc.AuthorizeHandler, authenticatedUser string) { + h.PendingStates().Store("random-state", &oidc.PendingState{ + ClientID: "test-client", + RedirectURI: "https://app.example.com/callback?from=x", + PKCEChallenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + PKCEChallengeMethod: "S256", + State: "random-state", + Scopes: []string{"openid", "profile", "email"}, + ExpiresAt: time.Now().Add(5 * time.Minute), + AuthenticatedUser: authenticatedUser, + }) +} + +func callbackReq() *http.Request { + return httptest.NewRequest(http.MethodGet, "/authorize/callback?code=authelia-code&state=random-state", nil) +} + +func mfaSubmitReq(otp string) *http.Request { + form := url.Values{"state": {"random-state"}, "mfa_token": {otp}} + req := httptest.NewRequest(http.MethodPost, "/authorize/callback", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + return req +} + +func TestSignInDiagnosis_FlagOff_RedirectUnchanged(t *testing.T) { + h := diagHandler(t, false, &mockMFAProvider{}, &captureEmitter{}) + seedDiagPending(h, "") + w := httptest.NewRecorder() + h.ServeHTTPCallback(w, callbackReq()) + + if w.Code != http.StatusFound { + t.Fatalf("flag off: want 302, got %d", w.Code) + } + loc, _ := url.Parse(w.Header().Get("Location")) + if loc.Host != "app.example.com" || loc.Query().Get("code") == "" || loc.Query().Get("state") != "random-state" { + t.Fatalf("flag off: unexpected redirect %q", loc) + } + if w.Body.Len() > 0 && strings.Contains(w.Body.String(), "assurance") { + t.Fatal("flag off: diagnosis page must not render") + } +} + +func diagPage(t *testing.T, w *httptest.ResponseRecorder) (string, string) { + t.Helper() + if w.Code != http.StatusOK { + t.Fatalf("flag on: want 200 page, got %d (Location %q)", w.Code, w.Header().Get("Location")) + } + body := w.Body.String() + i := strings.Index(body, `href="`) + if i < 0 { + t.Fatal("continue link missing") + } + rest := body[i+len(`href="`):] + href := rest[:strings.Index(rest, `"`)] + return body, strings.ReplaceAll(href, "&", "&") +} + +func TestSignInDiagnosis_FlagOn_ShowsFieldsAndContinue(t *testing.T) { + h := diagHandler(t, true, &mockMFAProvider{}, &captureEmitter{}) + seedDiagPending(h, "") + w := httptest.NewRecorder() + h.ServeHTTPCallback(w, callbackReq()) + body, href := diagPage(t, w) + + for _, want := range []string{ + `
alice
`, + `
Alpha <admins>, net-kingdom-users, zeta-team
`, + `
aal1
`, + `
test-client
`, + "Not this account?", + } { + if !strings.Contains(body, want) { + t.Errorf("page missing %q\n%s", want, body) + } + } + if strings.Contains(body, diagEmail) { + t.Error("page must not contain the email") + } + if strings.Count(body, "href=") != 1 || strings.Contains(body, "aal2`) { + t.Errorf("want aal2 after verifying OTP in this request\n%s", body) + } + if strings.Contains(body, "654321") { + t.Error("page must not contain the one-time password") + } + if got := emitter.last().Assurance; got != "aal2" { + t.Errorf("log assurance: want aal2, got %q", got) + } +} + +func TestSignInDiagnosis_ReusedAAL2SessionIsAAL1ForThisRequest(t *testing.T) { + required := true + emitter := &captureEmitter{} + h := diagHandler(t, true, &mockMFAProvider{}, emitter) + h.ClientConfig["test-client"].MFARequired = &required + prior := h.Logins.Create("alice", domain.AssuranceAAL2) + seedDiagPending(h, "") + req := callbackReq() + req.AddCookie(&http.Cookie{Name: "kc_login", Value: prior.ID}) + w := httptest.NewRecorder() + h.ServeHTTPCallback(w, req) + body, _ := diagPage(t, w) + if !strings.Contains(body, `
aal1
`) { + t.Errorf("reused aal2 session without OTP in this request must show aal1\n%s", body) + } + if got := emitter.last().Assurance; got != "aal1" { + t.Errorf("log assurance: want aal1, got %q", got) + } +} + +func TestSignInDiagnosis_AuthLogLineHasFieldsAndNoSecrets(t *testing.T) { + var buf bytes.Buffer + emitter := telemetry.NewLogEmitter(zerolog.New(&buf)) + mfa := &mockMFAProvider{required: true} + h := diagHandler(t, false, mfa, emitter) + seedDiagPending(h, "alice") + w := httptest.NewRecorder() + h.ServeHTTPCallback(w, mfaSubmitReq("918273")) + if w.Code != http.StatusFound { + t.Fatalf("want 302, got %d", w.Code) + } + loc, _ := url.Parse(w.Header().Get("Location")) + code := loc.Query().Get("code") + + var line string + for _, l := range strings.Split(buf.String(), "\n") { + if strings.Contains(l, `"event_type":"auth_success"`) { + line = l + } + } + if line == "" { + t.Fatalf("no auth_success line in %q", buf.String()) + } + for _, want := range []string{`"client_id":"test-client"`, `"uid":"alice"`, `"assurance":"aal2"`} { + if !strings.Contains(line, want) { + t.Errorf("log line missing %s: %s", want, line) + } + } + for _, secret := range []string{code, "918273", diagEmail, "authelia-code", "E9Melhoa2Ow"} { + if strings.Contains(buf.String(), secret) { + t.Errorf("log output contains secret material %q: %s", secret, buf.String()) + } + } +} diff --git a/src/internal/server/telemetry/emitter.go b/src/internal/server/telemetry/emitter.go index 54edcb5..ff11c05 100644 --- a/src/internal/server/telemetry/emitter.go +++ b/src/internal/server/telemetry/emitter.go @@ -74,6 +74,12 @@ func (l *LogEmitter) Emit(_ context.Context, ev Event) { if len(ev.Scopes) > 0 { e = e.Strs("scopes", ev.Scopes) } + if ev.UID != "" { + e = e.Str("uid", ev.UID) + } + if ev.Assurance != "" { + e = e.Str("assurance", ev.Assurance) + } if ev.GrantType != "" { e = e.Str("grant_type", ev.GrantType) } diff --git a/src/internal/server/telemetry/events.go b/src/internal/server/telemetry/events.go index c7032c1..793b79b 100644 --- a/src/internal/server/telemetry/events.go +++ b/src/internal/server/telemetry/events.go @@ -32,4 +32,9 @@ type Event struct { Environment string `json:"environment"` TraceID string `json:"trace_id"` EventType EventType `json:"event_type"` + // UID and Assurance are set on auth_success only (NK-WP-0051-T02): + // the uid KeyCape resolved and aal1/aal2 for this authorization request. + // Never put tokens, codes, email, or one-time passwords on an Event. + UID string `json:"uid,omitempty"` + Assurance string `json:"assurance,omitempty"` }