Keep JaWS session on login by default

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

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,51 +172,69 @@ 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)
 		oldSubject, _ := previous["sub"].(string)
 		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)
 	sess.Set(srv.SessionTokenKey, tokenSource)
 	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)
 			}
 		}

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()

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 {

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
 	}