Просмотр исходного кода

webapi: require a fresh login for every web client sessio

Sessions are now properly cancelled when the user is signed off.
Mike 2 дней назад
Родитель
Сommit
2f85b2d843

+ 64 - 112
server/webapi/handlers/auth.go

@@ -1,15 +1,16 @@
 package handlers
 package handlers
 
 
 import (
 import (
-	"bytes"
 	"context"
 	"context"
 	"crypto/rand"
 	"crypto/rand"
 	"encoding/base64"
 	"encoding/base64"
 	"encoding/json"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"fmt"
 	"log/slog"
 	"log/slog"
 	"net/http"
 	"net/http"
 	"net/url"
 	"net/url"
+	"strconv"
 	"strings"
 	"strings"
 	"time"
 	"time"
 
 
@@ -60,7 +61,6 @@ type RedirectData struct {
 // AuthHandler handles Web AIM API authentication endpoints.
 // AuthHandler handles Web AIM API authentication endpoints.
 type AuthHandler struct {
 type AuthHandler struct {
 	AuthService AuthService
 	AuthService AuthService
-	CookieBaker CookieBaker
 	Logger      *slog.Logger
 	Logger      *slog.Logger
 }
 }
 
 
@@ -82,9 +82,13 @@ func (h *AuthHandler) GetToken(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 	ctx := r.Context()
 	devID := r.URL.Query().Get("devId")
 	devID := r.URL.Query().Get("devId")
 
 
-	loginID, tokenBytes, ok := h.resolveGetTokenSession(ctx, r)
-	if !ok || loginID == "" {
-		h.Logger.DebugContext(ctx, "getToken: no session, returning redirect",
+	// The cookie is spent either way: consumed on success, and cleared on failure
+	// so a browser holding a dead token stops presenting it.
+	clearBOSTokenCookie(w)
+
+	loginID, authCookie, ok := h.resolveGetTokenSession(r)
+	if !ok {
+		h.Logger.DebugContext(ctx, "getToken: no token, returning redirect",
 			"devId", devID,
 			"devId", devID,
 			"host", r.Host)
 			"host", r.Host)
 		resp := BaseResponse{}
 		resp := BaseResponse{}
@@ -95,27 +99,14 @@ func (h *AuthHandler) GetToken(w http.ResponseWriter, r *http.Request) {
 		return
 		return
 	}
 	}
 
 
-	// Existence of the account is authoritatively enforced downstream by
-	// RegisterBOSSession (during startSession); a token minted here for an
-	// unknown screen name is inert, so no user lookup is needed at this point.
-	if len(tokenBytes) == 0 {
-		var err error
-		tokenBytes, err = h.issueAuthCookie(loginID, devID)
-		if err != nil {
-			h.Logger.ErrorContext(ctx, "getToken: failed to issue token", "error", err, "loginId", loginID)
-			SendError(w, r, http.StatusInternalServerError, "internal server error")
-			return
-		}
-	}
-
 	resp := BaseResponse{}
 	resp := BaseResponse{}
 	resp.Response.StatusCode = 200
 	resp.Response.StatusCode = 200
 	resp.Response.StatusText = "OK"
 	resp.Response.StatusText = "OK"
 	resp.Response.Data = &GetTokenData{
 	resp.Response.Data = &GetTokenData{
 		Token: AuthToken{
 		Token: AuthToken{
-			A: base64.URLEncoding.EncodeToString(tokenBytes),
+			A: base64.URLEncoding.EncodeToString(authCookie),
 			// A string, not a number: that is how the client is given it.
 			// A string, not a number: that is how the client is given it.
-			ExpiresIn: "86400", // todo check this assumption
+			ExpiresIn: strconv.Itoa(int(bosTokenTTL.Seconds())),
 		},
 		},
 		UserData: UserData{Attributes: UserAttributes{LoginID: string(loginID)}},
 		UserData: UserData{Attributes: UserAttributes{LoginID: string(loginID)}},
 	}
 	}
@@ -124,41 +115,19 @@ func (h *AuthHandler) GetToken(w http.ResponseWriter, r *http.Request) {
 	h.Logger.InfoContext(ctx, "getToken succeeded", "loginId", loginID, "devId", devID)
 	h.Logger.InfoContext(ctx, "getToken succeeded", "loginId", loginID, "devId", devID)
 }
 }
 
 
-func (h *AuthHandler) resolveGetTokenSession(ctx context.Context, r *http.Request) (state.DisplayScreenName, []byte, bool) {
-	if token := r.URL.Query().Get("a"); token != "" {
-		if loginID, cookie, ok := h.loginFromToken(token); ok {
-			return loginID, cookie, true
-		}
-	}
-
-	if c, err := r.Cookie("oldAimToken"); err == nil && c.Value != "" {
-		token, err := url.QueryUnescape(c.Value)
-		if err != nil {
-			token = c.Value
-		}
-		if loginID, cookie, ok := h.loginFromToken(token); ok {
-			return loginID, cookie, true
-		}
-	}
-
-	if c, err := r.Cookie("localAuthUser"); err == nil && c.Value != "" {
-		if loginID, ok := parseLocalAuthUser(c.Value); ok {
-			return loginID, nil, true
-		}
+// resolveGetTokenSession identifies the caller from the BOS token parked at
+// sign-in. That cookie is the only credential getToken ever receives: the client
+// sends no token of its own, only f, attributes, devId and r. A token past its
+// brief life fails to crack and reads the same as no token at all.
+func (h *AuthHandler) resolveGetTokenSession(r *http.Request) (state.DisplayScreenName, []byte, bool) {
+	c, err := r.Cookie(bosTokenCookie)
+	if err != nil || c.Value == "" {
+		return "", nil, false
 	}
 	}
-
-	for _, name := range []string{"RSP_USER", "RSP_LOCAL"} {
-		if c, err := r.Cookie(name); err == nil {
-			if loginID, ok := parseRSPCookie(c.Value); ok {
-				return loginID, nil, true
-			}
-		}
+	token, err := url.QueryUnescape(c.Value)
+	if err != nil {
+		token = c.Value
 	}
 	}
-
-	return "", nil, false
-}
-
-func (h *AuthHandler) loginFromToken(token string) (state.DisplayScreenName, []byte, bool) {
 	rawCookie, err := base64.URLEncoding.DecodeString(strings.TrimSpace(token))
 	rawCookie, err := base64.URLEncoding.DecodeString(strings.TrimSpace(token))
 	if err != nil {
 	if err != nil {
 		return "", nil, false
 		return "", nil, false
@@ -170,49 +139,41 @@ func (h *AuthHandler) loginFromToken(token string) (state.DisplayScreenName, []b
 	return serverCookie.ScreenName, rawCookie, true
 	return serverCookie.ScreenName, rawCookie, true
 }
 }
 
 
-func parseLocalAuthUser(value string) (state.DisplayScreenName, bool) {
-	parts := strings.SplitN(value, "||", 2)
-	loginID := strings.TrimSpace(parts[0])
-	if loginID == "" {
-		return "", false
-	}
-	return state.DisplayScreenName(loginID), true
-}
+// errInvalidCredentials reports that the auth service rejected the screen name or
+// password, as opposed to failing to answer at all.
+var errInvalidCredentials = errors.New("invalid screen name or password")
+
+// authenticateCredentials verifies the credentials and returns the auth cookie minted
+// by the OSCAR auth service. It returns errInvalidCredentials when the credentials are
+// rejected.
+func (h *AuthHandler) authenticateCredentials(ctx context.Context, username, password, clientID string) ([]byte, error) {
+	signonFrame := wire.FLAPSignonFrame{}
+	signonFrame.Append(wire.NewTLVBE(wire.LoginTLVTagsScreenName, username))
+	signonFrame.Append(wire.NewTLVBE(wire.LoginTLVTagsPlaintextPassword, password))
+	signonFrame.Append(wire.NewTLVBE(wire.LoginTLVTagsClientIdentity, clientID))
+	signonFrame.Append(wire.NewTLVBE(wire.LoginTLVTagsMultiConnFlags, wire.MultiConnFlagsRecentClient))
 
 
-func parseRSPCookie(value string) (state.DisplayScreenName, bool) {
-	value = strings.TrimSpace(value)
-	if value == "" {
-		return "", false
+	block, err := h.AuthService.FLAPLogin(ctx, signonFrame, "")
+	if err != nil {
+		return nil, fmt.Errorf("FLAPLogin: %w", err)
 	}
 	}
-	if decoded, err := url.QueryUnescape(value); err == nil && decoded != "" {
-		value = decoded
+	if block.HasTag(wire.LoginTLVTagsErrorSubcode) {
+		return nil, errInvalidCredentials
 	}
 	}
-	// RSP cookies typically contain the screen name directly.
-	if strings.ContainsAny(value, " \t\r\n") {
-		return "", false
+	authCookie, ok := block.Bytes(wire.LoginTLVTagsAuthorizationCookie)
+	if !ok {
+		return nil, fmt.Errorf("login response carries no authorization cookie")
 	}
 	}
-	return state.DisplayScreenName(value), true
+	return authCookie, nil
 }
 }
 
 
-func (h *AuthHandler) issueAuthCookie(screenName state.DisplayScreenName, devID string) ([]byte, error) {
-	if h.CookieBaker == nil {
-		return nil, fmt.Errorf("cookie baker not configured")
-	}
-	clientID := devID
-	if clientID == "" {
-		clientID = "WebAIM"
+// clientIDForDevID names the client on the session for callers that only know the
+// Web API devId.
+func clientIDForDevID(devID string) string {
+	if devID == "" {
+		return "WebAIM"
 	}
 	}
-	serverCookie := state.ServerCookie{
-		Service:       wire.BOS,
-		ScreenName:    screenName,
-		ClientID:      clientID,
-		MultiConnFlag: uint8(wire.MultiConnFlagsRecentClient),
-	}
-	buf := &bytes.Buffer{}
-	if err := wire.MarshalBE(serverCookie, buf); err != nil {
-		return nil, err
-	}
-	return h.CookieBaker.Issue(buf.Bytes())
+	return devID
 }
 }
 
 
 func (h *AuthHandler) loginRedirectURL(r *http.Request) string {
 func (h *AuthHandler) loginRedirectURL(r *http.Request) string {
@@ -223,9 +184,10 @@ func (h *AuthHandler) loginRedirectURL(r *http.Request) string {
 	return fmt.Sprintf("%s://%s/_cqr/login/login.psp", scheme, r.Host)
 	return fmt.Sprintf("%s://%s/_cqr/login/login.psp", scheme, r.Host)
 }
 }
 
 
+// Logout sends the browser to the login page. There is nothing to clear: the
+// token cookie is spent by the getToken that signed this client in, and nothing
+// else survives a request.
 func (h *AuthHandler) Logout(w http.ResponseWriter, r *http.Request) {
 func (h *AuthHandler) Logout(w http.ResponseWriter, r *http.Request) {
-	h.clearLoginPSPCookies(w)
-
 	loginURL := h.loginRedirectURL(r)
 	loginURL := h.loginRedirectURL(r)
 	q := url.Values{}
 	q := url.Values{}
 	if devID := r.URL.Query().Get("devId"); devID != "" {
 	if devID := r.URL.Query().Get("devId"); devID != "" {
@@ -260,6 +222,7 @@ func (h *AuthHandler) ClientLogin(w http.ResponseWriter, r *http.Request) {
 		}
 		}
 		username = req.Username
 		username = req.Username
 		password = req.Password
 		password = req.Password
+		devID = req.DevID
 	} else {
 	} else {
 		// Parse form-encoded or URL parameters
 		// Parse form-encoded or URL parameters
 		if err := r.ParseForm(); err != nil {
 		if err := r.ParseForm(); err != nil {
@@ -292,30 +255,19 @@ func (h *AuthHandler) ClientLogin(w http.ResponseWriter, r *http.Request) {
 		return
 		return
 	}
 	}
 
 
-	signonFrame := wire.FLAPSignonFrame{}
-	signonFrame.Append(wire.NewTLVBE(wire.LoginTLVTagsScreenName, username))
-	signonFrame.Append(wire.NewTLVBE(wire.LoginTLVTagsPlaintextPassword, password))
-	signonFrame.Append(wire.NewTLVBE(wire.LoginTLVTagsMultiConnFlags, wire.MultiConnFlagsRecentClient))
-
-	block, err := h.AuthService.FLAPLogin(r.Context(), signonFrame, "")
+	authCookie, err := h.authenticateCredentials(r.Context(), username, password, clientIDForDevID(devID))
 	if err != nil {
 	if err != nil {
-		h.Logger.DebugContext(r.Context(), err.Error())
+		h.Logger.DebugContext(r.Context(), "clientLogin failed", "username", username, "error", err)
+		if errors.Is(err, errInvalidCredentials) {
+			SendError(w, r, http.StatusUnauthorized, "username and password required")
+			return
+		}
 		SendError(w, r, http.StatusInternalServerError, "internal server error")
 		SendError(w, r, http.StatusInternalServerError, "internal server error")
 		return
 		return
 	}
 	}
 
 
-	if block.HasTag(wire.LoginTLVTagsErrorSubcode) {
-		h.Logger.DebugContext(r.Context(), "login failed")
-		SendError(w, r, http.StatusUnauthorized, "username and password required")
-		return
-	}
-
-	authCookie, ok := block.Bytes(wire.OServiceTLVTagsLoginCookie)
-	if !ok {
-		h.Logger.DebugContext(r.Context(), "login cookie not found")
-		SendError(w, r, http.StatusInternalServerError, "internal server error")
-		return
-	}
+	// No cookie here: this endpoint's caller receives the token in the response
+	// body and presents it to startSession itself.
 
 
 	// Generate session secret (for signing subsequent requests)
 	// Generate session secret (for signing subsequent requests)
 	sessionSecret, err := h.generateToken()
 	sessionSecret, err := h.generateToken()
@@ -332,14 +284,14 @@ func (h *AuthHandler) ClientLogin(w http.ResponseWriter, r *http.Request) {
 	resp.Response.Data = &ClientLoginData{
 	resp.Response.Data = &ClientLoginData{
 		Token: AuthToken{
 		Token: AuthToken{
 			A:         base64.URLEncoding.EncodeToString(authCookie),
 			A:         base64.URLEncoding.EncodeToString(authCookie),
-			ExpiresIn: "86400", // 24 hours in seconds
+			ExpiresIn: strconv.Itoa(int(bosTokenTTL.Seconds())),
 		},
 		},
 		LoginID:       username,
 		LoginID:       username,
 		ScreenName:    username,
 		ScreenName:    username,
 		SessionSecret: sessionSecret,
 		SessionSecret: sessionSecret,
 		HostTime:      time.Now().Unix(),
 		HostTime:      time.Now().Unix(),
 		// A number here where token.expiresIn is a string, as the client expects.
 		// A number here where token.expiresIn is a string, as the client expects.
-		TokenExpiresIn: 86400, // 24 hours in seconds
+		TokenExpiresIn: int(bosTokenTTL.Seconds()),
 	}
 	}
 
 
 	// Send response in requested format (JSON, JSONP, XML, or AMF)
 	// Send response in requested format (JSON, JSONP, XML, or AMF)

+ 169 - 44
server/webapi/handlers/auth_test.go

@@ -2,6 +2,7 @@ package handlers
 
 
 import (
 import (
 	"context"
 	"context"
+	"encoding/base64"
 	"errors"
 	"errors"
 	"log/slog"
 	"log/slog"
 	"net/http"
 	"net/http"
@@ -16,9 +17,11 @@ import (
 	"github.com/mk6i/open-oscar-server/wire"
 	"github.com/mk6i/open-oscar-server/wire"
 )
 )
 
 
-// testAuthService implements AuthService for ClientLogin tests (only FLAPLogin is exercised).
+// testAuthService implements AuthService for ClientLogin tests (only FLAPLogin and
+// CrackCookie are exercised).
 type testAuthService struct {
 type testAuthService struct {
-	flapLogin func(ctx context.Context, inFrame wire.FLAPSignonFrame, advertisedHost string) (wire.TLVRestBlock, error)
+	flapLogin   func(ctx context.Context, inFrame wire.FLAPSignonFrame, advertisedHost string) (wire.TLVRestBlock, error)
+	crackCookie func(authCookie []byte) (state.ServerCookie, error)
 }
 }
 
 
 func (t *testAuthService) BUCPChallenge(ctx context.Context, bodyIn wire.SNAC_0x17_0x06_BUCPChallengeRequest, newUUID func() uuid.UUID) (wire.SNACMessage, error) {
 func (t *testAuthService) BUCPChallenge(ctx context.Context, bodyIn wire.SNAC_0x17_0x06_BUCPChallengeRequest, newUUID func() uuid.UUID) (wire.SNACMessage, error) {
@@ -30,9 +33,27 @@ func (t *testAuthService) BUCPLogin(ctx context.Context, bodyIn wire.SNAC_0x17_0
 }
 }
 
 
 func (t *testAuthService) CrackCookie(authCookie []byte) (state.ServerCookie, error) {
 func (t *testAuthService) CrackCookie(authCookie []byte) (state.ServerCookie, error) {
+	if t.crackCookie != nil {
+		return t.crackCookie(authCookie)
+	}
 	return state.ServerCookie{}, nil
 	return state.ServerCookie{}, nil
 }
 }
 
 
+// signedCookieFor stands in for a CookieBaker-signed cookie naming screenName.
+func signedCookieFor(screenName string) []byte {
+	return []byte("signed:" + screenName)
+}
+
+// crackSignedCookie accepts only cookies produced by signedCookieFor, standing in
+// for the signature check the real baker performs.
+func crackSignedCookie(authCookie []byte) (state.ServerCookie, error) {
+	name, ok := strings.CutPrefix(string(authCookie), "signed:")
+	if !ok {
+		return state.ServerCookie{}, errors.New("bad signature")
+	}
+	return state.ServerCookie{ScreenName: state.DisplayScreenName(name)}, nil
+}
+
 func (t *testAuthService) RegisterBOSSession(ctx context.Context, authCookie state.ServerCookie, conf func(sess *state.Session)) (*state.SessionInstance, error) {
 func (t *testAuthService) RegisterBOSSession(ctx context.Context, authCookie state.ServerCookie, conf func(sess *state.Session)) (*state.SessionInstance, error) {
 	return nil, nil
 	return nil, nil
 }
 }
@@ -50,85 +71,95 @@ func (t *testAuthService) SignoutChat(ctx context.Context, sess *state.Session)
 
 
 func successfulLoginBlock() wire.TLVRestBlock {
 func successfulLoginBlock() wire.TLVRestBlock {
 	var b wire.TLVRestBlock
 	var b wire.TLVRestBlock
-	b.Append(wire.NewTLVBE(wire.OServiceTLVTagsLoginCookie, []byte("fake-auth-cookie-bytes")))
+	b.Append(wire.NewTLVBE(wire.LoginTLVTagsAuthorizationCookie, loginBlockCookie))
 	return b
 	return b
 }
 }
 
 
-func failedLoginBlock() wire.TLVRestBlock {
+// loginBlockCookie is the cookie successfulLoginBlock reports as minted by the auth
+// service. Handlers must hand this exact value back rather than mint their own.
+var loginBlockCookie = signedCookieFor("testuser")
+
+// blockWithoutCookie is a login response that reports neither an error nor a cookie.
+func blockWithoutCookie() wire.TLVRestBlock {
 	var b wire.TLVRestBlock
 	var b wire.TLVRestBlock
-	b.Append(wire.NewTLVBE(wire.LoginTLVTagsErrorSubcode, uint16(1)))
+	b.Append(wire.NewTLVBE(wire.LoginTLVTagsScreenName, "testuser"))
 	return b
 	return b
 }
 }
 
 
-type testCookieBaker struct {
-	issue func(data []byte) ([]byte, error)
-}
-
-func (t *testCookieBaker) Issue(data []byte) ([]byte, error) {
-	if t.issue != nil {
-		return t.issue(data)
-	}
-	return []byte("issued-cookie"), nil
-}
-
-func (t *testCookieBaker) Crack(data []byte) ([]byte, error) {
-	return data, nil
+func failedLoginBlock() wire.TLVRestBlock {
+	var b wire.TLVRestBlock
+	b.Append(wire.NewTLVBE(wire.LoginTLVTagsErrorSubcode, uint16(1)))
+	return b
 }
 }
 
 
 func TestAuthHandler_GetToken(t *testing.T) {
 func TestAuthHandler_GetToken(t *testing.T) {
+	validToken := base64.URLEncoding.EncodeToString(signedCookieFor("testuser"))
+
 	tests := []struct {
 	tests := []struct {
-		name         string
-		query        string
-		cookies      []*http.Cookie
-		checkBody    func(*testing.T, string)
-		expectedCode int
+		name      string
+		query     string
+		cookies   []*http.Cookie
+		checkBody func(*testing.T, string)
 	}{
 	}{
 		{
 		{
-			name:  "Success_LocalAuthUserCookie",
+			name:  "Success_TokenCookie",
 			query: "f=json&attributes=loginId&devId=ao1yOLlHVHhsa3o6&c=_callbacks_._0mq8wqdav",
 			query: "f=json&attributes=loginId&devId=ao1yOLlHVHhsa3o6&c=_callbacks_._0mq8wqdav",
 			cookies: []*http.Cookie{
 			cookies: []*http.Cookie{
-				{Name: "localAuthUser", Value: "testuser||Test User"},
+				{Name: bosTokenCookie, Value: validToken},
 			},
 			},
 			checkBody: func(t *testing.T, body string) {
 			checkBody: func(t *testing.T, body string) {
 				assert.Contains(t, body, "_callbacks_._0mq8wqdav(")
 				assert.Contains(t, body, "_callbacks_._0mq8wqdav(")
 				assert.Contains(t, body, `"statusCode":200`)
 				assert.Contains(t, body, `"statusCode":200`)
 				assert.Contains(t, body, `"loginId":"testuser"`)
 				assert.Contains(t, body, `"loginId":"testuser"`)
-				assert.Contains(t, body, `"a":`)
+				// The parked token is handed straight back, not re-minted.
+				assert.Contains(t, body, `"a":"`+validToken+`"`)
+				assert.Contains(t, body, `"expiresIn":"60"`)
 			},
 			},
-			expectedCode: http.StatusOK,
 		},
 		},
 		{
 		{
-			name:  "Unauthorized_NoSession",
+			name:  "Unauthorized_NoCookie",
 			query: "f=json&attributes=loginId&devId=ao1yOLlHVHhsa3o6&c=_callbacks_._abc",
 			query: "f=json&attributes=loginId&devId=ao1yOLlHVHhsa3o6&c=_callbacks_._abc",
 			checkBody: func(t *testing.T, body string) {
 			checkBody: func(t *testing.T, body string) {
 				assert.Contains(t, body, `"statusCode":401`)
 				assert.Contains(t, body, `"statusCode":401`)
 				assert.Contains(t, body, `"redirectURL"`)
 				assert.Contains(t, body, `"redirectURL"`)
 			},
 			},
-			expectedCode: http.StatusOK,
 		},
 		},
 		{
 		{
-			// getToken no longer checks account existence; an unknown screen name
-			// still receives a token. Existence is enforced later by
-			// RegisterBOSSession during startSession.
-			name:  "Success_UnknownUserStillIssuesToken",
+			// A token past its brief life no longer cracks, which is what makes a
+			// later visit sign in again.
+			name:  "Unauthorized_UnsignedToken",
 			query: "f=json&attributes=loginId&devId=dev123&c=_callbacks_._xyz",
 			query: "f=json&attributes=loginId&devId=dev123&c=_callbacks_._xyz",
 			cookies: []*http.Cookie{
 			cookies: []*http.Cookie{
-				{Name: "localAuthUser", Value: "missing||Missing User"},
+				{Name: bosTokenCookie, Value: base64.URLEncoding.EncodeToString([]byte("victim"))},
 			},
 			},
 			checkBody: func(t *testing.T, body string) {
 			checkBody: func(t *testing.T, body string) {
-				assert.Contains(t, body, `"statusCode":200`)
-				assert.Contains(t, body, `"loginId":"missing"`)
-				assert.Contains(t, body, `"a":`)
+				assert.Contains(t, body, `"statusCode":401`)
+				assert.Contains(t, body, `"redirectURL"`)
+				assert.NotContains(t, body, "victim")
+			},
+		},
+		{
+			// The screen name comes only from a signature-verified token, so these
+			// forgeable plaintext cookies must not authenticate anyone.
+			name:  "Unauthorized_ForgedSSOCookies",
+			query: "f=json&attributes=loginId&devId=dev123&c=_callbacks_._xyz",
+			cookies: []*http.Cookie{
+				{Name: "RSP_USER", Value: "victim"},
+				{Name: "RSP_LOCAL", Value: "victim"},
+				{Name: "localAuthUser", Value: "victim||victim"},
+			},
+			checkBody: func(t *testing.T, body string) {
+				assert.Contains(t, body, `"statusCode":401`)
+				assert.Contains(t, body, `"redirectURL"`)
+				assert.NotContains(t, body, "victim")
 			},
 			},
-			expectedCode: http.StatusOK,
 		},
 		},
 	}
 	}
 
 
 	for _, tt := range tests {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			handler := &AuthHandler{
 			handler := &AuthHandler{
-				AuthService: &testAuthService{},
-				CookieBaker: &testCookieBaker{},
+				AuthService: &testAuthService{crackCookie: crackSignedCookie},
 				Logger:      slog.Default(),
 				Logger:      slog.Default(),
 			}
 			}
 
 
@@ -141,14 +172,49 @@ func TestAuthHandler_GetToken(t *testing.T) {
 			rr := httptest.NewRecorder()
 			rr := httptest.NewRecorder()
 			handler.GetToken(rr, req)
 			handler.GetToken(rr, req)
 
 
-			assert.Equal(t, tt.expectedCode, rr.Code)
-			if tt.checkBody != nil {
-				tt.checkBody(t, rr.Body.String())
-			}
+			assert.Equal(t, http.StatusOK, rr.Code)
+			tt.checkBody(t, rr.Body.String())
+
+			// Spent either way, so a reload has nothing to sign in with.
+			assert.True(t, tokenCookieCleared(rr), "getToken should expire the token cookie")
 		})
 		})
 	}
 	}
 }
 }
 
 
+// tokenCookieCleared reports whether the response expires the token cookie.
+func tokenCookieCleared(rr *httptest.ResponseRecorder) bool {
+	for _, c := range rr.Result().Cookies() {
+		if c.Name == bosTokenCookie && c.MaxAge < 0 {
+			return true
+		}
+	}
+	return false
+}
+
+// A second getToken must fail even inside the token's own lifetime: the cookie is
+// gone after the first, so every reload lands on the login page.
+func TestAuthHandler_GetToken_IsOneShot(t *testing.T) {
+	handler := &AuthHandler{
+		AuthService: &testAuthService{crackCookie: crackSignedCookie},
+		Logger:      slog.Default(),
+	}
+	validToken := base64.URLEncoding.EncodeToString(signedCookieFor("testuser"))
+
+	get := func(withCookie bool) string {
+		req := httptest.NewRequest(http.MethodGet, "/auth/getToken?f=json&attributes=loginId&devId=dev1", nil)
+		if withCookie {
+			req.AddCookie(&http.Cookie{Name: bosTokenCookie, Value: validToken})
+		}
+		rr := httptest.NewRecorder()
+		handler.GetToken(rr, req)
+		return rr.Body.String()
+	}
+
+	assert.Contains(t, get(true), `"statusCode":200`)
+	// The browser dropped the cookie, so the follow-up presents nothing.
+	assert.Contains(t, get(false), `"statusCode":401`)
+}
+
 func TestAuthHandler_ClientLogin(t *testing.T) {
 func TestAuthHandler_ClientLogin(t *testing.T) {
 	tests := []struct {
 	tests := []struct {
 		name               string
 		name               string
@@ -172,6 +238,8 @@ func TestAuthHandler_ClientLogin(t *testing.T) {
 			expectedStatusCode: http.StatusOK,
 			expectedStatusCode: http.StatusOK,
 			checkResponse: func(t *testing.T, body string) {
 			checkResponse: func(t *testing.T, body string) {
 				assert.Contains(t, body, `"statusCode":200`)
 				assert.Contains(t, body, `"statusCode":200`)
+				// The token is the cookie the auth service minted, not a re-mint.
+				assert.Contains(t, body, `"a":"`+base64.URLEncoding.EncodeToString(loginBlockCookie)+`"`)
 				assert.Contains(t, body, `"loginId":"testuser"`)
 				assert.Contains(t, body, `"loginId":"testuser"`)
 				assert.Contains(t, body, `"screenName":"testuser"`)
 				assert.Contains(t, body, `"screenName":"testuser"`)
 				assert.Contains(t, body, `"token"`)
 				assert.Contains(t, body, `"token"`)
@@ -257,6 +325,21 @@ func TestAuthHandler_ClientLogin(t *testing.T) {
 				assert.Contains(t, body, "invalid JSON format")
 				assert.Contains(t, body, "invalid JSON format")
 			},
 			},
 		},
 		},
+		{
+			name:        "Error_LoginResponseHasNoCookie",
+			method:      "POST",
+			contentType: "application/json",
+			body:        `{"username":"testuser","password":"testpass"}`,
+			auth: &testAuthService{
+				flapLogin: func(ctx context.Context, inFrame wire.FLAPSignonFrame, advertisedHost string) (wire.TLVRestBlock, error) {
+					return blockWithoutCookie(), nil
+				},
+			},
+			expectedStatusCode: http.StatusInternalServerError,
+			checkResponse: func(t *testing.T, body string) {
+				assert.Contains(t, body, "internal server error")
+			},
+		},
 	}
 	}
 
 
 	for _, tt := range tests {
 	for _, tt := range tests {
@@ -285,3 +368,45 @@ func TestAuthHandler_ClientLogin(t *testing.T) {
 		})
 		})
 	}
 	}
 }
 }
+
+func TestAuthHandler_ClientLogin_SendsClientIdentity(t *testing.T) {
+	tests := []struct {
+		name             string
+		body             string
+		expectedClientID string
+	}{
+		{
+			name:             "DevIDNamesTheClient",
+			body:             `{"username":"testuser","password":"testpass","devId":"dev123"}`,
+			expectedClientID: "dev123",
+		},
+		{
+			name:             "MissingDevIDFallsBack",
+			body:             `{"username":"testuser","password":"testpass"}`,
+			expectedClientID: "WebAIM",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var got wire.FLAPSignonFrame
+			handler := &AuthHandler{
+				AuthService: &testAuthService{
+					flapLogin: func(ctx context.Context, inFrame wire.FLAPSignonFrame, advertisedHost string) (wire.TLVRestBlock, error) {
+						got = inFrame
+						return successfulLoginBlock(), nil
+					},
+				},
+				Logger: slog.Default(),
+			}
+
+			req := httptest.NewRequest(http.MethodPost, "/auth/clientLogin", strings.NewReader(tt.body))
+			req.Header.Set("Content-Type", "application/json")
+			handler.ClientLogin(httptest.NewRecorder(), req)
+
+			clientID, ok := got.String(wire.LoginTLVTagsClientIdentity)
+			assert.True(t, ok, "signon frame should carry a client identity")
+			assert.Equal(t, tt.expectedClientID, clientID)
+		})
+	}
+}

+ 0 - 17
server/webapi/handlers/buddylist_test.go

@@ -891,23 +891,6 @@ func TestRequireSession(t *testing.T) {
 			expectedResponse:   `{"response":{"statusCode":401,"statusText":"invalid or expired session","data":{}}}`,
 			expectedResponse:   `{"response":{"statusCode":401,"statusText":"invalid or expired session","data":{}}}`,
 			expectNextCalled:   false,
 			expectNextCalled:   false,
 		},
 		},
-		{
-			name:   "Error_NilOSCARSession",
-			aimsid: "no-oscar-session",
-			setupMocks: func(sm *MockWebAPISessionManager, aimsid string) {
-				sess := &state.WebAPISession{
-					AimSID:       aimsid,
-					ScreenName:   state.DisplayScreenName("someuser"),
-					LastAccessed: time.Now(),
-					// OSCARSession is nil - a broken server invariant, since
-					// startSession never creates a session without one.
-				}
-				sm.On("GetSession", mock.Anything, aimsid).Return(sess, nil)
-			},
-			expectedStatusCode: http.StatusInternalServerError,
-			expectedResponse:   `{"response":{"statusCode":500,"statusText":"internal server error","data":{}}}`,
-			expectNextCalled:   false,
-		},
 		{
 		{
 			name:   "Success_PassesSessionToNext",
 			name:   "Success_PassesSessionToNext",
 			aimsid: "valid-session",
 			aimsid: "valid-session",

+ 52 - 59
server/webapi/handlers/login_psp.go

@@ -1,19 +1,25 @@
 package handlers
 package handlers
 
 
 import (
 import (
-	"fmt"
+	"encoding/base64"
+	"errors"
 	"html/template"
 	"html/template"
 	"net"
 	"net"
 	"net/http"
 	"net/http"
 	"net/url"
 	"net/url"
 	"strings"
 	"strings"
 	"time"
 	"time"
-
-	"github.com/mk6i/open-oscar-server/state"
-	"github.com/mk6i/open-oscar-server/wire"
 )
 )
 
 
-const loginPSPCookieMaxAge = 86400
+// bosTokenCookie is the cookie the browser presents to getToken. The name is the
+// one AIM's own client knows, kept so a client running against the non-Web API
+// path finds what it expects.
+const bosTokenCookie = "oldAimToken"
+
+// bosTokenTTL mirrors the expiry stamped by HMACCookieBaker.Issue
+// (state/cookie.go). The token only has to survive login.psp -> getToken ->
+// startSession, so its brief life is what makes every later visit sign in again.
+const bosTokenTTL = time.Minute
 
 
 var loginPSPPage = template.Must(template.New("login.psp").Parse(`<!DOCTYPE html>
 var loginPSPPage = template.Must(template.New("login.psp").Parse(`<!DOCTYPE html>
 <html lang="en">
 <html lang="en">
@@ -98,17 +104,23 @@ func (h *AuthHandler) LoginPSP(w http.ResponseWriter, r *http.Request) {
 			return
 			return
 		}
 		}
 
 
-		if err := h.authenticateCredentials(r, loginID, password); err != nil {
-			h.Logger.DebugContext(r.Context(), "login.psp failed", "loginId", loginID, "error", err)
-			data.Error = "Invalid screen name or password."
-			h.renderLoginPSP(w, r, data)
+		authCookie, err := h.authenticateCredentials(r.Context(), loginID, password, clientIDForDevID(data.DevID))
+		if err != nil {
+			if errors.Is(err, errInvalidCredentials) {
+				h.Logger.DebugContext(r.Context(), "login.psp failed", "loginId", loginID)
+				data.Error = "Invalid screen name or password."
+				h.renderLoginPSP(w, r, data)
+				return
+			}
+			h.Logger.ErrorContext(r.Context(), "login.psp could not authenticate", "loginId", loginID, "error", err)
+			http.Error(w, "internal server error", http.StatusInternalServerError)
 			return
 			return
 		}
 		}
 
 
-		screenName := state.DisplayScreenName(loginID)
-		h.setLoginPSPCookies(w, screenName)
+		setBOSTokenCookie(w, authCookie)
+
 		redirectURL := safeLoginRedirectURL(r, data.SuccURL)
 		redirectURL := safeLoginRedirectURL(r, data.SuccURL)
-		h.Logger.InfoContext(r.Context(), "login.psp succeeded", "loginId", screenName, "redirect", redirectURL)
+		h.Logger.InfoContext(r.Context(), "login.psp succeeded", "loginId", loginID, "redirect", redirectURL)
 		http.Redirect(w, r, redirectURL, http.StatusFound)
 		http.Redirect(w, r, redirectURL, http.StatusFound)
 	default:
 	default:
 		http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
 		http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
@@ -126,55 +138,36 @@ func (h *AuthHandler) renderLoginPSP(w http.ResponseWriter, r *http.Request, dat
 	}
 	}
 }
 }
 
 
-func (h *AuthHandler) authenticateCredentials(r *http.Request, username, password string) error {
-	signonFrame := wire.FLAPSignonFrame{}
-	signonFrame.Append(wire.NewTLVBE(wire.LoginTLVTagsScreenName, username))
-	signonFrame.Append(wire.NewTLVBE(wire.LoginTLVTagsPlaintextPassword, password))
-	signonFrame.Append(wire.NewTLVBE(wire.LoginTLVTagsMultiConnFlags, wire.MultiConnFlagsRecentClient))
-
-	block, err := h.AuthService.FLAPLogin(r.Context(), signonFrame, "")
-	if err != nil {
-		return err
-	}
-	if block.HasTag(wire.LoginTLVTagsErrorSubcode) {
-		return fmt.Errorf("login failed")
-	}
-	return nil
-}
-
-func (h *AuthHandler) setLoginPSPCookies(w http.ResponseWriter, screenName state.DisplayScreenName) {
-	loginID := string(screenName)
-	expires := time.Now().Add(loginPSPCookieMaxAge * time.Second)
-	cookie := func(name, value string) *http.Cookie {
-		return &http.Cookie{
-			Name:     name,
-			Value:    value,
-			Path:     "/",
-			Expires:  expires,
-			MaxAge:   loginPSPCookieMaxAge,
-			HttpOnly: false,
-			SameSite: http.SameSiteLaxMode,
-		}
-	}
-	http.SetCookie(w, cookie("RSP_USER", loginID))
-	http.SetCookie(w, cookie("RSP_LOCAL", loginID))
-	http.SetCookie(w, cookie("localAuthUser", loginID+"||"+loginID))
+// setBOSTokenCookie hands the BOS token from the login response to the browser,
+// which carries it as far as the getToken that follows the redirect. It is a
+// bearer credential, so HttpOnly keeps it out of reach of page scripts, and its
+// MaxAge matches the token's own life so the browser drops it on the same
+// schedule the server stops honouring it.
+func setBOSTokenCookie(w http.ResponseWriter, authCookie []byte) {
+	http.SetCookie(w, &http.Cookie{
+		Name:     bosTokenCookie,
+		Value:    base64.URLEncoding.EncodeToString(authCookie),
+		Path:     "/",
+		Expires:  time.Now().Add(bosTokenTTL),
+		MaxAge:   int(bosTokenTTL.Seconds()),
+		HttpOnly: true,
+		SameSite: http.SameSiteLaxMode,
+	})
 }
 }
 
 
-// clearLoginPSPCookies expires the SSO cookies set by setLoginPSPCookies (plus
-// the oldAimToken cookie honored by getToken) so the browser is logged out.
-func (h *AuthHandler) clearLoginPSPCookies(w http.ResponseWriter) {
-	for _, name := range []string{"RSP_USER", "RSP_LOCAL", "localAuthUser", "oldAimToken"} {
-		http.SetCookie(w, &http.Cookie{
-			Name:     name,
-			Value:    "",
-			Path:     "/",
-			Expires:  time.Unix(0, 0),
-			MaxAge:   -1,
-			HttpOnly: false,
-			SameSite: http.SameSiteLaxMode,
-		})
-	}
+// clearBOSTokenCookie expires the token cookie. getToken calls it on every
+// request, spending the token whether or not it was any good, so a reload finds
+// nothing to sign in with.
+func clearBOSTokenCookie(w http.ResponseWriter) {
+	http.SetCookie(w, &http.Cookie{
+		Name:     bosTokenCookie,
+		Value:    "",
+		Path:     "/",
+		Expires:  time.Unix(0, 0),
+		MaxAge:   -1,
+		HttpOnly: true,
+		SameSite: http.SameSiteLaxMode,
+	})
 }
 }
 
 
 func defaultLoginSuccURL(r *http.Request) string {
 func defaultLoginSuccURL(r *http.Request) string {

+ 72 - 17
server/webapi/handlers/login_psp_test.go

@@ -2,6 +2,8 @@ package handlers
 
 
 import (
 import (
 	"context"
 	"context"
+	"encoding/base64"
+	"errors"
 	"log/slog"
 	"log/slog"
 	"net/http"
 	"net/http"
 	"net/http/httptest"
 	"net/http/httptest"
@@ -44,22 +46,16 @@ func TestAuthHandler_Logout(t *testing.T) {
 	assert.Equal(t, "dev1", loc.Query().Get("devId"))
 	assert.Equal(t, "dev1", loc.Query().Get("devId"))
 	assert.Equal(t, "http://localhost:8000/.client/", loc.Query().Get("succUrl"))
 	assert.Equal(t, "http://localhost:8000/.client/", loc.Query().Get("succUrl"))
 
 
-	// SSO cookies are expired so the browser is logged out.
-	cleared := map[string]bool{}
-	for _, c := range rr.Result().Cookies() {
-		if c.MaxAge < 0 {
-			cleared[c.Name] = true
-		}
-	}
-	for _, name := range []string{"RSP_USER", "RSP_LOCAL", "localAuthUser", "oldAimToken"} {
-		assert.True(t, cleared[name], "expected %s cookie to be cleared", name)
-	}
+	// Nothing to clear: getToken spent the token cookie signing this client in.
+	assert.Empty(t, rr.Result().Cookies())
 }
 }
 
 
 func TestAuthHandler_LoginPSP_POST_Success(t *testing.T) {
 func TestAuthHandler_LoginPSP_POST_Success(t *testing.T) {
+	var got wire.FLAPSignonFrame
 	handler := &AuthHandler{
 	handler := &AuthHandler{
 		AuthService: &testAuthService{
 		AuthService: &testAuthService{
 			flapLogin: func(ctx context.Context, inFrame wire.FLAPSignonFrame, advertisedHost string) (wire.TLVRestBlock, error) {
 			flapLogin: func(ctx context.Context, inFrame wire.FLAPSignonFrame, advertisedHost string) (wire.TLVRestBlock, error) {
+				got = inFrame
 				return successfulLoginBlock(), nil
 				return successfulLoginBlock(), nil
 			},
 			},
 		},
 		},
@@ -80,14 +76,73 @@ func TestAuthHandler_LoginPSP_POST_Success(t *testing.T) {
 	assert.Equal(t, http.StatusFound, rr.Code)
 	assert.Equal(t, http.StatusFound, rr.Code)
 	assert.Equal(t, "http://localhost:8000/", rr.Header().Get("Location"))
 	assert.Equal(t, "http://localhost:8000/", rr.Header().Get("Location"))
 
 
-	cookies := rr.Result().Cookies()
-	names := make(map[string]string, len(cookies))
-	for _, c := range cookies {
-		names[c.Name] = c.Value
+	set := make(map[string]*http.Cookie)
+	for _, c := range rr.Result().Cookies() {
+		set[c.Name] = c
+	}
+
+	// The cookie carries the BOS token from the login response, unchanged.
+	tokenCookie := set[bosTokenCookie]
+	if assert.NotNil(t, tokenCookie) {
+		assert.True(t, tokenCookie.HttpOnly)
+		raw, err := base64.URLEncoding.DecodeString(tokenCookie.Value)
+		assert.NoError(t, err)
+		assert.Equal(t, loginBlockCookie, raw)
+		// It outlives the redirect but little else.
+		assert.Equal(t, int(bosTokenTTL.Seconds()), tokenCookie.MaxAge)
+	}
+
+	for _, name := range []string{"RSP_USER", "RSP_LOCAL", "localAuthUser"} {
+		assert.NotContains(t, set, name)
+	}
+
+	// The devId names the client on the resulting session.
+	clientID, ok := got.String(wire.LoginTLVTagsClientIdentity)
+	assert.True(t, ok, "signon frame should carry a client identity")
+	assert.Equal(t, "dev1", clientID)
+}
+
+func TestAuthHandler_LoginPSP_POST_ServiceErrors(t *testing.T) {
+	tests := []struct {
+		name      string
+		flapLogin func(ctx context.Context, inFrame wire.FLAPSignonFrame, advertisedHost string) (wire.TLVRestBlock, error)
+	}{
+		{
+			name: "LoginResponseHasNoCookie",
+			flapLogin: func(ctx context.Context, inFrame wire.FLAPSignonFrame, advertisedHost string) (wire.TLVRestBlock, error) {
+				return blockWithoutCookie(), nil
+			},
+		},
+		{
+			name: "AuthServiceUnreachable",
+			flapLogin: func(ctx context.Context, inFrame wire.FLAPSignonFrame, advertisedHost string) (wire.TLVRestBlock, error) {
+				return wire.TLVRestBlock{}, errors.New("boom")
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			handler := &AuthHandler{
+				AuthService: &testAuthService{flapLogin: tt.flapLogin},
+				Logger:      slog.Default(),
+			}
+
+			form := url.Values{}
+			form.Set("loginId", "testuser")
+			form.Set("password", "secret")
+			req := httptest.NewRequest(http.MethodPost, "/_cqr/login/login.psp", strings.NewReader(form.Encode()))
+			req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+			rr := httptest.NewRecorder()
+
+			handler.LoginPSP(rr, req)
+
+			// A broken auth service must not read as a mistyped password.
+			assert.Equal(t, http.StatusInternalServerError, rr.Code)
+			assert.NotContains(t, rr.Body.String(), "Invalid screen name or password")
+			assert.Empty(t, rr.Result().Cookies())
+		})
 	}
 	}
-	assert.Equal(t, "testuser", names["RSP_USER"])
-	assert.Equal(t, "testuser", names["RSP_LOCAL"])
-	assert.Equal(t, "testuser||testuser", names["localAuthUser"])
 }
 }
 
 
 func TestAuthHandler_LoginPSP_POST_InvalidCredentials(t *testing.T) {
 func TestAuthHandler_LoginPSP_POST_InvalidCredentials(t *testing.T) {

+ 0 - 4
server/webapi/handlers/messaging_test.go

@@ -50,10 +50,6 @@ func (m *MockICBMService) OfflineRetrieve(ctx context.Context, instance *state.S
 }
 }
 
 
 // createTestSessionManager creates a WebAPISessionManager with a pre-populated session.
 // createTestSessionManager creates a WebAPISessionManager with a pre-populated session.
-func createTestSessionManager(screenName string) (*state.WebAPISessionManager, string) {
-	return createTestSessionManagerWithOSCAR(screenName, nil)
-}
-
 // createTestSessionManagerWithOSCAR creates a WebAPISessionManager with an OSCAR session instance set.
 // createTestSessionManagerWithOSCAR creates a WebAPISessionManager with an OSCAR session instance set.
 func createTestSessionManagerWithOSCAR(screenName string, oscarSession *state.SessionInstance) (*state.WebAPISessionManager, string) {
 func createTestSessionManagerWithOSCAR(screenName string, oscarSession *state.SessionInstance) (*state.WebAPISessionManager, string) {
 	mgr := state.NewWebAPISessionManager()
 	mgr := state.NewWebAPISessionManager()

+ 0 - 22
server/webapi/handlers/preference_test.go

@@ -237,28 +237,6 @@ func TestPermitDenyData_PDInfoModeWins(t *testing.T) {
 	assert.Equal(t, []string{"alloweduser"}, got.PermitList)
 	assert.Equal(t, []string{"alloweduser"}, got.PermitList)
 }
 }
 
 
-func TestPreferenceHandler_SetPreferences_NoOSCARSession(t *testing.T) {
-	fs := &MockFeedbagService{}
-	sessionMgr, aimsid := createTestSessionManager("webonly") // nil OSCARSession
-
-	handler := &PreferenceHandler{
-		SessionManager: sessionMgr,
-		FeedbagService: fs,
-		Logger:         slog.Default(),
-	}
-
-	req, _ := http.NewRequest("GET", "/preference/set?aimsid="+aimsid+"&playIMSound=1", nil)
-	rr := httptest.NewRecorder()
-	requireSession(handler.SessionManager, handler.SetPreferences).ServeHTTP(rr, req)
-
-	// A nil OSCARSession is a broken server invariant (guests are unsupported),
-	// so the session middleware rejects it with a 500 before the handler runs
-	// and no feedbag lookup occurs.
-	assert.Equal(t, http.StatusInternalServerError, rr.Code)
-	assert.Contains(t, rr.Body.String(), "internal server error")
-	fs.AssertNotCalled(t, "Query", mock.Anything, mock.Anything, mock.Anything)
-}
-
 // assertPref asserts a preference is carried and holds want.
 // assertPref asserts a preference is carried and holds want.
 func assertPref(t *testing.T, prefs *PreferenceData, name string, want int) {
 func assertPref(t *testing.T, prefs *PreferenceData, name string, want int) {
 	t.Helper()
 	t.Helper()

+ 0 - 3
server/webapi/handlers/presence.go

@@ -612,9 +612,6 @@ func currentWebState(instance *state.SessionInstance) string {
 // to "myInfo" events only, so state changes made via setState/setStatus are
 // to "myInfo" events only, so state changes made via setState/setStatus are
 // invisible in the user's own UI unless a myInfo event is delivered.
 // invisible in the user's own UI unless a myInfo event is delivered.
 func (h *PresenceHandler) pushMyInfo(session *state.WebAPISession, webState, awayMsg, statusMsg string) {
 func (h *PresenceHandler) pushMyInfo(session *state.WebAPISession, webState, awayMsg, statusMsg string) {
-	if session.EventQueue == nil {
-		return
-	}
 	if !session.IsSubscribedTo("myInfo") && !session.IsSubscribedTo("presence") {
 	if !session.IsSubscribedTo("myInfo") && !session.IsSubscribedTo("presence") {
 		return
 		return
 	}
 	}

+ 0 - 20
server/webapi/handlers/presence_test.go

@@ -528,26 +528,6 @@ func TestPresenceHandler_SetState_MyInfoNormalizesAimID(t *testing.T) {
 	assert.Equal(t, "Mike Kelly", myInfo.Friendly)
 	assert.Equal(t, "Mike Kelly", myInfo.Friendly)
 }
 }
 
 
-func TestPresenceHandler_SetState_NoOSCARSession_Rejected(t *testing.T) {
-	// A nil OSCARSession is a broken server invariant (guests are unsupported),
-	// so the session middleware rejects it with a 500 before the handler runs.
-	sessionMgr, aimsid := createTestSessionManager("testuser")
-
-	handler := &PresenceHandler{
-		SessionManager: sessionMgr,
-		Logger:         slog.Default(),
-	}
-
-	req, err := http.NewRequest("GET", "/presence/setState?aimsid="+aimsid+"&state=online", nil)
-	assert.NoError(t, err)
-
-	rr := httptest.NewRecorder()
-	requireSession(handler.SessionManager, handler.SetState).ServeHTTP(rr, req)
-
-	assert.Equal(t, http.StatusInternalServerError, rr.Code)
-	assert.Contains(t, rr.Body.String(), "internal server error")
-}
-
 func TestIsICQScreenName(t *testing.T) {
 func TestIsICQScreenName(t *testing.T) {
 	tests := []struct {
 	tests := []struct {
 		name       string
 		name       string

+ 8 - 13
server/webapi/handlers/session.go

@@ -473,12 +473,10 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 			// would silently fall back to the client's hidden default and, for
 			// would silently fall back to the client's hidden default and, for
 			// showGroups, hide group headers.
 			// showGroups, hide group headers.
 			prefPayload := &PreferenceData{}
 			prefPayload := &PreferenceData{}
-			if session.OSCARSession != nil {
-				if item, err := buddyPrefsItem(ctx, h.FeedbagService, session.OSCARSession); err != nil {
-					h.Logger.ErrorContext(ctx, "failed to get preferences", "err", err.Error())
-				} else {
-					prefPayload = effectiveBuddyPrefs(item.TLVList)
-				}
+			if item, err := buddyPrefsItem(ctx, h.FeedbagService, session.OSCARSession); err != nil {
+				h.Logger.ErrorContext(ctx, "failed to get preferences", "err", err.Error())
+			} else {
+				prefPayload = effectiveBuddyPrefs(item.TLVList)
 			}
 			}
 			data.Events.Preference = prefPayload
 			data.Events.Preference = prefPayload
 			session.EventQueue.Push(types.EventTypePreference, prefPayload)
 			session.EventQueue.Push(types.EventTypePreference, prefPayload)
@@ -488,13 +486,10 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 			// presence state read that model and no-op silently while it is
 			// presence state read that model and no-op silently while it is
 			// empty, so the session has to start with one.
 			// empty, so the session has to start with one.
 			var pdPayload interface{} = PermitDenyData{PDMode: "permitAll"}
 			var pdPayload interface{} = PermitDenyData{PDMode: "permitAll"}
-			if session.OSCARSession != nil {
-				pdd, err := session.PermitDenyRefresher(ctx)
-				if err != nil {
-					h.Logger.ErrorContext(ctx, "failed to get permit/deny settings", "err", err.Error())
-				} else {
-					pdPayload = pdd
-				}
+			if pdd, err := session.PermitDenyRefresher(ctx); err != nil {
+				h.Logger.ErrorContext(ctx, "failed to get permit/deny settings", "err", err.Error())
+			} else {
+				pdPayload = pdd
 			}
 			}
 			data.Events.PermitDeny = pdPayload
 			data.Events.PermitDeny = pdPayload
 			session.EventQueue.Push(types.EventTypePermitDeny, pdPayload)
 			session.EventQueue.Push(types.EventTypePermitDeny, pdPayload)

+ 0 - 6
server/webapi/middleware/auth.go

@@ -164,12 +164,6 @@ func (m *AuthMiddleware) RequireSession(sm WebAPISessionResolver, next func(http
 			m.sendSessionError(w, r, http.StatusUnauthorized, "invalid or expired session")
 			m.sendSessionError(w, r, http.StatusUnauthorized, "invalid or expired session")
 			return
 			return
 		}
 		}
-		// startSession no longer creates sessions without an OSCAR instance, so a
-		// nil here is a server-side invariant violation, not a bad request.
-		if session.OSCARSession == nil {
-			m.sendSessionError(w, r, http.StatusInternalServerError, "internal server error")
-			return
-		}
 		_ = sm.TouchSession(r.Context(), aimsid)
 		_ = sm.TouchSession(r.Context(), aimsid)
 		next(w, r, session)
 		next(w, r, session)
 	})
 	})

+ 0 - 1
server/webapi/server.go

@@ -24,7 +24,6 @@ func NewServer(listeners []string, logger *slog.Logger, handler Handler, apiKeyV
 
 
 	authHandler := &handlers.AuthHandler{
 	authHandler := &handlers.AuthHandler{
 		AuthService: handler.AuthService,
 		AuthService: handler.AuthService,
-		CookieBaker: handler.CookieBaker,
 		Logger:      logger,
 		Logger:      logger,
 	}
 	}
 
 

+ 12 - 11
state/webapi_session.go

@@ -178,10 +178,6 @@ func (s *WebAPISession) IsSubscribedTo(eventType string) bool {
 // StartListeningToOSCARSession starts a goroutine that listens to the OSCAR session's
 // StartListeningToOSCARSession starts a goroutine that listens to the OSCAR session's
 // message channel and converts SNAC messages into WebAPI events.
 // message channel and converts SNAC messages into WebAPI events.
 func (s *WebAPISession) StartListeningToOSCARSession() {
 func (s *WebAPISession) StartListeningToOSCARSession() {
-	if s.OSCARSession == nil {
-		return
-	}
-
 	s.closeMu.Lock()
 	s.closeMu.Lock()
 	defer s.closeMu.Unlock()
 	defer s.closeMu.Unlock()
 	if s.closed {
 	if s.closed {
@@ -201,7 +197,16 @@ func (s *WebAPISession) StartListeningToOSCARSession() {
 				}
 				}
 				s.handleSNACMessage(msg)
 				s.handleSNACMessage(msg)
 			case <-s.OSCARSession.Closed():
 			case <-s.OSCARSession.Closed():
-				// OSCAR session closed
+				// The OSCAR instance went away without this session asking — a
+				// boot, a rate-limit disconnect. Tell the client rather than
+				// leaving its parked fetcher to hang: a sessionEnded event
+				// releases the poll at once and the client signs off on the
+				// spot, instead of waiting out the reaper's next sweep.
+				//
+				// A teardown this session started needs no event, and gets
+				// none: Close closes the queue before it closes the instance,
+				// so this Push is a no-op on that path.
+				s.EventQueue.Push(types.EventTypeSessionEnded, struct{}{})
 				return
 				return
 			}
 			}
 		}
 		}
@@ -229,10 +234,6 @@ func (s *WebAPISession) Close() {
 
 
 // handleSNACMessage converts a SNAC message into WebAPI events and pushes them to the event queue.
 // handleSNACMessage converts a SNAC message into WebAPI events and pushes them to the event queue.
 func (s *WebAPISession) handleSNACMessage(msg wire.SNACMessage) {
 func (s *WebAPISession) handleSNACMessage(msg wire.SNACMessage) {
-	if s.EventQueue == nil {
-		return
-	}
-
 	// Convert SNAC message to WebAPI events based on food group and subgroup
 	// Convert SNAC message to WebAPI events based on food group and subgroup
 	switch msg.Frame.FoodGroup {
 	switch msg.Frame.FoodGroup {
 	case wire.ICBM:
 	case wire.ICBM:
@@ -678,7 +679,7 @@ func (m *WebAPISessionManager) GetSession(ctx context.Context, aimsid string) (*
 	// The aimsid must stop resolving at that point, otherwise a client told to
 	// The aimsid must stop resolving at that point, otherwise a client told to
 	// disconnect could keep issuing charged requests against a dead session (the
 	// disconnect could keep issuing charged requests against a dead session (the
 	// reaper only removes it on time expiry, up to a TTL later).
 	// reaper only removes it on time expiry, up to a TTL later).
-	if session.OSCARSession != nil && session.OSCARSession.IsClosed() {
+	if session.OSCARSession.IsClosed() {
 		return nil, ErrWebAPISessionExpired
 		return nil, ErrWebAPISessionExpired
 	}
 	}
 
 
@@ -763,7 +764,7 @@ func (m *WebAPISessionManager) reapExpired() {
 	now := time.Now()
 	now := time.Now()
 	var expired []*WebAPISession
 	var expired []*WebAPISession
 	for aimsid, session := range m.sessions {
 	for aimsid, session := range m.sessions {
-		if now.After(session.ExpiresAt) || (session.OSCARSession != nil && session.OSCARSession.IsClosed()) {
+		if now.After(session.ExpiresAt) || session.OSCARSession.IsClosed() {
 			delete(m.sessions, aimsid)
 			delete(m.sessions, aimsid)
 			expired = append(expired, session)
 			expired = append(expired, session)
 		}
 		}

+ 56 - 0
state/webapi_session_test.go

@@ -1156,3 +1156,59 @@ func TestWebAPISession_OfflineIM(t *testing.T) {
 		assert.Empty(t, sess.EventQueue.GetAllEvents())
 		assert.Empty(t, sess.EventQueue.GetAllEvents())
 	})
 	})
 }
 }
+
+// A boot closes the account's OSCAR session out from under its web session. The
+// client is parked on a long poll at that moment, so it must be released with a
+// sessionEnded event rather than left to hang until the reaper's next sweep —
+// which measured 26-28s against a running server.
+func TestWebAPISession_BootReleasesParkedFetcherWithSessionEnded(t *testing.T) {
+	mgr := NewWebAPISessionManager()
+	inst := NewSession().AddInstance()
+
+	sess, err := mgr.CreateSession(DisplayScreenName("mike"), "dev", []string{"presence"}, inst, "", slog.Default())
+	require.NoError(t, err)
+	sess.StartListeningToOSCARSession()
+
+	// Park a fetcher the way fetchEvents does, with nothing pending.
+	type result struct {
+		events []types.Event
+		err    error
+	}
+	done := make(chan result, 1)
+	go func() {
+		events, err := sess.EventQueue.Fetch(context.Background(), 0, 60*time.Second)
+		done <- result{events, err}
+	}()
+
+	// Let the fetcher block before the session is taken away.
+	time.Sleep(50 * time.Millisecond)
+
+	inst.Session().CloseSession()
+
+	select {
+	case got := <-done:
+		require.NoError(t, got.err)
+		require.Len(t, got.events, 1)
+		assert.Equal(t, types.EventTypeSessionEnded, got.events[0].Type)
+	case <-time.After(5 * time.Second):
+		t.Fatal("parked fetcher was not released by the boot")
+	}
+}
+
+// A session tearing itself down — endSession, or the idle reaper — needs no
+// sessionEnded event: the client already knows it is leaving. Close closes the
+// queue before the instance, so the listener's push lands on a closed queue.
+func TestWebAPISession_SelfCloseEmitsNoSessionEndedEvent(t *testing.T) {
+	mgr := NewWebAPISessionManager()
+	inst := NewSession().AddInstance()
+
+	sess, err := mgr.CreateSession(DisplayScreenName("mike"), "dev", []string{"presence"}, inst, "", slog.Default())
+	require.NoError(t, err)
+	sess.StartListeningToOSCARSession()
+
+	require.NoError(t, mgr.RemoveSession(context.Background(), sess.AimSID))
+
+	events, err := sess.EventQueue.Fetch(context.Background(), 0, time.Second)
+	require.NoError(t, err)
+	assert.Empty(t, events)
+}