webapi: deliver offline messages

fix a bug where messages are delivered to all session instances, now
only send to subscribers.

foodgroup/icbm.go

@@ -173,6 +173,12 @@ func (s *ICBMService) ChannelMsgToHost(ctx context.Context, instance *state.Sess
 			// Strip the store message directive.
 			continue
 		}
+		if tlv.Tag == wire.ICBMTLVSendTime {
+			// Only the server stamps a send time, and only on a message replayed
+			// out of the offline store. Forwarding the sender's would let them
+			// pass a live message off as a stored one and date it at will.
+			continue
+		}
 		if clientIM.ChannelID == wire.ICBMChannelRendezvous && tlv.Tag == wire.ICBMTLVData {
 			if tlv, err = addExternalIP(instance, tlv); err != nil {
 				return nil, fmt.Errorf("addExternalIP: %w", err)
@@ -566,11 +572,20 @@ func (s *ICBMService) OfflineRetrieve(ctx context.Context, instance *state.Sessi
 		}
 
 		for _, tlv := range event.Message.TLVList {
+			// The stored SNAC is whatever the sender sent. A send time it carries
+			// is not ours, and TLVList lookups return the first match, so it would
+			// shadow the stamp appended below.
+			if tlv.Tag == wire.ICBMTLVSendTime {
+				continue
+			}
 			clientIM.Append(tlv)
 		}
 		clientIM.Append(wire.NewTLVBE(wire.ICBMTLVSendTime, uint32(event.Sent.Unix())))
 
-		s.messageRelayer.RelayToScreenName(ctx, event.Recipient, wire.SNACMessage{
+		// Retrieval answers the instance that asked for it. Relaying to the screen
+		// name would hand this reply to every other instance of the account, which
+		// never requested it and cannot tell it apart from a live message.
+		s.messageRelayer.RelayToSelf(ctx, instance, wire.SNACMessage{
 			Frame: wire.SNACFrame{
 				FoodGroup: wire.ICBM,
 				SubGroup:  wire.ICBMChannelMsgToClient,

foodgroup/icbm_test.go

@@ -355,6 +355,73 @@ func TestICBMService_ChannelMsgToHost(t *testing.T) {
 			},
 			expectOutput: nil,
 		},
+		{
+			// Only the server stamps a send time, and only on a replay out of the
+			// offline store. Forwarding the sender's would let them pass a live
+			// message off as a stored one and date it at will.
+			name:     "strip sender-supplied send time from message relayed to online recipient",
+			instance: newTestInstance("sender-screen-name", sessOptWarning(10)),
+			mockParams: mockParams{
+				relationshipFetcherParams: relationshipFetcherParams{
+					relationshipParams: relationshipParams{
+						{
+							me:   state.NewIdentScreenName("sender-screen-name"),
+							them: state.NewIdentScreenName("recipient-screen-name"),
+							result: state.Relationship{
+								User: state.NewIdentScreenName("recipient-screen-name"),
+							},
+						},
+					},
+				},
+				sessionRetrieverParams: sessionRetrieverParams{
+					retrieveSessionParams{
+						{
+							screenName: state.NewIdentScreenName("recipient-screen-name"),
+							result:     newTestInstance("recipient-screen-name", sessOptWarning(20), sessOptSignonComplete).Session(),
+						},
+					},
+				},
+				messageRelayerParams: messageRelayerParams{
+					relayToScreenNameActiveOnlyParams: relayToScreenNameActiveOnlyParams{
+						{
+							screenName: state.NewIdentScreenName("recipient-screen-name"),
+							message: wire.SNACMessage{
+								Frame: wire.SNACFrame{
+									FoodGroup: wire.ICBM,
+									SubGroup:  wire.ICBMChannelMsgToClient,
+									RequestID: wire.ReqIDFromServer,
+								},
+								Body: wire.SNAC_0x04_0x07_ICBMChannelMsgToClient{
+									ChannelID:   wire.ICBMChannelIM,
+									TLVUserInfo: newTestInstance("sender-screen-name", sessOptWarning(10)).Session().TLVUserInfo(),
+									TLVRestBlock: wire.TLVRestBlock{
+										TLVList: wire.TLVList{
+											wire.NewTLVBE(wire.ICBMTLVData, []byte{1, 2, 3, 4}),
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			inputSNAC: wire.SNACMessage{
+				Frame: wire.SNACFrame{
+					RequestID: 1234,
+				},
+				Body: wire.SNAC_0x04_0x06_ICBMChannelMsgToHost{
+					ChannelID:  wire.ICBMChannelIM,
+					ScreenName: "recipient-screen-name",
+					TLVRestBlock: wire.TLVRestBlock{
+						TLVList: wire.TLVList{
+							wire.NewTLVBE(wire.ICBMTLVSendTime, uint32(time.Unix(1700000000, 0).Unix())),
+							wire.NewTLVBE(wire.ICBMTLVData, []byte{1, 2, 3, 4}),
+						},
+					},
+				},
+			},
+			expectOutput: nil,
+		},
 		{
 			name:     "don't transmit message from sender to recipient because sender has blocked recipient",
 			instance: newTestInstance("sender-screen-name", sessOptWarning(10)),
@@ -3083,10 +3150,17 @@ func TestICBMService_OfflineRetrieve(t *testing.T) {
 							recipIn: state.NewIdentScreenName("recipient"),
 							messagesOut: []state.OfflineMessage{
 								{
+									// The stored SNAC carries a send time the sender
+									// supplied. TLVList lookups return the first
+									// match, so it has to be dropped rather than
+									// shadow the Sent stamp appended on replay.
 									Message: wire.SNAC_0x04_0x06_ICBMChannelMsgToHost{
-										Cookie:       1234,
-										ChannelID:    wire.ICBMChannelIM,
-										TLVRestBlock: wire.TLVRestBlock{TLVList: []wire.TLV{wire.NewTLVBE(wire.ICBMTLVData, []byte{1, 2, 3})}},
+										Cookie:    1234,
+										ChannelID: wire.ICBMChannelIM,
+										TLVRestBlock: wire.TLVRestBlock{TLVList: []wire.TLV{
+											wire.NewTLVBE(wire.ICBMTLVSendTime, uint32(1)),
+											wire.NewTLVBE(wire.ICBMTLVData, []byte{1, 2, 3}),
+										}},
 									},
 									Recipient: state.NewIdentScreenName("recipient"),
 									Sender:    state.NewIdentScreenName("sender"),
@@ -3103,7 +3177,7 @@ func TestICBMService_OfflineRetrieve(t *testing.T) {
 					},
 				},
 				messageRelayerParams: messageRelayerParams{
-					relayToScreenNameParams: relayToScreenNameParams{
+					relayToSelfParams: relayToSelfParams{
 						{
 							screenName: state.NewIdentScreenName("recipient"),
 							message: wire.SNACMessage{
@@ -3158,7 +3232,7 @@ func TestICBMService_OfflineRetrieve(t *testing.T) {
 					deleteMessagesParams: deleteMessagesParams{},
 				},
 				messageRelayerParams: messageRelayerParams{
-					relayToScreenNameParams: relayToScreenNameParams{},
+					relayToSelfParams: relayToSelfParams{},
 				},
 			},
 		},
@@ -3197,7 +3271,7 @@ func TestICBMService_OfflineRetrieve(t *testing.T) {
 					},
 				},
 				messageRelayerParams: messageRelayerParams{
-					relayToScreenNameParams: relayToScreenNameParams{
+					relayToSelfParams: relayToSelfParams{
 						{
 							screenName: state.NewIdentScreenName("recipient"),
 							message: wire.SNACMessage{
@@ -3244,7 +3318,7 @@ func TestICBMService_OfflineRetrieve(t *testing.T) {
 					},
 				},
 				messageRelayerParams: messageRelayerParams{
-					relayToScreenNameParams: relayToScreenNameParams{},
+					relayToSelfParams: relayToSelfParams{},
 				},
 			},
 		},
@@ -3265,9 +3339,9 @@ func TestICBMService_OfflineRetrieve(t *testing.T) {
 			}
 
 			messageRelayer := newMockMessageRelayer(t)
-			for _, item := range tc.mockParams.relayToScreenNameParams {
+			for _, item := range tc.mockParams.relayToSelfParams {
 				messageRelayer.EXPECT().
-					RelayToScreenName(mock.Anything, item.screenName, item.message)
+					RelayToSelf(mock.Anything, matchSession(item.screenName), item.message)
 			}
 
 			svc := ICBMService{

foodgroup/icq.go

@@ -446,7 +446,7 @@ func (s *ICQService) OfflineMsgReq(ctx context.Context, inFrame wire.SNACFrame,
 			msgOut := wire.ICQMessageReplyEnvelope{
 				Message: reply,
 			}
-			if err := s.reply(ctx, instance, msgOut, inFrame.RequestID, wire.SNACFlagsMoreToCome); err != nil {
+			if err := s.replyToSelf(ctx, instance, msgOut, inFrame.RequestID, wire.SNACFlagsMoreToCome); err != nil {
 				return fmt.Errorf("sending offline message: %w", err)
 			}
 		} else {
@@ -462,11 +462,19 @@ func (s *ICQService) OfflineMsgReq(ctx context.Context, inFrame wire.SNACFrame,
 			}
 
 			for _, tlv := range msgIn.Message.TLVList {
+				// The stored SNAC is whatever the sender sent. A send time it
+				// carries is not ours, and TLVList lookups return the first match,
+				// so it would shadow the stamp appended below.
+				if tlv.Tag == wire.ICBMTLVSendTime {
+					continue
+				}
 				clientIM.Append(tlv)
 			}
 			clientIM.Append(wire.NewTLVBE(wire.ICBMTLVSendTime, uint32(msgIn.Sent.Unix())))
 
-			s.messageRelayer.RelayToScreenName(ctx, msgIn.Recipient, wire.SNACMessage{
+			// Retrieval answers the instance that asked for it, not every instance
+			// of the account.
+			s.messageRelayer.RelayToSelf(ctx, instance, wire.SNACMessage{
 				Frame: wire.SNACFrame{
 					FoodGroup: wire.ICBM,
 					SubGroup:  wire.ICBMChannelMsgToClient,
@@ -488,7 +496,7 @@ func (s *ICQService) OfflineMsgReq(ctx context.Context, inFrame wire.SNACFrame,
 		},
 	}
 
-	if err := s.reply(ctx, instance, eofMsg, inFrame.RequestID, 0); err != nil {
+	if err := s.replyToSelf(ctx, instance, eofMsg, inFrame.RequestID, 0); err != nil {
 		return fmt.Errorf("sending end of offline messages: %w", err)
 	}
 
@@ -1205,7 +1213,20 @@ func (s *ICQService) affiliations(ctx context.Context, instance *state.SessionIn
 }
 
 func (s *ICQService) reply(ctx context.Context, instance *state.SessionInstance, message wire.ICQMessageReplyEnvelope, requestID uint32, snacFlags uint16) error {
-	msg := wire.SNACMessage{
+	s.messageRelayer.RelayToScreenName(ctx, instance.IdentScreenName(), replySNAC(message, requestID, snacFlags))
+	return nil
+}
+
+// replyToSelf answers the instance that made the request rather than every
+// instance of the account.
+func (s *ICQService) replyToSelf(ctx context.Context, instance *state.SessionInstance, message wire.ICQMessageReplyEnvelope, requestID uint32, snacFlags uint16) error {
+	s.messageRelayer.RelayToSelf(ctx, instance, replySNAC(message, requestID, snacFlags))
+	return nil
+}
+
+// replySNAC wraps an ICQ metadata payload in a DB reply SNAC.
+func replySNAC(message wire.ICQMessageReplyEnvelope, requestID uint32, snacFlags uint16) wire.SNACMessage {
+	return wire.SNACMessage{
 		Frame: wire.SNACFrame{
 			FoodGroup: wire.ICQ,
 			SubGroup:  wire.ICQDBReply,
@@ -1220,9 +1241,6 @@ func (s *ICQService) reply(ctx context.Context, instance *state.SessionInstance,
 			},
 		},
 	}
-
-	s.messageRelayer.RelayToScreenName(ctx, instance.IdentScreenName(), msg)
-	return nil
 }
 
 func (s *ICQService) replySearchHit(ctx context.Context, instance *state.SessionInstance, record wire.ICQUserSearchRecord, requestID uint32, last bool, seq uint16) error {

foodgroup/icq_test.go

@@ -2373,7 +2373,7 @@ func TestICQService_OfflineMsgReq(t *testing.T) {
 					},
 				},
 				messageRelayerParams: messageRelayerParams{
-					relayToScreenNameParams: relayToScreenNameParams{
+					relayToSelfParams: relayToSelfParams{
 						{
 							screenName: state.NewIdentScreenName("11111111"),
 							message: wire.SNACMessage{
@@ -2511,7 +2511,7 @@ func TestICQService_OfflineMsgReq(t *testing.T) {
 					},
 				},
 				messageRelayerParams: messageRelayerParams{
-					relayToScreenNameParams: relayToScreenNameParams{
+					relayToSelfParams: relayToSelfParams{
 						{
 							screenName: state.NewIdentScreenName("11111111"),
 							message: wire.SNACMessage{
@@ -2576,7 +2576,9 @@ func TestICQService_OfflineMsgReq(t *testing.T) {
 					},
 				},
 				messageRelayerParams: messageRelayerParams{
-					relayToScreenNameParams: relayToScreenNameParams{
+					// Retrieval answers the requesting instance, both the message
+					// itself and the end-of-messages envelope.
+					relayToSelfParams: relayToSelfParams{
 						{
 							screenName: state.NewIdentScreenName("11111111"),
 							message: wire.SNACMessage{
@@ -2647,6 +2649,10 @@ func TestICQService_OfflineMsgReq(t *testing.T) {
 				messageRelayer.EXPECT().
 					RelayToScreenName(mock.Anything, params.screenName, params.message)
 			}
+			for _, params := range tt.mockParams.relayToSelfParams {
+				messageRelayer.EXPECT().
+					RelayToSelf(mock.Anything, matchSession(params.screenName), params.message)
+			}
 
 			var feedbagSenderCalls int
 

server/webapi/handlers/session.go

@@ -364,47 +364,6 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 	// icon propagates to the badge.
 	myIconURL := h.IconSource.PublishedURL(ctx, baseURL, screenName.IdentScreenName())
 
-	// Queue myInfo event for authenticated users
-	if authToken != "" {
-		for _, event := range events {
-			if event == "myInfo" || event == "presence" {
-				myInfoData := buildMyInfo(screenName, "online", myIconURL)
-				myInfoData["onlineTime"] = time.Now().Unix()
-				myInfoData["memberSince"] = time.Now().Unix() - 86400*30 // 30 days ago
-				session.EventQueue.Push(types.EventType("myInfo"), myInfoData)
-				break
-			}
-		}
-		for _, event := range events {
-			if event == "conversation" {
-				session.EventQueue.Push(types.EventTypeConversation,
-					types.ConversationEventData("list", nil))
-				break
-			}
-		}
-	}
-
-	// Drain messages stored while the user was signed off. The service relays them
-	// as ordinary ICBMChannelMsgToClient SNACs stamped with a send time, which the
-	// listener started above turns into offlineIM events. Retrieval deletes them
-	// from the store, so skip it when the client subscribes to neither event that
-	// can carry them rather than dropping the messages on the floor.
-	//
-	// This trails the conversation list queued above because the listener pushes
-	// from its own goroutine. The client rebuilds its list from scratch on the first
-	// "list" it sees, dropping conversations it knows only from an earlier "update",
-	// so a drained message must not reach the queue ahead of that event.
-	if slices.Contains(events, "offlineIM") || slices.Contains(events, "im") {
-		frame := wire.SNACFrame{
-			FoodGroup: wire.ICBM,
-			SubGroup:  wire.ICBMOfflineRetrieve,
-			RequestID: wire.ReqIDFromServer,
-		}
-		if _, err := h.ICBMService.OfflineRetrieve(ctx, instance, frame); err != nil {
-			h.Logger.ErrorContext(ctx, "failed to retrieve offline messages", "err", err.Error())
-		}
-	}
-
 	now := time.Now().Unix()
 
 	// Prepare response
@@ -425,41 +384,57 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 		"lifestreamApiBase": baseURL + "/",
 	}
 
-	if authToken != "" {
-		myInfoPayload := buildMyInfo(screenName, "online", myIconURL)
-		myInfoPayload["onlineTime"] = time.Now().Unix()
-		myInfoPayload["memberSince"] = time.Now().Unix() - 86400*30 // 30 days ago
-		myInfoPayload["self"] = map[string]interface{}{
-			"instNum":        1,
-			"loginTime":      time.Now().Unix(),
-			"sessionTimeout": 30,
-			"events":         events,
-			"assertCaps":     []string{},
-			"rightsInfo": map[string]interface{}{
-				"maxDenies":            500,
-				"maxPermits":           500,
-				"maxWatchers":          3000,
-				"maxBuddies":           500,
-				"maxTempBuddies":       160,
-				"maxIMSize":            3987,
-				"minInterIcbmInterval": 1000,
-				"maxSourceEvil":        900,
-				"maxDstEvil":           999,
-				"maxSigLen":            4096,
-			},
-		}
-		resp.Response.Data.MyInfo = myInfoPayload
-		if resp.Response.Data.Events == nil {
-			resp.Response.Data.Events = make(map[string]interface{})
-		}
-		resp.Response.Data.Events["myInfo"] = myInfoPayload
-	}
-
+	myInfoPayload := buildMyInfo(screenName, "online", myIconURL)
+	myInfoPayload["onlineTime"] = time.Now().Unix()
+	myInfoPayload["memberSince"] = time.Now().Unix() - 86400*30 // 30 days ago
+	myInfoPayload["self"] = map[string]interface{}{
+		"instNum":        1,
+		"loginTime":      time.Now().Unix(),
+		"sessionTimeout": 30,
+		"events":         events,
+		"assertCaps":     []string{},
+		"rightsInfo": map[string]interface{}{
+			"maxDenies":            500,
+			"maxPermits":           500,
+			"maxWatchers":          3000,
+			"maxBuddies":           500,
+			"maxTempBuddies":       160,
+			"maxIMSize":            3987,
+			"minInterIcbmInterval": 1000,
+			"maxSourceEvil":        900,
+			"maxDstEvil":           999,
+			"maxSigLen":            4096,
+		},
+	}
+	resp.Response.Data.MyInfo = myInfoPayload
+	if resp.Response.Data.Events == nil {
+		resp.Response.Data.Events = make(map[string]interface{})
+	}
+	resp.Response.Data.Events["myInfo"] = myInfoPayload
+
+	// Seeds that only queue an event are keyed off the subscription rather than
+	// iterated with it, so the server fixes their order and each is queued once.
+	// myInfo and presence render the identity badge from the same payload and the
+	// client subscribes to both, which a per-subscription loop would queue twice.
+	if slices.Contains(events, "myInfo") || slices.Contains(events, "presence") {
+		myInfoData := buildMyInfo(screenName, "online", myIconURL)
+		myInfoData["onlineTime"] = time.Now().Unix()
+		myInfoData["memberSince"] = time.Now().Unix() - 86400*30 // 30 days ago
+		session.EventQueue.Push(types.EventTypeMyInfo, myInfoData)
+	}
+
+	if slices.Contains(events, "conversation") {
+		session.EventQueue.Push(types.EventTypeConversation,
+			types.ConversationEventData("list", nil))
+	}
+
+	// The remaining seeds also populate the response payload, so they stay keyed off
+	// the subscription list they are rendered into.
 	for _, event := range events {
 		switch types.EventType(event) {
 		case types.EventTypeBuddyList:
 			buddyGroups := []WebAPIBuddyGroup{}
-			if authToken != "" && h.BuddyListManager != nil {
+			if h.BuddyListManager != nil {
 				var err error
 				buddyGroups, err = h.BuddyListManager.GetBuddyListForUser(ctx, session)
 				if err != nil {
@@ -475,9 +450,7 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 				resp.Response.Data.Events = make(map[string]interface{})
 			}
 			resp.Response.Data.Events["buddylist"] = blPayload
-			if authToken != "" {
-				session.EventQueue.Push(types.EventTypeBuddyList, blPayload)
-			}
+			session.EventQueue.Push(types.EventTypeBuddyList, blPayload)
 		case types.EventTypePreference:
 			// Seed the client with effective preference values: the user's stored
 			// prefs where set, and the server-side spec defaults otherwise. The
@@ -486,7 +459,7 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 			// would silently fall back to the client's hidden default and, for
 			// showGroups, hide group headers.
 			prefPayload := map[string]interface{}{}
-			if authToken != "" && session.OSCARSession != nil {
+			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 {
@@ -497,16 +470,14 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 				resp.Response.Data.Events = make(map[string]interface{})
 			}
 			resp.Response.Data.Events["preference"] = prefPayload
-			if authToken != "" {
-				session.EventQueue.Push(types.EventTypePreference, prefPayload)
-			}
+			session.EventQueue.Push(types.EventTypePreference, prefPayload)
 		case types.EventTypePermitDeny:
 			// The client keeps its privacy state solely in the model this event
 			// populates. Both the block/unblock menu action and the "blocked"
 			// presence state read that model and no-op silently while it is
 			// empty, so the session has to start with one.
 			var pdPayload interface{} = PermitDenyData{PDMode: "permitAll"}
-			if authToken != "" && session.OSCARSession != nil {
+			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())
@@ -518,9 +489,31 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 				resp.Response.Data.Events = make(map[string]interface{})
 			}
 			resp.Response.Data.Events["permitDeny"] = pdPayload
-			if authToken != "" {
-				session.EventQueue.Push(types.EventTypePermitDeny, pdPayload)
-			}
+			session.EventQueue.Push(types.EventTypePermitDeny, pdPayload)
+		}
+	}
+
+	// Drain messages stored while the user was signed off. The service relays them
+	// as ordinary ICBMChannelMsgToClient SNACs stamped with a send time, which the
+	// listener started above turns into offlineIM events. Retrieval deletes them
+	// from the store and the Web API has no ack, so this is the one delivery attempt.
+	// Skip it for a client that did not subscribe, which leaves the messages stored
+	// for a session that wants them rather than spending them on one that does not.
+	//
+	// This trails every event queued above because the listener pushes from its own
+	// goroutine, so anything drained here can overtake a later push. An offlineIM
+	// names its sender by bare aimId and leaves the client to resolve the display
+	// name against the buddy list it holds, and the conversation list is rebuilt from
+	// scratch on the first "list" the client sees. Both of those events are queued in
+	// the loop above, so the drain has to come after it.
+	if slices.Contains(events, "offlineIM") {
+		frame := wire.SNACFrame{
+			FoodGroup: wire.ICBM,
+			SubGroup:  wire.ICBMOfflineRetrieve,
+			RequestID: wire.ReqIDFromServer,
+		}
+		if _, err := h.ICBMService.OfflineRetrieve(ctx, instance, frame); err != nil {
+			h.Logger.ErrorContext(ctx, "failed to retrieve offline messages", "err", err.Error())
 		}
 	}
 
@@ -571,7 +564,7 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 			if event == "buddylist" || event == "myInfo" {
 				var buddyGroups []BuddyGroup
 
-				if authToken != "" && h.BuddyListManager != nil {
+				if h.BuddyListManager != nil {
 					// Fetch actual buddy list from service
 					webAPIGroups, err := h.BuddyListManager.GetBuddyListForUser(ctx, session)
 					if err != nil {

server/webapi/types/events.go

@@ -14,6 +14,7 @@ const (
 	EventTypeBuddyList    EventType = "buddylist"
 	EventTypeConversation EventType = "conversation"
 	EventTypeIM           EventType = "im"
+	EventTypeMyInfo       EventType = "myInfo"
 	EventTypeOfflineIM    EventType = "offlineIM"
 	EventTypePreference   EventType = "preference"
 	EventTypePresence     EventType = "presence"

state/webapi_session.go

@@ -334,10 +334,10 @@ func (s *WebAPISession) handleIncomingIM(msg wire.SNACMessage) {
 	// signed off, and carries the moment the sender actually sent it.
 	sentTime, isOffline := body.Uint32BE(wire.ICBMTLVSendTime)
 
-	// An offlineIM subscriber gets the dedicated event; a client that asked for
-	// im only still gets the message, just with no signal that it was stored.
-	asOfflineIM := isOffline && s.IsSubscribedTo("offlineIM")
-	if !asOfflineIM && !s.IsSubscribedTo("im") {
+	// Retrieval answers only the instance that asked, and StartSession asks only
+	// when the client subscribed to offlineIM, so a stamped message here is one
+	// this session requested. A live IM still needs the im subscription.
+	if !isOffline && !s.IsSubscribedTo("im") {
 		return
 	}
 
@@ -375,7 +375,7 @@ func (s *WebAPISession) handleIncomingIM(msg wire.SNACMessage) {
 	}
 	s.AddStoredIM(partnerAimID, partnerAimID, messageText, msgID, timestamp)
 
-	if asOfflineIM {
+	if isOffline {
 		s.EventQueue.Push(types.EventTypeOfflineIM, types.OfflineIMEvent{
 			AimID:     partnerAimID,
 			Message:   messageText,

state/webapi_session_test.go

@@ -1123,18 +1123,6 @@ func TestWebAPISession_OfflineIM(t *testing.T) {
 		assert.Equal(t, types.EventTypeIM, events[0].Type)
 	})
 
-	// Retrieval deletes the message from the store, so a client that subscribes to
-	// im alone still has to receive it.
-	t.Run("falls back to im when offlineIM is not subscribed", func(t *testing.T) {
-		sess := newSession("im")
-		sess.handleIncomingIM(storedMsg(t, true))
-
-		events := sess.EventQueue.GetAllEvents()
-		require.Len(t, events, 1)
-		assert.Equal(t, types.EventTypeIM, events[0].Type)
-		assert.Equal(t, float64(sentAt), events[0].Data.(types.IMEvent).Timestamp)
-	})
-
 	t.Run("offlineIM subscriber gets a conversation update", func(t *testing.T) {
 		sess := newSession("offlineIM", "conversation")
 		sess.handleIncomingIM(storedMsg(t, true))
@@ -1156,9 +1144,13 @@ func TestWebAPISession_OfflineIM(t *testing.T) {
 		assert.Equal(t, float64(sentAt), stored[0]["date"])
 	})
 
-	t.Run("no subscription drops the message", func(t *testing.T) {
+	// Only a live IM is filtered on subscription here. Retrieval answers the
+	// instance that asked and StartSession asks only for an offlineIM subscriber,
+	// so a stamped message reaching a session that did not subscribe is not a state
+	// this handler can be put in.
+	t.Run("no subscription drops a live message", func(t *testing.T) {
 		sess := newSession("presence")
-		sess.handleIncomingIM(storedMsg(t, true))
+		sess.handleIncomingIM(storedMsg(t, false))
 
 		assert.Empty(t, sess.EventQueue.GetAllEvents())
 	})