Various bugfixes, related to #2064 and #2066

* eventsAfter: Reject early if the contractID is not set * Fix instances causing such unset contractIDs * Fix group members list to include members with only a contractID * Don't set an empty contract state when receiving OP_KEY_SHARE * Files: laxer validation when removing to process individual files separately * Increase shelter header validity window
okTurtles · Jun 18, 2024 · 68f402a · 68f402a
1 parent 80c3ffd
commit 68f402a
Show file tree

Hide file tree

Showing 8 changed files with 47 additions and 23 deletions.
diff --git a/frontend/controller/actions/identity.js b/frontend/controller/actions/identity.js
@@ -761,24 +761,44 @@ export default (sbp('sbp/selectors/register', {
   }) => {
     const rootGetters = sbp('state/vuex/getters')
     const { identityContractID } = sbp('state/vuex/state').loggedIn
-    const { shouldDeleteFile, shouldDeleteToken } = option
+    const { shouldDeleteFile, shouldDeleteToken, maybeDeleted } = option
+    let deleteResult, toDelete
 
     if (shouldDeleteFile) {
       const credentials = Object.fromEntries(manifestCids.map(cid => {
+        // It could be that the file was already deleted, if we no longer have
+        // a delete token. In this case, omit those CIDs.
+        if (maybeDeleted && !rootGetters.currentIdentityState.fileDeleteTokens[cid]) {
+          return [cid, null]
+        };
         const credential = shouldDeleteToken
           ? { token: rootGetters.currentIdentityState.fileDeleteTokens[cid] }
           : { billableContractID: identityContractID }
         return [cid, credential]
       }))
-      await sbp('chelonia/fileDelete', manifestCids, credentials)
+      toDelete = maybeDeleted ? manifestCids.filter((cid) => !!credentials[cid]) : manifestCids
+      deleteResult = await sbp('chelonia/fileDelete', toDelete, credentials)
+    } else {
+      toDelete = manifestCids
     }
 
     if (shouldDeleteToken) {
       await sbp('gi.actions/identity/removeFileDeleteToken', {
         contractID: identityContractID,
-        data: { manifestCids }
+        data: {
+          manifestCids: deleteResult
+            ? toDelete.filter((_, i) => {
+              return deleteResult[i].status === 'fulfilled'
+            })
+            : toDelete
+        }
       })
     }
+
+    if (deleteResult?.some(r => r.status === 'rejected')) {
+      console.error('[gi.actions/identity/removeFiles] Some CIDs could not be deleted', deleteResult.map((r, i) => r.status === 'rejected' && toDelete[i]).filter(Boolean))
+      throw new Error('Some CIDs could not be deleted')
+    }
   },
   'gi.actions/identity/fetchChatRoomUnreadMessages': async () => {
     const { ourIdentityContractId } = sbp('state/vuex/getters')

diff --git a/frontend/model/contracts/chatroom.js b/frontend/model/contracts/chatroom.js
@@ -508,7 +508,8 @@ sbp('chelonia/defineContract', {
         if (data.manifestCids.length) {
           const option = {
             shouldDeleteFile: me === innerSigningContractID,
-            shouldDeleteToken: me === data.messageSender
+            shouldDeleteToken: me === data.messageSender,
+            maybeDeleted: true
           }
           deleteEncryptedFiles(data.manifestCids, option).catch(e => {
             console.error(`[gi.contracts/chatroom/deleteMessage/sideEffect] (${contractID}):`, e)

diff --git a/frontend/model/notifications/templates.js b/frontend/model/notifications/templates.js
@@ -251,12 +251,12 @@ export default ({
   },
   PAYMENT_THANKYOU_SENT (data: { creatorID: string, fromMemberID: string, toMemberID: string }) {
     return {
-      avatarUserID: data.creatorID,
+      avatarUserID: data.fromMemberID,
       body: L('{name} sent you a {strong_}thank you note{_strong} for your contribution.', {
         name: strong(userDisplayNameFromID(data.fromMemberID)),
         ...LTags('strong')
       }),
-      creatorID: data.creatorID,
+      creatorID: data.fromMemberID,
       icon: '',
       level: 'info',
       linkTo: `/payments?modal=ThankYouNoteModal&from=${data.fromMemberID}&to=${data.toMemberID}`,

diff --git a/frontend/model/state.js b/frontend/model/state.js
@@ -439,7 +439,7 @@ const getters = {
       .filter(memberID => getters.groupProfiles[memberID] ||
          getters.groupMembersPending[memberID].expires >= Date.now())
       .map(memberID => {
-        const { contractID, displayName, username } = getters.globalProfile(memberID) || groupMembersPending[memberID] || {}
+        const { contractID, displayName, username } = getters.globalProfile(memberID) || groupMembersPending[memberID] || (getters.groupProfiles[memberID] ? { contractID: memberID } : {})
         return {
           id: memberID, // common unique ID: it can be either the contract ID or the invite key
           contractID,

diff --git a/frontend/views/components/AvatarUser.vue b/frontend/views/components/AvatarUser.vue
@@ -18,7 +18,9 @@ export default ({
     picture: {
       type: [String, Object]
     },
-    contractID: String,
+    contractID: {
+      type: String
+    },
     alt: {
       type: String,
       default: ''

diff --git a/shared/domains/chelonia/files.js b/shared/domains/chelonia/files.js
@@ -369,26 +369,25 @@ export default (sbp('sbp/selectors/register', {
       throw new TypeError('A manifest CID must be provided')
     }
     if (!Array.isArray(manifestCid)) manifestCid = [manifestCid]
-    // Validation
-    manifestCid.forEach((cid) => {
+    return await Promise.allSettled(manifestCid.map(async (cid) => {
       const hasCredential = has(credentials, cid)
-      const hasToken = has(credentials[cid], 'token')
-      const hasBillableContractID = has(credentials[cid], 'billableContractID')
+      const hasToken = has(credentials[cid], 'token') && credentials[cid].token
+      const hasBillableContractID = has(credentials[cid], 'billableContractID') && credentials[cid].billableContractID
       if (!hasCredential || (!hasToken && hasToken === hasBillableContractID)) {
         throw new TypeError(`Either a token or a billable contract ID must be provided for ${cid}`)
       }
-    })
-    return await Promise.all(manifestCid.map(async (cid) => {
-      const { token, billableContractID } = credentials[cid]
+
       const response = await fetch(`${this.config.connectionURL}/deleteFile/${cid}`, {
         method: 'POST',
         signal: this.abortController.signal,
         headers: new Headers([
           ['authorization',
-            token
-              ? `bearer ${token}`
+            hasToken
+              // $FlowFixMe[incompatible-type]
+              ? `bearer ${credentials[cid].token}`
+              // $FlowFixMe[incompatible-type]
               // $FlowFixMe[incompatible-call]
-              : buildShelterAuthorizationHeader.call(this, billableContractID)]
+              : buildShelterAuthorizationHeader.call(this, credentials[cid].billableContractID)]
         ])
       })
       if (!response.ok) {

diff --git a/shared/domains/chelonia/internals.js b/shared/domains/chelonia/internals.js
@@ -723,9 +723,6 @@ export default (sbp('sbp/selectors/register', {
 
           const cheloniaState = sbp(self.config.stateSelector)
 
-          if (!cheloniaState[v.contractID]) {
-            config.reactiveSet(cheloniaState, v.contractID, Object.create(null))
-          }
           const targetState = cheloniaState[v.contractID]
 
           let newestEncryptionKeyHeight = Number.POSITIVE_INFINITY
@@ -743,7 +740,7 @@ export default (sbp('sbp/selectors/register', {
                     transient
                   }])
                   if (
-                    targetState._vm?.authorizedKeys?.[key.id]?._notBeforeHeight != null &&
+                    targetState?._vm?.authorizedKeys?.[key.id]?._notBeforeHeight != null &&
                       Array.isArray(targetState._vm.authorizedKeys[key.id].purpose) &&
                       targetState._vm.authorizedKeys[key.id].purpose.includes('enc')
                   ) {

diff --git a/shared/domains/chelonia/utils.js b/shared/domains/chelonia/utils.js
@@ -551,6 +551,11 @@ export const getContractIDfromKeyId = (contractID: string, signingKeyId: ?string
 }
 
 export function eventsAfter (contractID: string, sinceHeight: number, limit?: number, sinceHash?: string): ReadableStream {
+  if (!contractID) {
+    // Avoid making a network roundtrip to tell us what we already know
+    throw new Error('Missing contract ID')
+  }
+
   const fetchEventsStreamReader = async () => {
     requestLimit = Math.min(limit ?? MAX_EVENTS_AFTER, remainingEvents)
     const eventsResponse = await fetch(`${this.config.connectionURL}/eventsAfter/${contractID}/${sinceHeight}${Number.isInteger(requestLimit) ? `/${requestLimit}` : ''}`, { signal })
@@ -756,7 +761,7 @@ export function verifyShelterAuthorizationHeader (authorization: string, rootSta
   }
   // TODO: Remember nonces and reject already used ones
   const [, data, contractID, timestamp, , signature] = matches
-  if (Math.abs(parseInt(timestamp) - (Date.now() / 1e3 | 0)) > 2) {
+  if (Math.abs(parseInt(timestamp) - (Date.now() / 1e3 | 0)) > 60) {
     throw new Error('Invalid signature time range')
   }
   if (!rootState) rootState = sbp('chelonia/rootState')