From a8372f1b8c6ee269cab370d2d642c209e2589159 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Tue, 31 Dec 2024 11:11:05 +1100 Subject: [PATCH 1/5] fix: show dialog before deleting group also deal with leave/delete group silent retries on 401 --- .../overlay/OverlayRightPanelSettings.tsx | 25 ++++--- .../conversation-list-item/MessageItem.tsx | 5 +- .../menu/ConversationListItemContextMenu.tsx | 4 +- ts/components/menu/Menu.tsx | 8 +-- .../DeleteGroupMenuItem.tsx | 18 ++--- .../LeaveGroupMenuItem.tsx | 14 ++-- .../menu/items/LeaveAndDeleteGroup/guard.ts | 49 ++++++++----- ts/hooks/useParamSelector.ts | 2 +- ts/interactions/conversationInteractions.ts | 70 +++++++++++-------- ts/models/conversation.ts | 1 + ts/session/apis/snode_api/batchRequest.ts | 2 +- ts/session/apis/snode_api/onions.ts | 32 ++++----- ts/session/apis/snode_api/sessionRpc.ts | 7 +- .../conversations/ConversationController.ts | 18 +++-- ts/session/sending/MessageSender.ts | 14 ++-- ts/session/types/with.ts | 1 + .../utils/job_runners/jobs/GroupInviteJob.ts | 1 + .../jobs/GroupPendingRemovalsJob.ts | 12 ++-- .../utils/job_runners/jobs/GroupSyncJob.ts | 5 ++ .../utils/job_runners/jobs/UserSyncJob.ts | 1 + ts/state/ducks/metaGroups.ts | 6 ++ ts/state/selectors/section.ts | 7 +- .../group_sync_job/GroupSyncJob_test.ts | 3 + 23 files changed, 185 insertions(+), 120 deletions(-) diff --git a/ts/components/conversation/right-panel/overlay/OverlayRightPanelSettings.tsx b/ts/components/conversation/right-panel/overlay/OverlayRightPanelSettings.tsx index 8b2843062..c50d7b5e1 100644 --- a/ts/components/conversation/right-panel/overlay/OverlayRightPanelSettings.tsx +++ b/ts/components/conversation/right-panel/overlay/OverlayRightPanelSettings.tsx @@ -1,7 +1,7 @@ import { compact, flatten, isEqual } from 'lodash'; import { SessionDataTestId, useEffect, useState } from 'react'; -import { useDispatch, useSelector } from 'react-redux'; +import { useDispatch } from 'react-redux'; import useInterval from 'react-use/lib/useInterval'; import styled from 'styled-components'; import { Data } from '../../../../data/data'; @@ -11,13 +11,14 @@ import { useConversationUsername, useDisappearingMessageSettingText, useIsClosedGroup, + useIsGroupDestroyed, useIsKickedFromGroup, useIsPublic, - useLastMessageIsLeaveError, } from '../../../../hooks/useParamSelector'; import { useIsRightPanelShowing } from '../../../../hooks/useUI'; import { showAddModeratorsByConvoId, + showDeleteGroupByConvoId, showInviteContactByConvoId, showLeaveGroupByConvoId, showRemoveModeratorsByConvoId, @@ -57,7 +58,7 @@ import { showDeleteGroupItem, showLeaveGroupItem, } from '../../../menu/items/LeaveAndDeleteGroup/guard'; -import { getIsMessageRequestOverlayShown } from '../../../../state/selectors/section'; +import { useIsMessageRequestOverlayShown } from '../../../../state/selectors/section'; import { showLeaveCommunityItem } from '../../../menu/items/LeaveCommunity/guard'; async function getMediaGalleryProps(conversationId: string): Promise<{ @@ -229,16 +230,18 @@ const LeaveCommunityPanelButton = () => { const DeleteGroupPanelButton = () => { const convoId = useSelectedConversationKey(); const isGroup = useIsClosedGroup(convoId); - const isMessageRequestShown = useSelector(getIsMessageRequestOverlayShown); + const isMessageRequestShown = useIsMessageRequestOverlayShown(); const isKickedFromGroup = useIsKickedFromGroup(convoId) || false; - const lastMessageIsLeaveError = useLastMessageIsLeaveError(convoId); const selectedUsername = useConversationUsername(convoId) || convoId; + const isPublic = useIsPublic(convoId); + const isGroupDestroyed = useIsGroupDestroyed(convoId); const showItem = showDeleteGroupItem({ isGroup, isKickedFromGroup, isMessageRequestShown, - lastMessageIsLeaveError, + isPublic, + isGroupDestroyed, }); if (!showItem || !convoId) { @@ -251,7 +254,7 @@ const DeleteGroupPanelButton = () => { void showLeaveGroupByConvoId(convoId, selectedUsername)} + onClick={() => void showDeleteGroupByConvoId(convoId, selectedUsername)} color={'var(--danger-color)'} iconType={'delete'} /> @@ -262,15 +265,17 @@ const LeaveGroupPanelButton = () => { const selectedConvoKey = useSelectedConversationKey(); const isGroup = useIsClosedGroup(selectedConvoKey); const username = useConversationUsername(selectedConvoKey) || selectedConvoKey; - const isMessageRequestShown = useSelector(getIsMessageRequestOverlayShown); + const isMessageRequestShown = useIsMessageRequestOverlayShown(); const isKickedFromGroup = useIsKickedFromGroup(selectedConvoKey) || false; - const lastMessageIsLeaveError = useLastMessageIsLeaveError(selectedConvoKey); + const isPublic = useIsPublic(selectedConvoKey); + const isGroupDestroyed = useIsGroupDestroyed(selectedConvoKey); const showItem = showLeaveGroupItem({ isGroup, isKickedFromGroup, isMessageRequestShown, - lastMessageIsLeaveError, + isPublic, + isGroupDestroyed, }); if (!selectedConvoKey || !showItem) { diff --git a/ts/components/leftpane/conversation-list-item/MessageItem.tsx b/ts/components/leftpane/conversation-list-item/MessageItem.tsx index 78d240cdf..1273a0ae7 100644 --- a/ts/components/leftpane/conversation-list-item/MessageItem.tsx +++ b/ts/components/leftpane/conversation-list-item/MessageItem.tsx @@ -1,7 +1,6 @@ import classNames from 'classnames'; import { isEmpty } from 'lodash'; -import { useSelector } from 'react-redux'; import { useConvoIdFromContext } from '../../../contexts/ConvoIdContext'; import { useHasUnread, @@ -12,7 +11,7 @@ import { } from '../../../hooks/useParamSelector'; import { LastMessageStatusType } from '../../../state/ducks/types'; import { useIsSearching } from '../../../state/selectors/search'; -import { getIsMessageRequestOverlayShown } from '../../../state/selectors/section'; +import { useIsMessageRequestOverlayShown } from '../../../state/selectors/section'; import { assertUnreachable } from '../../../types/sqlSharedTypes'; import { TypingAnimation } from '../../conversation/TypingAnimation'; import { MessageBody } from '../../conversation/message/message-content/MessageBody'; @@ -26,7 +25,7 @@ export const MessageItem = () => { const hasUnread = useHasUnread(conversationId); const isConvoTyping = useIsTyping(conversationId); - const isMessageRequest = useSelector(getIsMessageRequestOverlayShown); + const isMessageRequest = useIsMessageRequestOverlayShown(); const isOutgoingRequest = useIsOutgoingRequest(conversationId); const isSearching = useIsSearching(); diff --git a/ts/components/menu/ConversationListItemContextMenu.tsx b/ts/components/menu/ConversationListItemContextMenu.tsx index 2ac35998f..83aeb1964 100644 --- a/ts/components/menu/ConversationListItemContextMenu.tsx +++ b/ts/components/menu/ConversationListItemContextMenu.tsx @@ -5,8 +5,8 @@ import { useConvoIdFromContext } from '../../contexts/ConvoIdContext'; import { useIsPinned, useIsPrivate, useIsPrivateAndFriend } from '../../hooks/useParamSelector'; import { ConvoHub } from '../../session/conversations'; import { - getIsMessageRequestOverlayShown, getIsMessageSection, + useIsMessageRequestOverlayShown, } from '../../state/selectors/section'; import { useIsSearching } from '../../state/selectors/search'; import { SessionContextMenuContainer } from '../SessionContextMenuContainer'; @@ -91,7 +91,7 @@ export const PinConversationMenuItem = (): JSX.Element | null => { const isPrivateAndFriend = useIsPrivateAndFriend(conversationId); const isPrivate = useIsPrivate(conversationId); const isPinned = useIsPinned(conversationId); - const isMessageRequest = useSelector(getIsMessageRequestOverlayShown); + const isMessageRequest = useIsMessageRequestOverlayShown(); if (isMessagesSection && !isMessageRequest && (!isPrivate || (isPrivate && isPrivateAndFriend))) { const conversation = ConvoHub.use().get(conversationId); diff --git a/ts/components/menu/Menu.tsx b/ts/components/menu/Menu.tsx index b03237548..2c4d0ae90 100644 --- a/ts/components/menu/Menu.tsx +++ b/ts/components/menu/Menu.tsx @@ -49,8 +49,8 @@ import { } from '../../state/ducks/modalDialog'; import { useConversationIdOrigin } from '../../state/selectors/conversations'; import { - getIsMessageRequestOverlayShown, getIsMessageSection, + useIsMessageRequestOverlayShown, } from '../../state/selectors/section'; import { useSelectedConversationKey } from '../../state/selectors/selectedConversation'; import type { LocalizerToken } from '../../types/localizer'; @@ -84,7 +84,7 @@ export const MarkConversationUnreadMenuItem = (): JSX.Element | null => { const isMessagesSection = useSelector(getIsMessageSection); const isPrivate = useIsPrivate(conversationId); const isPrivateAndFriend = useIsPrivateAndFriend(conversationId); - const isMessageRequestShown = useSelector(getIsMessageRequestOverlayShown); + const isMessageRequestShown = useIsMessageRequestOverlayShown(); if ( isMessagesSection && @@ -358,7 +358,7 @@ export const ChangeNicknameMenuItem = () => { */ export const DeleteMessagesMenuItem = () => { const convoId = useConvoIdFromContext(); - const isMessageRequestShown = useSelector(getIsMessageRequestOverlayShown); + const isMessageRequestShown = useIsMessageRequestOverlayShown(); if (!convoId || isMessageRequestShown) { return null; @@ -495,7 +495,7 @@ export const NotificationForConvoMenuItem = (): JSX.Element | null => { const isFriend = useIsPrivateAndFriend(convoId); const isPrivate = useIsPrivate(convoId); - const isMessageRequestShown = useSelector(getIsMessageRequestOverlayShown); + const isMessageRequestShown = useIsMessageRequestOverlayShown(); if ( !convoId || diff --git a/ts/components/menu/items/LeaveAndDeleteGroup/DeleteGroupMenuItem.tsx b/ts/components/menu/items/LeaveAndDeleteGroup/DeleteGroupMenuItem.tsx index 18411a807..9fdc5aaa9 100644 --- a/ts/components/menu/items/LeaveAndDeleteGroup/DeleteGroupMenuItem.tsx +++ b/ts/components/menu/items/LeaveAndDeleteGroup/DeleteGroupMenuItem.tsx @@ -1,14 +1,14 @@ -import { useSelector } from 'react-redux'; import { useConvoIdFromContext } from '../../../../contexts/ConvoIdContext'; import { useConversationUsername, useIsKickedFromGroup, useIsClosedGroup, - useLastMessageIsLeaveError, + useIsPublic, + useIsGroupDestroyed, } from '../../../../hooks/useParamSelector'; -import { showLeaveGroupByConvoId } from '../../../../interactions/conversationInteractions'; +import { showDeleteGroupByConvoId } from '../../../../interactions/conversationInteractions'; import { PubKey } from '../../../../session/types'; -import { getIsMessageRequestOverlayShown } from '../../../../state/selectors/section'; +import { useIsMessageRequestOverlayShown } from '../../../../state/selectors/section'; import { ItemWithDataTestId } from '../MenuItemWithDataTestId'; import { showDeleteGroupItem } from './guard'; import { Localizer } from '../../../basic/Localizer'; @@ -17,15 +17,17 @@ export const DeleteGroupMenuItem = () => { const convoId = useConvoIdFromContext(); const username = useConversationUsername(convoId) || convoId; const isGroup = useIsClosedGroup(convoId); - const isMessageRequestShown = useSelector(getIsMessageRequestOverlayShown); + const isMessageRequestShown = useIsMessageRequestOverlayShown(); const isKickedFromGroup = useIsKickedFromGroup(convoId) || false; - const lastMessageIsLeaveError = useLastMessageIsLeaveError(convoId); + const isPublic = useIsPublic(convoId); + const isGroupDestroyed = useIsGroupDestroyed(convoId); const showLeave = showDeleteGroupItem({ isGroup, isKickedFromGroup, isMessageRequestShown, - lastMessageIsLeaveError, + isPublic, + isGroupDestroyed, }); if (!showLeave) { @@ -37,7 +39,7 @@ export const DeleteGroupMenuItem = () => { return ( { - void showLeaveGroupByConvoId(convoId, username); + void showDeleteGroupByConvoId(convoId, username); }} > diff --git a/ts/components/menu/items/LeaveAndDeleteGroup/LeaveGroupMenuItem.tsx b/ts/components/menu/items/LeaveAndDeleteGroup/LeaveGroupMenuItem.tsx index 4facb6bbd..760273513 100644 --- a/ts/components/menu/items/LeaveAndDeleteGroup/LeaveGroupMenuItem.tsx +++ b/ts/components/menu/items/LeaveAndDeleteGroup/LeaveGroupMenuItem.tsx @@ -1,13 +1,13 @@ -import { useSelector } from 'react-redux'; import { useConvoIdFromContext } from '../../../../contexts/ConvoIdContext'; import { useConversationUsername, useIsKickedFromGroup, useIsClosedGroup, - useLastMessageIsLeaveError, + useIsPublic, + useIsGroupDestroyed, } from '../../../../hooks/useParamSelector'; import { showLeaveGroupByConvoId } from '../../../../interactions/conversationInteractions'; -import { getIsMessageRequestOverlayShown } from '../../../../state/selectors/section'; +import { useIsMessageRequestOverlayShown } from '../../../../state/selectors/section'; import { ItemWithDataTestId } from '../MenuItemWithDataTestId'; import { showLeaveGroupItem } from './guard'; import { Localizer } from '../../../basic/Localizer'; @@ -15,16 +15,18 @@ import { Localizer } from '../../../basic/Localizer'; export const LeaveGroupMenuItem = () => { const convoId = useConvoIdFromContext(); const isGroup = useIsClosedGroup(convoId); + const isPublic = useIsPublic(convoId); const username = useConversationUsername(convoId) || convoId; - const isMessageRequestShown = useSelector(getIsMessageRequestOverlayShown); + const isMessageRequestShown = useIsMessageRequestOverlayShown(); const isKickedFromGroup = useIsKickedFromGroup(convoId) || false; - const lastMessageIsLeaveError = useLastMessageIsLeaveError(convoId); + const isGroupDestroyed = useIsGroupDestroyed(convoId); const showLeave = showLeaveGroupItem({ isGroup, isMessageRequestShown, isKickedFromGroup, - lastMessageIsLeaveError, + isPublic, + isGroupDestroyed, }); if (!showLeave) { diff --git a/ts/components/menu/items/LeaveAndDeleteGroup/guard.ts b/ts/components/menu/items/LeaveAndDeleteGroup/guard.ts index e0dda44f5..baafe0d67 100644 --- a/ts/components/menu/items/LeaveAndDeleteGroup/guard.ts +++ b/ts/components/menu/items/LeaveAndDeleteGroup/guard.ts @@ -1,42 +1,53 @@ function sharedEnabled({ isGroup, + isPublic, isMessageRequestShown, -}: Pick[0], 'isGroup' | 'isMessageRequestShown'>) { - return isGroup && !isMessageRequestShown; +}: Pick< + Parameters[0], + 'isGroup' | 'isMessageRequestShown' | 'isPublic' +>) { + return isGroup && !isMessageRequestShown && !isPublic; } - +/** + * We can try leave a group if + * - we are an admin of the group (that group would be marked as destroyed on delete) + * and + * - we are a **not kicked** member (if we are kicked without knowing about it and try to leave, we will silently remove the group) + * + * Note: Those actions are hidden if the group is a group request (as we have other buttons to accept/decline a group request). + * + * Note: If we fail to leave the group but that error is retryable, we will keep the group displaying the "leave" option. + */ export function showLeaveGroupItem({ isGroup, + isPublic, isKickedFromGroup, isMessageRequestShown, - lastMessageIsLeaveError, + isGroupDestroyed, }: { isGroup: boolean; + isPublic: boolean; isMessageRequestShown: boolean; - lastMessageIsLeaveError: boolean; isKickedFromGroup: boolean; + isGroupDestroyed: boolean; }) { - // we can't try to leave the group if we were kicked from it, or if we've already tried to (lastMessageIsLeaveError is true) return ( - sharedEnabled({ isGroup, isMessageRequestShown }) && + sharedEnabled({ isGroup, isMessageRequestShown, isPublic }) && !isKickedFromGroup && - !lastMessageIsLeaveError + !isGroupDestroyed ); } -export function showDeleteGroupItem({ - isGroup, - isKickedFromGroup, - isMessageRequestShown, - lastMessageIsLeaveError, -}: { +/** + * We can try to delete a group only if the `showLeaveGroupItem` returns false. + * Note: those actions are hidden if the group is a group request (as we have other buttons to accept/decline a group request) + */ +export function showDeleteGroupItem(args: { isGroup: boolean; + isPublic: boolean; isMessageRequestShown: boolean; - lastMessageIsLeaveError: boolean; isKickedFromGroup: boolean; + isGroupDestroyed: boolean; }) { - return ( - sharedEnabled({ isGroup, isMessageRequestShown }) && - (isKickedFromGroup || lastMessageIsLeaveError) - ); + return sharedEnabled(args) && !showLeaveGroupItem(args); } diff --git a/ts/hooks/useParamSelector.ts b/ts/hooks/useParamSelector.ts index 66a712354..e35ae40dd 100644 --- a/ts/hooks/useParamSelector.ts +++ b/ts/hooks/useParamSelector.ts @@ -212,7 +212,7 @@ export function useIsKickedFromGroup(convoId?: string) { export function useIsGroupDestroyed(convoId?: string) { const libIsDestroyed = useLibGroupDestroyed(convoId); if (convoId && PubKey.is03Pubkey(convoId)) { - return libIsDestroyed; + return libIsDestroyed || false; } return false; } diff --git a/ts/interactions/conversationInteractions.ts b/ts/interactions/conversationInteractions.ts index cb1734caf..1d4540276 100644 --- a/ts/interactions/conversationInteractions.ts +++ b/ts/interactions/conversationInteractions.ts @@ -431,21 +431,6 @@ async function leaveGroupOrCommunityByConvoId({ } } -/** - * Returns true if we the convo is a 03 group and if we can try to send a leave message. - */ -async function hasLeavingDetails(convoId: string) { - if (!PubKey.is03Pubkey(convoId)) { - return true; - } - - const group = await UserGroupsWrapperActions.getGroup(convoId); - - // we need the authData or the secretKey to be able to attempt to leave, - // otherwise we won't be able to even try - return group && (!isEmpty(group.authData) || !isEmpty(group.secretKey)); -} - export async function showLeaveGroupByConvoId(conversationId: string, name: string | undefined) { const conversation = ConvoHub.use().get(conversationId); @@ -462,20 +447,6 @@ export async function showLeaveGroupByConvoId(conversationId: string, name: stri (PubKey.is05Pubkey(conversationId) || PubKey.is03Pubkey(conversationId)) && isAdmin && admins.length === 1; - const lastMessageInteractionType = conversation.get('lastMessageInteractionType'); - const lastMessageInteractionStatus = conversation.get('lastMessageInteractionStatus'); - - const canTryToLeave = await hasLeavingDetails(conversationId); - - if ( - !isPublic && - ((lastMessageInteractionType === ConversationInteractionType.Leave && - lastMessageInteractionStatus === ConversationInteractionStatus.Error) || - !canTryToLeave) // if we don't have any key to send our leave message, no need to try - ) { - await leaveGroupOrCommunityByConvoId({ conversationId, isPublic, sendLeaveMessage: false }); - return; - } // if this is a community, or we legacy group are not admin, we can just show a confirmation dialog @@ -525,6 +496,46 @@ export async function showLeaveGroupByConvoId(conversationId: string, name: stri } } +/** + * Can be used to show a dialog asking confirmation about deleting a group. + * Communities are explicitly forbidden. + * This function won't attempt to send a leave message. Use `showLeaveGroupByConvoId` for that purpose + */ +export async function showDeleteGroupByConvoId(conversationId: string, name: string | undefined) { + const conversation = ConvoHub.use().get(conversationId); + + const isPublic = conversation.isPublic(); + + if (!conversation.isGroup() || isPublic) { + throw new Error('showDeleteGroupByConvoId() called with a non group convo.'); + } + + const onClickClose = () => { + window?.inboxStore?.dispatch(updateConfirmModal(null)); + }; + + const onClickOk = async () => { + await leaveGroupOrCommunityByConvoId({ + conversationId, + isPublic, // we check for isPublic above, and throw if it's true + sendLeaveMessage: false, + onClickClose, + }); + }; + + window?.inboxStore?.dispatch( + updateConfirmModal({ + title: window.i18n('groupDelete'), + i18nMessage: { token: 'groupDeleteDescriptionMember', args: { group_name: name ?? '' } }, + onClickOk, + okText: window.i18n('delete'), + okTheme: SessionButtonColor.Danger, + onClickClose, + conversationId, + }) + ); +} + export function showInviteContactByConvoId(conversationId: string) { window.inboxStore?.dispatch(updateInviteContactModal({ conversationId })); } @@ -1027,6 +1038,7 @@ export async function promoteUsersInGroup({ method: 'batch', sortedSubRequests: storeRequests, abortSignal: controller.signal, + allow401s: false, }), 2 * DURATION.MINUTES, controller diff --git a/ts/models/conversation.ts b/ts/models/conversation.ts index 09a2e3fa2..93a88876a 100644 --- a/ts/models/conversation.ts +++ b/ts/models/conversation.ts @@ -1148,6 +1148,7 @@ export class ConversationModel extends Backbone.Model { await GroupSync.pushChangesToGroupSwarmIfNeeded({ groupPk: this.id, extraStoreRequests, + allow401s: false, }); await GroupSync.queueNewJobIfNeeded(this.id); diff --git a/ts/session/apis/snode_api/batchRequest.ts b/ts/session/apis/snode_api/batchRequest.ts index 2ce77e9bc..64652bcc4 100644 --- a/ts/session/apis/snode_api/batchRequest.ts +++ b/ts/session/apis/snode_api/batchRequest.ts @@ -15,6 +15,7 @@ import { } from './SnodeRequestTypes'; import { NotEmptyArrayOfBatchResults } from './BatchResultEntry'; import { MergedAbortSignal, WithTimeoutMs } from './requestWith'; +import { WithAllow401s } from '../../types/with'; function logSubRequests(requests: Array) { return `[${requests.map(builtRequestToLoggingId).join(', ')}]`; @@ -22,7 +23,6 @@ function logSubRequests(requests: Array) { type WithTargetNode = { targetNode: Snode }; type WithAssociatedWith = { associatedWith: string | null }; -type WithAllow401s = { allow401s: boolean }; /** * This is the equivalent to the batch send on sogs. The target node runs each sub request and returns a list of all the sub status and bodies. diff --git a/ts/session/apis/snode_api/onions.ts b/ts/session/apis/snode_api/onions.ts index 6ef56204a..a99e36252 100644 --- a/ts/session/apis/snode_api/onions.ts +++ b/ts/session/apis/snode_api/onions.ts @@ -21,6 +21,7 @@ import { fileServerHost } from '../file_server_api/FileServerApi'; import { hrefPnServerProd } from '../push_notification_api/PnServer'; import { ERROR_CODE_NO_CONNECT } from './SNodeAPI'; import { MergedAbortSignal, WithAbortSignal, WithTimeoutMs } from './requestWith'; +import { WithAllow401s } from '../../types/with'; // hold the ed25519 key of a snode against the time it fails. Used to remove a snode only after a few failures (snodeFailureThreshold failures) let snodeFailureCount: Record = {}; @@ -310,12 +311,11 @@ export async function processOnionRequestErrorAtDestination({ destinationSnodeEd25519, associatedWith, allow401s, -}: { +}: WithAllow401s & { statusCode: number; body: string; destinationSnodeEd25519?: string; associatedWith?: string; - allow401s: boolean; }) { if (statusCode === 200) { return; @@ -526,14 +526,14 @@ async function processOnionResponse({ associatedWith, destinationSnodeEd25519, allow401s, -}: Partial & { - response?: { text: () => Promise; status: number }; - symmetricKey?: ArrayBuffer; - guardNode: Snode; - destinationSnodeEd25519?: string; - associatedWith?: string; - allow401s: boolean; -}): Promise { +}: Partial & + WithAllow401s & { + response?: { text: () => Promise; status: number }; + symmetricKey?: ArrayBuffer; + guardNode: Snode; + destinationSnodeEd25519?: string; + associatedWith?: string; + }): Promise { let ciphertext = ''; processAbortedRequest(abortSignal); @@ -828,7 +828,8 @@ async function sendOnionRequestHandlingSnodeEjectNoRetries({ allow401s, timeoutMs, }: WithAbortSignal & - WithTimeoutMs & { + WithTimeoutMs & + WithAllow401s & { nodePath: Array; destSnodeX25519: string; finalDestOptions: FinalDestOptions; @@ -836,7 +837,6 @@ async function sendOnionRequestHandlingSnodeEjectNoRetries({ associatedWith?: string; useV4: boolean; throwErrors: boolean; - allow401s: boolean; }): Promise { // this sendOnionRequestNoRetries() call has to be the only one like this. // If you need to call it, call it through sendOnionRequestHandlingSnodeEjectNoRetries because this is the one handling path rebuilding and known errors @@ -1118,12 +1118,12 @@ async function sendOnionRequestSnodeDestNoRetries({ timeoutMs, associatedWith, }: WithTimeoutMs & - WithAbortSignal & { + WithAbortSignal & + WithAllow401s & { onionPath: Array; targetNode: Snode; headers: Record; plaintext: string | null; - allow401s: boolean; associatedWith?: string; }) { return Onions.sendOnionRequestHandlingSnodeEjectNoRetries({ @@ -1155,12 +1155,12 @@ async function lokiOnionFetchNoRetries({ abortSignal, timeoutMs, }: WithTimeoutMs & - WithAbortSignal & { + WithAbortSignal & + WithAllow401s & { targetNode: Snode; headers: Record; body: string | null; associatedWith?: string; - allow401s: boolean; }): Promise { try { // Get a path excluding `targetNode`: diff --git a/ts/session/apis/snode_api/sessionRpc.ts b/ts/session/apis/snode_api/sessionRpc.ts index 0303d1a7a..5a499f1bd 100644 --- a/ts/session/apis/snode_api/sessionRpc.ts +++ b/ts/session/apis/snode_api/sessionRpc.ts @@ -10,6 +10,7 @@ import { HTTPError, NotFoundError } from '../../utils/errors'; import { APPLICATION_JSON } from '../../../types/MIME'; import { ERROR_421_HANDLED_RETRY_REQUEST, Onions, snodeHttpsAgent, SnodeResponse } from './onions'; import { WithAbortSignal, WithTimeoutMs } from './requestWith'; +import { WithAllow401s } from '../../types/with'; export interface LokiFetchOptions { method: 'GET' | 'POST'; @@ -32,12 +33,12 @@ async function doRequestNoRetries({ allow401s, abortSignal, }: WithTimeoutMs & - WithAbortSignal & { + WithAbortSignal & + WithAllow401s & { url: string; options: LokiFetchOptions; targetNode?: Snode; associatedWith: string | null; - allow401s: boolean; }): Promise { const method = options.method || 'GET'; @@ -126,12 +127,12 @@ async function snodeRpcNoRetries( timeoutMs, abortSignal, }: WithTimeoutMs & + WithAllow401s & WithAbortSignal & { method: string; params: Record | Array>; targetNode: Snode; associatedWith: string | null; - allow401s: boolean; } // the user pubkey this call is for. if the onion request fails, this is used to handle the error for this user swarm for instance ): Promise { const url = `https://${targetNode.ip}:${targetNode.port}/storage_rpc/v1`; diff --git a/ts/session/conversations/ConversationController.ts b/ts/session/conversations/ConversationController.ts index 5353cb42a..f293f706e 100644 --- a/ts/session/conversations/ConversationController.ts +++ b/ts/session/conversations/ConversationController.ts @@ -286,8 +286,8 @@ class ConvoController { const groupInUserGroup = await UserGroupsWrapperActions.getGroup(groupPk); // send the leave message before we delete everything for this group (including the key!) - // Note: if we were kicked, we already lost the authData/secretKey for it, so no need to try to send our message. - if (sendLeaveMessage && !groupInUserGroup?.kicked) { + + if (sendLeaveMessage) { const failedToSendLeaveMessage = await leaveClosedGroup(groupPk, fromSyncMessage); if (PubKey.is03Pubkey(groupPk) && failedToSendLeaveMessage) { // this is caught and is adding an interaction notification message @@ -369,6 +369,7 @@ class ConvoController { groupPk, deleteAllMessagesSubRequest, extraStoreRequests: [], + allow401s: false, }); await LibSessionUtil.saveDumpsToDb(groupPk); @@ -573,7 +574,7 @@ class ConvoController { throw new Error(`ConvoHub.${deleteType} needs complete initial fetch`); } - window.log.info(`${deleteType} with ${ed25519Str(convoId)}`); + window.log.info(`deleteConvoInitialChecks: type ${deleteType} with ${ed25519Str(convoId)}`); const conversation = this.conversations.get(convoId); if (!conversation) { @@ -693,7 +694,7 @@ async function leaveClosedGroup(groupPk: PubkeyType | GroupPubkeyType, fromSyncM window?.log?.info( `We are leaving the group ${ed25519Str(groupPk)}. Sending our leaving messages.` ); - let failedToSent03LeaveMessage = false; + let failedToSent03LeaveMessage = true; // We might not be able to send our leaving messages (no encryption key pair, we were already removed, no network, etc). // If that happens, we should just remove everything from our current user. try { @@ -711,23 +712,28 @@ async function leaveClosedGroup(groupPk: PubkeyType | GroupPubkeyType, fromSyncM sortedSubRequests: storeRequests, method: 'sequence', abortSignal: controller.signal, + allow401s: true, // we want "allow" 401s so we don't throw }), 30 * DURATION.SECONDS, controller ); - if (results?.[0].code !== 200) { + if (results?.[0].code === 401) { + window.log.info( + `leaveClosedGroup for ${ed25519Str(groupPk)} failed with 401. Assuming we've been revoked.` + ); + } else if (results?.[0].code !== 200) { throw new Error( `Even with the retries, leaving message for group ${ed25519Str( groupPk )} failed to be sent...` ); } + failedToSent03LeaveMessage = false; } catch (e) { window?.log?.warn( `failed to send our leaving messages for ${ed25519Str(groupPk)}:${e.message}` ); - failedToSent03LeaveMessage = true; } // the rest of the cleaning of that conversation is done in the `deleteClosedGroup()` diff --git a/ts/session/sending/MessageSender.ts b/ts/session/sending/MessageSender.ts index 9472ab7a1..9705d7347 100644 --- a/ts/session/sending/MessageSender.ts +++ b/ts/session/sending/MessageSender.ts @@ -57,6 +57,7 @@ import { SaveSeenMessageHash, stringify } from '../../types/sqlSharedTypes'; import { OpenGroupRequestCommonType } from '../../data/types'; import { NetworkTime } from '../../util/NetworkTime'; import { MergedAbortSignal } from '../apis/snode_api/requestWith'; +import { WithAllow401s } from '../types/with'; // ================ SNODE STORE ================ @@ -420,7 +421,8 @@ async function sendMessagesDataToSnode({ sortedSubRequests, method, abortSignal, -}: { + allow401s, +}: WithAllow401s & { sortedSubRequests: SortedSubRequestsType; associatedWith: T; method: MethodBatchType; @@ -448,7 +450,7 @@ async function sendMessagesDataToSnode({ targetNode, timeoutMs: 6 * DURATION.SECONDS, associatedWith, - allow401s: false, + allow401s, method, abortSignal, }); @@ -473,7 +475,9 @@ async function sendMessagesDataToSnode({ 'first result status is not 200 for sendMessagesDataToSnode but: ', firstResult.code ); - throw new Error('sendMessagesDataToSnode: Invalid status code'); + if (!allow401s || firstResult.code !== 401) { + throw new Error('sendMessagesDataToSnode: Invalid status code'); + } } GetNetworkTime.handleTimestampOffsetFromNetwork('store', firstResult.body.t); @@ -509,7 +513,8 @@ async function sendEncryptedDataToSnode( sortedSubRequests, method, abortSignal, -}: { + allow401s, +}: WithAllow401s & { sortedSubRequests: SortedSubRequestsType; // keeping those as an array because the order needs to be enforced for some (group keys for instance) destination: T; method: MethodBatchType; @@ -523,6 +528,7 @@ async function sendEncryptedDataToSnode( associatedWith: destination, method, abortSignal, + allow401s, }); }, { diff --git a/ts/session/types/with.ts b/ts/session/types/with.ts index b7d6ff3ae..37db18903 100644 --- a/ts/session/types/with.ts +++ b/ts/session/types/with.ts @@ -25,3 +25,4 @@ export type WithLocalMessageDeletionType = { deletionType: 'complete' | 'markDel export type ShortenOrExtend = 'extend' | 'shorten' | ''; export type WithShortenOrExtend = { shortenOrExtend: ShortenOrExtend }; export type WithMessagesHashes = { messagesHashes: Array }; +export type WithAllow401s = { allow401s: boolean }; diff --git a/ts/session/utils/job_runners/jobs/GroupInviteJob.ts b/ts/session/utils/job_runners/jobs/GroupInviteJob.ts index 83f8949bd..ac84a7283 100644 --- a/ts/session/utils/job_runners/jobs/GroupInviteJob.ts +++ b/ts/session/utils/job_runners/jobs/GroupInviteJob.ts @@ -200,6 +200,7 @@ class GroupInviteJob extends PersistedJob { groupPk, unrevokeSubRequest, extraStoreRequests: [], + allow401s: false, }); if (sequenceResult !== RunJobResult.Success) { await LibSessionUtil.saveDumpsToDb(groupPk); diff --git a/ts/session/utils/job_runners/jobs/GroupPendingRemovalsJob.ts b/ts/session/utils/job_runners/jobs/GroupPendingRemovalsJob.ts index 17857b945..ce4f65d6a 100644 --- a/ts/session/utils/job_runners/jobs/GroupPendingRemovalsJob.ts +++ b/ts/session/utils/job_runners/jobs/GroupPendingRemovalsJob.ts @@ -68,13 +68,10 @@ async function getPendingRevokeParams({ const revokeChanges: RevokeChanges = []; const unrevokeChanges: RevokeChanges = []; - for (let index = 0; index < withoutHistory.length; index++) { - const m = withoutHistory[index]; - const token = await MetaGroupWrapperActions.swarmSubAccountToken(groupPk, m); - unrevokeChanges.push({ action: 'unrevoke_subaccount', tokenToRevokeHex: token }); - } - for (let index = 0; index < withHistory.length; index++) { - const m = withHistory[index]; + const toUnrevoke = withoutHistory.concat(withHistory); + + for (let index = 0; index < toUnrevoke.length; index++) { + const m = toUnrevoke[index]; const token = await MetaGroupWrapperActions.swarmSubAccountToken(groupPk, m); unrevokeChanges.push({ action: 'unrevoke_subaccount', tokenToRevokeHex: token }); } @@ -198,6 +195,7 @@ class GroupPendingRemovalsJob extends PersistedJob { return await GroupSync.pushChangesToGroupSwarmIfNeeded({ groupPk: thisJobDestination, extraStoreRequests: [], + allow401s: false, }); } catch (e) { window.log.warn('GroupSyncJob failed with', e.message); diff --git a/ts/session/utils/job_runners/jobs/UserSyncJob.ts b/ts/session/utils/job_runners/jobs/UserSyncJob.ts index 7e4500917..9b861ba4e 100644 --- a/ts/session/utils/job_runners/jobs/UserSyncJob.ts +++ b/ts/session/utils/job_runners/jobs/UserSyncJob.ts @@ -126,6 +126,7 @@ async function pushChangesToUserSwarmIfNeeded() { destination: us, method: 'sequence', abortSignal: controller.signal, + allow401s: false, // user swarm push, we shouldn't need to allow 401s }), 30 * DURATION.SECONDS, controller diff --git a/ts/state/ducks/metaGroups.ts b/ts/state/ducks/metaGroups.ts index ad1bb38df..05bb277ec 100644 --- a/ts/state/ducks/metaGroups.ts +++ b/ts/state/ducks/metaGroups.ts @@ -225,6 +225,7 @@ const initNewGroupInWrapper = createAsyncThunk( const result = await GroupSync.pushChangesToGroupSwarmIfNeeded({ groupPk, extraStoreRequests, + allow401s: false, }); if (result !== RunJobResult.Success) { window.log.warn('GroupSync.pushChangesToGroupSwarmIfNeeded during create failed'); @@ -670,6 +671,7 @@ async function handleMemberAddedFromUI({ revokeSubRequest, unrevokeSubRequest, extraStoreRequests, + allow401s: false, }); if (sequenceResult !== RunJobResult.Success) { await LibSessionUtil.saveDumpsToDb(groupPk); @@ -795,6 +797,7 @@ async function handleMemberRemovedFromUI({ const sequenceResult = await GroupSync.pushChangesToGroupSwarmIfNeeded({ groupPk, extraStoreRequests, + allow401s: false, }); if (sequenceResult !== RunJobResult.Success) { throw new Error( @@ -880,6 +883,7 @@ async function handleNameChangeFromUI({ const batchResult = await GroupSync.pushChangesToGroupSwarmIfNeeded({ groupPk, extraStoreRequests, + allow401s: false, }); if (batchResult !== RunJobResult.Success) { @@ -1005,6 +1009,7 @@ const triggerFakeAvatarUpdate = createAsyncThunk( const batchResult = await GroupSync.pushChangesToGroupSwarmIfNeeded({ groupPk, extraStoreRequests, + allow401s: false, }); if (!batchResult) { window.log.warn(`failed to send avatarChange message for group ${ed25519Str(groupPk)}`); @@ -1060,6 +1065,7 @@ const triggerFakeDeleteMsgBeforeNow = createAsyncThunk( const batchResult = await GroupSync.pushChangesToGroupSwarmIfNeeded({ groupPk, extraStoreRequests, + allow401s: false, }); if (!batchResult) { window.log.warn( diff --git a/ts/state/selectors/section.ts b/ts/state/selectors/section.ts index 39992174e..c74db0f53 100644 --- a/ts/state/selectors/section.ts +++ b/ts/state/selectors/section.ts @@ -1,5 +1,6 @@ import { createSelector } from '@reduxjs/toolkit'; +import { useSelector } from 'react-redux'; import { LeftOverlayMode, SectionStateType, SectionType } from '../ducks/section'; import { StateType } from '../reducer'; import type { SessionSettingCategory } from '../../types/ReduxTypes'; @@ -34,9 +35,13 @@ export const getRightOverlayMode = (state: StateType) => { return state.section.rightOverlayMode; }; -export const getIsMessageRequestOverlayShown = (state: StateType) => { +const getIsMessageRequestOverlayShown = (state: StateType) => { const focusedSection = getFocusedSection(state); const leftOverlayMode = getLeftOverlayMode(state); return focusedSection === SectionType.Message && leftOverlayMode === 'message-requests'; }; + +export function useIsMessageRequestOverlayShown() { + return useSelector(getIsMessageRequestOverlayShown); +} diff --git a/ts/test/session/unit/utils/job_runner/group_sync_job/GroupSyncJob_test.ts b/ts/test/session/unit/utils/job_runner/group_sync_job/GroupSyncJob_test.ts index 9d7ed628b..a1cd62785 100644 --- a/ts/test/session/unit/utils/job_runner/group_sync_job/GroupSyncJob_test.ts +++ b/ts/test/session/unit/utils/job_runner/group_sync_job/GroupSyncJob_test.ts @@ -297,6 +297,7 @@ describe('GroupSyncJob pushChangesToGroupSwarmIfNeeded', () => { const result = await GroupSync.pushChangesToGroupSwarmIfNeeded({ groupPk, extraStoreRequests: [], + allow401s: false, }); expect(result).to.be.eq(RunJobResult.Success); expect(sendStub.callCount).to.be.eq(0); @@ -322,6 +323,7 @@ describe('GroupSyncJob pushChangesToGroupSwarmIfNeeded', () => { const result = await GroupSync.pushChangesToGroupSwarmIfNeeded({ groupPk, extraStoreRequests: [], + allow401s: false, }); sendStub.resolves(undefined); @@ -369,6 +371,7 @@ describe('GroupSyncJob pushChangesToGroupSwarmIfNeeded', () => { const result = await GroupSync.pushChangesToGroupSwarmIfNeeded({ groupPk, extraStoreRequests: [], + allow401s: false, }); expect(sendStub.callCount).to.be.eq(1); From 3e34b0f77f3cfe1135f8c02c26ad41d33959abb7 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Mon, 6 Jan 2025 12:03:07 +1100 Subject: [PATCH 2/5] fix: use more withs for onion.ts file --- ts/hooks/useParamSelector.ts | 2 +- ts/session/apis/snode_api/batchRequest.ts | 6 +- ts/session/apis/snode_api/onions.ts | 74 +++++++++++------------ ts/session/types/with.ts | 8 +++ 4 files changed, 44 insertions(+), 46 deletions(-) diff --git a/ts/hooks/useParamSelector.ts b/ts/hooks/useParamSelector.ts index e35ae40dd..6f5954029 100644 --- a/ts/hooks/useParamSelector.ts +++ b/ts/hooks/useParamSelector.ts @@ -212,7 +212,7 @@ export function useIsKickedFromGroup(convoId?: string) { export function useIsGroupDestroyed(convoId?: string) { const libIsDestroyed = useLibGroupDestroyed(convoId); if (convoId && PubKey.is03Pubkey(convoId)) { - return libIsDestroyed || false; + return !!libIsDestroyed; } return false; } diff --git a/ts/session/apis/snode_api/batchRequest.ts b/ts/session/apis/snode_api/batchRequest.ts index 64652bcc4..e9aecca64 100644 --- a/ts/session/apis/snode_api/batchRequest.ts +++ b/ts/session/apis/snode_api/batchRequest.ts @@ -2,7 +2,6 @@ import { isArray } from 'lodash'; import { AbortController } from 'abort-controller'; import { MessageSender } from '../../sending'; -import { Snode } from '../../../data/types'; import { SnodeResponseError } from '../../utils/errors'; import { processOnionRequestErrorAtDestination, SnodeResponse } from './onions'; import { SessionRpc } from './sessionRpc'; @@ -15,15 +14,12 @@ import { } from './SnodeRequestTypes'; import { NotEmptyArrayOfBatchResults } from './BatchResultEntry'; import { MergedAbortSignal, WithTimeoutMs } from './requestWith'; -import { WithAllow401s } from '../../types/with'; +import { WithAllow401s, WithAssociatedWith, WithTargetNode } from '../../types/with'; function logSubRequests(requests: Array) { return `[${requests.map(builtRequestToLoggingId).join(', ')}]`; } -type WithTargetNode = { targetNode: Snode }; -type WithAssociatedWith = { associatedWith: string | null }; - /** * This is the equivalent to the batch send on sogs. The target node runs each sub request and returns a list of all the sub status and bodies. * If the global status code is not 200, an exception is thrown. diff --git a/ts/session/apis/snode_api/onions.ts b/ts/session/apis/snode_api/onions.ts index a99e36252..73c4e955b 100644 --- a/ts/session/apis/snode_api/onions.ts +++ b/ts/session/apis/snode_api/onions.ts @@ -21,7 +21,13 @@ import { fileServerHost } from '../file_server_api/FileServerApi'; import { hrefPnServerProd } from '../push_notification_api/PnServer'; import { ERROR_CODE_NO_CONNECT } from './SNodeAPI'; import { MergedAbortSignal, WithAbortSignal, WithTimeoutMs } from './requestWith'; -import { WithAllow401s } from '../../types/with'; +import { + WithAllow401s, + WithAssociatedWith, + WithDestinationEd25519, + WithGuardNode, + WithSymmetricKey, +} from '../../types/with'; // hold the ed25519 key of a snode against the time it fails. Used to remove a snode only after a few failures (snodeFailureThreshold failures) let snodeFailureCount: Record = {}; @@ -311,12 +317,11 @@ export async function processOnionRequestErrorAtDestination({ destinationSnodeEd25519, associatedWith, allow401s, -}: WithAllow401s & { - statusCode: number; - body: string; - destinationSnodeEd25519?: string; - associatedWith?: string; -}) { +}: WithAllow401s & + Partial & { + statusCode: number; + body: string; + }) { if (statusCode === 200) { return; } @@ -328,13 +333,13 @@ export async function processOnionRequestErrorAtDestination({ process401Error(statusCode); } processOxenServerError(statusCode, body); - await process421Error(statusCode, body, associatedWith, destinationSnodeEd25519); + await process421Error(statusCode, body, associatedWith || undefined, destinationSnodeEd25519); if (destinationSnodeEd25519) { await processAnyOtherErrorAtDestination( statusCode, body, destinationSnodeEd25519, - associatedWith + associatedWith || undefined ); } } @@ -342,9 +347,8 @@ export async function processOnionRequestErrorAtDestination({ async function handleNodeNotFound({ ed25519NotFound, associatedWith, -}: { +}: Partial & { ed25519NotFound: string; - associatedWith?: string; }) { const shortNodeNotFound = ed25519Str(ed25519NotFound); window?.log?.warn('Handling NODE NOT FOUND with: ', shortNodeNotFound); @@ -526,13 +530,10 @@ async function processOnionResponse({ associatedWith, destinationSnodeEd25519, allow401s, -}: Partial & - WithAllow401s & { +}: Partial & + WithAllow401s & + WithGuardNode & { response?: { text: () => Promise; status: number }; - symmetricKey?: ArrayBuffer; - guardNode: Snode; - destinationSnodeEd25519?: string; - associatedWith?: string; }): Promise { let ciphertext = ''; @@ -549,7 +550,7 @@ async function processOnionResponse({ ciphertext, guardNode.pubkey_ed25519, destinationSnodeEd25519, - associatedWith + associatedWith || undefined ); if (!ciphertext) { @@ -598,7 +599,7 @@ async function processOnionResponse({ } return value; }) as Record; - + // TODO: type those status const status = jsonRes.status_code || jsonRes.status; await processOnionRequestErrorAtDestination({ @@ -642,13 +643,10 @@ async function processOnionResponseV4({ guardNode, destinationSnodeEd25519, associatedWith, -}: Partial & { - response?: Response; - symmetricKey?: ArrayBuffer; - guardNode: Snode; - destinationSnodeEd25519?: string; - associatedWith?: string; -}): Promise { +}: Partial & + WithGuardNode & { + response?: Response; + }): Promise { processAbortedRequest(abortSignal); const validSymmetricKey = await processNoSymmetricKeyError(guardNode, symmetricKey); @@ -669,7 +667,7 @@ async function processOnionResponseV4({ cipherText, guardNode.pubkey_ed25519, destinationSnodeEd25519, - associatedWith + associatedWith || undefined ); const plaintextBuffer = await callUtilsWorker( @@ -705,9 +703,8 @@ export type FinalRelayOptions = { port?: number; // default to 443 }; -export type DestinationContext = { +export type DestinationContext = WithSymmetricKey & { ciphertext: Uint8Array; - symmetricKey: ArrayBuffer; ephemeralKey: ArrayBuffer; }; @@ -721,10 +718,8 @@ async function handle421InvalidSwarm({ body, destinationSnodeEd25519, associatedWith, -}: { +}: Partial & { body: string; - destinationSnodeEd25519?: string; - associatedWith?: string; }) { if (!destinationSnodeEd25519 || !associatedWith) { // The snode isn't associated with the given public key anymore @@ -784,9 +779,8 @@ async function handle421InvalidSwarm({ async function incrementBadSnodeCountOrDrop({ snodeEd25519, associatedWith, -}: { +}: Partial & { snodeEd25519: string; - associatedWith?: string; }) { const oldFailureCount = snodeFailureCount[snodeEd25519] || 0; const newFailureCount = oldFailureCount + 1; @@ -829,12 +823,12 @@ async function sendOnionRequestHandlingSnodeEjectNoRetries({ timeoutMs, }: WithAbortSignal & WithTimeoutMs & - WithAllow401s & { + WithAllow401s & + Partial & { nodePath: Array; destSnodeX25519: string; finalDestOptions: FinalDestOptions; finalRelayOptions?: FinalRelayOptions; - associatedWith?: string; useV4: boolean; throwErrors: boolean; }): Promise { @@ -1119,12 +1113,12 @@ async function sendOnionRequestSnodeDestNoRetries({ associatedWith, }: WithTimeoutMs & WithAbortSignal & - WithAllow401s & { + WithAllow401s & + Partial & { onionPath: Array; targetNode: Snode; headers: Record; plaintext: string | null; - associatedWith?: string; }) { return Onions.sendOnionRequestHandlingSnodeEjectNoRetries({ nodePath: onionPath, @@ -1156,11 +1150,11 @@ async function lokiOnionFetchNoRetries({ timeoutMs, }: WithTimeoutMs & WithAbortSignal & - WithAllow401s & { + WithAllow401s & + Partial & { targetNode: Snode; headers: Record; body: string | null; - associatedWith?: string; }): Promise { try { // Get a path excluding `targetNode`: diff --git a/ts/session/types/with.ts b/ts/session/types/with.ts index 37db18903..edc9f8bd0 100644 --- a/ts/session/types/with.ts +++ b/ts/session/types/with.ts @@ -1,4 +1,5 @@ import { PubkeyType } from 'libsession_util_nodejs'; +import { Snode } from '../../data/types'; export type WithMessageHash = { messageHash: string }; export type WithTimestamp = { timestamp: number }; @@ -26,3 +27,10 @@ export type ShortenOrExtend = 'extend' | 'shorten' | ''; export type WithShortenOrExtend = { shortenOrExtend: ShortenOrExtend }; export type WithMessagesHashes = { messagesHashes: Array }; export type WithAllow401s = { allow401s: boolean }; + +export type WithDestinationEd25519 = { destinationSnodeEd25519: string }; +export type WithAssociatedWith = { associatedWith: string | null }; +export type WithTargetNode = { targetNode: Snode }; +export type WithGuardNode = { guardNode: Snode }; + +export type WithSymmetricKey = { symmetricKey: ArrayBuffer }; From 53e579427a515318e3041bc40013e5d71a206c7b Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Mon, 6 Jan 2025 13:27:19 +1100 Subject: [PATCH 3/5] fix: do not schedule store update when deleting msgs --- ts/data/data.ts | 4 ++-- ts/models/message.ts | 4 ++-- ts/session/disappearing_messages/index.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ts/data/data.ts b/ts/data/data.ts index 99ce43b28..a7bc155b4 100644 --- a/ts/data/data.ts +++ b/ts/data/data.ts @@ -263,7 +263,7 @@ async function removeMessage(id: string): Promise { // it needs to delete all associated on-disk files along with the database delete. if (message) { await channels.removeMessage(id); - await message.cleanup(); + await message.cleanup(true); } } @@ -554,7 +554,7 @@ async function removeAllMessagesInConversation(conversationId: string): Promise< for (let index = 0; index < messages.length; index++) { const message = messages.at(index); // eslint-disable-next-line no-await-in-loop - await message.cleanup(); + await message.cleanup(false); // not triggering UI updates, as we remove them from the store just below } window.log.info( `removeAllMessagesInConversation messages.cleanup() ${conversationId} took ${ diff --git a/ts/models/message.ts b/ts/models/message.ts index 0beff6b15..79b96fddc 100644 --- a/ts/models/message.ts +++ b/ts/models/message.ts @@ -472,10 +472,10 @@ export class MessageModel extends Backbone.Model { return ''; } - public async cleanup() { + public async cleanup(triggerUIUpdate: boolean) { const changed = await deleteExternalMessageFiles(this.attributes); if (changed) { - await this.commit(); + await this.commit(triggerUIUpdate); } } diff --git a/ts/session/disappearing_messages/index.ts b/ts/session/disappearing_messages/index.ts index 4a2f69afb..860916d2a 100644 --- a/ts/session/disappearing_messages/index.ts +++ b/ts/session/disappearing_messages/index.ts @@ -46,7 +46,7 @@ export async function destroyMessagesAndUpdateRedux( // TODO make this use getMessagesById and not getMessageById const message = await Data.getMessageById(messageIds[i]); - await message?.cleanup(); + await message?.cleanup(false); // not triggering UI updates, as we remove them from the store just below /* eslint-enable no-await-in-loop */ } From 2ca2dd4790361fa971055416785dcc89707ed64e Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Tue, 7 Jan 2025 16:51:52 +1100 Subject: [PATCH 4/5] fix: allow promoted state to be reset to NOT_SENT --- package.json | 2 +- preload.js | 2 +- ts/components/MemberListItem.tsx | 4 +- .../utils/job_runners/jobs/GroupInviteJob.ts | 29 ++++-------- .../utils/job_runners/jobs/GroupPromoteJob.ts | 5 +-- ts/state/ducks/metaGroups.ts | 44 ------------------- yarn.lock | 6 +-- 7 files changed, 16 insertions(+), 76 deletions(-) diff --git a/package.json b/package.json index e8537644e..da9e1a73c 100644 --- a/package.json +++ b/package.json @@ -91,7 +91,7 @@ "fs-extra": "9.0.0", "glob": "10.3.10", "image-type": "^4.1.0", - "libsession_util_nodejs": "https://github.com/session-foundation/libsession-util-nodejs/releases/download/v0.4.10/libsession_util_nodejs-v0.4.10.tar.gz", + "libsession_util_nodejs": "https://github.com/session-foundation/libsession-util-nodejs/releases/download/v0.4.11/libsession_util_nodejs-v0.4.11.tar.gz", "libsodium-wrappers-sumo": "^0.7.9", "linkify-it": "^4.0.1", "lodash": "^4.17.21", diff --git a/preload.js b/preload.js index 7857da846..30a43299f 100644 --- a/preload.js +++ b/preload.js @@ -41,7 +41,7 @@ window.sessionFeatureFlags = { useOnionRequests: true, useTestNet: isTestNet() || isTestIntegration(), useClosedGroupV2: true, // TODO DO NOT MERGE Remove after QA - useClosedGroupV2QAButtons: false, // TODO DO NOT MERGE Remove after QA + useClosedGroupV2QAButtons: true, // TODO DO NOT MERGE Remove after QA replaceLocalizedStringsWithKeys: false, debug: { debugLogging: !_.isEmpty(process.env.SESSION_DEBUG), diff --git a/ts/components/MemberListItem.tsx b/ts/components/MemberListItem.tsx index d879d57c2..3f54d4276 100644 --- a/ts/components/MemberListItem.tsx +++ b/ts/components/MemberListItem.tsx @@ -259,14 +259,12 @@ const ResendButton = ({ groupPk, pubkey }: { pubkey: PubkeyType; groupPk: GroupP window.log.warn('tried to resend invite but we do not have correct details'); return; } - await MetaGroupWrapperActions.memberSetInviteNotSent(groupPk, pubkey); // if we tried to invite that member as admin right away, let's retry it as such. - const inviteAsAdmin = member.nominatedAdmin; await GroupInvite.addJob({ groupPk, member: pubkey, - inviteAsAdmin, + inviteAsAdmin: member.nominatedAdmin, forceUnrevoke: true, }); }} diff --git a/ts/session/utils/job_runners/jobs/GroupInviteJob.ts b/ts/session/utils/job_runners/jobs/GroupInviteJob.ts index ac84a7283..3bf752cfb 100644 --- a/ts/session/utils/job_runners/jobs/GroupInviteJob.ts +++ b/ts/session/utils/job_runners/jobs/GroupInviteJob.ts @@ -68,7 +68,15 @@ async function addJob({ groupPk, member, inviteAsAdmin, forceUnrevoke }: JobExtr forceUnrevoke, nextAttemptTimestamp: Date.now(), }); - window.log.debug(`addGroupInviteJob: adding group invite for ${groupPk}:${member} `); + window.log.debug( + `addGroupInviteJob: adding group invite for ${groupPk}:${member}. inviteAsAdmin:${inviteAsAdmin} ` + ); + if (inviteAsAdmin) { + // this resets the promotion status to not_sent, so that the sending state can be applied + await MetaGroupWrapperActions.memberSetPromoted(groupPk, member); + } else { + await MetaGroupWrapperActions.memberSetInviteNotSent(groupPk, member); + } window?.inboxStore?.dispatch( groupInfoActions.refreshGroupDetailsFromWrapper({ groupPk }) as any @@ -76,16 +84,6 @@ async function addJob({ groupPk, member, inviteAsAdmin, forceUnrevoke }: JobExtr await LibSessionUtil.saveDumpsToDb(groupPk); await runners.groupInviteJobRunner.addJob(groupInviteJob); - - if (inviteAsAdmin) { - window?.inboxStore?.dispatch( - groupInfoActions.setPromotionPending({ groupPk, pubkey: member, sending: true }) - ); - } else { - window?.inboxStore?.dispatch( - groupInfoActions.setInvitePending({ groupPk, pubkey: member, sending: true }) - ); - } } } @@ -279,15 +277,6 @@ class GroupInviteJob extends PersistedJob { updateFailedStateForMember(groupPk, member, failed); - if (inviteAsAdmin) { - window?.inboxStore?.dispatch( - groupInfoActions.setPromotionPending({ groupPk, pubkey: member, sending: false }) - ); - } else { - window?.inboxStore?.dispatch( - groupInfoActions.setInvitePending({ groupPk, pubkey: member, sending: false }) - ); - } window?.inboxStore?.dispatch( groupInfoActions.refreshGroupDetailsFromWrapper({ groupPk }) as any ); diff --git a/ts/session/utils/job_runners/jobs/GroupPromoteJob.ts b/ts/session/utils/job_runners/jobs/GroupPromoteJob.ts index ff76a2395..2346c3ab7 100644 --- a/ts/session/utils/job_runners/jobs/GroupPromoteJob.ts +++ b/ts/session/utils/job_runners/jobs/GroupPromoteJob.ts @@ -46,7 +46,7 @@ async function addJob({ groupPk, member }: JobExtraArgs) { window.log.debug(`addGroupPromoteJob: adding group promote for ${groupPk}:${member} `); await runners.groupPromoteJobRunner.addJob(groupPromoteJob); window?.inboxStore?.dispatch( - groupInfoActions.setPromotionPending({ groupPk, pubkey: member, sending: true }) + groupInfoActions.refreshGroupDetailsFromWrapper({ groupPk }) as any ); } } @@ -111,9 +111,6 @@ class GroupPromoteJob extends PersistedJob { failed = false; } } finally { - window?.inboxStore?.dispatch( - groupInfoActions.setPromotionPending({ groupPk, pubkey: member, sending: false }) - ); try { if (failed) { await MetaGroupWrapperActions.memberSetPromotionFailed(groupPk, member); diff --git a/ts/state/ducks/metaGroups.ts b/ts/state/ducks/metaGroups.ts index 05bb277ec..66499cacd 100644 --- a/ts/state/ducks/metaGroups.ts +++ b/ts/state/ducks/metaGroups.ts @@ -7,7 +7,6 @@ import { PubkeyType, UserGroupsGet, WithGroupPubkey, - WithPubkey, } from 'libsession_util_nodejs'; import { concat, intersection, isEmpty, uniq } from 'lodash'; import { from_hex } from 'libsodium-wrappers-sumo'; @@ -1212,36 +1211,6 @@ function deleteGroupPkEntriesFromState(state: GroupState, groupPk: GroupPubkeyTy delete state.members[groupPk]; } -function applySendingStateChange({ - groupPk, - pubkey, - sending, - state, - changeType, -}: WithGroupPubkey & - WithPubkey & { sending: boolean; changeType: 'invite' | 'promote'; state: GroupState }) { - if (changeType === 'invite' && !state.membersInviteSending[groupPk]) { - state.membersInviteSending[groupPk] = []; - } else if (changeType === 'promote' && !state.membersPromoteSending[groupPk]) { - state.membersPromoteSending[groupPk] = []; - } - const arrRef = - changeType === 'invite' - ? state.membersInviteSending[groupPk] - : state.membersPromoteSending[groupPk]; - - const foundAt = arrRef.findIndex(p => p === pubkey); - - if (sending && foundAt === -1) { - arrRef.push(pubkey); - return state; - } - if (!sending && foundAt >= 0) { - arrRef.splice(foundAt, 1); - } - return state; -} - function refreshConvosModelProps(convoIds: Array) { /** * @@ -1263,19 +1232,6 @@ const metaGroupSlice = createSlice({ name: 'metaGroup', initialState: initialGroupState, reducers: { - setInvitePending( - state: GroupState, - { payload }: PayloadAction<{ sending: boolean } & WithGroupPubkey & WithPubkey> - ) { - return applySendingStateChange({ changeType: 'invite', ...payload, state }); - }, - - setPromotionPending( - state: GroupState, - { payload }: PayloadAction<{ pubkey: PubkeyType; groupPk: GroupPubkeyType; sending: boolean }> - ) { - return applySendingStateChange({ changeType: 'promote', ...payload, state }); - }, removeGroupDetailsFromSlice( state: GroupState, { payload }: PayloadAction<{ groupPk: GroupPubkeyType }> diff --git a/yarn.lock b/yarn.lock index 9f21297c9..a77540814 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4944,9 +4944,9 @@ levn@~0.3.0: prelude-ls "~1.1.2" type-check "~0.3.2" -"libsession_util_nodejs@https://github.com/session-foundation/libsession-util-nodejs/releases/download/v0.4.10/libsession_util_nodejs-v0.4.10.tar.gz": - version "0.4.10" - resolved "https://github.com/session-foundation/libsession-util-nodejs/releases/download/v0.4.10/libsession_util_nodejs-v0.4.10.tar.gz#9a420fa0ad4dc9067de17b2ec9fa3676a1c10056" +"libsession_util_nodejs@https://github.com/session-foundation/libsession-util-nodejs/releases/download/v0.4.11/libsession_util_nodejs-v0.4.11.tar.gz": + version "0.4.11" + resolved "https://github.com/session-foundation/libsession-util-nodejs/releases/download/v0.4.11/libsession_util_nodejs-v0.4.11.tar.gz#ea6ab3fc11088ede3c79ddc5b702b17af842f774" dependencies: cmake-js "7.2.1" node-addon-api "^6.1.0" From 82abfedad5bb7e87e2eb31e9250c91d7600ef7aa Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 9 Jan 2025 09:24:51 +1100 Subject: [PATCH 5/5] fix: sorting order for group depending on statuses --- ts/components/MemberListItem.tsx | 2 +- ts/state/ducks/metaGroups.ts | 5 +++++ ts/state/selectors/groups.ts | 32 ++++++++++++++++++++------------ 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/ts/components/MemberListItem.tsx b/ts/components/MemberListItem.tsx index 3f54d4276..1bfc66f31 100644 --- a/ts/components/MemberListItem.tsx +++ b/ts/components/MemberListItem.tsx @@ -174,7 +174,7 @@ function localisedStatusFromMemberStatus(memberStatus: MemberStateGroupV2) { case 'REMOVED_UNKNOWN': // fallback, hopefully won't happen in production case 'REMOVED_MEMBER': // we want pending removal members at the end of the "invite" states case 'REMOVED_MEMBER_AND_MESSAGES': - return null; // no text for those 3 pending removal states + return window.i18n('groupPendingRemoval'); // no text for those 3 pending removal states case 'PROMOTION_FAILED': return window.i18n('adminPromotionFailed'); case 'PROMOTION_NOT_SENT': diff --git a/ts/state/ducks/metaGroups.ts b/ts/state/ducks/metaGroups.ts index 66499cacd..e0fe15256 100644 --- a/ts/state/ducks/metaGroups.ts +++ b/ts/state/ducks/metaGroups.ts @@ -423,6 +423,10 @@ const refreshGroupDetailsFromWrapper = createAsyncThunk( const infos = await MetaGroupWrapperActions.infoGet(groupPk); const members = await MetaGroupWrapperActions.memberGetAll(groupPk); + if (window.sessionFeatureFlags.debug.debugLibsessionDumps) { + window.log.info(`groupInfo after refreshGroupDetailsFromWrapper: ${stringify(infos)}`); + window.log.info(`groupMembers after refreshGroupDetailsFromWrapper: ${stringify(members)}`); + } return { groupPk, infos, members }; } catch (e) { window.log.warn('refreshGroupDetailsFromWrapper failed with ', e.message); @@ -748,6 +752,7 @@ async function handleMemberRemovedFromUI({ // We don't revoke the member's token right away. Instead we schedule a `GroupPendingRemovals` // which will deal with the revokes of all of them together. await GroupPendingRemovals.addJob({ groupPk }); + window.inboxStore?.dispatch(refreshGroupDetailsFromWrapper({ groupPk }) as any); // Build a GroupUpdateMessage to be sent if that member was kicked by us. const createAtNetworkTimestamp = NetworkTime.now(); diff --git a/ts/state/selectors/groups.ts b/ts/state/selectors/groups.ts index f5efbe451..ec3017f64 100644 --- a/ts/state/selectors/groups.ts +++ b/ts/state/selectors/groups.ts @@ -272,19 +272,27 @@ export function useStateOf03GroupMembers(convoId?: string) { ); const sorted = useMemo(() => { - // needing an index like this outside of lodash is not pretty, - // but sortBy doesn't provide the index in the callback - let index = 0; - return sortBy(unsortedMembers, item => { - const stateSortingOrder = getSortingOrderForStatus(item.memberStatus); - const sortingOrder = [ - stateSortingOrder, + // damn this is overkill + return sortBy( + unsortedMembers, + item => { + const sortingOrder = getSortingOrderForStatus(item.memberStatus); + return sortingOrder; + }, + item => { // per section, we want "us" first, then "nickname || displayName || pubkey" - item.pubkeyHex === us ? -1 : names[index], - ]; - index++; - return sortingOrder; - }); + + if (item.pubkeyHex === us) { + return -1; + } + const index = unsortedMembers.findIndex(p => p.pubkeyHex === item.pubkeyHex); + if (index < 0 || index >= names.length) { + throw new Error('this should never happen'); + } + + return names[index].toLowerCase(); + } + ); }, [unsortedMembers, us, names]); return sorted; }