Skip to content

Commit

Permalink
Use deep equality comparisons when searching for outgoing key request…
Browse files Browse the repository at this point in the history
…s by target (#2623)
  • Loading branch information
duxovni authored Aug 27, 2022
1 parent c1160f4 commit 1c77816
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 21 deletions.
46 changes: 29 additions & 17 deletions spec/unit/crypto/outgoing-room-key-requests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import {
IndexedDBCryptoStore,
} from '../../../src/crypto/store/indexeddb-crypto-store';
import { CryptoStore } from '../../../src/crypto/store/base';
import { IndexedDBCryptoStore } from '../../../src/crypto/store/indexeddb-crypto-store';
import { LocalStorageCryptoStore } from '../../../src/crypto/store/localStorage-crypto-store';
import { MemoryCryptoStore } from '../../../src/crypto/store/memory-crypto-store';
import { RoomKeyRequestState } from '../../../src/crypto/OutgoingRoomKeyRequestManager';

Expand All @@ -26,36 +26,39 @@ import 'jest-localstorage-mock';
const requests = [
{
requestId: "A",
requestBody: { session_id: "A", room_id: "A" },
requestBody: { session_id: "A", room_id: "A", sender_key: "A", algorithm: "m.megolm.v1.aes-sha2" },
state: RoomKeyRequestState.Sent,
recipients: [
{ userId: "@alice:example.com", deviceId: "*" },
{ userId: "@becca:example.com", deviceId: "foobarbaz" },
],
},
{
requestId: "B",
requestBody: { session_id: "B", room_id: "B" },
requestBody: { session_id: "B", room_id: "B", sender_key: "B", algorithm: "m.megolm.v1.aes-sha2" },
state: RoomKeyRequestState.Sent,
recipients: [
{ userId: "@alice:example.com", deviceId: "*" },
{ userId: "@carrie:example.com", deviceId: "barbazquux" },
],
},
{
requestId: "C",
requestBody: { session_id: "C", room_id: "C" },
requestBody: { session_id: "C", room_id: "C", sender_key: "B", algorithm: "m.megolm.v1.aes-sha2" },
state: RoomKeyRequestState.Unsent,
recipients: [
{ userId: "@becca:example.com", deviceId: "foobarbaz" },
],
},
];

describe.each([
["IndexedDBCryptoStore",
() => new IndexedDBCryptoStore(global.indexedDB, "tests")],
["LocalStorageCryptoStore",
() => new IndexedDBCryptoStore(undefined, "tests")],
["MemoryCryptoStore", () => {
const store = new IndexedDBCryptoStore(undefined, "tests");
// @ts-ignore set private properties
store.backend = new MemoryCryptoStore();
// @ts-ignore
store.backendPromise = Promise.resolve(store.backend);
return store;
}],
["LocalStorageCryptoStore", () => new LocalStorageCryptoStore(localStorage)],
["MemoryCryptoStore", () => new MemoryCryptoStore()],
])("Outgoing room key requests [%s]", function(name, dbFactory) {
let store;
let store: CryptoStore;

beforeAll(async () => {
store = dbFactory();
Expand All @@ -75,6 +78,15 @@ describe.each([
});
});

it("getOutgoingRoomKeyRequestsByTarget retrieves all entries with a given target",
async () => {
const r = await store.getOutgoingRoomKeyRequestsByTarget(
"@becca:example.com", "foobarbaz", [RoomKeyRequestState.Sent],
);
expect(r).toHaveLength(1);
expect(r[0]).toEqual(requests[0]);
});

test("getOutgoingRoomKeyRequestByState retrieves any entry in a given state",
async () => {
const r =
Expand Down
6 changes: 4 additions & 2 deletions src/crypto/store/indexeddb-crypto-store-backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
Mode,
OutgoingRoomKeyRequest,
} from "./base";
import { IRoomKeyRequestBody } from "../index";
import { IRoomKeyRequestBody, IRoomKeyRequestRecipient } from "../index";
import { ICrossSigningKey } from "../../client";
import { IOlmDevice } from "../algorithms/megolm";
import { IRoomEncryption } from "../RoomList";
Expand Down Expand Up @@ -261,7 +261,9 @@ export class Backend implements CryptoStore {
const cursor = this.result;
if (cursor) {
const keyReq = cursor.value;
if (keyReq.recipients.includes({ userId, deviceId })) {
if (keyReq.recipients.some((recipient: IRoomKeyRequestRecipient) =>
recipient.userId === userId && recipient.deviceId === deviceId,

This comment has been minimized.

Copy link
@3nprob

3nprob Aug 28, 2022

Contributor

Is there any specific reason not to use the already existing utils.deepCompare(recipient, target) here? If so it could be worth adding a comment to prevent a future oblivious refactor? Otherwise I guess it's preferred for consistency.

Note that it currently has a bug addressed in #2586

)) {
results.push(keyReq);
}
cursor.continue();
Expand Down
6 changes: 4 additions & 2 deletions src/crypto/store/memory-crypto-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,13 @@ export class MemoryCryptoStore implements CryptoStore {
deviceId: string,
wantedStates: number[],
): Promise<OutgoingRoomKeyRequest[]> {
const results = [];
const results: OutgoingRoomKeyRequest[] = [];

for (const req of this.outgoingRoomKeyRequests) {
for (const state of wantedStates) {
if (req.state === state && req.recipients.includes({ userId, deviceId })) {
if (req.state === state && req.recipients.some(
(recipient) => recipient.userId === userId && recipient.deviceId === deviceId,
)) {
results.push(req);
}
}
Expand Down

0 comments on commit 1c77816

Please sign in to comment.