From e8ffe751b24faeae3e0f2f443d85dc92d312a33a Mon Sep 17 00:00:00 2001 From: Johan Lindh Date: Sun, 4 Oct 2026 19:19:55 +0200 Subject: [PATCH 1/2] fix: make verified admin email an opt-in policy --- README.md | 11 ++++++ adminpolicy_test.go | 91 +++++++++++++++++++++++++++++++++++++++++++++ authsession.go | 6 +-- authsession_test.go | 2 +- jawsauth.go | 2 +- oauth2.go | 29 ++++++++++++--- oauth2_test.go | 4 +- server.go | 53 ++++++++++++++++++++------ wrapper.go | 3 +- 9 files changed, 176 insertions(+), 25 deletions(-) create mode 100644 adminpolicy_test.go diff --git a/README.md b/README.md index dca9f6e..daecd82 100644 --- a/README.md +++ b/README.md @@ -17,3 +17,14 @@ 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 2048 bytes fall back to `/`. + +Admin allowlists trust the configured identity provider's email claims by default, +including providers such as Microsoft Entra ID that omit `email_verified`. +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. +`Server.IsAdmin(email)` only checks address membership. With strict mode enabled, +`mail` and `public_email` remain display fallbacks but cannot establish verified +email ownership. An empty admin list continues to allow every authenticated user. +UserInfo fallback requires a matching non-empty `sub`; its verification flag is +used only when that response supplies or confirms the same email. diff --git a/adminpolicy_test.go b/adminpolicy_test.go new file mode 100644 index 0000000..1852f20 --- /dev/null +++ b/adminpolicy_test.go @@ -0,0 +1,91 @@ +package jawsauth + +import ( + "net/http" + "net/http/httptest" + "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}, + } { + 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"}) + 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}, + {"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}, + } { + 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) + } + }) + } +} diff --git a/authsession.go b/authsession.go index d5fb736..4da66eb 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,12 +113,12 @@ func (srv *Server) storeSessionAuthClaims(ctx context.Context, sess *jaws.Sessio return } } - verified := extractEmailVerified(claims) + email, verified := srv.extractEmail(claims) claims["email_verified"] = verified 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.Jaws.Dirty(sess) srv.scheduleSessionAuthTimer(sess, expiry) 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..729d940 100644 --- a/jawsauth.go +++ b/jawsauth.go @@ -42,7 +42,7 @@ 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 5e4cc86..42b0009 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 != "" && other != "" && normalizeEmail(email) == normalizeEmail(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 normalizeEmail(m.Address), k == "email" && extractEmailVerified(claims) } } } diff --git a/oauth2_test.go b/oauth2_test.go index 297f233..65db384 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) @@ -1210,7 +1210,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..a3a5ef1 100644 --- a/server.go +++ b/server.go @@ -28,7 +28,12 @@ func normalizeEmail(s string) (email string) { if m, e := mail.ParseAddress(s); e == nil { s = m.Address } - email = strings.ToLower(strings.TrimSpace(s)) + email = strings.Map(func(r rune) rune { + if r >= 'A' && r <= 'Z' { + return r + ('a' - 'A') + } + return r + }, strings.TrimSpace(s)) return } @@ -77,16 +82,22 @@ type Server struct { 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 for providers such as Microsoft Entra ID that omit it. + // Set before serving requests. It does not restrict ordinary authenticated users. + RequireVerifiedAdminEmail bool + + 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 } // NewDebug behaves like New but can override the scheme and host of cfg.RedirectURL. @@ -155,6 +166,8 @@ 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. +// This address-only lookup cannot verify email ownership. WrapAdmin, HandlerAdmin +// and JawsAuth.IsAdmin also enforce RequireVerifiedAdminEmail on session claims. func (srv *Server) IsAdmin(email string) (yes bool) { yes = true if srv != nil { @@ -167,6 +180,24 @@ func (srv *Server) IsAdmin(email string) (yes bool) { return } +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) + _, listed := srv.admins[normalizeEmail(email)] + return listed && (!srv.RequireVerifiedAdminEmail || verified) +} + // SetAdmins sets the emails of administrators. If empty, everyone is considered an administrator. func (srv *Server) SetAdmins(emails []string) { if srv != nil { 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() } } From 4d7d1cba8b484237f8de80e238c179356f5995ee Mon Sep 17 00:00:00 2001 From: Johan Lindh Date: Mon, 5 Oct 2026 11:47:07 +0200 Subject: [PATCH 2/2] fix: preserve email identity and verification consistency --- README.md | 21 ++++++++++------ adminpolicy_test.go | 59 +++++++++++++++++++++++++++++++++++++++++++-- authsession.go | 5 ++++ jawsauth.go | 6 ++++- oauth2.go | 4 +-- oauth2_test.go | 15 ++++++++++++ server.go | 46 +++++++++++++++++++++++------------ 7 files changed, 129 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index daecd82..41df877 100644 --- a/README.md +++ b/README.md @@ -16,15 +16,22 @@ OIDC-verified authentication for [JaWS](https://github.com/linkdata/jaws) sessio 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 2048 bytes fall back to `/`. +Post-login return targets longer than 8192 bytes fall back to `/`. -Admin allowlists trust the configured identity provider's email claims by default, -including providers such as Microsoft Entra ID that omit `email_verified`. +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. -`Server.IsAdmin(email)` only checks address membership. With strict mode enabled, -`mail` and `public_email` remain display fallbacks but cannot establish verified -email ownership. An empty admin list continues to allow every authenticated user. +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 or confirms the same email. +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 index 1852f20..7a35646 100644 --- a/adminpolicy_test.go +++ b/adminpolicy_test.go @@ -3,6 +3,7 @@ package jawsauth import ( "net/http" "net/http/httptest" + "sync" "testing" "time" @@ -21,6 +22,9 @@ func TestAdminEmailPolicy(t *testing.T) { {"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() @@ -29,7 +33,7 @@ func TestAdminEmailPolicy(t *testing.T) { } defer jw.Close() srv := newTimerTestServer(t, jw, "https://issuer.example", &testAuthTimerFactory{}) - srv.SetAdmins([]string{"admin@example.com", "kate@example.com"}) + 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 { @@ -75,10 +79,15 @@ func TestUserInfoClaimProvenance(t *testing.T) { {"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}, + {"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) @@ -89,3 +98,49 @@ func TestUserInfoClaimProvenance(t *testing.T) { }) } } + +func TestAdminEmailPolicyConcurrentUpdates(t *testing.T) { + 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 4da66eb..df90f47 100644 --- a/authsession.go +++ b/authsession.go @@ -115,11 +115,14 @@ func (srv *Server) storeSessionAuthClaims(ctx context.Context, sess *jaws.Sessio } 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, 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/jawsauth.go b/jawsauth.go index 729d940..6ad8e76 100644 --- a/jawsauth.go +++ b/jawsauth.go @@ -19,6 +19,8 @@ func (a *JawsAuth) Data() (x map[string]any) { } // Email returns the authenticated email stored in the session, or an empty string. +// +// Address parsing and case matching follow [Server.SetAdmins]. // 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 { @@ -37,7 +39,9 @@ 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 diff --git a/oauth2.go b/oauth2.go index 42b0009..512be42 100644 --- a/oauth2.go +++ b/oauth2.go @@ -203,7 +203,7 @@ func mergeUserInfoClaims(dst, src map[string]any) { } else if _, hasVerified := dst["email_verified"]; !hasVerified { email, _ := dst["email"].(string) other, _ := src["email"].(string) - if email != "" && other != "" && normalizeEmail(email) == normalizeEmail(other) { + if email != "" && email == other { dst["email_verified"] = src["email_verified"] } } @@ -221,7 +221,7 @@ func (srv *Server) extractEmail(claims map[string]any) (sessEmailValue any, veri if s, ok := claims[k].(string); ok { if s = strings.TrimSpace(s); s != "" { if m, err := mail.ParseAddress(s); err == nil { - return normalizeEmail(m.Address), k == "email" && extractEmailVerified(claims) + return foldEmailCase(m.Address), k == "email" && extractEmailVerified(claims) } } } diff --git a/oauth2_test.go b/oauth2_test.go index 65db384..39df9d4 100644 --- a/oauth2_test.go +++ b/oauth2_test.go @@ -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 "}, diff --git a/server.go b/server.go index a3a5ef1..255d474 100644 --- a/server.go +++ b/server.go @@ -24,16 +24,21 @@ import ( // to be non-nil for the lifetime of the Server. var ErrServerNilJaws = errors.New("jawsauth: nil *jaws.Jaws") -func normalizeEmail(s string) (email string) { - if m, e := mail.ParseAddress(s); e == nil { - s = m.Address - } - email = strings.Map(func(r rune) rune { +func foldEmailCase(s string) string { + return strings.Map(func(r rune) rune { if r >= 'A' && r <= 'Z' { return r + ('a' - 'A') } return r - }, strings.TrimSpace(s)) + }, s) +} + +func normalizeEmail(s string) (email string) { + s = strings.TrimSpace(s) + if m, e := mail.ParseAddress(s); e == nil { + s = m.Address + } + email = foldEmailCase(s) return } @@ -75,7 +80,7 @@ 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 @@ -84,8 +89,10 @@ type Server struct { Options []oauth2.AuthCodeOption // options to use, see https://pkg.go.dev/golang.org/x/oauth2#AuthCodeOption // RequireVerifiedAdminEmail requires email_verified for a non-empty admin list. - // It defaults to false for providers such as Microsoft Entra ID that omit it. - // Set before serving requests. It does not restrict ordinary authenticated users. + // + // 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 @@ -93,7 +100,7 @@ type Server struct { userinfoUrl string httpClient *http.Client ishttps bool - mu sync.Mutex // protects following + 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 @@ -166,8 +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. -// This address-only lookup cannot verify email ownership. WrapAdmin, HandlerAdmin -// and JawsAuth.IsAdmin also enforce RequireVerifiedAdminEmail on session claims. +// +// Address parsing and case matching follow [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 { @@ -194,11 +203,16 @@ func (srv *Server) sessionIsAdmin(sess *jaws.Session) bool { } email, _ := sess.Get(srv.SessionEmailKey).(string) verified, _ := sess.Get(srv.SessionEmailVerifiedKey).(bool) - _, listed := srv.admins[normalizeEmail(email)] + // 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. If empty, everyone is considered an administrator. +// 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() @@ -278,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) } @@ -297,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 {