diff --git a/README.md b/README.md index 7325eac..04307fb 100644 --- a/README.md +++ b/README.md @@ -33,10 +33,15 @@ 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, -login returns 503 with `Retry-After: 60` and preserves the existing session. +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 +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, +the default response is 503 with `Retry-After: 60`, and the existing session +remains available. ## Multiple providers @@ -53,18 +58,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. - Starting a link replaces this server's pending flow, including a running - login callback. + 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 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. - jawsauth holds it during flow start, rotation, 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 @@ -94,15 +100,16 @@ 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. 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, +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 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 diff --git a/authsession.go b/authsession.go index c2f26f9..3683c05 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 } @@ -118,6 +117,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 @@ -165,28 +172,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) @@ -194,7 +213,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) @@ -202,14 +222,19 @@ 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) - if loginCanceled != nil && srv.LoginEvent != nil { + 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 { srv.loginEvents = make(map[*jaws.Session]*pendingLoginEvent) } 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 { @@ -222,31 +247,33 @@ 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) { - if srv.LoginEvent == nil { - return - } srv.mu.Lock() pending := srv.loginEvents[sess] if pending == nil { srv.mu.Unlock() return } - pending.started = true + if pending.cleared { + delete(srv.loginEvents, sess) + srv.mu.Unlock() + return + } srv.mu.Unlock() defer func() { srv.mu.Lock() 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 +542,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 +552,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..b9fda18 100644 --- a/jawsauth.go +++ b/jawsauth.go @@ -6,16 +6,22 @@ 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. -// Treat the returned session map as read-only, and recheck before protected writes -// because the session may close after this call. +// CurrentClaims returns this provider's live authentication claims. +// +// 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 +// 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 } 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..9ca850b 100644 --- a/oauth2.go +++ b/oauth2.go @@ -149,7 +149,9 @@ 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. // Non-GET requests receive 405 with Allow: GET. func (srv *Server) HandleLogin(hw http.ResponseWriter, hr *http.Request) { @@ -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. 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: @@ -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,42 @@ 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) { + 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() + } + status, err = http.StatusForbidden, ErrOAuth2MissingSession + var delay time.Duration + var replaced bool + 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() + 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 + } + } + if rotationLock != nil { + rotationLock.Unlock() + } + 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) { status, err := oauth2CallbackError(http.StatusBadRequest, hr) if err != nil { @@ -641,14 +687,15 @@ 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 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. +// 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. // -// 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 +729,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..36bbad2 100644 --- a/server.go +++ b/server.go @@ -64,17 +64,21 @@ 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]. 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. -// 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 OAuth callback failures and flow conflicts. +// +// 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 @@ -105,13 +109,20 @@ 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 flow conflicts; see FailedFunc 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, 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 Options []oauth2.AuthCodeOption // authorization URL options, e.g. oauth2.AccessTypeOffline; never sent to token exchange + // 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. + // 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..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) } @@ -226,6 +275,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 +429,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..2ee2de9 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) } @@ -30,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, _ := w.server.sessionAuthStatus(sess, time.Now); !current || (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()) } }