fix: Bug - Users doesn't receive dm messages
Match direct messages against every local identity alias (home id and provisioned signal-server actor ids) so recipients accept traffic addressed to their per-server presence id instead of silently dropping it. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -4,6 +4,7 @@ import {
|
||||
createGroupConversation,
|
||||
directMessageEventIncludesUser,
|
||||
directMessageSyncIncludesUser,
|
||||
directMessageConversationIncludesUser,
|
||||
createDirectCallStartedMessage,
|
||||
getDirectConversationId,
|
||||
isGroupDirectConversation,
|
||||
@@ -137,6 +138,32 @@ describe('DirectMessageService domain flow', () => {
|
||||
expect(directMessageEventIncludesUser(payload, 'charlie')).toBe(false);
|
||||
});
|
||||
|
||||
it('recognises direct-message recipients across identity aliases', () => {
|
||||
const payload = {
|
||||
message: createMessage('message-1', 'SENT', getDirectConversationId('alice', 'bob-foreign'), ['bob-foreign']),
|
||||
participants: [alice, { ...bob, userId: 'bob-foreign' }],
|
||||
sender: alice
|
||||
};
|
||||
const bobIds = new Set(['bob', 'bob-foreign']);
|
||||
|
||||
expect(directMessageEventIncludesUser(payload, bobIds)).toBe(true);
|
||||
expect(directMessageEventIncludesUser(payload, 'bob')).toBe(false);
|
||||
});
|
||||
|
||||
it('recognises conversation participants across identity aliases', () => {
|
||||
const conversation = {
|
||||
...createDirectConversation(alice, bob, 10),
|
||||
participants: ['alice', 'bob-foreign'],
|
||||
participantProfiles: {
|
||||
alice,
|
||||
'bob-foreign': { ...bob, userId: 'bob-foreign' }
|
||||
}
|
||||
};
|
||||
|
||||
expect(directMessageConversationIncludesUser(conversation, new Set(['bob', 'bob-foreign']))).toBe(true);
|
||||
expect(directMessageConversationIncludesUser(conversation, 'bob')).toBe(false);
|
||||
});
|
||||
|
||||
it('recognises only declared sync participants', () => {
|
||||
const payload = {
|
||||
conversationId: 'dm-group-test',
|
||||
|
||||
@@ -14,6 +14,7 @@ import { OfflineMessageQueueService } from './offline-message-queue.service';
|
||||
import { PeerDeliveryService } from './peer-delivery.service';
|
||||
import { AttachmentFacade } from '../../../attachment';
|
||||
import { CustomEmojiService } from '../../../custom-emoji';
|
||||
import { SignalServerCredentialStoreService } from '../../../authentication/application/services/signal-server-credential-store.service';
|
||||
import {
|
||||
advanceDirectMessageStatus,
|
||||
createDirectConversation,
|
||||
@@ -27,6 +28,7 @@ import {
|
||||
updateMessageStatusInConversation,
|
||||
upsertDirectMessage
|
||||
} from '../../domain/logic/direct-message.logic';
|
||||
import { collectDirectMessageSelfUserIds, isSelfDirectMessageSender } from '../../domain/logic/direct-message-identity.rules';
|
||||
import {
|
||||
DirectMessage,
|
||||
DirectMessageConversation,
|
||||
@@ -67,6 +69,7 @@ export class DirectMessageService {
|
||||
private readonly delivery = inject(PeerDeliveryService);
|
||||
private readonly attachments = inject(AttachmentFacade);
|
||||
private readonly customEmoji = inject(CustomEmojiService);
|
||||
private readonly credentialStore = inject(SignalServerCredentialStoreService);
|
||||
private readonly store = inject(Store);
|
||||
private readonly router = inject(Router);
|
||||
private readonly currentUser = this.store.selectSignal(selectCurrentUser);
|
||||
@@ -501,8 +504,9 @@ export class DirectMessageService {
|
||||
private async handleIncomingMessage(payload: DirectMessageEventPayload): Promise<void> {
|
||||
const ownerId = this.getCurrentUserIdOrThrow();
|
||||
const currentUser = this.requireCurrentUser();
|
||||
const selfUserIds = this.getSelfUserIds();
|
||||
|
||||
if (!directMessageEventIncludesUser(payload, ownerId) || payload.sender.userId === ownerId || payload.message.senderId === ownerId) {
|
||||
if (!directMessageEventIncludesUser(payload, selfUserIds) || isSelfDirectMessageSender(payload, selfUserIds)) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -571,8 +575,9 @@ export class DirectMessageService {
|
||||
private async handleIncomingMutation(payload: DirectMessageMutationEventPayload): Promise<void> {
|
||||
const ownerId = this.getCurrentUserIdOrThrow();
|
||||
const conversation = await this.findConversation(ownerId, payload.conversationId);
|
||||
const selfUserIds = this.getSelfUserIds();
|
||||
|
||||
if (!conversation || !directMessageConversationIncludesUser(conversation, ownerId)) {
|
||||
if (!conversation || !directMessageConversationIncludesUser(conversation, selfUserIds)) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -580,16 +585,16 @@ export class DirectMessageService {
|
||||
}
|
||||
|
||||
private handleIncomingTyping(payload: DirectMessageTypingEventPayload): void {
|
||||
const currentUserId = this.getCurrentUserId();
|
||||
const selfUserIds = this.getSelfUserIds();
|
||||
|
||||
if (!currentUserId || payload.sender.userId === currentUserId) {
|
||||
if (selfUserIds.size === 0 || selfUserIds.has(payload.sender.userId)) {
|
||||
return;
|
||||
}
|
||||
|
||||
const conversation = this.conversationsSignal().find((entry) => entry.id === payload.conversationId);
|
||||
|
||||
if (!conversation
|
||||
|| !directMessageConversationIncludesUser(conversation, currentUserId)
|
||||
|| !directMessageConversationIncludesUser(conversation, selfUserIds)
|
||||
|| !directMessageConversationIncludesUser(conversation, payload.sender.userId)) {
|
||||
return;
|
||||
}
|
||||
@@ -621,10 +626,11 @@ export class DirectMessageService {
|
||||
const ownerId = this.getCurrentUserIdOrThrow();
|
||||
const currentUser = this.requireCurrentUser();
|
||||
const conversation = await this.findConversation(ownerId, payload.conversationId);
|
||||
const selfUserIds = this.getSelfUserIds();
|
||||
|
||||
if (!conversation
|
||||
|| payload.sender.userId === ownerId
|
||||
|| !directMessageConversationIncludesUser(conversation, ownerId)
|
||||
|| selfUserIds.has(payload.sender.userId)
|
||||
|| !directMessageConversationIncludesUser(conversation, selfUserIds)
|
||||
|| !directMessageConversationIncludesUser(conversation, payload.sender.userId)) {
|
||||
return;
|
||||
}
|
||||
@@ -647,12 +653,13 @@ export class DirectMessageService {
|
||||
const ownerId = this.getCurrentUserIdOrThrow();
|
||||
const currentUser = this.requireCurrentUser();
|
||||
const currentParticipant = toDirectMessageParticipant(currentUser);
|
||||
const selfUserIds = this.getSelfUserIds();
|
||||
|
||||
if (payload.sender.userId === ownerId) {
|
||||
if (selfUserIds.has(payload.sender.userId)) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (!directMessageSyncIncludesUser(payload, ownerId) || !directMessageSyncIncludesUser(payload, payload.sender.userId)) {
|
||||
if (!directMessageSyncIncludesUser(payload, selfUserIds) || !directMessageSyncIncludesUser(payload, payload.sender.userId)) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -929,7 +936,9 @@ export class DirectMessageService {
|
||||
return [];
|
||||
}
|
||||
|
||||
return conversation.participants.filter((participantId) => participantId !== currentUserId);
|
||||
const selfUserIds = this.getSelfUserIds();
|
||||
|
||||
return conversation.participants.filter((participantId) => !selfUserIds.has(participantId));
|
||||
}
|
||||
|
||||
private conversationKind(conversation: DirectMessageConversation): 'direct' | 'group' {
|
||||
@@ -991,4 +1000,16 @@ export class DirectMessageService {
|
||||
|
||||
return ownerId;
|
||||
}
|
||||
|
||||
private getSelfUserIds(): ReadonlySet<string> {
|
||||
const currentUser = this.currentUser();
|
||||
|
||||
if (!currentUser) {
|
||||
return new Set();
|
||||
}
|
||||
|
||||
const actorUserIds = this.credentialStore.listValidCredentials().map((credential) => credential.userId);
|
||||
|
||||
return collectDirectMessageSelfUserIds(currentUser, actorUserIds);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -9,7 +9,8 @@ import {
|
||||
} from 'rxjs';
|
||||
import { RealtimeSessionFacade } from '../../../../core/realtime';
|
||||
import { selectAllUsers } from '../../../../store/users/users.selectors';
|
||||
import type { ChatEvent, User } from '../../../../shared-kernel';
|
||||
import { buildUserIdentityLookup, resolveUserByIdentity } from '../../../../store/users/user-identity-lookup.rules';
|
||||
import type { ChatEvent } from '../../../../shared-kernel';
|
||||
|
||||
@Injectable({ providedIn: 'root' })
|
||||
export class PeerDeliveryService {
|
||||
@@ -87,13 +88,13 @@ export class PeerDeliveryService {
|
||||
return recipientId;
|
||||
}
|
||||
|
||||
const user = this.users().find((candidate: User) =>
|
||||
candidate.id === recipientId || candidate.oderId === recipientId || candidate.peerId === recipientId
|
||||
);
|
||||
const lookup = buildUserIdentityLookup(this.users());
|
||||
const user = resolveUserByIdentity(lookup, recipientId);
|
||||
const candidates = [
|
||||
user?.oderId,
|
||||
user?.peerId,
|
||||
user?.id
|
||||
user?.id,
|
||||
recipientId
|
||||
].filter((candidate): candidate is string => !!candidate);
|
||||
|
||||
return candidates.find((candidate) => connectedPeerIds.has(candidate)) ?? null;
|
||||
@@ -135,9 +136,8 @@ export class PeerDeliveryService {
|
||||
}
|
||||
|
||||
private resolveCandidateIds(recipientId: string): string[] {
|
||||
const user = this.users().find((candidate: User) =>
|
||||
candidate.id === recipientId || candidate.oderId === recipientId || candidate.peerId === recipientId
|
||||
);
|
||||
const lookup = buildUserIdentityLookup(this.users());
|
||||
const user = resolveUserByIdentity(lookup, recipientId);
|
||||
|
||||
return [
|
||||
recipientId,
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
import {
|
||||
collectDirectMessageSelfUserIds,
|
||||
directMessageConversationIncludesAnyUser,
|
||||
directMessageEventIncludesAnyUser,
|
||||
isSelfDirectMessageSender
|
||||
} from './direct-message-identity.rules';
|
||||
import type { DirectMessageConversation, DirectMessageParticipant } from '../models/direct-message.model';
|
||||
|
||||
const aliceHome: DirectMessageParticipant = {
|
||||
userId: 'alice-home',
|
||||
username: 'alice',
|
||||
displayName: 'Alice'
|
||||
};
|
||||
const bobHome: DirectMessageParticipant = {
|
||||
userId: 'bob-home',
|
||||
username: 'bob',
|
||||
displayName: 'Bob'
|
||||
};
|
||||
|
||||
describe('direct-message-identity.rules', () => {
|
||||
it('collects home and provisioned actor ids for the local user', () => {
|
||||
const ids = collectDirectMessageSelfUserIds(
|
||||
{ id: 'alice-home', oderId: 'alice-home' },
|
||||
['alice-foreign']
|
||||
);
|
||||
|
||||
expect(Array.from(ids).sort()).toEqual(['alice-foreign', 'alice-home']);
|
||||
});
|
||||
|
||||
it('accepts incoming direct messages addressed to a provisioned actor id', () => {
|
||||
const payload = {
|
||||
message: {
|
||||
id: 'message-1',
|
||||
conversationId: 'dm-alice-home--bob-foreign',
|
||||
senderId: 'alice-home',
|
||||
recipientId: 'bob-foreign',
|
||||
recipientIds: ['bob-foreign'],
|
||||
content: 'hello',
|
||||
timestamp: 1,
|
||||
status: 'SENT' as const
|
||||
},
|
||||
sender: aliceHome,
|
||||
participants: [aliceHome, { ...bobHome, userId: 'bob-foreign' }]
|
||||
};
|
||||
|
||||
expect(directMessageEventIncludesAnyUser(payload, collectDirectMessageSelfUserIds(
|
||||
{ id: 'bob-home', oderId: 'bob-home' },
|
||||
['bob-foreign']
|
||||
))).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects incoming direct messages that do not target any local identity', () => {
|
||||
const payload = {
|
||||
message: {
|
||||
id: 'message-1',
|
||||
conversationId: 'dm-alice-home--charlie',
|
||||
senderId: 'alice-home',
|
||||
recipientId: 'charlie',
|
||||
recipientIds: ['charlie'],
|
||||
content: 'hello',
|
||||
timestamp: 1,
|
||||
status: 'SENT' as const
|
||||
},
|
||||
sender: aliceHome,
|
||||
participants: [aliceHome]
|
||||
};
|
||||
|
||||
expect(directMessageEventIncludesAnyUser(payload, collectDirectMessageSelfUserIds(
|
||||
{ id: 'bob-home', oderId: 'bob-home' },
|
||||
['bob-foreign']
|
||||
))).toBe(false);
|
||||
});
|
||||
|
||||
it('treats any local identity as the sender for echo suppression', () => {
|
||||
const payload = {
|
||||
message: {
|
||||
id: 'message-1',
|
||||
conversationId: 'dm-alice-home--bob-foreign',
|
||||
senderId: 'alice-foreign',
|
||||
recipientId: 'bob-foreign',
|
||||
recipientIds: ['bob-foreign'],
|
||||
content: 'hello',
|
||||
timestamp: 1,
|
||||
status: 'SENT' as const
|
||||
},
|
||||
sender: { ...aliceHome, userId: 'alice-foreign' }
|
||||
};
|
||||
const selfIds = collectDirectMessageSelfUserIds(
|
||||
{ id: 'alice-home', oderId: 'alice-home' },
|
||||
['alice-foreign']
|
||||
);
|
||||
|
||||
expect(isSelfDirectMessageSender(payload, selfIds)).toBe(true);
|
||||
});
|
||||
|
||||
it('matches conversations stored under a different participant alias', () => {
|
||||
const conversation: DirectMessageConversation = {
|
||||
id: 'dm-alice-home--bob-foreign',
|
||||
participants: ['alice-home', 'bob-foreign'],
|
||||
participantProfiles: {
|
||||
'alice-home': aliceHome,
|
||||
'bob-foreign': { ...bobHome, userId: 'bob-foreign' }
|
||||
},
|
||||
messages: [],
|
||||
lastMessageAt: 1,
|
||||
unreadCount: 0
|
||||
};
|
||||
|
||||
expect(directMessageConversationIncludesAnyUser(
|
||||
conversation,
|
||||
collectDirectMessageSelfUserIds({ id: 'bob-home', oderId: 'bob-home' }, ['bob-foreign'])
|
||||
)).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,49 @@
|
||||
import type { User } from '../../../../shared-kernel';
|
||||
import { directMessageConversationIncludesUser, directMessageEventIncludesUser } from './direct-message.logic';
|
||||
import type { DirectMessageConversation, DirectMessageEventPayload } from '../models/direct-message.model';
|
||||
|
||||
type UserIdentityFields = Pick<User, 'id' | 'oderId' | 'peerId'>;
|
||||
|
||||
/** Collect every id that can represent the local user in direct-message traffic. */
|
||||
export function collectDirectMessageSelfUserIds(
|
||||
user: UserIdentityFields,
|
||||
additionalUserIds: readonly string[] = []
|
||||
): ReadonlySet<string> {
|
||||
const ids = new Set<string>();
|
||||
|
||||
for (const candidate of [
|
||||
user.id,
|
||||
user.oderId,
|
||||
user.peerId,
|
||||
...additionalUserIds
|
||||
]) {
|
||||
const normalized = candidate?.trim();
|
||||
|
||||
if (normalized) {
|
||||
ids.add(normalized);
|
||||
}
|
||||
}
|
||||
|
||||
return ids;
|
||||
}
|
||||
|
||||
export function directMessageEventIncludesAnyUser(
|
||||
payload: DirectMessageEventPayload,
|
||||
userIds: ReadonlySet<string>
|
||||
): boolean {
|
||||
return directMessageEventIncludesUser(payload, userIds);
|
||||
}
|
||||
|
||||
export function isSelfDirectMessageSender(
|
||||
payload: DirectMessageEventPayload,
|
||||
userIds: ReadonlySet<string>
|
||||
): boolean {
|
||||
return userIds.has(payload.sender.userId) || userIds.has(payload.message.senderId);
|
||||
}
|
||||
|
||||
export function directMessageConversationIncludesAnyUser(
|
||||
conversation: Pick<DirectMessageConversation, 'participantProfiles' | 'participants'>,
|
||||
userIds: ReadonlySet<string>
|
||||
): boolean {
|
||||
return directMessageConversationIncludesUser(conversation, userIds);
|
||||
}
|
||||
@@ -100,23 +100,46 @@ export function isGroupDirectConversation(conversation: DirectMessageConversatio
|
||||
|
||||
export function directMessageConversationIncludesUser(
|
||||
conversation: Pick<DirectMessageConversation, 'participantProfiles' | 'participants'>,
|
||||
userId: string
|
||||
userId: string | ReadonlySet<string>
|
||||
): boolean {
|
||||
return conversation.participants.includes(userId) || !!conversation.participantProfiles[userId];
|
||||
const userIds = typeof userId === 'string' ? new Set([userId]) : userId;
|
||||
|
||||
if (conversation.participants.some((participantId) => userIds.has(participantId))) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (Object.keys(conversation.participantProfiles).some((participantId) => userIds.has(participantId))) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return Object.values(conversation.participantProfiles).some((participant) =>
|
||||
userIds.has(participant.userId)
|
||||
);
|
||||
}
|
||||
|
||||
export function directMessageEventIncludesUser(
|
||||
payload: DirectMessageEventPayload,
|
||||
userId: string
|
||||
userId: string | ReadonlySet<string>
|
||||
): boolean {
|
||||
return collectDirectMessageEventParticipantIds(payload).has(userId);
|
||||
const userIds = typeof userId === 'string' ? new Set([userId]) : userId;
|
||||
const participantIds = collectDirectMessageEventParticipantIds(payload);
|
||||
|
||||
for (const candidate of userIds) {
|
||||
if (participantIds.has(candidate)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
export function directMessageSyncIncludesUser(
|
||||
payload: DirectMessageSyncEventPayload,
|
||||
userId: string
|
||||
userId: string | ReadonlySet<string>
|
||||
): boolean {
|
||||
return payload.participants.some((participant) => participant.userId === userId);
|
||||
const userIds = typeof userId === 'string' ? new Set([userId]) : userId;
|
||||
|
||||
return payload.participants.some((participant) => userIds.has(participant.userId));
|
||||
}
|
||||
|
||||
export function upsertDirectMessage(
|
||||
|
||||
Reference in New Issue
Block a user