webapi: fail fast when BOS registration errors, drop getToken existence check and unused deps

cmd/server/factory.go

@@ -534,14 +534,6 @@ func WebAPI(deps Container) *webapi.Server {
 	)
 
 	handler := webapi.Handler{
-		AdminService: foodgroup.NewAdminService(
-			deps.sqLiteUserStore,
-			deps.sqLiteUserStore,
-			deps.sqLiteUserStore,
-			deps.inMemorySessionManager,
-			deps.inMemorySessionManager,
-			deps.logger,
-		),
 		AuthService: foodgroup.NewAuthService(
 			deps.cfg,
 			deps.inMemorySessionManager,
@@ -558,17 +550,8 @@ func WebAPI(deps Container) *webapi.Server {
 			logger,
 		),
 		BuddyListRegistry: deps.sqLiteUserStore,
-		BuddyService: foodgroup.NewBuddyService(
-			deps.inMemorySessionManager,
-			deps.sqLiteUserStore,
-			deps.sqLiteUserStore,
-			deps.inMemorySessionManager,
-			deps.sqLiteUserStore,
-			deps.sqLiteUserStore,
-		),
-		CookieBaker:      deps.hmacCookieBaker,
-		DirSearchService: foodgroup.NewODirService(logger, deps.sqLiteUserStore),
-		ICBMService:      deps.icbmSvc,
+		CookieBaker:       deps.hmacCookieBaker,
+		ICBMService:       deps.icbmSvc,
 		LocateService: foodgroup.NewLocateService(
 			deps.sqLiteUserStore,
 			deps.inMemorySessionManager,
@@ -593,24 +576,11 @@ func WebAPI(deps Container) *webapi.Server {
 			deps.sqLiteUserStore,
 			deps.sqLiteUserStore,
 		),
-		PermitDenyService: foodgroup.NewPermitDenyService(
-			deps.sqLiteUserStore,
-			deps.sqLiteUserStore,
-			deps.sqLiteUserStore,
-			deps.inMemorySessionManager,
-			deps.inMemorySessionManager,
-		),
-		TOCConfigStore: deps.sqLiteUserStore,
-		ChatService:    foodgroup.NewChatService(deps.chatSessionManager),
-		ChatNavService: foodgroup.NewChatNavService(logger, deps.sqLiteUserStore),
-		SNACRateLimits: deps.snacRateLimits,
 		// New fields for WebAPI handlers
 		SessionRetriever: deps.inMemorySessionManager,
 		// Phase 2 additions
-		MessageRelayer:        deps.inMemorySessionManager,
-		OfflineMessageManager: deps.sqLiteUserStore,
-		BuddyBroadcaster:      oscarBuddyBroadcaster,
-		ProfileManager:        deps.sqLiteUserStore,
+		BuddyBroadcaster: oscarBuddyBroadcaster,
+		ProfileManager:   deps.sqLiteUserStore,
 		// Phase 3 additions
 		PreferenceManager: deps.sqLiteUserStore.NewWebPreferenceManager(),
 		// Phase 4 additions for OSCAR Bridge

server/webapi/handler.go

@@ -8,32 +8,21 @@ import (
 	"net/http"
 
 	"github.com/mk6i/open-oscar-server/state"
-	"github.com/mk6i/open-oscar-server/wire"
 )
 
 type Handler struct {
-	AdminService      AdminService
 	AuthService       AuthService
 	BuddyListRegistry BuddyListRegistry
-	BuddyService      BuddyService
-	ChatNavService    ChatNavService
-	ChatService       ChatService
 	CookieBaker       CookieBaker
-	DirSearchService  DirSearchService
 	ICBMService       ICBMService
 	LocateService     LocateService
 	Logger            *slog.Logger
 	OServiceService   OServiceService
-	PermitDenyService PermitDenyService
-	TOCConfigStore    TOCConfigStore
-	SNACRateLimits    wire.SNACRateLimits
 	// New fields for WebAPI handlers
 	SessionRetriever SessionRetriever
 	// Phase 2 additions
-	MessageRelayer        MessageRelayer
-	OfflineMessageManager OfflineMessageManager
-	BuddyBroadcaster      BuddyBroadcaster
-	ProfileManager        ProfileManager
+	BuddyBroadcaster BuddyBroadcaster
+	ProfileManager   ProfileManager
 	// Phase 3 additions
 	PreferenceManager PreferenceManager
 	// Phase 4 additions for OSCAR Bridge

server/webapi/handlers/auth.go

@@ -21,15 +21,9 @@ import (
 type AuthHandler struct {
 	AuthService AuthService
 	CookieBaker CookieBaker
-	UserManager UserRetriever
 	Logger      *slog.Logger
 }
 
-// UserRetriever looks up local AIM accounts.
-type UserRetriever interface {
-	User(ctx context.Context, screenName state.IdentScreenName) (*state.User, error)
-}
-
 type OServiceService interface {
 	ClientOnline(ctx context.Context, service uint16, inBody wire.SNAC_0x01_0x02_OServiceClientOnline, instance *state.SessionInstance) error
 }
@@ -62,26 +56,9 @@ func (h *AuthHandler) GetToken(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if h.UserManager != nil {
-		user, err := h.UserManager.User(ctx, loginID.IdentScreenName())
-		if err != nil {
-			h.Logger.ErrorContext(ctx, "getToken: user lookup failed", "error", err, "loginId", loginID)
-			SendError(w, http.StatusInternalServerError, "internal server error")
-			return
-		}
-		if user == nil {
-			h.Logger.DebugContext(ctx, "getToken: user not found", "loginId", loginID)
-			resp := BaseResponse{}
-			resp.Response.StatusCode = 401
-			resp.Response.StatusText = "Unauthorized"
-			resp.Response.Data = map[string]interface{}{
-				"redirectURL": h.loginRedirectURL(r),
-			}
-			SendResponse(w, r, resp, h.Logger)
-			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)
@@ -98,7 +75,7 @@ func (h *AuthHandler) GetToken(w http.ResponseWriter, r *http.Request) {
 	resp.Response.Data = map[string]interface{}{
 		"token": map[string]interface{}{
 			"a":         base64.URLEncoding.EncodeToString(tokenBytes),
-			"expiresIn": "86400",
+			"expiresIn": "86400", // todo check this assumption
 		},
 		"userData": map[string]interface{}{
 			"attributes": map[string]interface{}{

server/webapi/handlers/auth_test.go

@@ -75,24 +75,11 @@ func (t *testCookieBaker) Crack(data []byte) ([]byte, error) {
 	return data, nil
 }
 
-type testUserRetriever struct {
-	user *state.User
-	err  error
-}
-
-func (t *testUserRetriever) User(ctx context.Context, screenName state.IdentScreenName) (*state.User, error) {
-	if t.err != nil {
-		return nil, t.err
-	}
-	return t.user, nil
-}
-
 func TestAuthHandler_GetToken(t *testing.T) {
 	tests := []struct {
 		name         string
 		query        string
 		cookies      []*http.Cookie
-		user         *state.User
 		checkBody    func(*testing.T, string)
 		expectedCode int
 	}{
@@ -102,7 +89,6 @@ func TestAuthHandler_GetToken(t *testing.T) {
 			cookies: []*http.Cookie{
 				{Name: "localAuthUser", Value: "testuser||Test User"},
 			},
-			user: &state.User{},
 			checkBody: func(t *testing.T, body string) {
 				assert.Contains(t, body, "_callbacks_._0mq8wqdav(")
 				assert.Contains(t, body, `"statusCode":200`)
@@ -121,15 +107,18 @@ func TestAuthHandler_GetToken(t *testing.T) {
 			expectedCode: http.StatusOK,
 		},
 		{
-			name:  "Unauthorized_UnknownUser",
-			query: "f=json&attributes=loginId&devId=dev123",
+			// 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",
+			query: "f=json&attributes=loginId&devId=dev123&c=_callbacks_._xyz",
 			cookies: []*http.Cookie{
 				{Name: "localAuthUser", Value: "missing||Missing User"},
 			},
-			user: nil,
 			checkBody: func(t *testing.T, body string) {
-				assert.Contains(t, body, `"statusCode":401`)
-				assert.Contains(t, body, `"redirectURL"`)
+				assert.Contains(t, body, `"statusCode":200`)
+				assert.Contains(t, body, `"loginId":"missing"`)
+				assert.Contains(t, body, `"a":`)
 			},
 			expectedCode: http.StatusOK,
 		},
@@ -140,7 +129,6 @@ func TestAuthHandler_GetToken(t *testing.T) {
 			handler := &AuthHandler{
 				AuthService: &testAuthService{},
 				CookieBaker: &testCookieBaker{},
-				UserManager: &testUserRetriever{user: tt.user},
 				Logger:      slog.Default(),
 			}
 

server/webapi/handlers/session.go

@@ -249,66 +249,68 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 		oscarInstance, err = h.OSCARAuthService.RegisterBOSSession(ctx, cookie, fnCfg)
 
 		if err != nil {
+			// A failed BOS registration (e.g. the per-user session cap) leaves no
+			// OSCAR session. Downstream steps (buddy list, feedbag, presence) all
+			// dereference it, so fail the request instead of continuing half-set-up.
 			h.Logger.ErrorContext(ctx, "failed to create OSCAR session", "err", err.Error())
-			// Continue without OSCAR session - WebAPI can work standalone
-			// todo wat
-			oscarInstance = nil
-		} else {
-			if err = oscarInstance.Session().RunOnce(func() error {
-				// make buddy list visible to other users
-				if err := h.BuddyListRegistry.RegisterBuddyList(ctx, oscarInstance.IdentScreenName()); err != nil {
-					return fmt.Errorf("unable to init buddy list: %w", err)
-				}
-				// restore warning level from last session
-				if err := h.RecalcWarning(ctx, oscarInstance); err != nil {
-					return fmt.Errorf("failed to recalculate warning level: %w", err)
-				}
-				// periodically decay warning level
-				go h.LowerWarnLevel(ctx, oscarInstance)
-				return nil
-			}); err != nil {
-				h.Logger.ErrorContext(ctx, "failed to init session", "err", err.Error())
-				h.sendError(w, r, http.StatusInternalServerError, "internal server error")
-				return
+			h.sendError(w, r, http.StatusServiceUnavailable, "unable to establish session")
+			return
+		}
+
+		if err = oscarInstance.Session().RunOnce(func() error {
+			// make buddy list visible to other users
+			if err := h.BuddyListRegistry.RegisterBuddyList(ctx, oscarInstance.IdentScreenName()); err != nil {
+				return fmt.Errorf("unable to init buddy list: %w", err)
 			}
+			// restore warning level from last session
+			if err := h.RecalcWarning(ctx, oscarInstance); err != nil {
+				return fmt.Errorf("failed to recalculate warning level: %w", err)
+			}
+			// periodically decay warning level
+			go h.LowerWarnLevel(ctx, oscarInstance)
+			return nil
+		}); err != nil {
+			h.Logger.ErrorContext(ctx, "failed to init session", "err", err.Error())
+			h.sendError(w, r, http.StatusInternalServerError, "internal server error")
+			return
+		}
 
-			// Update user visibility when an instance closes, as the user's overall status may change.
-			// Example: With 1 away and 1 non-away instance, the user appears available. If the non-away
-			// instance closes, the user should appear away.
-			oscarInstance.OnClose(func() {
-				if shuttingDown(ctx) {
-					return
+		// Update user visibility when an instance closes, as the user's overall status may change.
+		// Example: With 1 away and 1 non-away instance, the user appears available. If the non-away
+		// instance closes, the user should appear away.
+		oscarInstance.OnClose(func() {
+			if shuttingDown(ctx) {
+				return
+			}
+			if oscarInstance.Session().Invisible() {
+				if err := h.BuddyBroadcaster.BroadcastBuddyDeparted(ctx, oscarInstance.IdentScreenName()); err != nil {
+					h.Logger.ErrorContext(ctx, "error sending buddy departure notifications", "err", err.Error())
 				}
-				if oscarInstance.Session().Invisible() {
-					if err := h.BuddyBroadcaster.BroadcastBuddyDeparted(ctx, oscarInstance.IdentScreenName()); err != nil {
-						h.Logger.ErrorContext(ctx, "error sending buddy departure notifications", "err", err.Error())
-					}
-				} else {
-					if err := h.BuddyBroadcaster.BroadcastBuddyArrived(ctx, oscarInstance.IdentScreenName(), oscarInstance.Session().TLVUserInfo()); err != nil {
-						h.Logger.ErrorContext(ctx, "error sending buddy arrival notifications", "err", err.Error())
-					}
+			} else {
+				if err := h.BuddyBroadcaster.BroadcastBuddyArrived(ctx, oscarInstance.IdentScreenName(), oscarInstance.Session().TLVUserInfo()); err != nil {
+					h.Logger.ErrorContext(ctx, "error sending buddy arrival notifications", "err", err.Error())
 				}
-			})
-
-			if err := h.FeedbagService.Use(ctx, oscarInstance); err != nil {
-				h.Logger.ErrorContext(ctx, "failed to use feedbag", "err", err.Error())
 			}
+		})
 
-			// A web client signals that it wants typing events through its event
-			// subscription, not through a stored feedbag buddy pref. Reflect that
-			// on the OSCAR session so ICBMService attaches the WantEvents TLV to
-			// outgoing IMs, prompting recipients to send typing notifications
-			// back. This must run after FeedbagService.Use, which otherwise
-			// overwrites the flag from stored prefs the web user may not have set.
-			oscarInstance.Session().SetTypingEventsEnabled(slices.Contains(events, "typing"))
+		if err := h.FeedbagService.Use(ctx, oscarInstance); err != nil {
+			h.Logger.ErrorContext(ctx, "failed to use feedbag", "err", err.Error())
+		}
 
-			oscarInstance.SetSignonComplete()
+		// A web client signals that it wants typing events through its event
+		// subscription, not through a stored feedbag buddy pref. Reflect that
+		// on the OSCAR session so ICBMService attaches the WantEvents TLV to
+		// outgoing IMs, prompting recipients to send typing notifications
+		// back. This must run after FeedbagService.Use, which otherwise
+		// overwrites the flag from stored prefs the web user may not have set.
+		oscarInstance.Session().SetTypingEventsEnabled(slices.Contains(events, "typing"))
 
-			if err := h.OServiceService.ClientOnline(ctx, wire.BOS, wire.SNAC_0x01_0x02_OServiceClientOnline{}, oscarInstance); err != nil {
-				h.Logger.ErrorContext(ctx, "failed to set client online", "err", err.Error())
-				h.sendError(w, r, http.StatusInternalServerError, "internal server error")
-				return
-			}
+		oscarInstance.SetSignonComplete()
+
+		if err := h.OServiceService.ClientOnline(ctx, wire.BOS, wire.SNAC_0x01_0x02_OServiceClientOnline{}, oscarInstance); err != nil {
+			h.Logger.ErrorContext(ctx, "failed to set client online", "err", err.Error())
+			h.sendError(w, r, http.StatusInternalServerError, "internal server error")
+			return
 		}
 	}
 

server/webapi/server.go

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

server/webapi/types.go

@@ -10,24 +10,6 @@ import (
 	"github.com/mk6i/open-oscar-server/wire"
 )
 
-type BuddyService interface {
-	AddBuddies(ctx context.Context, instance *state.SessionInstance, inFrame wire.SNACFrame, inBody wire.SNAC_0x03_0x04_BuddyAddBuddies) (*wire.SNACMessage, error)
-	BroadcastBuddyDeparted(ctx context.Context, screenName state.IdentScreenName) error
-	DelBuddies(ctx context.Context, instance *state.SessionInstance, inBody wire.SNAC_0x03_0x05_BuddyDelBuddies) error
-	RightsQuery(ctx context.Context, inFrame wire.SNACFrame) wire.SNACMessage
-}
-
-type ChatService interface {
-	ChannelMsgToHost(ctx context.Context, instance *state.SessionInstance, inFrame wire.SNACFrame, inBody wire.SNAC_0x0E_0x05_ChatChannelMsgToHost) (*wire.SNACMessage, error)
-}
-
-type ChatNavService interface {
-	CreateRoom(ctx context.Context, instance *state.SessionInstance, inFrame wire.SNACFrame, inBody wire.SNAC_0x0E_0x02_ChatRoomInfoUpdate) (wire.SNACMessage, error)
-	ExchangeInfo(ctx context.Context, inFrame wire.SNACFrame, inBody wire.SNAC_0x0D_0x03_ChatNavRequestExchangeInfo) (wire.SNACMessage, error)
-	RequestChatRights(ctx context.Context, inFrame wire.SNACFrame) wire.SNACMessage
-	RequestRoomInfo(ctx context.Context, inFrame wire.SNACFrame, inBody wire.SNAC_0x0D_0x04_ChatNavRequestRoomInfo) (wire.SNACMessage, error)
-}
-
 type ICBMService interface {
 	ChannelMsgToHost(ctx context.Context, instance *state.SessionInstance, inFrame wire.SNACFrame, inBody wire.SNAC_0x04_0x06_ICBMChannelMsgToHost) (*wire.SNACMessage, error)
 	ClientEvent(ctx context.Context, instance *state.SessionInstance, inFrame wire.SNACFrame, inBody wire.SNAC_0x04_0x14_ICBMClientEvent) error
@@ -61,18 +43,6 @@ type LocateService interface {
 	DirInfo(ctx context.Context, inFrame wire.SNACFrame, inBody wire.SNAC_0x02_0x0B_LocateGetDirInfo) (wire.SNACMessage, error)
 }
 
-type DirSearchService interface {
-	InfoQuery(ctx context.Context, inFrame wire.SNACFrame, inBody wire.SNAC_0x0F_0x02_InfoQuery) (wire.SNACMessage, error)
-}
-
-type PermitDenyService interface {
-	AddDenyListEntries(ctx context.Context, instance *state.SessionInstance, inBody wire.SNAC_0x09_0x07_PermitDenyAddDenyListEntries) error
-	AddPermListEntries(ctx context.Context, instance *state.SessionInstance, inBody wire.SNAC_0x09_0x05_PermitDenyAddPermListEntries) error
-	DelDenyListEntries(ctx context.Context, instance *state.SessionInstance, inBody wire.SNAC_0x09_0x08_PermitDenyDelDenyListEntries) error
-	DelPermListEntries(ctx context.Context, instance *state.SessionInstance, inBody wire.SNAC_0x09_0x06_PermitDenyDelPermListEntries) error
-	RightsQuery(ctx context.Context, inFrame wire.SNACFrame) wire.SNACMessage
-}
-
 // BuddyListRegistry is the interface for keeping track of users with active
 // buddy lists. Once registered, a user becomes visible to other users' buddy
 // lists and vice versa.
@@ -81,14 +51,6 @@ type BuddyListRegistry interface {
 	UnregisterBuddyList(ctx context.Context, user state.IdentScreenName) error
 }
 
-type TOCConfigStore interface {
-	// SetTOCConfig sets the user's TOC config. The TOC config is the server-side
-	// buddy list functionality for TOC. This configuration is not available to
-	// OSCAR clients.
-	SetTOCConfig(ctx context.Context, user state.IdentScreenName, config string) error
-	User(ctx context.Context, screenName state.IdentScreenName) (*state.User, error)
-}
-
 // CookieBaker defines methods for issuing and verifying AIM authentication tokens ("cookies").
 // These tokens are used for authenticating client sessions with AIM services.
 type CookieBaker interface {
@@ -101,10 +63,6 @@ type CookieBaker interface {
 	Issue(data []byte) ([]byte, error)
 }
 
-type AdminService interface {
-	InfoChangeRequest(ctx context.Context, instance *state.SessionInstance, inFrame wire.SNACFrame, inBody wire.SNAC_0x07_0x04_AdminInfoChangeRequest) (wire.SNACMessage, error)
-}
-
 // SessionRetriever provides methods to retrieve OSCAR sessions.
 type SessionRetriever interface {
 	AllSessions() []*state.Session
@@ -123,20 +81,6 @@ type FeedbagService interface {
 	Use(ctx context.Context, instance *state.SessionInstance) error
 }
 
-// Phase 2: Additional interfaces for messaging and presence
-
-// MessageRelayer relays messages between users
-type MessageRelayer interface {
-	RelayToScreenName(ctx context.Context, recipient state.IdentScreenName, msg wire.SNACMessage)
-}
-
-// OfflineMessageManager manages offline message storage and retrieval
-type OfflineMessageManager interface {
-	SaveMessage(ctx context.Context, msg state.OfflineMessage) (int, error)
-	RetrieveMessages(ctx context.Context, recipient state.IdentScreenName) ([]state.OfflineMessage, error)
-	DeleteMessages(ctx context.Context, recipient state.IdentScreenName) error
-}
-
 // BuddyBroadcaster broadcasts buddy presence updates
 type BuddyBroadcaster interface {
 	BroadcastBuddyArrived(ctx context.Context, screenName state.IdentScreenName, userInfo wire.TLVUserInfo) error