Support opt-in MFA per browser client with authoritative enrollment checks
All checks were successful
Build and Publish Container Image / build-and-push (push) Successful in 40s
All checks were successful
Build and Publish Container Image / build-and-push (push) Successful in 40s
Assistant: codex Assistant-Model: gpt-6-astra Assistant-Session: 01a092fe-13b1-7f12-ac74-7d258af4d79c
This commit is contained in:
parent
e1e292919a
commit
ac8ed65203
14 changed files with 298 additions and 12 deletions
|
|
@ -410,6 +410,7 @@ func buildClientRegistry(cfgClients []config.ClientConfig) (map[string]*domain.C
|
|||
Roles: c.Roles,
|
||||
TokenLifetime: clientTokenLifetime,
|
||||
MFARequired: c.MFARequired,
|
||||
MFAOptional: c.MFAOptional,
|
||||
RegistrationURL: c.RegistrationURL,
|
||||
EnrollmentURL: c.EnrollmentURL,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -57,12 +57,14 @@ func (a *PrivacyIDEAAdapter) hasActiveToken(ctx context.Context, userID string)
|
|||
q := url.Values{}
|
||||
q.Set("user", userID)
|
||||
q.Set("realm", a.cfg.realm())
|
||||
// Filter before pagination so an active factor cannot be hidden on page two.
|
||||
q.Set("active", "True")
|
||||
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint+"?"+q.Encode(), nil)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("privacyidea: build token list request: %w", err)
|
||||
}
|
||||
req.Header.Set("Authorization", "Bearer "+a.cfg.AdminToken)
|
||||
req.Header.Set("Authorization", a.cfg.AdminToken)
|
||||
|
||||
resp, err := a.client.Do(req)
|
||||
if err != nil {
|
||||
|
|
@ -84,11 +86,20 @@ func (a *PrivacyIDEAAdapter) hasActiveToken(ctx context.Context, userID string)
|
|||
return false, fmt.Errorf("privacyidea: decode token list response: %w", err)
|
||||
}
|
||||
|
||||
if !parsed.Result.Status || parsed.Result.Value.Tokens == nil || parsed.Result.Value.Count == nil || *parsed.Result.Value.Count < len(parsed.Result.Value.Tokens) {
|
||||
return false, fmt.Errorf("privacyidea: incomplete or unsuccessful token list response")
|
||||
}
|
||||
for _, tok := range parsed.Result.Value.Tokens {
|
||||
if tok.Active {
|
||||
if tok.Active == nil {
|
||||
return false, fmt.Errorf("privacyidea: token missing active state")
|
||||
}
|
||||
if *tok.Active {
|
||||
return true, nil
|
||||
}
|
||||
}
|
||||
if *parsed.Result.Value.Count > len(parsed.Result.Value.Tokens) {
|
||||
return false, fmt.Errorf("privacyidea: incomplete token list page")
|
||||
}
|
||||
return false, nil
|
||||
}
|
||||
|
||||
|
|
@ -110,7 +121,7 @@ func (a *PrivacyIDEAAdapter) ValidateMFAToken(ctx context.Context, userID, token
|
|||
return fmt.Errorf("privacyidea: build validate request: %w", err)
|
||||
}
|
||||
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
req.Header.Set("Authorization", "Bearer "+a.cfg.AdminToken)
|
||||
req.Header.Set("Authorization", a.cfg.AdminToken)
|
||||
|
||||
resp, err := a.client.Do(req)
|
||||
if err != nil {
|
||||
|
|
@ -148,6 +159,7 @@ type tokenListResponse struct {
|
|||
Status bool `json:"status"`
|
||||
Value struct {
|
||||
Tokens []tokenEntry `json:"tokens"`
|
||||
Count *int `json:"count"`
|
||||
} `json:"value"`
|
||||
} `json:"result"`
|
||||
}
|
||||
|
|
@ -155,7 +167,7 @@ type tokenListResponse struct {
|
|||
// tokenEntry represents a single token entry in the token list response.
|
||||
type tokenEntry struct {
|
||||
Serial string `json:"serial"`
|
||||
Active bool `json:"active"`
|
||||
Active *bool `json:"active"`
|
||||
}
|
||||
|
||||
// validateResponse models the privacyIDEA POST /validate/check response envelope.
|
||||
|
|
|
|||
|
|
@ -64,7 +64,7 @@ func tokenListResponse(tokens []map[string]interface{}) string {
|
|||
tokenJSON += fmt.Sprintf(`{"serial":"TOK%d","active":%v}`, i, active)
|
||||
}
|
||||
tokenJSON += "]"
|
||||
return fmt.Sprintf(`{"result":{"status":true,"value":{"tokens":%s}}}`, tokenJSON)
|
||||
return fmt.Sprintf(`{"result":{"status":true,"value":{"tokens":%s,"count":%d}}}`, tokenJSON, len(tokens))
|
||||
}
|
||||
|
||||
// validateResponse builds a privacyIDEA /validate/check JSON response.
|
||||
|
|
@ -243,8 +243,8 @@ func TestCheckMFARequired_SendsAdminToken(t *testing.T) {
|
|||
client := &mockHTTPClient{
|
||||
doFn: func(req *http.Request) (*http.Response, error) {
|
||||
auth := req.Header.Get("Authorization")
|
||||
if !strings.HasPrefix(auth, "Bearer ") {
|
||||
t.Errorf("expected Bearer token in Authorization, got %q", auth)
|
||||
if auth != "service-jwt" {
|
||||
t.Errorf("expected raw service JWT in Authorization, got %q", auth)
|
||||
}
|
||||
if !strings.Contains(auth, "service-jwt") {
|
||||
t.Errorf("expected admin token in Authorization header, got %q", auth)
|
||||
|
|
|
|||
43
src/internal/adapters/privacyidea/optional_mfa_test.go
Normal file
43
src/internal/adapters/privacyidea/optional_mfa_test.go
Normal file
|
|
@ -0,0 +1,43 @@
|
|||
package privacyidea_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"keycape/internal/adapters/privacyidea"
|
||||
"net/http"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestEnrollmentLookupRejectsIndeterminateResponses(t *testing.T) {
|
||||
for _, body := range []string{
|
||||
`{}`, `{"result":{"status":true,"value":{"tokens":[{}],"count":1}}}`, `{"result":{"status":false,"value":{"tokens":[],"count":0}}}`,
|
||||
`{"result":{"status":true,"value":{"count":0}}}`,
|
||||
`{"result":{"status":true,"value":{"tokens":[]}}}`,
|
||||
`{"result":{"status":true,"value":{"tokens":null,"count":0}}}`,
|
||||
`{"result":{"status":true,"value":{"tokens":[],"count":1}}}`,
|
||||
`{"result":{"status":true,"value":{"tokens":[],"count":-1}}}`,
|
||||
} {
|
||||
t.Run(body, func(t *testing.T) {
|
||||
adapter := privacyidea.New(testConfig(), &mockHTTPClient{doFn: func(*http.Request) (*http.Response, error) { return jsonResponse(body), nil }})
|
||||
required, err := adapter.HasEnrolledFactor(context.Background(), "alice")
|
||||
if err == nil || required {
|
||||
t.Fatalf("indeterminate lookup accepted: required=%v error=%v", required, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
func TestEnrollmentLookupFiltersBeforePagination(t *testing.T) {
|
||||
adapter := privacyidea.New(testConfig(), &mockHTTPClient{doFn: func(req *http.Request) (*http.Response, error) {
|
||||
q := req.URL.Query()
|
||||
if q.Get("user") != "alice" || q.Get("realm") != "netkingdom" || q.Get("active") != "True" {
|
||||
t.Fatalf("incorrect filter: %v", q)
|
||||
}
|
||||
if req.Header.Get("Authorization") != "service-jwt" {
|
||||
t.Fatal("provider requires raw JWT")
|
||||
}
|
||||
return jsonResponse(`{"result":{"status":true,"value":{"tokens":[{"active":true}],"count":20}}}`), nil
|
||||
}})
|
||||
required, err := adapter.HasEnrolledFactor(context.Background(), "alice")
|
||||
if err != nil || !required {
|
||||
t.Fatalf("active factor missed: %v %v", required, err)
|
||||
}
|
||||
}
|
||||
|
|
@ -61,6 +61,7 @@ type ClientConfig struct {
|
|||
Tenant string `yaml:"tenant,omitempty"`
|
||||
Roles []string `yaml:"roles,omitempty"`
|
||||
TokenLifetime string `yaml:"tokenLifetime,omitempty"`
|
||||
MFAOptional bool `yaml:"mfaOptional,omitempty"`
|
||||
MFARequired *bool `yaml:"mfaRequired,omitempty"`
|
||||
RegistrationURL string `yaml:"registrationUrl,omitempty"`
|
||||
EnrollmentURL string `yaml:"enrollmentUrl,omitempty"`
|
||||
|
|
@ -130,6 +131,7 @@ func (c *Config) Registrations() ([]domain.Client, error) {
|
|||
Roles: cc.Roles,
|
||||
TokenLifetime: lifetime,
|
||||
MFARequired: cc.MFARequired,
|
||||
MFAOptional: cc.MFAOptional,
|
||||
RegistrationURL: cc.RegistrationURL,
|
||||
EnrollmentURL: cc.EnrollmentURL,
|
||||
})
|
||||
|
|
|
|||
40
src/internal/config/optional_mfa_test.go
Normal file
40
src/internal/config/optional_mfa_test.go
Normal file
|
|
@ -0,0 +1,40 @@
|
|||
package config_test
|
||||
|
||||
import (
|
||||
"keycape/internal/config"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestOptionalMFALoadAndRegistration(t *testing.T) {
|
||||
cfg, err := config.Load(writeTempFile(t, "clients:\n - clientId: demo\n mfaOptional: true\n"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
clients, err := cfg.Registrations()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !clients[0].MFAOptional || clients[0].MFARequired != nil {
|
||||
t.Fatal("optional MFA policy lost in registration")
|
||||
}
|
||||
}
|
||||
func TestOptionalMFAValidation(t *testing.T) {
|
||||
for _, explicit := range []bool{false, true} {
|
||||
cfg := validConfig(writeTempFile(t, "placeholder"))
|
||||
cfg.Clients[0].MFAOptional = true
|
||||
if err := config.ValidateConfig(cfg); len(err) != 0 {
|
||||
t.Fatal(err)
|
||||
}
|
||||
cfg.Clients[0].MFARequired = &explicit
|
||||
if err := config.ValidateConfig(cfg); len(err) == 0 || !strings.Contains(strings.Join(err, ";"), "mfaOptional cannot be combined") {
|
||||
t.Fatalf("ambiguous policy accepted: %v", err)
|
||||
}
|
||||
}
|
||||
cfg := validConfig(writeTempFile(t, "placeholder"))
|
||||
cfg.Clients[0].MFAOptional = true
|
||||
cfg.Clients[0].GrantTypes = []string{"client_credentials"}
|
||||
if err := config.ValidateConfig(cfg); len(err) == 0 || !strings.Contains(strings.Join(err, ";"), "mfaOptional is only supported") {
|
||||
t.Fatalf("service policy accepted: %v", err)
|
||||
}
|
||||
}
|
||||
|
|
@ -78,6 +78,12 @@ func ValidateConfig(cfg *Config) []string {
|
|||
if strings.TrimSpace(c.Audience) != c.Audience || strings.ContainsAny(c.Audience, " \t\r\n") {
|
||||
errs = append(errs, prefix+": audience must be a single non-whitespace identifier")
|
||||
}
|
||||
if c.MFAOptional && c.MFARequired != nil {
|
||||
errs = append(errs, prefix+": mfaOptional cannot be combined with mfaRequired")
|
||||
}
|
||||
if c.MFAOptional && contains(c.GrantTypes, "client_credentials") {
|
||||
errs = append(errs, prefix+": mfaOptional is only supported for browser clients")
|
||||
}
|
||||
hasAuthorizationCode := contains(c.GrantTypes, "authorization_code")
|
||||
hasClientCredentials := contains(c.GrantTypes, "client_credentials")
|
||||
if (hasAuthorizationCode || !hasClientCredentials) && len(c.RedirectURIs) == 0 {
|
||||
|
|
|
|||
|
|
@ -63,10 +63,12 @@ type Client struct {
|
|||
Roles []string `yaml:"roles,omitempty" json:"roles,omitempty"`
|
||||
// TokenLifetime overrides the server default for this confidential client.
|
||||
// It is internal runtime policy, not identity data serialized into tokens.
|
||||
TokenLifetime time.Duration `yaml:"-" json:"-"`
|
||||
MFARequired *bool `yaml:"mfaRequired,omitempty" json:"mfaRequired,omitempty"`
|
||||
RegistrationURL string `yaml:"registrationUrl,omitempty" json:"registrationUrl,omitempty"`
|
||||
EnrollmentURL string `yaml:"enrollmentUrl,omitempty" json:"enrollmentUrl,omitempty"`
|
||||
TokenLifetime time.Duration `yaml:"-" json:"-"`
|
||||
// MFAOptional requires MFA for enrolled users, independently of RequireForAll.
|
||||
MFAOptional bool `yaml:"mfaOptional,omitempty" json:"mfaOptional,omitempty"`
|
||||
MFARequired *bool `yaml:"mfaRequired,omitempty" json:"mfaRequired,omitempty"`
|
||||
RegistrationURL string `yaml:"registrationUrl,omitempty" json:"registrationUrl,omitempty"`
|
||||
EnrollmentURL string `yaml:"enrollmentUrl,omitempty" json:"enrollmentUrl,omitempty"`
|
||||
}
|
||||
|
||||
// Membership links a user to a group.
|
||||
|
|
|
|||
|
|
@ -372,6 +372,10 @@ func mapClient(c domain.Client) (KeycloakClient, []string) {
|
|||
if c.EnrollmentURL != "" {
|
||||
kc.Attributes["keycape.enrollmentUrl"] = c.EnrollmentURL
|
||||
}
|
||||
if c.MFAOptional {
|
||||
kc.Attributes["keycape.mfaOptional"] = "true"
|
||||
unpreserved = append(unpreserved, fmt.Sprintf("client %q: mfaOptional requires a manually verified conditional MFA authentication flow; enrollment policy is not enforced by import", c.ClientID))
|
||||
}
|
||||
if c.MFARequired != nil && *c.MFARequired {
|
||||
// Keycloak expresses this as an authentication flow binding, which a
|
||||
// realm import cannot synthesise from a boolean.
|
||||
|
|
|
|||
|
|
@ -375,7 +375,11 @@ func (h *AuthorizeHandler) decideAssurance(ctx context.Context, ps *PendingState
|
|||
providerRequired := false
|
||||
if !domain.ACRRequiresAAL2(ps.ACRValues) && (client == nil || client.MFARequired == nil) {
|
||||
var err error
|
||||
providerRequired, err = h.MFA.CheckMFARequired(ctx, username)
|
||||
if client != nil && client.MFAOptional {
|
||||
providerRequired, err = h.MFA.HasEnrolledFactor(ctx, username)
|
||||
} else {
|
||||
providerRequired, err = h.MFA.CheckMFARequired(ctx, username)
|
||||
}
|
||||
if err != nil {
|
||||
return domain.AssuranceDecision{}, err
|
||||
}
|
||||
|
|
|
|||
50
src/internal/server/oidc/optional_mfa_test.go
Normal file
50
src/internal/server/oidc/optional_mfa_test.go
Normal file
|
|
@ -0,0 +1,50 @@
|
|||
package oidc_test
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"keycape/internal/domain"
|
||||
"keycape/internal/server/oidc"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestOptionalMFAFollowsEnrollment(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
enrolled bool
|
||||
lookupErr error
|
||||
acr []string
|
||||
want int
|
||||
challenge bool
|
||||
}{
|
||||
{name: "unenrolled despite global requirement", want: http.StatusFound},
|
||||
{name: "enrolled", enrolled: true, want: http.StatusOK, challenge: true},
|
||||
{name: "lookup unavailable", lookupErr: errors.New("provider unavailable"), want: http.StatusInternalServerError},
|
||||
{name: "explicit step up", acr: []string{"aal2"}, lookupErr: errors.New("must not query enrollment"), want: http.StatusOK, challenge: true},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
sessions := oidc.NewSessionStore()
|
||||
h := &oidc.AuthorizeHandler{
|
||||
ClientConfig: map[string]*domain.Client{"test-client": {ClientID: "test-client", MFAOptional: true}},
|
||||
Auth: &mockAuthProvider{callbackResult: &domain.AuthResult{Username: "alice"}},
|
||||
MFA: &mockMFAProvider{required: true, requiredErr: errors.New("global policy must not be queried"), enrolled: tc.enrolled, enrolledErr: tc.lookupErr},
|
||||
Sessions: sessions, Emitter: &captureEmitter{},
|
||||
}
|
||||
h.PendingStates().Store("optional", &oidc.PendingState{ClientID: "test-client", RedirectURI: "https://app.example/callback", State: "optional", ACRValues: tc.acr, ExpiresAt: time.Now().Add(time.Minute)})
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTPCallback(rec, httptest.NewRequest(http.MethodGet, "/authorize/callback?code=x&state=optional", nil))
|
||||
if rec.Code != tc.want {
|
||||
t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if strings.Contains(rec.Body.String(), "KeyCape MFA") != tc.challenge {
|
||||
t.Fatal("unexpected MFA challenge state")
|
||||
}
|
||||
if tc.want != http.StatusFound && strings.Contains(rec.Header().Get("Location"), "code=") {
|
||||
t.Fatal("issued authorization code before MFA")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue