From eda025817dae1cd1b873c58d622aab5d521a3259 Mon Sep 17 00:00:00 2001 From: Johan Lindh Date: Fri, 6 Nov 2026 10:36:12 +0100 Subject: [PATCH 1/6] Keep JaWS session across login by default --- README.md | 40 ++++++++------ authsession.go | 30 ++++++---- authsession_test.go | 1 + concurrent_providers_test.go | 40 ++++++++++++++ jawsauth.go | 9 ++- oauth2.go | 59 ++++++++++++++++---- oauth2_callback_edge_test.go | 77 ++++++++++++++------------ oauth2_flow_guards_test.go | 104 +++++++++++++++++++++++++++++++++++ oauth2_test.go | 23 +++----- oauth_flow_grace_test.go | 1 + profile_link_test.go | 37 +++++++++++++ server.go | 25 ++++++--- sessionlifecycle_test.go | 2 + wrapper.go | 24 ++++++-- 14 files changed, 370 insertions(+), 102 deletions(-) diff --git a/README.md b/README.md index 7325eac..15a0cfd 100644 --- a/README.md +++ b/README.md @@ -33,9 +33,13 @@ An explicit `expires_in: 0` in the token response is rejected. Set `Jaws.MaxSessions` and `Jaws.MaxSessionsPerIP` to bound sessions created by unauthenticated requests; both limits are unlimited by default. A login start, including an unauthenticated protected GET, can retain its session for about ten -minutes even if the client never reaches the provider. Login rotates the session -and discards its application data; rotation needs a free slot under both limits. -`MaxSessionsPerIP = 1` prevents login. If a slot is unavailable, +minutes even if the client never reaches the provider. By default, login keeps +the same session and its application data, so a one-session limit permits login. +Set `Server.RotateSessionOnLogin` before serving requests when an attacker can +plant or know a pre-login cookie and use the same client IP JaWS sees for its +owner. +Rotation discards pre-login application data and needs a free slot under both +limits; `MaxSessionsPerIP = 1` then prevents login. If a slot is unavailable, login returns 503 with `Retry-After: 60` and preserves the existing session. ## Multiple providers @@ -53,18 +57,19 @@ login returns 503 with `Retry-After: 60` and preserves the existing session. - Register `HandleLink` as a POST handler to enable linking; `New` does not register it. Authorize the session before calling it. In `LinkEvent`, recheck the original provider's `CurrentClaims` before writing the link. Linking keeps - the session; an ordinary login rotates it and signs out other providers. - A re-login replacing existing auth calls its `LogoutEvent` before storing - the new sign-in. Calling `Logout` on that event session cancels it. Other - providers' `LogoutEvent` may run later with a nil request. Before using one to - log out another provider, check its `CurrentClaims`; the session may have rotated. + the session; ordinary login keeps it and leaves other providers signed in. + With `RotateSessionOnLogin`, login signs out other providers, and re-login + calls its `LogoutEvent` on the old session before storing the new sign-in. + Calling `Logout` on that event session cancels it. Other providers' + `LogoutEvent` may run later with a nil request. Before using one to log out + another provider, check its `CurrentClaims`. Starting a link replaces this server's pending flow, including a running login callback. - `New` shares one process-wide `SessionRotationLock` by default. To coordinate with other authentication methods or isolate independent JaWS instances, set a custom lock before serving; servers sharing a JaWS must use the same lock. - jawsauth holds it during flow start, rotation, authentication clearing and - `LinkEvent`. + jawsauth holds it during flow start, login claim storage, authentication + clearing and `LinkEvent`. Do not take the lock around jawsauth handlers or `Server.Logout`, or within `LinkEvent`; do not call `Server.Logout` from `LinkEvent`. Bound `LinkEvent` I/O with a timeout; other sessions sharing the lock wait. One provider's logout @@ -97,12 +102,15 @@ Pending OAuth flows expire after ten minutes. jawsauth extends the original JaWS session's grace for the pending flow. Unauthenticated protected GETs reuse a pending login's state, return target and grace deadline while more than two minutes remain; near expiry, they start a new flow and -extend grace. While a login callback runs before session rotation, protected GETs -with that session cookie return 409 unless `LoginFailed` handles them. After -rotation, the old cookie starts a new flow and cannot cancel that sign-in. Logout -and auth expiry clear this server's credentials and reload every JaWS page in the -session. Auth expiry preserves a pending login but cancels a pending link. Explicit -logout on the live session cancels either flow. A new login on the same server, +extend grace. While a login callback runs, protected GETs with that session +cookie return 409 unless `LoginFailed` handles them. During in-place login, +`CurrentClaims` also remains nil until `LoginEvent` returns. With +`RotateSessionOnLogin`, the old cookie starts a new flow after rotation and +cannot cancel that sign-in. Without rotation, successful login reloads active +JaWS pages after `LoginEvent`. Logout and auth expiry clear this server's +credentials and reload every JaWS page in the session. Auth expiry preserves a +pending login but cancels a pending link. Explicit logout on the live session +cancels either flow. A new login on the same server, including one started by `Wrap` or `Handler`, cancels a pending link. Use an HTTP endpoint for a post-logout redirect. When `Server.Logout` clears diff --git a/authsession.go b/authsession.go index c2f26f9..cc8a683 100644 --- a/authsession.go +++ b/authsession.go @@ -118,6 +118,14 @@ func (srv *Server) sessionAuthStatus(sess *jaws.Session, now func() time.Time) ( return } +func (srv *Server) sessionAccessStatus(sess *jaws.Session) (current, present, pending bool) { + srv.mu.Lock() + current, present = srv.sessionAuthStatus(sess, time.Now) + pending = !srv.RotateSessionOnLogin && srv.loginEvents[sess] != nil + srv.mu.Unlock() + return +} + func sessionAlive(sess *jaws.Session) bool { cookie := sess.Cookie() return cookie != nil && cookie.MaxAge >= 0 @@ -203,7 +211,8 @@ func (srv *Server) storeSessionAuthClaimsIfActive(sess *jaws.Session, claims map sess.Set(srv.SessionEmailKey, email) sess.Set(srv.SessionEmailVerifiedKey, verified) delay, replaced := srv.scheduleSessionAuthTimerLocked(sess, expiry) - if loginCanceled != nil && srv.LoginEvent != nil { + if loginCanceled != nil { + // Keep a new flow from replacing this login before its callback finishes. if srv.loginEvents == nil { srv.loginEvents = make(map[*jaws.Session]*pendingLoginEvent) } @@ -226,15 +235,17 @@ func (srv *Server) storeSessionAuthClaimsIfActive(sess *jaws.Session, claims map } func (srv *Server) emitLoginEvent(sess *jaws.Session, hr *http.Request) { - if srv.LoginEvent == nil { - return - } srv.mu.Lock() pending := srv.loginEvents[sess] if pending == nil { srv.mu.Unlock() return } + if pending.cleared { + delete(srv.loginEvents, sess) + srv.mu.Unlock() + return + } pending.started = true srv.mu.Unlock() defer func() { @@ -242,11 +253,13 @@ func (srv *Server) emitLoginEvent(sess *jaws.Session, hr *http.Request) { delete(srv.loginEvents, sess) cleared := pending.cleared srv.mu.Unlock() - if cleared && srv.LogoutEvent != nil { + if cleared && srv.LoginEvent != nil && srv.LogoutEvent != nil { srv.LogoutEvent(sess, nil) } }() - srv.LoginEvent(sess, hr) + if srv.LoginEvent != nil { + srv.LoginEvent(sess, hr) + } } func (srv *Server) setSessionAuthFromToken(ctx context.Context, sess *jaws.Session, tokenSource oauth2.TokenSource, token *oauth2.Token, minExpiry time.Time, entry *authTimerState) error { @@ -515,9 +528,6 @@ func (srv *Server) clearSessionAuth(sess *jaws.Session, hr *http.Request, entry requests = sess.Requests() if pending != nil { pending.cleared = true - if !pending.started { - delete(srv.loginEvents, sess) - } } } } @@ -528,7 +538,7 @@ func (srv *Server) clearSessionAuth(sess *jaws.Session, hr *http.Request, entry if cleared { if hadAuth { reloadAuthRequests(requests) - if pending == nil && srv.LogoutEvent != nil { + if (pending == nil || srv.LoginEvent == nil) && srv.LogoutEvent != nil { srv.LogoutEvent(sess, hr) } } diff --git a/authsession_test.go b/authsession_test.go index 8ab0acd..6f2cca0 100644 --- a/authsession_test.go +++ b/authsession_test.go @@ -1254,6 +1254,7 @@ func TestAbandonedLoginClearDropsPendingEvent(t *testing.T) { if !srv.Logout(sess, nil) { t.Fatal("pending login was not cleared") } + srv.emitLoginEvent(sess, req) srv.mu.Lock() pending := len(srv.loginEvents) srv.mu.Unlock() diff --git a/concurrent_providers_test.go b/concurrent_providers_test.go index e40a8a2..70781c6 100644 --- a/concurrent_providers_test.go +++ b/concurrent_providers_test.go @@ -39,6 +39,8 @@ func TestDefaultRotationLockSerializesProvidersSharingJaws(t *testing.T) { })} a := newConcurrentProfileProvider(t, jw, client, "a") b := newConcurrentProfileProvider(t, jw, client, "b") + a.RotateSessionOnLogin = true + b.RotateSessionOnLogin = true if a.SessionRotationLock == nil || a.SessionRotationLock != b.SessionRotationLock { t.Fatal("providers sharing JaWS do not share the default rotation lock") } @@ -82,6 +84,43 @@ func TestDefaultRotationLockSerializesProvidersSharingJaws(t *testing.T) { } } +func TestDefaultLoginKeepsOtherProvider(t *testing.T) { + jw := newTestJaws(t) + jw.MaxSessions = 1 + jw.MaxSessionsPerIP = 1 + client := &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + if r.URL.Path == "/token" { + return jsonResponse(`{"access_token":"proof","token_type":"Bearer"}`), nil + } + return jsonResponse(`{"id":42}`), nil + })} + a := newConcurrentProfileProvider(t, jw, client, "a") + b := newConcurrentProfileProvider(t, jw, client, "b") + cookie, stateA := profileStart(t, a, http.MethodGet, "/auth/a/login", nil) + _, stateB := profileStart(t, b, http.MethodGet, "/auth/b/login", cookie) + r := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + r.AddCookie(cookie) + sess := jw.GetSession(r) + if sess == nil { + t.Fatal("pending session missing") + } + for _, tc := range []struct { + server *Server + state string + }{ + {a, stateA}, + {b, stateB}, + } { + response := profileCallback(tc.server, cookie, tc.state, "proof") + if response.Code != http.StatusFound || len(response.Result().Cookies()) != 0 || jw.GetSession(r) != sess { + t.Fatalf("provider login replaced session: status=%d cookies=%v", response.Code, response.Result().Cookies()) + } + } + if a.CurrentClaims(sess) == nil || b.CurrentClaims(sess) == nil || jw.SessionCount() != 1 { + t.Fatal("provider login discarded another provider or created a session") + } +} + // holdFirstRotationLock lets a later caller acquire the lock before the first. type holdFirstRotationLock struct { mu sync.Mutex @@ -112,6 +151,7 @@ func TestLinkedProviderLogoutBeforeLinkWrite(t *testing.T) { })} a := newConcurrentProfileProvider(t, jw, client, "a") b := newConcurrentProfileProvider(t, jw, client, "b") + a.RotateSessionOnLogin = true cookie, state := profileStart(t, a, http.MethodGet, "/auth/a/login", nil) login := profileCallback(a, cookie, state, "proof") if login.Code != http.StatusFound || len(login.Result().Cookies()) != 1 { diff --git a/jawsauth.go b/jawsauth.go index 842fb97..8d65180 100644 --- a/jawsauth.go +++ b/jawsauth.go @@ -6,8 +6,10 @@ import ( "github.com/linkdata/jaws" ) -// CurrentClaims returns this provider's claims only while authentication and -// the JaWS session are live. It is useful when several providers share JaWS. +// CurrentClaims returns this provider's live authentication claims. +// +// It returns nil until [Server.LoginEvent] completes for in-place login. +// It is useful when several providers share JaWS. // Treat the returned session map as read-only, and recheck before protected writes // because the session may close after this call. func (srv *Server) CurrentClaims(sess *jaws.Session) map[string]any { @@ -16,6 +18,9 @@ func (srv *Server) CurrentClaims(sess *jaws.Session) map[string]any { } srv.mu.Lock() defer srv.mu.Unlock() + if !srv.RotateSessionOnLogin && srv.loginEvents[sess] != nil { + return nil + } if current, _ := srv.sessionAuthStatus(sess, time.Now); !current { return nil } diff --git a/oauth2.go b/oauth2.go index bffa3ab..786758b 100644 --- a/oauth2.go +++ b/oauth2.go @@ -151,6 +151,8 @@ func (srv *Server) returnTarget(hr *http.Request, location string) string { // session, then responds with a 302 redirect to the provider's authorization URL. // It replaces any pending flow on this server, canceling a running callback or link. // If no usable session is available, it returns 503 with Retry-After instead. +// Without [Server.RotateSessionOnLogin], it returns 409 while a completed +// callback delivers its login event. // Non-GET requests receive 405 with Allow: GET. func (srv *Server) HandleLogin(hw http.ResponseWriter, hr *http.Request) { if hr.Method != http.MethodGet { @@ -166,8 +168,9 @@ func (srv *Server) HandleLogin(hw http.ResponseWriter, hr *http.Request) { // recheck that identity in LinkEvent when the provider returns. On success it // responds with a 303 redirect to the provider. It returns 405 with Allow: POST // for other methods, 403 for a cross-origin or missing session, and 500 when -// linking is not configured. Starting a link replaces any pending flow on this -// server, including a running login callback. +// linking is not configured. Without [Server.RotateSessionOnLogin], it returns +// 409 while a completed login callback delivers its event. Starting a link +// replaces any pending flow on this server, including a running login callback. func (srv *Server) HandleLink(hw http.ResponseWriter, hr *http.Request) { switch { case hr.Method != http.MethodPost: @@ -249,6 +252,9 @@ func (srv *Server) prepareOAuthFlow(hw http.ResponseWriter, hr *http.Request, li } srv.mu.Lock() defer srv.mu.Unlock() + if !srv.RotateSessionOnLogin && srv.loginEvents[sess] != nil { + return flow, sess, http.StatusConflict + } if fromWrapper && srv.pendingOrdinaryLoginLocked(sess, 2*time.Minute) { // Reissue one pending transaction without replacing the first tab's state. flow, _ = sess.Get(srv.sessionKey(oauth2FlowKey)).(pendingOAuthFlow) @@ -582,6 +588,10 @@ func (srv *Server) completeLink(sess *jaws.Session, hr *http.Request, claims map func (srv *Server) completeLogin(hw http.ResponseWriter, hr *http.Request, old *jaws.Session, flow pendingOAuthFlow, claims map[string]any, tokenSource oauth2.TokenSource, expiry time.Time) (sess *jaws.Session, status int, err error) { sess, status, err = old, http.StatusServiceUnavailable, ErrOAuth2MissingSession + if !srv.RotateSessionOnLogin { + status, err = srv.completeLoginInPlace(hr, old, flow, claims, tokenSource, expiry) + return + } rotated, hadTimer, stale := srv.rotateOAuthSession(hw, hr, old, flow) if rotated == nil { if stale { @@ -608,6 +618,30 @@ func (srv *Server) completeLogin(hw http.ResponseWriter, hr *http.Request, old * return sess, http.StatusFound, nil } +func (srv *Server) completeLoginInPlace(hr *http.Request, sess *jaws.Session, flow pendingOAuthFlow, claims map[string]any, tokenSource oauth2.TokenSource, expiry time.Time) (status int, err error) { + rotationLock := srv.SessionRotationLock + if rotationLock != nil { + rotationLock.Lock() + defer rotationLock.Unlock() + } + if flow.revoked() || srv.Jaws.GetSession(hr) != sess { + return http.StatusForbidden, ErrOAuth2MissingSession + } + err = srv.storeSessionAuthClaimsIfActive(sess, claims, tokenSource, expiry, nil, flow.canceled) + if err != nil { + if errors.Is(err, ErrOAuth2MissingSession) { + return http.StatusForbidden, err + } + return http.StatusUnauthorized, err + } + srv.mu.Lock() + if srv.oauthCallbacks[sess].canceled == flow.canceled { + delete(srv.oauthCallbacks, sess) + } + srv.mu.Unlock() + return http.StatusFound, nil +} + func (srv *Server) completeOAuthCallback(ctx context.Context, hw http.ResponseWriter, hr *http.Request, sess *jaws.Session, flow pendingOAuthFlow) (*jaws.Session, int, error) { status, err := oauth2CallbackError(http.StatusBadRequest, hr) if err != nil { @@ -642,13 +676,14 @@ func (srv *Server) completeOAuthCallback(ctx context.Context, hw http.ResponseWr // // For GET requests it validates the state, exchanges the authorization code using the // stored PKCE verifier, verifies the ID token or authenticated profile, and stores -// claims in a new session. A link flow instead invokes [Server.LinkEvent] -// and keeps the original session. It invokes [Server.LoginEvent] after a -// new sign-in or [Server.LoginFailed] on login or link failure. Non-GET requests receive 405. +// claims in the existing session. A link flow instead invokes [Server.LinkEvent]. +// It invokes [Server.LoginEvent] after a new sign-in or [Server.LoginFailed] +// on login or link failure. Non-GET requests receive 405. // -// Rotation discards pre-login application data and requires a free JaWS session slot. -// If no slot is available, existing authentication is preserved and the default -// response is 503. The user must restart login after capacity becomes available. +// With [Server.RotateSessionOnLogin], login creates a new session and discards +// pre-login application data. If no session slot is available, existing +// authentication is preserved and the default response is 503. The user must +// restart login after capacity becomes available. func (srv *Server) HandleAuthResponse(hw http.ResponseWriter, hr *http.Request) { if hr.Method != http.MethodGet { hw.Header().Set("Allow", http.MethodGet) @@ -682,10 +717,14 @@ func (srv *Server) HandleAuthResponse(hw http.ResponseWriter, hr *http.Request) srv.emitLoginEvent(sess, hr) current, _ := srv.sessionAuthStatus(sess, time.Now) if flow.revoked() || !current || !sessionAlive(sess) { - srv.clearSessionAuth(sess, hr, nil, false) - http.SetCookie(hw, sess.Close()) + if srv.RotateSessionOnLogin { + srv.clearSessionAuth(sess, hr, nil, false) + http.SetCookie(hw, sess.Close()) + } err = ErrOAuth2MissingSession statusCode = http.StatusForbidden + } else if !srv.RotateSessionOnLogin { + reloadAuthRequests(sess.Requests()) } } if err == nil { diff --git a/oauth2_callback_edge_test.go b/oauth2_callback_edge_test.go index fbc1ecf..a91cab3 100644 --- a/oauth2_callback_edge_test.go +++ b/oauth2_callback_edge_test.go @@ -224,39 +224,48 @@ func TestLoginEventCanLogoutItsSession(t *testing.T) { } func TestLogoutDuringLoginEventDefersLogoutEvent(t *testing.T) { - jw := newTestJaws(t) - srv := newProfileTestServer(t, jw, nil) - entered := make(chan *jaws.Session, 1) - release := make(chan struct{}) - releaseOnce := sync.OnceFunc(func() { close(release) }) - defer releaseOnce() - events := make(chan string, 2) - logoutRequests := make(chan *http.Request, 1) - srv.LoginEvent = func(sess *jaws.Session, _ *http.Request) { - events <- "login" - entered <- sess - <-release - } - srv.LogoutEvent = func(_ *jaws.Session, request *http.Request) { - events <- "logout" - logoutRequests <- request - } - cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) - result := make(chan *httptest.ResponseRecorder, 1) - go func() { result <- profileCallback(srv, cookie, state, "first") }() - rotated := waitFor(t, entered, "LoginEvent did not start") - loggedOut := make(chan bool, 1) - logoutRequest := httptest.NewRequest(http.MethodGet, "https://app.example/logout", nil) - go func() { loggedOut <- srv.Logout(rotated, logoutRequest) }() - if !waitFor(t, loggedOut, "concurrent logout blocked") || len(events) != 1 { - t.Fatalf("logout before LoginEvent returned: events=%d", len(events)) - } - releaseOnce() - response := waitFor(t, result, "callback did not finish") - first := waitFor(t, events, "missing LoginEvent") - second := waitFor(t, events, "missing LogoutEvent") - deferredRequest := waitFor(t, logoutRequests, "missing LogoutEvent request") - if response.Code != http.StatusForbidden || first != "login" || second != "logout" || deferredRequest != nil || srv.CurrentClaims(rotated) != nil { - t.Fatalf("status=%d events=%s,%s request=%v claims=%v", response.Code, first, second, deferredRequest, srv.CurrentClaims(rotated)) + for _, rotate := range []bool{false, true} { + name := "default" + if rotate { + name = "rotation" + } + t.Run(name, func(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + srv.RotateSessionOnLogin = rotate + entered := make(chan *jaws.Session, 1) + release := make(chan struct{}) + releaseOnce := sync.OnceFunc(func() { close(release) }) + defer releaseOnce() + events := make(chan string, 2) + logoutRequests := make(chan *http.Request, 1) + srv.LoginEvent = func(sess *jaws.Session, _ *http.Request) { + events <- "login" + entered <- sess + <-release + } + srv.LogoutEvent = func(_ *jaws.Session, request *http.Request) { + events <- "logout" + logoutRequests <- request + } + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + result := make(chan *httptest.ResponseRecorder, 1) + go func() { result <- profileCallback(srv, cookie, state, "first") }() + sess := waitFor(t, entered, "LoginEvent did not start") + loggedOut := make(chan bool, 1) + logoutRequest := httptest.NewRequest(http.MethodGet, "https://app.example/logout", nil) + go func() { loggedOut <- srv.Logout(sess, logoutRequest) }() + if !waitFor(t, loggedOut, "concurrent logout blocked") || len(events) != 1 { + t.Fatalf("logout before LoginEvent returned: events=%d", len(events)) + } + releaseOnce() + response := waitFor(t, result, "callback did not finish") + first := waitFor(t, events, "missing LoginEvent") + second := waitFor(t, events, "missing LogoutEvent") + deferredRequest := waitFor(t, logoutRequests, "missing LogoutEvent request") + if response.Code != http.StatusForbidden || first != "login" || second != "logout" || deferredRequest != nil || srv.CurrentClaims(sess) != nil { + t.Fatalf("status=%d events=%s,%s request=%v claims=%v", response.Code, first, second, deferredRequest, srv.CurrentClaims(sess)) + } + }) } } diff --git a/oauth2_flow_guards_test.go b/oauth2_flow_guards_test.go index a8394b0..26d8862 100644 --- a/oauth2_flow_guards_test.go +++ b/oauth2_flow_guards_test.go @@ -70,6 +70,110 @@ func TestLoginStartDuringRotationKeepsFirstCallback(t *testing.T) { } } +func TestDefaultLoginStartDuringLoginEventConflicts(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + srv.RotateSessionOnLogin = false + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + r := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + r.AddCookie(cookie) + sess := jw.GetSession(r) + entered := make(chan struct{}) + release := make(chan struct{}) + releaseOnce := sync.OnceFunc(func() { close(release) }) + defer releaseOnce() + srv.LoginEvent = func(current *jaws.Session, _ *http.Request) { + if current != sess { + t.Error("LoginEvent received another session") + } + close(entered) + <-release + } + callbackDone := make(chan *httptest.ResponseRecorder, 1) + go func() { callbackDone <- profileCallback(srv, cookie, state, "first") }() + waitFor(t, entered, "LoginEvent did not start") + second := httptest.NewRequest(http.MethodGet, "https://app.example/auth/login", nil) + second.AddCookie(cookie) + w := httptest.NewRecorder() + srv.HandleLogin(w, second) + if w.Code != http.StatusConflict || len(w.Result().Cookies()) != 0 || sess.Get(srv.sessionKey(oauth2FlowKey)) != nil { + t.Fatalf("overlapping login: status=%d cookies=%v flow=%v", w.Code, w.Result().Cookies(), sess.Get(srv.sessionKey(oauth2FlowKey))) + } + releaseOnce() + callback := waitFor(t, callbackDone, "first callback did not finish") + if callback.Code != http.StatusFound || len(callback.Result().Cookies()) != 0 || jw.GetSession(r) != sess || srv.CurrentClaims(sess) == nil { + t.Fatalf("first login: status=%d cookies=%v claims=%v", callback.Code, callback.Result().Cookies(), srv.CurrentClaims(sess)) + } + profileStart(t, srv, http.MethodGet, "/auth/login", cookie) +} + +func TestDefaultPendingLoginIsNotAuthorized(t *testing.T) { + for _, revoke := range []bool{false, true} { + name := "committed" + if revoke { + name = "revoked" + } + t.Run(name, func(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + srv.RotateSessionOnLogin = false + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + request := httptest.NewRequest(http.MethodGet, "https://app.example/protected", nil) + request.AddCookie(cookie) + sess := jw.GetSession(request) + entered := make(chan struct{}) + release := make(chan struct{}) + releaseOnce := sync.OnceFunc(func() { close(release) }) + defer releaseOnce() + srv.LoginEvent = func(current *jaws.Session, _ *http.Request) { + if current != sess { + t.Error("LoginEvent received another session") + } + close(entered) + <-release + if revoke && !srv.Logout(current, nil) { + t.Error("LoginEvent could not revoke authentication") + } + } + callbackDone := make(chan *httptest.ResponseRecorder, 1) + go func() { callbackDone <- profileCallback(srv, cookie, state, "first") }() + waitFor(t, entered, "LoginEvent did not start") + if sess.Get(srv.SessionKey) == nil || srv.CurrentClaims(sess) != nil { + t.Fatal("pending login exposed claims") + } + handled := 0 + protected := srv.Wrap(http.HandlerFunc(func(hw http.ResponseWriter, _ *http.Request) { + handled++ + hw.WriteHeader(http.StatusOK) + })) + pending := httptest.NewRecorder() + protected.ServeHTTP(pending, request) + if pending.Code != http.StatusConflict || handled != 0 { + t.Fatalf("pending protected request: status=%d handled=%d", pending.Code, handled) + } + releaseOnce() + callback := waitFor(t, callbackDone, "callback did not finish") + if revoke { + post := httptest.NewRequest(http.MethodPost, "https://app.example/protected", nil) + post.AddCookie(cookie) + denied := httptest.NewRecorder() + protected.ServeHTTP(denied, post) + if callback.Code != http.StatusForbidden || denied.Code != http.StatusForbidden || + handled != 0 || srv.CurrentClaims(sess) != nil { + t.Fatalf("revoked login: callback=%d protected=%d handled=%d", callback.Code, denied.Code, handled) + } + return + } + authorized := httptest.NewRecorder() + protected.ServeHTTP(authorized, request) + if callback.Code != http.StatusFound || authorized.Code != http.StatusOK || + handled != 1 || srv.CurrentClaims(sess) == nil { + t.Fatalf("committed login: callback=%d protected=%d handled=%d", callback.Code, authorized.Code, handled) + } + }) + } +} + func TestCallbackRevokedAfterStoreBalancesEvents(t *testing.T) { jw := newTestJaws(t) srv := newProfileTestServer(t, jw, nil) diff --git a/oauth2_test.go b/oauth2_test.go index 162102f..93f53dc 100644 --- a/oauth2_test.go +++ b/oauth2_test.go @@ -463,17 +463,13 @@ func Test_handleAuthResponseUsesPKCEVerifier(t *testing.T) { sess := jw.NewSession(rec, req) setTestOAuthFlow(sess, srv, pendingOAuthFlow{state: wantState, expiry: time.Now().Add(oauth2FlowLifetime), verifier: verifier, nonce: wantNonce, referrer: "/secure"}) - oldSession := sess - srv.HandleAuthResponse(rec, req) - sess = jw.GetSession(req) - if sess == nil || sess == oldSession || oldSession.Cookie().MaxAge >= 0 { - t.Fatal("login did not rotate the session") - } - if oldSession.Get(srv.SessionKey) != nil { - t.Fatal("old session retained authentication") + callback := httptest.NewRecorder() + srv.HandleAuthResponse(callback, req) + if jw.GetSession(req) != sess || sess.Cookie().MaxAge < 0 || len(callback.Result().Cookies()) != 0 { + t.Fatal("login replaced the existing session or cookie") } - resp := rec.Result() + resp := callback.Result() if resp.StatusCode != http.StatusFound { closeResponseBody(t, resp) t.Fatal(resp.Status) @@ -580,14 +576,9 @@ func Test_handleAuthResponseStoredTokenSourceSurvivesRequestCancel(t *testing.T) sess := jw.NewSession(rec, req) setTestOAuthFlow(sess, srv, pendingOAuthFlow{state: "state", expiry: time.Now().Add(oauth2FlowLifetime), verifier: oauth2.GenerateVerifier(), nonce: wantNonce}) - oldSession := sess srv.HandleAuthResponse(rec, req) - sess = jw.GetSession(req) - if sess == nil || sess == oldSession || oldSession.Cookie().MaxAge >= 0 { - t.Fatal("login did not rotate the session") - } - if oldSession.Get(srv.SessionKey) != nil { - t.Fatal("old session retained authentication") + if jw.GetSession(req) != sess || sess.Cookie().MaxAge < 0 { + t.Fatal("login replaced the existing session") } if status := rec.Result().StatusCode; status != http.StatusFound { diff --git a/oauth_flow_grace_test.go b/oauth_flow_grace_test.go index d6cbdc1..5ac59b2 100644 --- a/oauth_flow_grace_test.go +++ b/oauth_flow_grace_test.go @@ -192,6 +192,7 @@ func TestOIDCInitialCallbackAfterProviderVisit(t *testing.T) { Body: io.NopCloser(strings.NewReader(`{"access_token":"access","token_type":"Bearer","id_token":"` + idToken + `"}`))}, nil })}, } + srv.RotateSessionOnLogin = true start := httptest.NewRequest(http.MethodGet, "https://app.example/auth/login", nil) response := httptest.NewRecorder() srv.HandleLogin(response, start) diff --git a/profile_link_test.go b/profile_link_test.go index 8ce42cd..3e4f1e4 100644 --- a/profile_link_test.go +++ b/profile_link_test.go @@ -739,6 +739,7 @@ func newProfileTestServer(t *testing.T, jw *jaws.Jaws, calls *int) *Server { if err != nil || !srv.Valid() { t.Fatalf("profile server: %v", err) } + srv.RotateSessionOnLogin = true return srv } @@ -801,6 +802,42 @@ func profileLogin(t *testing.T, srv *Server) *http.Cookie { return cookies[0] } +func TestOAuthProfileLoginKeepsSessionByDefault(t *testing.T) { + jw := newTestJaws(t) + jw.MaxSessions = 1 + jw.MaxSessionsPerIP = 1 + srv := newProfileTestServer(t, jw, nil) + srv.RotateSessionOnLogin = false + logins, logouts := 0, 0 + srv.LoginEvent = func(*jaws.Session, *http.Request) { logins++ } + srv.LogoutEvent = func(*jaws.Session, *http.Request) { logouts++ } + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + r := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + r.AddCookie(cookie) + sess := jw.GetSession(r) + if sess == nil { + t.Fatal("pending session missing") + } + sess.Set("cart", "kept") + for _, tc := range []struct{ code, subject string }{ + {"first", "9007199254740993"}, + {"second", "99"}, + } { + response := profileCallback(srv, cookie, state, tc.code) + if response.Code != http.StatusFound || len(response.Result().Cookies()) != 0 || + jw.GetSession(r) != sess || sess.Get("cart") != "kept" || + sess.Get(srv.SessionKey).(map[string]any)["sub"] != tc.subject { + t.Fatalf("login %q replaced session or lost data: status=%d cookies=%v", tc.code, response.Code, response.Result().Cookies()) + } + if tc.code == "first" { + _, state = profileStart(t, srv, http.MethodGet, "/auth/login", cookie) + } + } + if logins != 2 || logouts != 0 || jw.SessionCount() != 1 { + t.Fatalf("login events=%d logout events=%d sessions=%d", logins, logouts, jw.SessionCount()) + } +} + func TestOAuthProfileSignInAndDualProofLink(t *testing.T) { synctest.Test(t, func(t *testing.T) { jw := newTestJaws(t) diff --git a/server.go b/server.go index d40fa6e..f0ce179 100644 --- a/server.go +++ b/server.go @@ -64,17 +64,20 @@ type HandleFunc func(uri string, handler http.Handler) // jawsauth calls [Server.LogoutEvent] after releasing SessionRotationLock. // When LoginEvent is set, clearing auth before it begins suppresses both // events; clearing auth during it defers LogoutEvent until it returns. +// LoginEvent can read claims from sess with [Server.SessionKey]. For in-place +// login, [Server.CurrentClaims] becomes available after the event returns. // LogoutEvent receives a nil request for timer cleanup, deferred delivery, or // a call to [Server.Logout] with a nil request. -// When re-login replaces existing auth, its LogoutEvent runs on the old session -// before the new sign-in is stored; calling Logout there cancels the new sign-in. +// With [Server.RotateSessionOnLogin], re-login calls LogoutEvent on the old +// session before storing the new sign-in; calling Logout there cancels it. // For cross-provider cleanup, check the other Server's CurrentClaims before -// calling Logout on the old session; that call can revoke a running sign-in. +// calling Logout; that call can revoke a running sign-in. type EventFunc func(sess *jaws.Session, hr *http.Request) -// FailedFunc is called on callback failures and when a protected request meets -// a running login callback before session rotation. Other flow-start failures -// write their own response. +// FailedFunc handles login and callback failures. +// +// It also handles protected requests during a login callback. Other flow-start +// failures write their own response. // // It returns true if it wrote the HTTP response itself, in which case jawsauth writes // nothing further. Errors can contain untrusted browser-supplied text. Quote errors @@ -105,13 +108,19 @@ type Server struct { HandledPaths map[string]struct{} // URI paths excluded from OAuth return targets LoginEvent EventFunc // called after claims are stored; revocation may suppress or follow it LogoutEvent EventFunc // auth-clear callback; see EventFunc for ordering and nil requests - LoginFailed FailedFunc // callback failures and protected requests during a login callback before rotation + LoginFailed FailedFunc // callback failures and protected requests during a login callback LinkEvent LinkFunc // required by HandleLink; register its POST route separately - // SessionRotationLock is held during flow start, rotation, auth clearing, and LinkEvent. + // SessionRotationLock is held during flow start, login claim storage, auth clearing, and LinkEvent. // New supplies a shared package lock; set the same custom lock on servers using // one JaWS instance when coordinating with other authentication methods. SessionRotationLock sync.Locker Options []oauth2.AuthCodeOption // authorization URL options, e.g. oauth2.AccessTypeOffline; never sent to token exchange + // RotateSessionOnLogin replaces the JaWS Session after a successful login. + // + // It defaults to false. When enabled, login publishes a new cookie, closes + // the old Session, and requires a free slot under JaWS session limits. + // Set it before serving requests. + RotateSessionOnLogin bool // RequireVerifiedAdminEmail requires email_verified for a non-empty admin list. // diff --git a/sessionlifecycle_test.go b/sessionlifecycle_test.go index ade716a..a5cd41b 100644 --- a/sessionlifecycle_test.go +++ b/sessionlifecycle_test.go @@ -226,6 +226,7 @@ func TestLoginRotationExistingAuth(t *testing.T) { jw.MaxSessions = limit factory := &testAuthTimerFactory{} srv := newTimerTestServer(t, jw, "https://issuer.example", factory) + srv.RotateSessionOnLogin = true expiry := time.Now().Add(time.Hour) raw := makeIDToken(t, map[string]any{"iss": "https://issuer.example", "aud": "client", "sub": "one", "exp": expiry.Unix(), "nonce": "nonce", "email": "user@example.com"}) srv.httpClient = &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { @@ -379,6 +380,7 @@ func TestLoginUserInfoDeadlineKeepsOIDCFallbackOptional(t *testing.T) { jw := newTestJaws(t) go jw.ServeWithTimeout(2 * time.Minute) srv := newWrapperTestServer(jw, "https://issuer.example") + srv.RotateSessionOnLogin = true expiry := time.Now().Add(time.Hour) hr := httptest.NewRequest(http.MethodGet, "/callback?state=state&code=code", nil) sess := jw.NewSession(httptest.NewRecorder(), hr) diff --git a/wrapper.go b/wrapper.go index 005573b..67f2938 100644 --- a/wrapper.go +++ b/wrapper.go @@ -1,9 +1,6 @@ package jawsauth -import ( - "net/http" - "time" -) +import "net/http" type wrapper struct { server *Server @@ -13,7 +10,22 @@ type wrapper struct { func (w wrapper) ServeHTTP(hw http.ResponseWriter, hr *http.Request) { sess := w.server.Jaws.GetSession(hr) - if current, present := w.server.sessionAuthStatus(sess, time.Now); !current { + current, present, pending := w.server.sessionAccessStatus(sess) + if pending { + if hr.Method == http.MethodGet { + if w.server.LoginFailed != nil { + email, _ := sess.Get(w.server.SessionEmailKey).(string) + if w.server.LoginFailed(hw, hr, http.StatusConflict, ErrOAuth2LoginInProgress, email) { + return + } + } + w.server.writeResult(hw, http.StatusConflict, ErrOAuth2LoginInProgress) + } else { + w.server.writeResult(hw, http.StatusForbidden, nil) + } + return + } + if !current { if present { w.server.clearSessionAuth(sess, hr, nil, true) } @@ -32,7 +44,7 @@ func (w wrapper) ServeHTTP(hw http.ResponseWriter, hr *http.Request) { w.handler.ServeHTTP(hw, hr) // A revocation may have taken its request snapshot before rendering attached // a new JaWS request. Retire that request before this response completes. - if current, _ := w.server.sessionAuthStatus(sess, time.Now); !current || (w.admin && !w.server.sessionIsAdmin(sess)) { + if current, _, pending := w.server.sessionAccessStatus(sess); !current || pending || (w.admin && !w.server.sessionIsAdmin(sess)) { reloadAuthRequests(sess.Requests()) } } From 23642f40f6a330e1d2e4b17a46d1e0862cb5b1c7 Mon Sep 17 00:00:00 2001 From: Johan Lindh Date: Fri, 6 Nov 2026 11:01:01 +0100 Subject: [PATCH 2/6] Keep in-place login session alive through redirect --- README.md | 7 +++--- authsession.go | 2 -- oauth2.go | 9 +++---- oauth_flow_grace_test.go | 51 ++++++++++++++++++++++++++++++++++++++++ server.go | 3 ++- 5 files changed, 62 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 15a0cfd..96f656d 100644 --- a/README.md +++ b/README.md @@ -68,8 +68,8 @@ login returns 503 with `Retry-After: 60` and preserves the existing session. - `New` shares one process-wide `SessionRotationLock` by default. To coordinate with other authentication methods or isolate independent JaWS instances, set a custom lock before serving; servers sharing a JaWS must use the same lock. - jawsauth holds it during flow start, login claim storage, authentication - clearing and `LinkEvent`. + jawsauth holds it during flow start, rotation, in-place login claim storage, + authentication clearing and `LinkEvent`. Do not take the lock around jawsauth handlers or `Server.Logout`, or within `LinkEvent`; do not call `Server.Logout` from `LinkEvent`. Bound `LinkEvent` I/O with a timeout; other sessions sharing the lock wait. One provider's logout @@ -107,7 +107,8 @@ cookie return 409 unless `LoginFailed` handles them. During in-place login, `CurrentClaims` also remains nil until `LoginEvent` returns. With `RotateSessionOnLogin`, the old cookie starts a new flow after rotation and cannot cancel that sign-in. Without rotation, successful login reloads active -JaWS pages after `LoginEvent`. Logout and auth expiry clear this server's +JaWS pages after `LoginEvent` and gives the Session at least one minute of grace +for the return redirect. Logout and auth expiry clear this server's credentials and reload every JaWS page in the session. Auth expiry preserves a pending login but cancels a pending link. Explicit logout on the live session cancels either flow. A new login on the same server, diff --git a/authsession.go b/authsession.go index cc8a683..68292f2 100644 --- a/authsession.go +++ b/authsession.go @@ -33,7 +33,6 @@ type authTimerState struct { // pendingLoginEvent orders callbacks when authentication is cleared before login notification finishes. type pendingLoginEvent struct { - started bool cleared bool } @@ -246,7 +245,6 @@ func (srv *Server) emitLoginEvent(sess *jaws.Session, hr *http.Request) { srv.mu.Unlock() return } - pending.started = true srv.mu.Unlock() defer func() { srv.mu.Lock() diff --git a/oauth2.go b/oauth2.go index 786758b..066e2ce 100644 --- a/oauth2.go +++ b/oauth2.go @@ -624,7 +624,7 @@ func (srv *Server) completeLoginInPlace(hr *http.Request, sess *jaws.Session, fl rotationLock.Lock() defer rotationLock.Unlock() } - if flow.revoked() || srv.Jaws.GetSession(hr) != sess { + if flow.revoked() || srv.Jaws.GetSession(hr) != sess || !sess.ExtendGrace(time.Minute) { return http.StatusForbidden, ErrOAuth2MissingSession } err = srv.storeSessionAuthClaimsIfActive(sess, claims, tokenSource, expiry, nil, flow.canceled) @@ -675,8 +675,8 @@ func (srv *Server) completeOAuthCallback(ctx context.Context, hw http.ResponseWr // HandleAuthResponse handles the OIDC or OAuth2 profile callback endpoint. // // For GET requests it validates the state, exchanges the authorization code using the -// stored PKCE verifier, verifies the ID token or authenticated profile, and stores -// claims in the existing session. A link flow instead invokes [Server.LinkEvent]. +// stored PKCE verifier, verifies the ID token or authenticated profile, and by +// default stores claims in the existing session. A link flow invokes [Server.LinkEvent]. // It invokes [Server.LoginEvent] after a new sign-in or [Server.LoginFailed] // on login or link failure. Non-GET requests receive 405. // @@ -716,7 +716,8 @@ func (srv *Server) HandleAuthResponse(hw http.ResponseWriter, hr *http.Request) if err == nil && !flow.link { srv.emitLoginEvent(sess, hr) current, _ := srv.sessionAuthStatus(sess, time.Now) - if flow.revoked() || !current || !sessionAlive(sess) { + // Leave a full redirect window after a possibly slow LoginEvent. + if flow.revoked() || !current || !sessionAlive(sess) || (!srv.RotateSessionOnLogin && !sess.ExtendGrace(time.Minute)) { if srv.RotateSessionOnLogin { srv.clearSessionAuth(sess, hr, nil, false) http.SetCookie(hw, sess.Close()) diff --git a/oauth_flow_grace_test.go b/oauth_flow_grace_test.go index 5ac59b2..55727fd 100644 --- a/oauth_flow_grace_test.go +++ b/oauth_flow_grace_test.go @@ -54,6 +54,57 @@ func TestOAuthProviderVisitKeepsOnlyPendingSessionAlive(t *testing.T) { }) } +func TestOAuthInPlaceLoginKeepsSessionForReturnRedirectNearGraceDeadline(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + srv.RotateSessionOnLogin = false + srv.LoginEvent = func(*jaws.Session, *http.Request) { + time.Sleep(45 * time.Second) + } + srv.httpClient.Timeout = oauth2ProviderTimeout + transport := srv.httpClient.Transport + srv.httpClient.Transport = roundTripFunc(func(r *http.Request) (*http.Response, error) { + if r.URL.Path == "/token" { + time.Sleep(oauth2ProviderTimeout - time.Second) + } + return transport.RoundTrip(r) + }) + + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + request := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + request.AddCookie(cookie) + sess := jw.GetSession(request) + if sess == nil { + t.Fatal("pending session missing") + } + time.Sleep(oauth2FlowLifetime - time.Second) + response := profileCallback(srv, cookie, state, "first") + if response.Code != http.StatusFound || len(response.Result().Cookies()) != 0 { + t.Fatalf("near-deadline callback: status=%d cookies=%v body=%q", response.Code, response.Result().Cookies(), response.Body.String()) + } + + // Provider exchange ends three seconds before pending grace; LoginEvent uses 45 seconds of the renewed grace. + time.Sleep(time.Minute - time.Second) + if got := jw.GetSession(request); got != sess { + t.Fatalf("session expired before return redirect: got %p, want %p", got, sess) + } + location := response.Header().Get("Location") + if location == "" { + t.Fatal("return redirect missing") + } + redirect := httptest.NewRequest(http.MethodGet, "https://app.example"+location, nil) + redirect.AddCookie(cookie) + if page := jw.NewRequest(httptest.NewRecorder(), redirect); page.Session() != sess { + t.Fatal("return redirect did not attach to authenticated session") + } + claims, ok := sess.Get(srv.SessionKey).(map[string]any) + if !ok || claims["sub"] != "9007199254740993" { + t.Fatalf("return redirect lost authentication: %v", claims) + } + }) +} + func TestPendingLoginProtectedRequestDoesNotRestartOrRenewGrace(t *testing.T) { synctest.Test(t, func(t *testing.T) { jw := newTestJaws(t) diff --git a/server.go b/server.go index f0ce179..3cfc767 100644 --- a/server.go +++ b/server.go @@ -110,7 +110,8 @@ type Server struct { LogoutEvent EventFunc // auth-clear callback; see EventFunc for ordering and nil requests LoginFailed FailedFunc // callback failures and protected requests during a login callback LinkEvent LinkFunc // required by HandleLink; register its POST route separately - // SessionRotationLock is held during flow start, login claim storage, auth clearing, and LinkEvent. + // SessionRotationLock is held during flow start, rotation, in-place login + // claim storage, auth clearing, and LinkEvent. // New supplies a shared package lock; set the same custom lock on servers using // one JaWS instance when coordinating with other authentication methods. SessionRotationLock sync.Locker From b461335cfc2fa53c3a9a9ad1890a64086fc20b81 Mon Sep 17 00:00:00 2001 From: Johan Lindh Date: Fri, 6 Nov 2026 11:21:51 +0100 Subject: [PATCH 3/6] Retire stale pages and keep logger callbacks outside login lock --- README.md | 4 +-- authsession.go | 39 +++++++++++++++++------- jawsauth.go | 5 ++-- oauth2.go | 40 ++++++++++++++++--------- oauth2_flow_guards_test.go | 61 ++++++++++++++++++++++++++++++++++++++ sessionlifecycle_test.go | 49 ++++++++++++++++++++++++++++++ wrapper.go | 8 ++--- 7 files changed, 174 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 96f656d..2aa37c4 100644 --- a/README.md +++ b/README.md @@ -36,8 +36,8 @@ including an unauthenticated protected GET, can retain its session for about ten minutes even if the client never reaches the provider. By default, login keeps the same session and its application data, so a one-session limit permits login. Set `Server.RotateSessionOnLogin` before serving requests when an attacker can -plant or know a pre-login cookie and use the same client IP JaWS sees for its -owner. +make a victim use an attacker-known pre-login cookie, reach JaWS from the same +client IP JaWS sees for the victim, and cannot learn the later cookie. Rotation discards pre-login application data and needs a free slot under both limits; `MaxSessionsPerIP = 1` then prevents login. If a slot is unavailable, login returns 503 with `Retry-After: 60` and preserves the existing session. diff --git a/authsession.go b/authsession.go index 68292f2..afbd348 100644 --- a/authsession.go +++ b/authsession.go @@ -117,10 +117,13 @@ func (srv *Server) sessionAuthStatus(sess *jaws.Session, now func() time.Time) ( return } -func (srv *Server) sessionAccessStatus(sess *jaws.Session) (current, present, pending bool) { +func (srv *Server) sessionAccessStatus(sess *jaws.Session) (current, present, pending bool, entry *authTimerState) { srv.mu.Lock() current, present = srv.sessionAuthStatus(sess, time.Now) pending = !srv.RotateSessionOnLogin && srv.loginEvents[sess] != nil + if sess != nil { + entry = srv.authTimers[sess.ID()] + } srv.mu.Unlock() return } @@ -172,28 +175,40 @@ func (srv *Server) storeSessionAuthClaimsIfActive(sess *jaws.Session, claims map } email, verified := srv.extractEmail(claims) // Logger callbacks must run outside srv.mu. claims["email_verified"] = verified + delay, replaced, err := srv.storePreparedSessionAuthClaimsIfActive(sess, claims, tokenSource, expiry, entry, loginCanceled, email, verified) + if err == nil { + srv.notifyStoredSessionAuthClaims(sess, expiry, delay, replaced) + } + return err +} + +func (srv *Server) storePreparedSessionAuthClaimsIfActive(sess *jaws.Session, claims map[string]any, tokenSource oauth2.TokenSource, expiry time.Time, entry *authTimerState, loginCanceled <-chan struct{}, email any, verified bool) (delay time.Duration, replaced bool, err error) { srv.mu.Lock() select { case <-loginCanceled: srv.mu.Unlock() - return ErrOAuth2MissingSession + err = ErrOAuth2MissingSession + return default: } - if err := srv.identityExpiryError(expiry); err != nil { + if err = srv.identityExpiryError(expiry); err != nil { srv.mu.Unlock() - return err + return } if entry != nil && srv.authTimers[sess.ID()] != entry { srv.mu.Unlock() - return errAuthTimerStale + err = errAuthTimerStale + return } if !sessionAlive(sess) { srv.mu.Unlock() if entry == nil { - return ErrOAuth2MissingSession + err = ErrOAuth2MissingSession + return } srv.clearSessionAuth(sess, nil, entry, true) - return errAuthTimerStale + err = errAuthTimerStale + return } if entry != nil { previous, _ := sess.Get(srv.SessionKey).(map[string]any) @@ -201,7 +216,8 @@ func (srv *Server) storeSessionAuthClaimsIfActive(sess *jaws.Session, claims map newSubject, _ := claims["sub"].(string) if oldSubject == "" || oldSubject != newSubject { srv.mu.Unlock() - return errOIDC{kind: ErrOIDCInvalidIDToken, cause: errOIDCSubjectMismatch} + err = errOIDC{kind: ErrOIDCInvalidIDToken, cause: errOIDCSubjectMismatch} + return } } sess.Set(srv.SessionKey, claims) @@ -209,7 +225,7 @@ func (srv *Server) storeSessionAuthClaimsIfActive(sess *jaws.Session, claims map sess.Set(srv.sessionKey(oauth2AuthExpiryKey), expiry) sess.Set(srv.SessionEmailKey, email) sess.Set(srv.SessionEmailVerifiedKey, verified) - delay, replaced := srv.scheduleSessionAuthTimerLocked(sess, expiry) + delay, replaced = srv.scheduleSessionAuthTimerLocked(sess, expiry) if loginCanceled != nil { // Keep a new flow from replacing this login before its callback finishes. if srv.loginEvents == nil { @@ -218,6 +234,10 @@ func (srv *Server) storeSessionAuthClaimsIfActive(sess *jaws.Session, claims map srv.loginEvents[sess] = &pendingLoginEvent{} } srv.mu.Unlock() + return +} + +func (srv *Server) notifyStoredSessionAuthClaims(sess *jaws.Session, expiry time.Time, delay time.Duration, replaced bool) { srv.Jaws.Dirty(sess) mode := "oidc" if srv.profileSubject != nil { @@ -230,7 +250,6 @@ func (srv *Server) storeSessionAuthClaimsIfActive(sess *jaws.Session, claims map "delay", delay, "replaced_existing", replaced, ) - return nil } func (srv *Server) emitLoginEvent(sess *jaws.Session, hr *http.Request) { diff --git a/jawsauth.go b/jawsauth.go index 8d65180..97f9af9 100644 --- a/jawsauth.go +++ b/jawsauth.go @@ -10,8 +10,9 @@ import ( // // It returns nil until [Server.LoginEvent] completes for in-place login. // It is useful when several providers share JaWS. -// Treat the returned session map as read-only, and recheck before protected writes -// because the session may close after this call. +// Treat the returned map as read-only. The session may close, and an in-place +// login may change its identity or permissions after this call. Recheck the +// session and claims before protected writes. func (srv *Server) CurrentClaims(sess *jaws.Session) map[string]any { if srv == nil || srv.Jaws == nil || srv.SessionKey == "" || sess == nil || sess.Jaws() != srv.Jaws { return nil diff --git a/oauth2.go b/oauth2.go index 066e2ce..84638b9 100644 --- a/oauth2.go +++ b/oauth2.go @@ -619,27 +619,39 @@ func (srv *Server) completeLogin(hw http.ResponseWriter, hr *http.Request, old * } func (srv *Server) completeLoginInPlace(hr *http.Request, sess *jaws.Session, flow pendingOAuthFlow, claims map[string]any, tokenSource oauth2.TokenSource, expiry time.Time) (status int, err error) { + if flow.revoked() { + return http.StatusForbidden, ErrOAuth2MissingSession + } + // A logger can call Logout, which takes SessionRotationLock. + email, verified := srv.extractEmail(claims) + claims["email_verified"] = verified rotationLock := srv.SessionRotationLock if rotationLock != nil { rotationLock.Lock() - defer rotationLock.Unlock() - } - if flow.revoked() || srv.Jaws.GetSession(hr) != sess || !sess.ExtendGrace(time.Minute) { - return http.StatusForbidden, ErrOAuth2MissingSession } - err = srv.storeSessionAuthClaimsIfActive(sess, claims, tokenSource, expiry, nil, flow.canceled) - if err != nil { - if errors.Is(err, ErrOAuth2MissingSession) { - return http.StatusForbidden, err + status, err = http.StatusForbidden, ErrOAuth2MissingSession + var delay time.Duration + var replaced bool + if !flow.revoked() && srv.Jaws.GetSession(hr) == sess && sess.ExtendGrace(time.Minute) { + delay, replaced, err = srv.storePreparedSessionAuthClaimsIfActive(sess, claims, tokenSource, expiry, nil, flow.canceled, email, verified) + if err == nil { + srv.mu.Lock() + if srv.oauthCallbacks[sess].canceled == flow.canceled { + delete(srv.oauthCallbacks, sess) + } + srv.mu.Unlock() + status = http.StatusFound + } else if !errors.Is(err, ErrOAuth2MissingSession) { + status = http.StatusUnauthorized } - return http.StatusUnauthorized, err } - srv.mu.Lock() - if srv.oauthCallbacks[sess].canceled == flow.canceled { - delete(srv.oauthCallbacks, sess) + if rotationLock != nil { + rotationLock.Unlock() } - srv.mu.Unlock() - return http.StatusFound, nil + if err == nil { + srv.notifyStoredSessionAuthClaims(sess, expiry, delay, replaced) + } + return } func (srv *Server) completeOAuthCallback(ctx context.Context, hw http.ResponseWriter, hr *http.Request, sess *jaws.Session, flow pendingOAuthFlow) (*jaws.Session, int, error) { diff --git a/oauth2_flow_guards_test.go b/oauth2_flow_guards_test.go index 26d8862..e4b0a97 100644 --- a/oauth2_flow_guards_test.go +++ b/oauth2_flow_guards_test.go @@ -174,6 +174,67 @@ func TestDefaultPendingLoginIsNotAuthorized(t *testing.T) { } } +func TestDefaultReloginRetiresLateProtectedRequest(t *testing.T) { + jw := newTestJaws(t) + go jw.Serve() + srv := newProfileTestServer(t, jw, nil) + srv.RotateSessionOnLogin = false + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + if response := profileCallback(srv, cookie, state, "first"); response.Code != http.StatusFound { + t.Fatalf("first login: %d", response.Code) + } + request := httptest.NewRequest(http.MethodGet, "https://app.example/protected", nil) + request.AddCookie(cookie) + sess := jw.GetSession(request) + if sess == nil { + t.Fatal("authenticated session missing") + } + + entered := make(chan string, 1) + resume := make(chan struct{}) + release := sync.OnceFunc(func() { close(resume) }) + defer release() + type latePage struct { + request *jaws.Request + attached bool + } + page := make(chan latePage, 1) + done := make(chan struct{}) + protected := srv.Wrap(http.HandlerFunc(func(hw http.ResponseWriter, hr *http.Request) { + claims := srv.CurrentClaims(sess) + if claims == nil { + entered <- "" + return + } + entered <- claims["sub"].(string) + <-resume + rq := newLiveAuthRequest(t, jw, hr) + page <- latePage{rq, rq.Session() == sess} + hw.WriteHeader(http.StatusOK) + })) + go func() { + protected.ServeHTTP(httptest.NewRecorder(), request) + close(done) + }() + if subject := waitFor(t, entered, "protected request did not start"); subject != "9007199254740993" { + t.Fatalf("initial subject: %q", subject) + } + _, state = profileStart(t, srv, http.MethodGet, "/auth/login", cookie) + if response := profileCallback(srv, cookie, state, "second"); response.Code != http.StatusFound { + t.Fatalf("second login: %d", response.Code) + } + if claims := srv.CurrentClaims(sess); claims == nil || claims["sub"] != "99" { + t.Fatalf("second identity missing: %v", claims) + } + release() + late := waitFor(t, page, "late protected page was not created") + waitFor(t, done, "protected request did not finish") + if !late.attached { + t.Fatal("late page did not attach to the existing session") + } + waitAuthRequestCancelled(t, late.request.Context()) +} + func TestCallbackRevokedAfterStoreBalancesEvents(t *testing.T) { jw := newTestJaws(t) srv := newProfileTestServer(t, jw, nil) diff --git a/sessionlifecycle_test.go b/sessionlifecycle_test.go index a5cd41b..774b495 100644 --- a/sessionlifecycle_test.go +++ b/sessionlifecycle_test.go @@ -7,6 +7,7 @@ import ( "net/http" "net/http/httptest" "strings" + "sync" "testing" "testing/synctest" "time" @@ -46,6 +47,54 @@ func TestLogoutDuringClaimPreparation(t *testing.T) { } } +func TestInPlaceLoginLoggerCanLogout(t *testing.T) { + for _, debug := range []bool{false, true} { + name := "warning" + if debug { + name = "debug" + } + t.Run(name, func(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + srv.RotateSessionOnLogin = false + srv.SessionRotationLock = &sync.Mutex{} + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + if response := profileCallback(srv, cookie, state, "first"); response.Code != http.StatusFound { + t.Fatalf("first login: %d", response.Code) + } + req := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + req.AddCookie(cookie) + sess := jw.GetSession(req) + if sess == nil || srv.CurrentClaims(sess) == nil { + t.Fatal("first login did not authenticate the session") + } + _, state = profileStart(t, srv, http.MethodGet, "/auth/login", cookie) + + loggedOut := false + if debug { + jw.Debug = true + jw.Logger = infoHookLogger(func(msg string) { + if msg == "jawsauth: scheduled auth timer" { + loggedOut = srv.Logout(sess, nil) + } + }) + } else { + jw.Logger = logoutOnWarnLogger{onWarn: func() { loggedOut = srv.Logout(sess, nil) }} + } + result := make(chan *httptest.ResponseRecorder, 1) + go func() { result <- profileCallback(srv, cookie, state, "second") }() + select { + case response := <-result: + if !loggedOut || response.Code != http.StatusForbidden || srv.CurrentClaims(sess) != nil || len(srv.authTimers) != 0 { + t.Fatalf("logout=%t status=%d claims=%v timers=%d", loggedOut, response.Code, srv.CurrentClaims(sess), len(srv.authTimers)) + } + case <-time.After(5 * time.Second): + t.Fatal("login blocked in reentrant logger") + } + }) + } +} + func TestAuthTimerDiscardsClosedSession(t *testing.T) { synctest.Test(t, testAuthTimerDiscardsClosedSession) } diff --git a/wrapper.go b/wrapper.go index 67f2938..fbe88b8 100644 --- a/wrapper.go +++ b/wrapper.go @@ -10,7 +10,7 @@ type wrapper struct { func (w wrapper) ServeHTTP(hw http.ResponseWriter, hr *http.Request) { sess := w.server.Jaws.GetSession(hr) - current, present, pending := w.server.sessionAccessStatus(sess) + current, present, pending, entry := w.server.sessionAccessStatus(sess) if pending { if hr.Method == http.MethodGet { if w.server.LoginFailed != nil { @@ -42,9 +42,9 @@ func (w wrapper) ServeHTTP(hw http.ResponseWriter, hr *http.Request) { return } w.handler.ServeHTTP(hw, hr) - // A revocation may have taken its request snapshot before rendering attached - // a new JaWS request. Retire that request before this response completes. - if current, _, pending := w.server.sessionAccessStatus(sess); !current || pending || (w.admin && !w.server.sessionIsAdmin(sess)) { + // Revocation or in-place re-login may take a request snapshot before rendering + // attaches a new JaWS request. Retire it before this response completes. + if current, _, pending, latestEntry := w.server.sessionAccessStatus(sess); !current || pending || latestEntry != entry || (w.admin && !w.server.sessionIsAdmin(sess)) { reloadAuthRequests(sess.Requests()) } } From 8376717cdcdc57201a3ed704ee4092cad862a9da Mon Sep 17 00:00:00 2001 From: Johan Lindh Date: Fri, 6 Nov 2026 11:27:04 +0100 Subject: [PATCH 4/6] Describe protected requests during login event precisely --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 2aa37c4..7691c16 100644 --- a/README.md +++ b/README.md @@ -102,9 +102,9 @@ Pending OAuth flows expire after ten minutes. jawsauth extends the original JaWS session's grace for the pending flow. Unauthenticated protected GETs reuse a pending login's state, return target and grace deadline while more than two minutes remain; near expiry, they start a new flow and -extend grace. While a login callback runs, protected GETs with that session -cookie return 409 unless `LoginFailed` handles them. During in-place login, -`CurrentClaims` also remains nil until `LoginEvent` returns. With +extend grace. After in-place login stores claims, protected GETs with that +session cookie return 409 while `LoginEvent` runs unless `LoginFailed` handles +them. `CurrentClaims` also remains nil until the event returns. With `RotateSessionOnLogin`, the old cookie starts a new flow after rotation and cannot cancel that sign-in. Without rotation, successful login reloads active JaWS pages after `LoginEvent` and gives the Session at least one minute of grace From 061cca2ab4c13ce5cd28fd01599b5638bccce994 Mon Sep 17 00:00:00 2001 From: Johan Lindh Date: Fri, 6 Nov 2026 11:35:19 +0100 Subject: [PATCH 5/6] Keep login handling focused on concrete failures --- README.md | 21 +++++++------ authsession.go | 5 +--- jawsauth.go | 3 +- oauth2.go | 5 ++-- oauth2_flow_guards_test.go | 61 -------------------------------------- oauth_flow_grace_test.go | 51 ------------------------------- server.go | 7 +++-- wrapper.go | 8 ++--- 8 files changed, 23 insertions(+), 138 deletions(-) diff --git a/README.md b/README.md index 7691c16..23e3bf9 100644 --- a/README.md +++ b/README.md @@ -102,17 +102,16 @@ Pending OAuth flows expire after ten minutes. jawsauth extends the original JaWS session's grace for the pending flow. Unauthenticated protected GETs reuse a pending login's state, return target and grace deadline while more than two minutes remain; near expiry, they start a new flow and -extend grace. After in-place login stores claims, protected GETs with that -session cookie return 409 while `LoginEvent` runs unless `LoginFailed` handles -them. `CurrentClaims` also remains nil until the event returns. With -`RotateSessionOnLogin`, the old cookie starts a new flow after rotation and -cannot cancel that sign-in. Without rotation, successful login reloads active -JaWS pages after `LoginEvent` and gives the Session at least one minute of grace -for the return redirect. Logout and auth expiry clear this server's -credentials and reload every JaWS page in the session. Auth expiry preserves a -pending login but cancels a pending link. Explicit logout on the live session -cancels either flow. A new login on the same server, -including one started by `Wrap` or `Handler`, cancels a pending link. +extend grace. Once in-place login stores claims, protected GETs with that +session cookie return 409 until login notification completes, unless +`LoginFailed` handles them. `CurrentClaims` remains nil during that interval. +With `RotateSessionOnLogin`, the old cookie starts a new flow after +rotation and cannot cancel that sign-in. Without rotation, successful login +reloads active JaWS pages after `LoginEvent`. Logout and auth expiry clear this +server's credentials and reload every JaWS page in the session. Auth expiry +preserves a pending login but cancels a pending link. Explicit logout on the +live session cancels either flow. A new login on the same server, including +one started by `Wrap` or `Handler`, cancels a pending link. Use an HTTP endpoint for a post-logout redirect. When `Server.Logout` clears authentication, it reloads pages and closes their sockets, so a later JaWS diff --git a/authsession.go b/authsession.go index afbd348..3683c05 100644 --- a/authsession.go +++ b/authsession.go @@ -117,13 +117,10 @@ func (srv *Server) sessionAuthStatus(sess *jaws.Session, now func() time.Time) ( return } -func (srv *Server) sessionAccessStatus(sess *jaws.Session) (current, present, pending bool, entry *authTimerState) { +func (srv *Server) sessionAccessStatus(sess *jaws.Session) (current, present, pending bool) { srv.mu.Lock() current, present = srv.sessionAuthStatus(sess, time.Now) pending = !srv.RotateSessionOnLogin && srv.loginEvents[sess] != nil - if sess != nil { - entry = srv.authTimers[sess.ID()] - } srv.mu.Unlock() return } diff --git a/jawsauth.go b/jawsauth.go index 97f9af9..2ca4f6a 100644 --- a/jawsauth.go +++ b/jawsauth.go @@ -8,7 +8,8 @@ import ( // CurrentClaims returns this provider's live authentication claims. // -// It returns nil until [Server.LoginEvent] completes for in-place login. +// During in-place login, it returns nil from claim storage until +// [Server.LoginEvent] completes. // It is useful when several providers share JaWS. // Treat the returned map as read-only. The session may close, and an in-place // login may change its identity or permissions after this call. Recheck the diff --git a/oauth2.go b/oauth2.go index 84638b9..c3d65d2 100644 --- a/oauth2.go +++ b/oauth2.go @@ -632,7 +632,7 @@ func (srv *Server) completeLoginInPlace(hr *http.Request, sess *jaws.Session, fl status, err = http.StatusForbidden, ErrOAuth2MissingSession var delay time.Duration var replaced bool - if !flow.revoked() && srv.Jaws.GetSession(hr) == sess && sess.ExtendGrace(time.Minute) { + if !flow.revoked() && srv.Jaws.GetSession(hr) == sess { delay, replaced, err = srv.storePreparedSessionAuthClaimsIfActive(sess, claims, tokenSource, expiry, nil, flow.canceled, email, verified) if err == nil { srv.mu.Lock() @@ -728,8 +728,7 @@ func (srv *Server) HandleAuthResponse(hw http.ResponseWriter, hr *http.Request) if err == nil && !flow.link { srv.emitLoginEvent(sess, hr) current, _ := srv.sessionAuthStatus(sess, time.Now) - // Leave a full redirect window after a possibly slow LoginEvent. - if flow.revoked() || !current || !sessionAlive(sess) || (!srv.RotateSessionOnLogin && !sess.ExtendGrace(time.Minute)) { + if flow.revoked() || !current || !sessionAlive(sess) { if srv.RotateSessionOnLogin { srv.clearSessionAuth(sess, hr, nil, false) http.SetCookie(hw, sess.Close()) diff --git a/oauth2_flow_guards_test.go b/oauth2_flow_guards_test.go index e4b0a97..26d8862 100644 --- a/oauth2_flow_guards_test.go +++ b/oauth2_flow_guards_test.go @@ -174,67 +174,6 @@ func TestDefaultPendingLoginIsNotAuthorized(t *testing.T) { } } -func TestDefaultReloginRetiresLateProtectedRequest(t *testing.T) { - jw := newTestJaws(t) - go jw.Serve() - srv := newProfileTestServer(t, jw, nil) - srv.RotateSessionOnLogin = false - cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) - if response := profileCallback(srv, cookie, state, "first"); response.Code != http.StatusFound { - t.Fatalf("first login: %d", response.Code) - } - request := httptest.NewRequest(http.MethodGet, "https://app.example/protected", nil) - request.AddCookie(cookie) - sess := jw.GetSession(request) - if sess == nil { - t.Fatal("authenticated session missing") - } - - entered := make(chan string, 1) - resume := make(chan struct{}) - release := sync.OnceFunc(func() { close(resume) }) - defer release() - type latePage struct { - request *jaws.Request - attached bool - } - page := make(chan latePage, 1) - done := make(chan struct{}) - protected := srv.Wrap(http.HandlerFunc(func(hw http.ResponseWriter, hr *http.Request) { - claims := srv.CurrentClaims(sess) - if claims == nil { - entered <- "" - return - } - entered <- claims["sub"].(string) - <-resume - rq := newLiveAuthRequest(t, jw, hr) - page <- latePage{rq, rq.Session() == sess} - hw.WriteHeader(http.StatusOK) - })) - go func() { - protected.ServeHTTP(httptest.NewRecorder(), request) - close(done) - }() - if subject := waitFor(t, entered, "protected request did not start"); subject != "9007199254740993" { - t.Fatalf("initial subject: %q", subject) - } - _, state = profileStart(t, srv, http.MethodGet, "/auth/login", cookie) - if response := profileCallback(srv, cookie, state, "second"); response.Code != http.StatusFound { - t.Fatalf("second login: %d", response.Code) - } - if claims := srv.CurrentClaims(sess); claims == nil || claims["sub"] != "99" { - t.Fatalf("second identity missing: %v", claims) - } - release() - late := waitFor(t, page, "late protected page was not created") - waitFor(t, done, "protected request did not finish") - if !late.attached { - t.Fatal("late page did not attach to the existing session") - } - waitAuthRequestCancelled(t, late.request.Context()) -} - func TestCallbackRevokedAfterStoreBalancesEvents(t *testing.T) { jw := newTestJaws(t) srv := newProfileTestServer(t, jw, nil) diff --git a/oauth_flow_grace_test.go b/oauth_flow_grace_test.go index 55727fd..5ac59b2 100644 --- a/oauth_flow_grace_test.go +++ b/oauth_flow_grace_test.go @@ -54,57 +54,6 @@ func TestOAuthProviderVisitKeepsOnlyPendingSessionAlive(t *testing.T) { }) } -func TestOAuthInPlaceLoginKeepsSessionForReturnRedirectNearGraceDeadline(t *testing.T) { - synctest.Test(t, func(t *testing.T) { - jw := newTestJaws(t) - srv := newProfileTestServer(t, jw, nil) - srv.RotateSessionOnLogin = false - srv.LoginEvent = func(*jaws.Session, *http.Request) { - time.Sleep(45 * time.Second) - } - srv.httpClient.Timeout = oauth2ProviderTimeout - transport := srv.httpClient.Transport - srv.httpClient.Transport = roundTripFunc(func(r *http.Request) (*http.Response, error) { - if r.URL.Path == "/token" { - time.Sleep(oauth2ProviderTimeout - time.Second) - } - return transport.RoundTrip(r) - }) - - cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) - request := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) - request.AddCookie(cookie) - sess := jw.GetSession(request) - if sess == nil { - t.Fatal("pending session missing") - } - time.Sleep(oauth2FlowLifetime - time.Second) - response := profileCallback(srv, cookie, state, "first") - if response.Code != http.StatusFound || len(response.Result().Cookies()) != 0 { - t.Fatalf("near-deadline callback: status=%d cookies=%v body=%q", response.Code, response.Result().Cookies(), response.Body.String()) - } - - // Provider exchange ends three seconds before pending grace; LoginEvent uses 45 seconds of the renewed grace. - time.Sleep(time.Minute - time.Second) - if got := jw.GetSession(request); got != sess { - t.Fatalf("session expired before return redirect: got %p, want %p", got, sess) - } - location := response.Header().Get("Location") - if location == "" { - t.Fatal("return redirect missing") - } - redirect := httptest.NewRequest(http.MethodGet, "https://app.example"+location, nil) - redirect.AddCookie(cookie) - if page := jw.NewRequest(httptest.NewRecorder(), redirect); page.Session() != sess { - t.Fatal("return redirect did not attach to authenticated session") - } - claims, ok := sess.Get(srv.SessionKey).(map[string]any) - if !ok || claims["sub"] != "9007199254740993" { - t.Fatalf("return redirect lost authentication: %v", claims) - } - }) -} - func TestPendingLoginProtectedRequestDoesNotRestartOrRenewGrace(t *testing.T) { synctest.Test(t, func(t *testing.T) { jw := newTestJaws(t) diff --git a/server.go b/server.go index 3cfc767..fde209f 100644 --- a/server.go +++ b/server.go @@ -65,7 +65,8 @@ type HandleFunc func(uri string, handler http.Handler) // When LoginEvent is set, clearing auth before it begins suppresses both // events; clearing auth during it defers LogoutEvent until it returns. // LoginEvent can read claims from sess with [Server.SessionKey]. For in-place -// login, [Server.CurrentClaims] becomes available after the event returns. +// login, new claims become available through [Server.CurrentClaims] after the +// event returns. // LogoutEvent receives a nil request for timer cleanup, deferred delivery, or // a call to [Server.Logout] with a nil request. // With [Server.RotateSessionOnLogin], re-login calls LogoutEvent on the old @@ -76,8 +77,8 @@ type EventFunc func(sess *jaws.Session, hr *http.Request) // FailedFunc handles login and callback failures. // -// It also handles protected requests during a login callback. Other flow-start -// failures write their own response. +// It also handles protected requests during a login callback and login or link +// start conflicts. Other flow-start failures write their own response. // // It returns true if it wrote the HTTP response itself, in which case jawsauth writes // nothing further. Errors can contain untrusted browser-supplied text. Quote errors diff --git a/wrapper.go b/wrapper.go index fbe88b8..2ee2de9 100644 --- a/wrapper.go +++ b/wrapper.go @@ -10,7 +10,7 @@ type wrapper struct { func (w wrapper) ServeHTTP(hw http.ResponseWriter, hr *http.Request) { sess := w.server.Jaws.GetSession(hr) - current, present, pending, entry := w.server.sessionAccessStatus(sess) + current, present, pending := w.server.sessionAccessStatus(sess) if pending { if hr.Method == http.MethodGet { if w.server.LoginFailed != nil { @@ -42,9 +42,9 @@ func (w wrapper) ServeHTTP(hw http.ResponseWriter, hr *http.Request) { return } w.handler.ServeHTTP(hw, hr) - // Revocation or in-place re-login may take a request snapshot before rendering - // attaches a new JaWS request. Retire it before this response completes. - if current, _, pending, latestEntry := w.server.sessionAccessStatus(sess); !current || pending || latestEntry != entry || (w.admin && !w.server.sessionIsAdmin(sess)) { + // A revocation may take its request snapshot before rendering attaches a + // JaWS request. Retire that request before this response completes. + if current, _, pending := w.server.sessionAccessStatus(sess); !current || pending || (w.admin && !w.server.sessionIsAdmin(sess)) { reloadAuthRequests(sess.Requests()) } } From d8c2ed418a8f146b3b96620bd0c39e171872faba Mon Sep 17 00:00:00 2001 From: Johan Lindh Date: Fri, 6 Nov 2026 13:00:53 +0100 Subject: [PATCH 6/6] Clarify login session documentation --- README.md | 25 ++++++++++++------------- jawsauth.go | 3 +-- oauth2.go | 12 ++++++------ server.go | 16 ++++++++-------- 4 files changed, 27 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index 23e3bf9..04307fb 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,8 @@ make a victim use an attacker-known pre-login cookie, reach JaWS from the same client IP JaWS sees for the victim, and cannot learn the later cookie. Rotation discards pre-login application data and needs a free slot under both limits; `MaxSessionsPerIP = 1` then prevents login. If a slot is unavailable, -login returns 503 with `Retry-After: 60` and preserves the existing session. +the default response is 503 with `Retry-After: 60`, and the existing session +remains available. ## Multiple providers @@ -63,8 +64,8 @@ login returns 503 with `Retry-After: 60` and preserves the existing session. Calling `Logout` on that event session cancels it. Other providers' `LogoutEvent` may run later with a nil request. Before using one to log out another provider, check its `CurrentClaims`. - Starting a link replaces this server's pending flow, including a running - login callback. + Starting a link replaces this server's pending flow in the same session, + including a running login callback. - `New` shares one process-wide `SessionRotationLock` by default. To coordinate with other authentication methods or isolate independent JaWS instances, set a custom lock before serving; servers sharing a JaWS must use the same lock. @@ -99,19 +100,17 @@ method proves a live login; check `CurrentClaims` before protected actions. ## Session lifetime Pending OAuth flows expire after ten minutes. jawsauth extends the original -JaWS session's grace for the pending flow. Unauthenticated -protected GETs reuse a pending login's state, return target and grace deadline -while more than two minutes remain; near expiry, they start a new flow and -extend grace. Once in-place login stores claims, protected GETs with that -session cookie return 409 until login notification completes, unless -`LoginFailed` handles them. `CurrentClaims` remains nil during that interval. -With `RotateSessionOnLogin`, the old cookie starts a new flow after -rotation and cannot cancel that sign-in. Without rotation, successful login +JaWS session's grace for the pending flow. Unauthenticated protected GETs reuse +a pending login's state, return target and grace deadline while more than two +minutes remain; near expiry, they start a new flow and extend grace. Once +in-place login stores claims, protected GETs with that session cookie return +409 until login notification completes, unless `LoginFailed` handles them. +`CurrentClaims` remains nil during that interval. Successful in-place login reloads active JaWS pages after `LoginEvent`. Logout and auth expiry clear this server's credentials and reload every JaWS page in the session. Auth expiry preserves a pending login but cancels a pending link. Explicit logout on the -live session cancels either flow. A new login on the same server, including -one started by `Wrap` or `Handler`, cancels a pending link. +live session cancels either flow. A new login on the same server and session, +including one started by `Wrap` or `Handler`, cancels a pending link. Use an HTTP endpoint for a post-logout redirect. When `Server.Logout` clears authentication, it reloads pages and closes their sockets, so a later JaWS diff --git a/jawsauth.go b/jawsauth.go index 2ca4f6a..b9fda18 100644 --- a/jawsauth.go +++ b/jawsauth.go @@ -8,8 +8,7 @@ import ( // CurrentClaims returns this provider's live authentication claims. // -// During in-place login, it returns nil from claim storage until -// [Server.LoginEvent] completes. +// While an in-place login notification is pending, it returns nil. // It is useful when several providers share JaWS. // Treat the returned map as read-only. The session may close, and an in-place // login may change its identity or permissions after this call. Recheck the diff --git a/oauth2.go b/oauth2.go index c3d65d2..9ca850b 100644 --- a/oauth2.go +++ b/oauth2.go @@ -149,10 +149,10 @@ func (srv *Server) returnTarget(hr *http.Request, location string) string { // // For GET requests it stores the state, PKCE verifier and, for OIDC, nonce in the // session, then responds with a 302 redirect to the provider's authorization URL. -// It replaces any pending flow on this server, canceling a running callback or link. +// It replaces any pending flow for this server and session, canceling a running +// callback or link, unless an in-place login notification is pending; then the +// default response is 409. // If no usable session is available, it returns 503 with Retry-After instead. -// Without [Server.RotateSessionOnLogin], it returns 409 while a completed -// callback delivers its login event. // Non-GET requests receive 405 with Allow: GET. func (srv *Server) HandleLogin(hw http.ResponseWriter, hr *http.Request) { if hr.Method != http.MethodGet { @@ -168,9 +168,9 @@ func (srv *Server) HandleLogin(hw http.ResponseWriter, hr *http.Request) { // recheck that identity in LinkEvent when the provider returns. On success it // responds with a 303 redirect to the provider. It returns 405 with Allow: POST // for other methods, 403 for a cross-origin or missing session, and 500 when -// linking is not configured. Without [Server.RotateSessionOnLogin], it returns -// 409 while a completed login callback delivers its event. Starting a link -// replaces any pending flow on this server, including a running login callback. +// linking is not configured. The default response is 409 while an in-place login +// notification is pending. Otherwise, starting a link replaces any pending +// flow for this server and session, including a running login callback. func (srv *Server) HandleLink(hw http.ResponseWriter, hr *http.Request) { switch { case hr.Method != http.MethodPost: diff --git a/server.go b/server.go index fde209f..36bbad2 100644 --- a/server.go +++ b/server.go @@ -64,9 +64,8 @@ type HandleFunc func(uri string, handler http.Handler) // jawsauth calls [Server.LogoutEvent] after releasing SessionRotationLock. // When LoginEvent is set, clearing auth before it begins suppresses both // events; clearing auth during it defers LogoutEvent until it returns. -// LoginEvent can read claims from sess with [Server.SessionKey]. For in-place -// login, new claims become available through [Server.CurrentClaims] after the -// event returns. +// LoginEvent can read claims from sess with [Server.SessionKey]. During in-place +// login, [Server.CurrentClaims] returns nil until the event ends. // LogoutEvent receives a nil request for timer cleanup, deferred delivery, or // a call to [Server.Logout] with a nil request. // With [Server.RotateSessionOnLogin], re-login calls LogoutEvent on the old @@ -75,10 +74,11 @@ type HandleFunc func(uri string, handler http.Handler) // calling Logout; that call can revoke a running sign-in. type EventFunc func(sess *jaws.Session, hr *http.Request) -// FailedFunc handles login and callback failures. +// FailedFunc handles OAuth callback failures and flow conflicts. // -// It also handles protected requests during a login callback and login or link -// start conflicts. Other flow-start failures write their own response. +// It receives login and link start conflicts, and protected GETs while an +// in-place login notification is pending. Other flow-start failures write their +// own response. // // It returns true if it wrote the HTTP response itself, in which case jawsauth writes // nothing further. Errors can contain untrusted browser-supplied text. Quote errors @@ -109,7 +109,7 @@ type Server struct { HandledPaths map[string]struct{} // URI paths excluded from OAuth return targets LoginEvent EventFunc // called after claims are stored; revocation may suppress or follow it LogoutEvent EventFunc // auth-clear callback; see EventFunc for ordering and nil requests - LoginFailed FailedFunc // callback failures and protected requests during a login callback + LoginFailed FailedFunc // callback failures and flow conflicts; see FailedFunc LinkEvent LinkFunc // required by HandleLink; register its POST route separately // SessionRotationLock is held during flow start, rotation, in-place login // claim storage, auth clearing, and LinkEvent. @@ -117,7 +117,7 @@ type Server struct { // one JaWS instance when coordinating with other authentication methods. SessionRotationLock sync.Locker Options []oauth2.AuthCodeOption // authorization URL options, e.g. oauth2.AccessTypeOffline; never sent to token exchange - // RotateSessionOnLogin replaces the JaWS Session after a successful login. + // RotateSessionOnLogin replaces the JaWS Session during login. // // It defaults to false. When enabled, login publishes a new cookie, closes // the old Session, and requires a free slot under JaWS session limits.