diff --git a/README.md b/README.md index e39e5de..41df877 100644 --- a/README.md +++ b/README.md @@ -17,3 +17,21 @@ Protected routes create JaWS sessions for unauthenticated visitors. Set `Jaws.MaxSessions` and `Jaws.MaxSessionsPerIP` to bound session counts. When a session cannot be created, login returns 503 with `Retry-After: 60`. Post-login return targets longer than 8192 bytes fall back to `/`. + +Admin allowlists match parsed addresses with ASCII case ignored; non-ASCII +characters are matched exactly. Session email values use the same case rules. +Email verification is not required by default. This supports providers such as +Microsoft Entra ID that omit `email_verified`, but relies on the provider's policy +for who may claim an address. Microsoft [advises against using its email claim for +authorization](https://learn.microsoft.com/en-us/entra/identity-platform/id-token-claims-reference). +To require verified email for a non-empty admin allowlist, set +`Server.RequireVerifiedAdminEmail = true` before serving requests. This affects +`WrapAdmin`, `HandlerAdmin`, and `JawsAuth.IsAdmin`; ordinary login stays available. +Providers that omit the verification claim cannot grant admin access in this mode. +`Server.IsAdmin(email)` only checks address membership. `mail` and `public_email` +are always considered unverified; they can match the allowlist only when strict +mode is disabled. +An empty admin list allows every authenticated user through the HTTP admin gates. +UserInfo fallback requires a matching non-empty `sub`; its verification flag is +used only when that response supplies the email or confirms the exact same email +claim, including case and whitespace. diff --git a/adminpolicy_test.go b/adminpolicy_test.go new file mode 100644 index 0000000..b4e4cad --- /dev/null +++ b/adminpolicy_test.go @@ -0,0 +1,147 @@ +package jawsauth + +import ( + "net/http" + "net/http/httptest" + "sync" + "testing" + "time" + + "github.com/linkdata/jaws" +) + +func TestAdminEmailPolicy(t *testing.T) { + for _, tc := range []struct { + name string + claims map[string]any + listed, verified bool + }{ + {"verified", map[string]any{"email": "Admin@Example.COM", "email_verified": true}, true, true}, + {"unverified", map[string]any{"email": "admin@example.com", "email_verified": false}, true, false}, + {"entra", map[string]any{"email": "admin@example.com"}, true, false}, + {"mail", map[string]any{"mail": "admin@example.com", "email_verified": true}, true, false}, + {"publicEmail", map[string]any{"email": "invalid", "public_email": "admin@example.com", "email_verified": true}, true, false}, + {"unicode", map[string]any{"email": "Kate@example.com", "email_verified": true}, false, true}, + {"unicodeListed", map[string]any{"email": "ÅSA@example.com", "email_verified": true}, true, true}, + {"unicodeDifferentCase", map[string]any{"email": "åsa@example.com", "email_verified": true}, false, true}, + {"quotedLocalPart", map[string]any{"email": `"\"quoted\""@example.com`, "email_verified": true}, true, true}, + } { + t.Run(tc.name, func(t *testing.T) { + jw, err := jaws.New() + if err != nil { + t.Fatal(err) + } + defer jw.Close() + srv := newTimerTestServer(t, jw, "https://issuer.example", &testAuthTimerFactory{}) + srv.SetAdmins([]string{"admin@example.com", "kate@example.com", "Åsa@example.com", `"\"quoted\""@example.com`}) + req := httptest.NewRequest(http.MethodGet, "/", nil) + sess := jw.NewSession(httptest.NewRecorder(), req) + if err = srv.storeSessionAuthClaims(t.Context(), sess, tc.claims, nil, time.Now().Add(time.Hour), nil); err != nil { + t.Fatal(err) + } + auth := &JawsAuth{server: srv, sess: sess} + if auth.EmailVerified() != tc.verified { + t.Fatal("incorrect verification provenance") + } + for _, strict := range []bool{false, true} { + srv.RequireVerifiedAdminEmail = strict + want := tc.listed && (!strict || tc.verified) + if auth.IsAdmin() != want { + t.Fatalf("strict=%v: template admin=%v", strict, auth.IsAdmin()) + } + rec := httptest.NewRecorder() + srv.WrapAdmin(testStatusHandler{http.StatusOK}).ServeHTTP(rec, req) + if (rec.Code == http.StatusOK) != want { + t.Fatalf("strict=%v: status=%d", strict, rec.Code) + } + rec = httptest.NewRecorder() + srv.Wrap(testStatusHandler{http.StatusOK}).ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatal("ordinary authentication was restricted") + } + } + srv.SetAdmins(nil) + if !auth.IsAdmin() { + t.Fatal("empty list should allow authenticated users") + } + }) + } +} + +func TestUserInfoClaimProvenance(t *testing.T) { + for _, tc := range []struct { + name string + id, info map[string]any + email string + verified bool + }{ + {"matching", map[string]any{"sub": "one"}, map[string]any{"sub": "one", "email": "user@example.com", "email_verified": true}, "user@example.com", true}, + {"differentSubject", map[string]any{"sub": "one"}, map[string]any{"sub": "two", "email": "user@example.com", "email_verified": true}, "", false}, + {"missingSubject", map[string]any{"sub": "one"}, map[string]any{"email": "user@example.com", "email_verified": true}, "", false}, + {"missingIDSubject", map[string]any{}, map[string]any{"sub": "one", "email": "user@example.com", "email_verified": true}, "", false}, + {"sameEmail", map[string]any{"sub": "one", "email": "User@Example.com"}, map[string]any{"sub": "one", "email": "User@Example.com", "email_verified": true}, "User@Example.com", true}, + {"differentLocalCase", map[string]any{"sub": "one", "email": "User@example.com"}, map[string]any{"sub": "one", "email": "user@example.com", "email_verified": true}, "User@example.com", false}, + {"differentDomainCase", map[string]any{"sub": "one", "email": "user@Example.com"}, map[string]any{"sub": "one", "email": "user@example.com", "email_verified": true}, "user@Example.com", false}, + {"differentFormatting", map[string]any{"sub": "one", "email": "User "}, map[string]any{"sub": "one", "email": "user@example.com", "email_verified": true}, "User ", false}, + {"differentWhitespace", map[string]any{"sub": "one", "email": " user@example.com"}, map[string]any{"sub": "one", "email": "user@example.com", "email_verified": true}, " user@example.com", false}, + {"differentEmail", map[string]any{"sub": "one", "email": "id@example.com"}, map[string]any{"sub": "one", "email": "info@example.com", "email_verified": true}, "id@example.com", false}, + {"unpairedFlag", map[string]any{"sub": "one", "email_verified": true}, map[string]any{"sub": "one", "email": "info@example.com"}, "info@example.com", false}, + {"IDWins", map[string]any{"sub": "one", "email": "id@example.com", "email_verified": true}, map[string]any{"sub": "one", "email": "info@example.com", "email_verified": false}, "id@example.com", true}, + {"IDUnverified", map[string]any{"sub": "one", "email": "user@example.com", "email_verified": false}, map[string]any{"sub": "one", "email": "user@example.com", "email_verified": true}, "user@example.com", false}, + } { + t.Run(tc.name, func(t *testing.T) { + mergeUserInfoClaims(tc.id, tc.info) + email, _ := tc.id["email"].(string) + if email != tc.email || extractEmailVerified(tc.id) != tc.verified { + t.Fatalf("unexpected merged claims: %v", tc.id) + } + }) + } +} + +func TestAdminEmailPolicyConcurrentUpdates(t *testing.T) { + // Exercise real concurrent updates and reads without forcing an interleaving. + jw, err := jaws.New() + if err != nil { + t.Fatal(err) + } + defer jw.Close() + srv, err := New(jw, nil, nil) + if err != nil { + t.Fatal(err) + } + srv.RequireVerifiedAdminEmail = true + srv.SetAdmins([]string{"admin@example.com"}) + sess := jw.NewSession(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/", nil)) + defer srv.Logout(sess, nil) + auth := &JawsAuth{server: srv, sess: sess} + expiry := time.Now().Add(time.Hour) + start := make(chan struct{}) + var workers sync.WaitGroup + workers.Go(func() { + <-start + for range 1000 { + for _, state := range []struct { + email string + verified bool + }{{"user@example.com", true}, {"admin@example.com", false}} { + claims := map[string]any{"sub": "user", "email": state.email, "email_verified": state.verified} + if err := srv.storeSessionAuthClaims(t.Context(), sess, claims, nil, expiry, nil); err != nil { + t.Error(err) + return + } + } + } + }) + workers.Go(func() { + <-start + for range 1000 { + if auth.IsAdmin() { + t.Error("admin check combined email and verification from different updates") + return + } + } + }) + close(start) + workers.Wait() +} diff --git a/authsession.go b/authsession.go index d5fb736..df90f47 100644 --- a/authsession.go +++ b/authsession.go @@ -105,7 +105,7 @@ func (srv *Server) storeSessionAuthClaims(ctx context.Context, sess *jaws.Sessio err = errOIDC{kind: ErrOIDCInvalidIDToken, cause: errOIDCInvalidExpiry} if !expiry.IsZero() { if fallback, e := srv.fetchUserInfo(ctx, srv.userinfoUrl, tokenSource); srv.Jaws.Log(e) == nil { - mergeMissingClaims(claims, fallback) + mergeUserInfoClaims(claims, fallback) } if entry != nil { if !srv.sessionAuthTimerCurrent(sess, entry) { @@ -113,13 +113,16 @@ func (srv *Server) storeSessionAuthClaims(ctx context.Context, sess *jaws.Sessio return } } - verified := extractEmailVerified(claims) + email, verified := srv.extractEmail(claims) claims["email_verified"] = verified + // Publish the email and its verification together for admin checks. + srv.mu.Lock() sess.Set(srv.SessionKey, claims) sess.Set(srv.SessionTokenKey, tokenSource) sess.Set(oauth2IDTokenExpiryKey, expiry) - sess.Set(srv.SessionEmailKey, srv.extractEmail(claims)) + sess.Set(srv.SessionEmailKey, email) sess.Set(srv.SessionEmailVerifiedKey, verified) + srv.mu.Unlock() srv.Jaws.Dirty(sess) srv.scheduleSessionAuthTimer(sess, expiry) err = nil @@ -360,11 +363,13 @@ func (srv *Server) clearSessionAuth(sess *jaws.Session, hr *http.Request, callLo if srv != nil && sess != nil { if srv.stopSessionAuthTimer(sess, entry) { clearSessionOAuthFlow(sess) + srv.mu.Lock() sess.Set(srv.SessionKey, nil) sess.Set(srv.SessionTokenKey, nil) sess.Set(oauth2IDTokenExpiryKey, nil) sess.Set(srv.SessionEmailKey, nil) sess.Set(srv.SessionEmailVerifiedKey, nil) + srv.mu.Unlock() if callLogout && srv.LogoutEvent != nil { srv.LogoutEvent(sess, hr) } diff --git a/authsession_test.go b/authsession_test.go index b7f40f0..3aab2f9 100644 --- a/authsession_test.go +++ b/authsession_test.go @@ -431,7 +431,7 @@ func TestAuthTimerRefreshesCachedTokenByForcingRefresh(t *testing.T) { userinfoAuth = hr.Header.Get("Authorization") mu.Unlock() hw.Header().Set("Content-Type", "application/json") - _, _ = hw.Write([]byte(`{"name":"Profile Name"}`)) + _, _ = hw.Write([]byte(`{"sub":"sub-123","name":"Profile Name"}`)) default: hw.WriteHeader(http.StatusNotFound) } diff --git a/jawsauth.go b/jawsauth.go index 8a43383..a52f7eb 100644 --- a/jawsauth.go +++ b/jawsauth.go @@ -19,6 +19,9 @@ func (a *JawsAuth) Data() (x map[string]any) { } // Email returns the authenticated email stored in the session, or an empty string. +// +// The value is a parsed address with ASCII letters lowercased. +// Use [JawsAuth.IsAdmin] to check administrator status. // It is safe to call on a nil or zero-value JawsAuth. func (a *JawsAuth) Email() (s string) { if a != nil && a.server != nil && a.sess != nil { @@ -27,7 +30,9 @@ func (a *JawsAuth) Email() (s string) { return } -// EmailVerified returns whether the authenticated email was marked verified. +// EmailVerified reports whether the session address came from a verified email claim. +// +// Addresses taken from mail or public_email claims are unverified. // It is safe to call on a nil or zero-value JawsAuth. func (a *JawsAuth) EmailVerified() (yes bool) { if a != nil && a.server != nil && a.sess != nil { @@ -37,12 +42,14 @@ func (a *JawsAuth) EmailVerified() (yes bool) { } // IsAdmin reports whether the authenticated email is an administrator. -// A nil or zero-value JawsAuth follows Server.IsAdmin's nil-server behavior and returns true. +// +// It applies [Server.RequireVerifiedAdminEmail] to non-empty admin lists. +// A nil or zero-value JawsAuth follows [Server.IsAdmin]'s nil-server behavior and returns true. func (a *JawsAuth) IsAdmin() (yes bool) { if a == nil || a.server == nil { yes = true } else { - yes = a.server.IsAdmin(a.Email()) + yes = a.server.sessionIsAdmin(a.sess) } return } diff --git a/oauth2.go b/oauth2.go index 9205652..c350910 100644 --- a/oauth2.go +++ b/oauth2.go @@ -187,9 +187,28 @@ func randomHexString() string { return hex.EncodeToString(b[:]) } -func mergeMissingClaims(dst, src map[string]any) { - if dst != nil { - for k, v := range src { +// mergeUserInfoClaims accepts fallback claims only for the verified subject. +// Email verification stays paired with the email from the same response. +func mergeUserInfoClaims(dst, src map[string]any) { + subject, _ := dst["sub"].(string) + if other, _ := src["sub"].(string); subject == "" || other != subject { + return + } + _, hasEmail := dst["email"] + if !hasEmail { + if email, ok := src["email"]; ok { + dst["email"] = email + dst["email_verified"] = src["email_verified"] + } + } else if _, hasVerified := dst["email_verified"]; !hasVerified { + email, _ := dst["email"].(string) + other, _ := src["email"].(string) + if email != "" && email == other { + dst["email_verified"] = src["email_verified"] + } + } + for k, v := range src { + if k != "email_verified" { if _, ok := dst[k]; !ok { dst[k] = v } @@ -197,12 +216,12 @@ func mergeMissingClaims(dst, src map[string]any) { } } -func (srv *Server) extractEmail(claims map[string]any) (sessEmailValue any) { +func (srv *Server) extractEmail(claims map[string]any) (sessEmailValue any, verified bool) { for _, k := range []string{"email", "mail", "public_email"} { if s, ok := claims[k].(string); ok { if s = strings.TrimSpace(s); s != "" { if m, err := mail.ParseAddress(s); err == nil { - return strings.ToLower(strings.TrimSpace(m.Address)) + return foldEmailCase(m.Address), k == "email" && extractEmailVerified(claims) } } } diff --git a/oauth2_test.go b/oauth2_test.go index 1327ec6..4aa655b 100644 --- a/oauth2_test.go +++ b/oauth2_test.go @@ -384,7 +384,7 @@ func Test_handleAuthResponseUsesPKCEVerifier(t *testing.T) { gotAuth = hr.Header.Get("Authorization") mu.Unlock() hw.Header().Set("Content-Type", "application/json") - _, _ = hw.Write([]byte(`{"email":"userinfo@example.com","name":"Profile Name"}`)) + _, _ = hw.Write([]byte(`{"sub":"sub-123","email":"userinfo@example.com","name":"Profile Name"}`)) default: setProviderErr(fmt.Errorf("unexpected provider path %s", hr.URL.Path)) hw.WriteHeader(http.StatusNotFound) @@ -1182,6 +1182,21 @@ func TestServerExtractEmail(t *testing.T) { userinfo: map[string]any{"email": `"Test User" `}, want: "testuser@example.com", }, + { + name: "literalQuotesInLocalPart", + userinfo: map[string]any{"email": `"\"Quoted\""@Example.com`}, + want: `"quoted"@example.com`, + }, + { + name: "literalSpacesInLocalPart", + userinfo: map[string]any{"email": `" User "@Example.com`}, + want: " user @example.com", + }, + { + name: "nonASCIICasePreserved", + userinfo: map[string]any{"email": "ÅSA@Example.com"}, + want: "Åsa@example.com", + }, { name: "mailFieldFallback", userinfo: map[string]any{"mail": "Secondary@Example.com "}, @@ -1210,7 +1225,7 @@ func TestServerExtractEmail(t *testing.T) { handler := &recordingHandler{} jw.Logger = slog.New(handler) srv := &Server{Jaws: jw} - got := srv.extractEmail(tc.userinfo) + got, _ := srv.extractEmail(tc.userinfo) if tc.expectNil { if got != nil { t.Fatalf("expected nil email value, got %v", got) diff --git a/server.go b/server.go index e08a465..693466c 100644 --- a/server.go +++ b/server.go @@ -24,11 +24,21 @@ import ( // to be non-nil for the lifetime of the Server. var ErrServerNilJaws = errors.New("jawsauth: nil *jaws.Jaws") +func foldEmailCase(s string) string { + return strings.Map(func(r rune) rune { + if r >= 'A' && r <= 'Z' { + return r + ('a' - 'A') + } + return r + }, s) +} + func normalizeEmail(s string) (email string) { + s = strings.TrimSpace(s) if m, e := mail.ParseAddress(s); e == nil { s = m.Address } - email = strings.ToLower(strings.TrimSpace(s)) + email = foldEmailCase(s) return } @@ -70,23 +80,31 @@ type Server struct { Jaws *jaws.Jaws SessionKey string // default is "oidc_claims", value will be of type map[string]any SessionTokenKey string // default is "oauth2_tokensource", value will be of type oauth2.TokenSource - SessionEmailKey string // default is "email", value will be of type string + SessionEmailKey string // default is "email", value is a parsed string address with ASCII letters lowercased SessionEmailVerifiedKey string // default is "email_verified", value will be of type bool HandledPaths map[string]struct{} // URI paths we have registered handlers for LoginEvent EventFunc // if not nil, called after a successful login LogoutEvent EventFunc // if not nil, called before logout; hr may be nil for timer-driven logout LoginFailed FailedFunc // if not nil, called on failed login Options []oauth2.AuthCodeOption // options to use, see https://pkg.go.dev/golang.org/x/oauth2#AuthCodeOption - oauth2cfg *oauth2.Config - idTokenVerifier *oidc.IDTokenVerifier - userinfoUrl string - httpClient *http.Client - ishttps bool - mu sync.Mutex // protects following - admins map[string]struct{} // if not empty, emails of admins - handle403 http.Handler // handler for 403 Forbidden - authTimers map[uint64]*authTimerState - authTimerAfterFunc authTimerAfterFunc + + // RequireVerifiedAdminEmail requires email_verified for a non-empty admin list. + // + // It defaults to false. When true, providers that omit email_verified cannot + // grant admin access through a non-empty list. Set before serving requests. + // Ordinary login is unaffected. + RequireVerifiedAdminEmail bool + + oauth2cfg *oauth2.Config + idTokenVerifier *oidc.IDTokenVerifier + userinfoUrl string + httpClient *http.Client + ishttps bool + mu sync.Mutex // protects following fields and session email/verification pairs + admins map[string]struct{} // if not empty, emails of admins + handle403 http.Handler // handler for 403 Forbidden + authTimers map[uint64]*authTimerState + authTimerAfterFunc authTimerAfterFunc } // NewDebug behaves like New but can override the scheme and host of cfg.RedirectURL. @@ -155,6 +173,10 @@ func (srv *Server) handlePath(p string, handleFn HandleFunc, h http.Handler) { } // IsAdmin returns true if email belongs to an admin, if the list of admins is empty, or if srv is nil. +// +// It parses a raw address using the rules in [Server.SetAdmins]. This lookup cannot +// verify email ownership. [Server.WrapAdmin], [Server.HandlerAdmin] and +// [JawsAuth.IsAdmin] also enforce [Server.RequireVerifiedAdminEmail]. func (srv *Server) IsAdmin(email string) (yes bool) { yes = true if srv != nil { @@ -167,7 +189,30 @@ func (srv *Server) IsAdmin(email string) (yes bool) { return } -// SetAdmins sets the emails of administrators. If empty, everyone is considered an administrator. +func (srv *Server) sessionIsAdmin(sess *jaws.Session) bool { + if srv == nil { + return true + } + srv.mu.Lock() + defer srv.mu.Unlock() + if len(srv.admins) == 0 { + return true + } + if sess == nil { + return false + } + email, _ := sess.Get(srv.SessionEmailKey).(string) + verified, _ := sess.Get(srv.SessionEmailVerifiedKey).(bool) + // Session emails are already parsed; parsing again can change quoted local parts. + _, listed := srv.admins[foldEmailCase(email)] + return listed && (!srv.RequireVerifiedAdminEmail || verified) +} + +// SetAdmins sets the emails of administrators. +// +// Addresses are parsed with [mail.ParseAddress] and ASCII letters are lowercased. +// Non-ASCII characters are matched exactly. If empty, everyone is considered an +// administrator. Session checks also apply [Server.RequireVerifiedAdminEmail]. func (srv *Server) SetAdmins(emails []string) { if srv != nil { srv.mu.Lock() @@ -247,7 +292,8 @@ func (srv *Server) wrap(h http.Handler, admin bool) (rh http.Handler) { // // Unauthenticated requests are redirected into the OIDC login flow (HandleLogin); // authenticated users whose email is not an admin (see SetAdmins and IsAdmin) are -// served the 403 handler instead of h. If the Server is not Valid, returns h. +// served the 403 handler instead of h. [Server.RequireVerifiedAdminEmail] also +// applies. If the Server is not Valid, returns h. func (srv *Server) WrapAdmin(h http.Handler) (rh http.Handler) { return srv.wrap(h, true) } @@ -266,6 +312,7 @@ func (srv *Server) Wrap(h http.Handler) (rh http.Handler) { // // Unauthenticated requests are redirected into the OIDC login flow (HandleLogin); // authenticated non-admins (see SetAdmins and IsAdmin) are served the 403 handler. +// [Server.RequireVerifiedAdminEmail] also applies. // If the Server is not Valid, the template handler is returned without the // authentication requirement. func (srv *Server) HandlerAdmin(name string, dot any) http.Handler { diff --git a/wrapper.go b/wrapper.go index 289850f..fcd90ed 100644 --- a/wrapper.go +++ b/wrapper.go @@ -23,8 +23,7 @@ func (w wrapper) ServeHTTP(hw http.ResponseWriter, hr *http.Request) { } if w.admin { - email, _ := sess.Get(w.server.SessionEmailKey).(string) - if !w.server.IsAdmin(email) { + if !w.server.sessionIsAdmin(sess) { h = w.server.get403Handler() } }