diff --git a/README.md b/README.md index e6f8012..ed16c57 100644 --- a/README.md +++ b/README.md @@ -45,14 +45,16 @@ rotation when setting limits. If no slot is available, the default response is 5 and the existing session's authentication and refresh timer are preserved. Restart login after capacity becomes available. -Logout and auth expiry cancel the session's live JaWS requests. Protected renders -also recheck authorization when the HTTP handler returns, cancelling requests -attached to that session during a concurrent revocation. Already-running handlers -can finish; applications needing authorization at each event must recheck current -claims, expiry, and `JawsAuth.IsAdmin` before acting. Changing `SetAdmins` also cancels -every page of sessions that lose admin access; their ordinary login remains valid. -Clients reconnect and reload. Use an HTTP endpoint for logout redirects: calling -`Request.Redirect` after `Server.Logout` in a JaWS event cannot send the redirect -because that request has been cancelled. Closed or expired JaWS sessions have -their tokens and timers discarded at the next scheduled refresh, which calls +Logout and auth expiry clear authentication and reload the session's JaWS pages. +Connected pages receive the reload command before their sockets close; pending +pages reload when they connect. Protected renders also recheck authorization when +the HTTP handler returns, retiring requests attached during a concurrent revocation. +Events and callbacks run normally until disconnection. Protected operations must +check current claims and expiry, plus `JawsAuth.IsAdmin` for administrator actions. +Changing `SetAdmins` also reloads every page of sessions that lose admin access; +their ordinary login remains valid. +Use an HTTP endpoint for logout redirects: calling `Request.Redirect` after +`Server.Logout` in a JaWS event cannot send the redirect because Reload closes the +connection and discards later messages. Closed or expired JaWS sessions have their +tokens and timers discarded at the next scheduled refresh, which calls `LogoutEvent` with a nil HTTP request. diff --git a/authsession.go b/authsession.go index 99ba1a6..4ac39f3 100644 --- a/authsession.go +++ b/authsession.go @@ -345,13 +345,13 @@ func clearSessionOAuthFlow(sess *jaws.Session) { // Logout clears all authentication state for sess. // // It stops the auth-refresh timer, clears the OIDC claims, token source, email, expiry -// and any in-flight OAuth flow keys, then cancels live JaWS requests, calls -// [Server.LogoutEvent] (if set), and marks the session dirty. It returns false for a +// and any in-flight OAuth flow keys, then reloads its pages with [jaws.Request.Reload], +// calls [Server.LogoutEvent] (if set), and marks the session dirty. It returns false for a // nil receiver or nil session, and true otherwise. The hr argument may be nil. // // It performs no HTTP redirect. An HTTP handler can build its own post-logout // response. A JaWS event handler cannot use [jaws.Request.Redirect] after Logout -// cancels its request; use an HTTP logout endpoint instead. +// queues Reload, which closes the connection; use an HTTP logout endpoint instead. func (srv *Server) Logout(sess *jaws.Session, hr *http.Request) (cleared bool) { return srv.clearSessionAuth(sess, hr, nil) } @@ -374,7 +374,7 @@ func (srv *Server) clearSessionAuth(sess *jaws.Session, hr *http.Request, entry } srv.mu.Unlock() if cleared { - cancelAuthRequests(requests) + reloadAuthRequests(requests) if srv.LogoutEvent != nil { srv.LogoutEvent(sess, hr) } @@ -389,10 +389,10 @@ func (srv *Server) clearSessionAuth(sess *jaws.Session, hr *http.Request, entry return } -func cancelAuthRequests(requests []*jaws.Request) { - // ponytail: JaWS v0.805.0 never reuses Request identities; replace this snapshot - // cancellation with a Session-level API when JaWS provides one. +func reloadAuthRequests(requests []*jaws.Request) { + // ponytail: JaWS never reuses Request identities; replace this snapshot + // reload with a Session-level API when JaWS provides one. for _, rq := range requests { - rq.Cancel(nil) + rq.Reload() } } diff --git a/cmd/go.mod b/cmd/go.mod index 0191e90..40baa1d 100644 --- a/cmd/go.mod +++ b/cmd/go.mod @@ -4,7 +4,7 @@ go 1.27.0 require ( github.com/linkdata/deadlock v0.5.5 - github.com/linkdata/jaws v0.805.0 + github.com/linkdata/jaws v0.808.0 github.com/linkdata/jawsauth v0.0.0 github.com/linkdata/webserv v1.4.2 github.com/moby/moby/api v1.55.0 diff --git a/cmd/go.sum b/cmd/go.sum index 15a56e8..5ecccc5 100644 --- a/cmd/go.sum +++ b/cmd/go.sum @@ -54,8 +54,8 @@ github.com/klauspost/compress v1.19.2 h1:hMRETovs/pu/dVWN7zIT1PGG8t509MwT6bO7XSi github.com/klauspost/compress v1.19.2/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= github.com/linkdata/deadlock v0.5.5 h1:d6O+rzEqasSfamGDA8u7bjtaq7hOX8Ha4Zn36Wxrkvo= github.com/linkdata/deadlock v0.5.5/go.mod h1:tXb28stzAD3trzEEK0UJWC+rZKuobCoPktPYzebb1u0= -github.com/linkdata/jaws v0.805.0 h1:aeMSBFbqaVVQYPTfD8chW+9cZKFu0Bm3hPhJitDllWM= -github.com/linkdata/jaws v0.805.0/go.mod h1:WAjBu+Fgwzf8ydzQJDP6x+WL/5/w5O3bYyFrjtvazAc= +github.com/linkdata/jaws v0.808.0 h1:63JUxe/dARqNIb6FsPKyWO/Aiso3cSgG4T5An+QywnY= +github.com/linkdata/jaws v0.808.0/go.mod h1:WAjBu+Fgwzf8ydzQJDP6x+WL/5/w5O3bYyFrjtvazAc= github.com/linkdata/jq v0.6.0 h1:yxAeWsknIWmt7Wbr5CIEkj2i1WwC2DnJDZlOrPxJkr0= github.com/linkdata/jq v0.6.0/go.mod h1:b76MMuWybyXrVEKSHJkv+r5IEattnghPf1bNRJ5PNlE= github.com/linkdata/secureheaders v1.5.0 h1:4kI290hrVUYqluxF8pQfRYpbZuorSd9+8GlMgrLcrK4= diff --git a/go.mod b/go.mod index a5034a3..b82500d 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,9 @@ module github.com/linkdata/jawsauth go 1.27.0 require ( + github.com/coder/websocket v1.8.15 github.com/coreos/go-oidc/v3 v3.20.0 - github.com/linkdata/jaws v0.805.0 + github.com/linkdata/jaws v0.808.0 github.com/linkdata/secureheaders v1.5.0 golang.org/x/oauth2 v0.36.0 ) @@ -12,7 +13,6 @@ require ( // replace github.com/linkdata/jaws => ../jaws require ( - github.com/coder/websocket v1.8.15 // indirect github.com/go-jose/go-jose/v4 v4.1.4 // indirect github.com/linkdata/deadlock v0.5.5 // indirect github.com/linkdata/jq v0.6.0 // indirect diff --git a/go.sum b/go.sum index ce9090f..6a0be3b 100644 --- a/go.sum +++ b/go.sum @@ -6,8 +6,8 @@ github.com/go-jose/go-jose/v4 v4.1.4 h1:moDMcTHmvE6Groj34emNPLs/qtYXRVcd6S7NHbHz github.com/go-jose/go-jose/v4 v4.1.4/go.mod h1:x4oUasVrzR7071A4TnHLGSPpNOm2a21K9Kf04k1rs08= github.com/linkdata/deadlock v0.5.5 h1:d6O+rzEqasSfamGDA8u7bjtaq7hOX8Ha4Zn36Wxrkvo= github.com/linkdata/deadlock v0.5.5/go.mod h1:tXb28stzAD3trzEEK0UJWC+rZKuobCoPktPYzebb1u0= -github.com/linkdata/jaws v0.805.0 h1:aeMSBFbqaVVQYPTfD8chW+9cZKFu0Bm3hPhJitDllWM= -github.com/linkdata/jaws v0.805.0/go.mod h1:WAjBu+Fgwzf8ydzQJDP6x+WL/5/w5O3bYyFrjtvazAc= +github.com/linkdata/jaws v0.808.0 h1:63JUxe/dARqNIb6FsPKyWO/Aiso3cSgG4T5An+QywnY= +github.com/linkdata/jaws v0.808.0/go.mod h1:WAjBu+Fgwzf8ydzQJDP6x+WL/5/w5O3bYyFrjtvazAc= github.com/linkdata/jq v0.6.0 h1:yxAeWsknIWmt7Wbr5CIEkj2i1WwC2DnJDZlOrPxJkr0= github.com/linkdata/jq v0.6.0/go.mod h1:b76MMuWybyXrVEKSHJkv+r5IEattnghPf1bNRJ5PNlE= github.com/linkdata/secureheaders v1.5.0 h1:4kI290hrVUYqluxF8pQfRYpbZuorSd9+8GlMgrLcrK4= diff --git a/logoutreload_test.go b/logoutreload_test.go new file mode 100644 index 0000000..2e01259 --- /dev/null +++ b/logoutreload_test.go @@ -0,0 +1,77 @@ +package jawsauth + +import ( + "bytes" + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/coder/websocket" + "github.com/linkdata/jaws" + "github.com/linkdata/jaws/lib/what" + "github.com/linkdata/jaws/lib/wire" +) + +func TestLogoutReloadsOtherTabs(t *testing.T) { + jw, err := jaws.New() + if err != nil { + t.Fatal(err) + } + go jw.Serve() + defer jw.Close() + hs := httptest.NewServer(jw) + defer hs.Close() + srv := newTimerTestServer(t, jw, "https://issuer.example", &testAuthTimerFactory{}) + srv.SetAdmins([]string{"admin@example.com"}) + hr := httptest.NewRequest(http.MethodGet, hs.URL+"/", nil) + hr.RemoteAddr = "127.0.0.1:1" + sess := jw.NewSession(httptest.NewRecorder(), hr) + if err = srv.storeSessionAuthClaims(sess, map[string]any{"email": "admin@example.com"}, nil, time.Now().Add(time.Hour), nil); err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(t.Context(), 3*time.Second) + defer cancel() + var conns []*websocket.Conn + for _, admin := range []bool{false, true} { + var rq *jaws.Request + ready := make(chan struct{}) + srv.wrap(http.HandlerFunc(func(hw http.ResponseWriter, hr *http.Request) { + rq = jw.NewRequest(hw, hr) + rq.SetConnectFn(func(*jaws.Request) error { close(ready); return nil }) + }), admin).ServeHTTP(httptest.NewRecorder(), hr.Clone(ctx)) + if rq == nil { + t.Fatal("protected page did not render") + } + conn, _, err := websocket.Dial(ctx, "ws"+strings.TrimPrefix(hs.URL, "http")+"/jaws/"+rq.JawsKeyString(), + &websocket.DialOptions{HTTPHeader: http.Header{"Origin": {hs.URL}}}) + if err != nil { + t.Fatal(err) + } + defer func() { _ = conn.CloseNow() }() + conns = append(conns, conn) + select { + case <-ready: + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + } + srv.Logout(sess, hr) + for tab, conn := range conns { + _, data, err := conn.Read(ctx) + if err != nil { + t.Fatalf("tab %d disconnected without reloading: %v", tab, err) + } + reload := false + for record := range bytes.Lines(data) { + if msg, ok := wire.Parse(record); ok && msg.What == what.Reload { + reload = true + } + } + if !reload { + t.Fatalf("tab %d received %q, want Reload", tab, data) + } + } +} diff --git a/oauth2.go b/oauth2.go index 18d8ecf..7865ef2 100644 --- a/oauth2.go +++ b/oauth2.go @@ -353,7 +353,7 @@ func (srv *Server) HandleAuthResponse(hw http.ResponseWriter, hr *http.Request) hadTimer := srv.authTimers[sess.ID()] != nil srv.stopSessionAuthTimerLocked(sess, nil) srv.mu.Unlock() - cancelAuthRequests(requests) + reloadAuthRequests(requests) if hadAuth && hadTimer && srv.LogoutEvent != nil { srv.LogoutEvent(sess, hr) } diff --git a/server.go b/server.go index 196945c..874d8ea 100644 --- a/server.go +++ b/server.go @@ -218,7 +218,7 @@ func (srv *Server) sessionIsAdminLocked(sess *jaws.Session, admins map[string]st // Non-ASCII characters are matched exactly. If empty, everyone is considered an // administrator. Session checks also apply [Server.RequireVerifiedAdminEmail]. // -// Sessions losing admin access have all their live JaWS requests cancelled; +// Sessions losing admin access have their JaWS requests retired with a page reload; // they remain authenticated and can reopen non-admin pages. func (srv *Server) SetAdmins(emails []string) { if srv == nil { @@ -243,7 +243,7 @@ func (srv *Server) SetAdmins(emails []string) { } srv.admins = admins srv.mu.Unlock() - cancelAuthRequests(requests) + reloadAuthRequests(requests) } // GetAdmins returns a sorted list of the administrator emails. If empty, everyone is considered an administrator. diff --git a/sessionlifecycle_test.go b/sessionlifecycle_test.go index 6699f38..fb3c39f 100644 --- a/sessionlifecycle_test.go +++ b/sessionlifecycle_test.go @@ -12,6 +12,8 @@ import ( "time" "github.com/linkdata/jaws" + "github.com/linkdata/jaws/jawstest" + "github.com/linkdata/jaws/lib/what" "golang.org/x/oauth2" ) @@ -93,7 +95,7 @@ func TestAuthorizationWithdrawalCancelsRequests(t *testing.T) { t.Fatal(err) } go jw.Serve() - defer jw.Close() + t.Cleanup(jw.Close) factory := &testAuthTimerFactory{} srv := newTimerTestServer(t, jw, "https://issuer.example", factory) if action != "restrictEveryone" { @@ -108,7 +110,7 @@ func TestAuthorizationWithdrawalCancelsRequests(t *testing.T) { if err = srv.storeSessionAuthClaims(sess, map[string]any{"email": "user@example.com"}, nil, expiry, nil); err != nil { t.Fatal(err) } - rq := jw.NewRequest(httptest.NewRecorder(), req) + rq := newLiveAuthRequest(t, jw, req) ctx := rq.Context() if rq.Session() != sess || len(sess.Requests()) != 1 { t.Fatal("request not attached") @@ -123,6 +125,7 @@ func TestAuthorizationWithdrawalCancelsRequests(t *testing.T) { default: srv.SetAdmins([]string{"other@example.com"}) } + waitAuthRequestCancelled(t, ctx) if !errors.Is(context.Cause(ctx), context.Canceled) { t.Fatalf("request cancellation cause: %v", context.Cause(ctx)) } @@ -218,6 +221,8 @@ func TestWrapperRevocationDuringRender(t *testing.T) { if err != nil { t.Fatal(err) } + go jw.Serve() + t.Cleanup(jw.Close) srv := newTimerTestServer(t, jw, "https://issuer.example", &testAuthTimerFactory{}) srv.SetAdmins([]string{"user@example.com"}) req := httptest.NewRequest(http.MethodGet, "/protected", nil) @@ -232,13 +237,15 @@ func TestWrapperRevocationDuringRender(t *testing.T) { } else { srv.Logout(sess, hr) } - ctx = jw.NewRequest(hw, hr).Context() + ctx = newLiveAuthRequest(t, jw, hr).Context() }) srv.wrap(h, admin).ServeHTTP(httptest.NewRecorder(), req) + if ctx != nil { + waitAuthRequestCancelled(t, ctx) + } if ctx == nil || ctx.Err() == nil { t.Fatal("request attached after revocation was not cancelled") } - jw.Close() } } @@ -262,7 +269,7 @@ func TestSetAdminsUsesSessionPolicy(t *testing.T) { t.Fatal(err) } go jw.Serve() - defer jw.Close() + t.Cleanup(jw.Close) srv := newWrapperTestServer(jw, "https://issuer.example") srv.RequireVerifiedAdminEmail = tc.strict srv.SetAdmins(tc.before) @@ -274,12 +281,15 @@ func TestSetAdminsUsesSessionPolicy(t *testing.T) { defer srv.Logout(sess, nil) var ctx context.Context srv.WrapAdmin(http.HandlerFunc(func(hw http.ResponseWriter, hr *http.Request) { - ctx = jw.NewRequest(hw, hr).Context() + ctx = newLiveAuthRequest(t, jw, hr).Context() })).ServeHTTP(httptest.NewRecorder(), hr) if ctx == nil || ctx.Err() != nil { t.Fatal("initial admin request not live") } srv.SetAdmins(tc.after) + if !tc.wantAdmin { + waitAuthRequestCancelled(t, ctx) + } if srv.sessionIsAdmin(sess) != tc.wantAdmin || errors.Is(ctx.Err(), context.Canceled) == tc.wantAdmin { t.Fatalf("admin=%v cancellation=%v, want admin=%v", srv.sessionIsAdmin(sess), ctx.Err(), tc.wantAdmin) } @@ -299,7 +309,7 @@ func TestLoginPreparesUserInfoBeforeRotation(t *testing.T) { t.Fatal(err) } go jw.ServeWithTimeout(2 * time.Minute) - defer jw.Close() + t.Cleanup(jw.Close) srv := newWrapperTestServer(jw, "https://issuer.example") expiry := time.Now().Add(time.Hour) hr := httptest.NewRequest(http.MethodGet, "/callback?state=state&code=code", nil) @@ -310,7 +320,7 @@ func TestLoginPreparesUserInfoBeforeRotation(t *testing.T) { defer srv.Logout(sess, nil) var ctx context.Context srv.Wrap(http.HandlerFunc(func(hw http.ResponseWriter, hr *http.Request) { - ctx = jw.NewRequest(hw, hr).Context() + ctx = newLiveAuthRequest(t, jw, hr).Context() })).ServeHTTP(httptest.NewRecorder(), hr) raw := makeIDToken(t, map[string]any{"iss": "https://issuer.example", "aud": "client", "sub": "user", "exp": expiry.Unix(), "nonce": "nonce", "email": "user@example.com"}) userinfoCalls := 0 @@ -339,6 +349,7 @@ func TestLoginPreparesUserInfoBeforeRotation(t *testing.T) { if hw.Code != http.StatusFound || logins != 1 || logouts != 1 || userinfoCalls != 1 { t.Fatalf("status=%d logins=%d logouts=%d userinfo=%d", hw.Code, logins, logouts, userinfoCalls) } + waitAuthRequestCancelled(t, ctx) if current == nil || current == sess || sess.Cookie().MaxAge >= 0 || !errors.Is(ctx.Err(), context.Canceled) { t.Fatal("login did not retire the old session and request") } @@ -347,3 +358,37 @@ func TestLoginPreparesUserInfoBeforeRotation(t *testing.T) { } }) } + +func newLiveAuthRequest(t *testing.T, jw *jaws.Jaws, hr *http.Request) *jaws.Request { + t.Helper() + tr := jawstest.NewTestRequest(jw, hr) + written := make(chan struct{}) + go func() { + defer close(written) + for msg := range tr.OutCh { + if msg.What == what.Reload { + tr.Cancel(nil) // Simulate the writer disconnecting after Reload. + } + } + }() + t.Cleanup(func() { + tr.Close() + <-written + <-tr.DoneCh + }) + select { + case <-tr.ReadyCh: + case <-time.After(3 * time.Second): + t.Fatal("request did not start") + } + return tr.Request +} + +func waitAuthRequestCancelled(t *testing.T, ctx context.Context) { + t.Helper() + select { + case <-ctx.Done(): + case <-time.After(3 * time.Second): + t.Fatal("request did not disconnect after Reload") + } +} diff --git a/wrapper.go b/wrapper.go index aab9836..e29625c 100644 --- a/wrapper.go +++ b/wrapper.go @@ -29,6 +29,6 @@ func (w wrapper) ServeHTTP(hw http.ResponseWriter, hr *http.Request) { // 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)) { - cancelAuthRequests(sess.Requests()) + reloadAuthRequests(sess.Requests()) } }