From 112b02cab630ea5d02452a9799df127e7b1771b2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 22:53:37 +0000 Subject: [PATCH 1/3] Add disconnect method to freighter-api Lets a connected dApp revoke its own access without a confirmation prompt. The calling domain is removed from the allowlist for the active account on the active network, so the next requestAccess or setAllowed call prompts the user again. Returns a locked error when the wallet is locked, since the active public key isn't known then. Requests from versions of freighter-api that include disconnect resolve with an error after 2s against older extensions, which drop the unknown message type. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01M6pbgqeBtJ7cL5biPuxBJ1 --- @shared/api/external.ts | 20 +++ .../__tests__/extensionMessaging.test.ts | 27 ++++ @shared/api/helpers/extensionMessaging.ts | 16 +++ @shared/constants/services.ts | 1 + .../src/__tests__/disconnect.test.js | 19 +++ @stellar/freighter-api/src/disconnect.ts | 12 ++ @stellar/freighter-api/src/index.ts | 3 + .../__tests__/disconnect.test.ts | 131 ++++++++++++++++++ .../freighterApiMessageListener.ts | 35 +++++ 9 files changed, 264 insertions(+) create mode 100644 @shared/api/helpers/__tests__/extensionMessaging.test.ts create mode 100644 @stellar/freighter-api/src/__tests__/disconnect.test.js create mode 100644 @stellar/freighter-api/src/disconnect.ts create mode 100644 extension/src/background/messageListener/__tests__/disconnect.test.ts diff --git a/@shared/api/external.ts b/@shared/api/external.ts index fbeae6a43c..061e928820 100644 --- a/@shared/api/external.ts +++ b/@shared/api/external.ts @@ -303,3 +303,23 @@ export const setAllowedStatus = async (): Promise<{ return { isAllowed, error: response?.apiError }; }; + +export const disconnect = async (): Promise<{ + error?: FreighterApiError; +}> => { + let response; + + try { + response = await sendMessageToContentScript({ + type: EXTERNAL_SERVICE_TYPES.DISCONNECT, + }); + } catch (e) { + console.error(e); + } + + if (!response) { + return { error: FreighterApiInternalError }; + } + + return response.apiError ? { error: response.apiError } : {}; +}; diff --git a/@shared/api/helpers/__tests__/extensionMessaging.test.ts b/@shared/api/helpers/__tests__/extensionMessaging.test.ts new file mode 100644 index 0000000000..87717a32ca --- /dev/null +++ b/@shared/api/helpers/__tests__/extensionMessaging.test.ts @@ -0,0 +1,27 @@ +import { EXTERNAL_SERVICE_TYPES } from "../../../constants/services"; +import { + FreighterApiInternalError, + sendMessageToContentScript, +} from "../extensionMessaging"; + +describe("sendMessageToContentScript", () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it("resolves DISCONNECT with an error when the extension never responds", async () => { + const response = sendMessageToContentScript({ + type: EXTERNAL_SERVICE_TYPES.DISCONNECT, + }); + + jest.advanceTimersByTime(2000); + + await expect(response).resolves.toEqual({ + apiError: FreighterApiInternalError, + }); + }); +}); diff --git a/@shared/api/helpers/extensionMessaging.ts b/@shared/api/helpers/extensionMessaging.ts index 9a6a6dec42..db871add30 100644 --- a/@shared/api/helpers/extensionMessaging.ts +++ b/@shared/api/helpers/extensionMessaging.ts @@ -55,6 +55,17 @@ export const sendMessageToContentScript = (msg: Msg): Promise => { }, 2000); } + /* + Versions of Freighter that predate DISCONNECT drop the message without + responding, so resolve with an error rather than hanging forever. + */ + if (msg.type === EXTERNAL_SERVICE_TYPES.DISCONNECT) { + requestTimeout = setTimeout(() => { + resolve({ apiError: FreighterApiInternalError } as Response); + window.removeEventListener("message", messageListener); + }, 2000); + } + const messageListener = (event: { source: any; data: Response }) => { // We only accept messages from ourselves if (event.source !== window) return; @@ -97,6 +108,11 @@ export const FreighterApiInternalError = { "The wallet encountered an internal error. Please try again or contact the wallet if the problem persists.", }; +export const FreighterApiLockedError = { + code: -1, + message: "The wallet is locked. Unlock Freighter and try again.", +}; + export const FreighterApiDeclinedError = { code: -4, message: "The user rejected this request.", diff --git a/@shared/constants/services.ts b/@shared/constants/services.ts index 9c967543a3..5a3bf66b98 100644 --- a/@shared/constants/services.ts +++ b/@shared/constants/services.ts @@ -97,6 +97,7 @@ export enum EXTERNAL_SERVICE_TYPES { REQUEST_ALLOWED_STATUS = "REQUEST_ALLOWED_STATUS", SET_ALLOWED_STATUS = "SET_ALLOWED_STATUS", REQUEST_USER_INFO = "REQUEST_USER_INFO", + DISCONNECT = "DISCONNECT", } export const EXTERNAL_MSG_REQUEST = "FREIGHTER_EXTERNAL_MSG_REQUEST"; diff --git a/@stellar/freighter-api/src/__tests__/disconnect.test.js b/@stellar/freighter-api/src/__tests__/disconnect.test.js new file mode 100644 index 0000000000..4bb5b47887 --- /dev/null +++ b/@stellar/freighter-api/src/__tests__/disconnect.test.js @@ -0,0 +1,19 @@ +import * as extensionMessaging from "@shared/api/helpers/extensionMessaging"; +import { disconnect } from "../disconnect"; + +describe("disconnect", () => { + it("returns an empty object on success", async () => { + extensionMessaging.sendMessageToContentScript = jest + .fn() + .mockReturnValue({}); + const res = await disconnect(); + expect(res).toEqual({}); + }); + it("returns an error", async () => { + extensionMessaging.sendMessageToContentScript = jest + .fn() + .mockReturnValue({ apiError: "error" }); + const res = await disconnect(); + expect(res).toEqual({ error: "error" }); + }); +}); diff --git a/@stellar/freighter-api/src/disconnect.ts b/@stellar/freighter-api/src/disconnect.ts new file mode 100644 index 0000000000..f1ed8d2bd3 --- /dev/null +++ b/@stellar/freighter-api/src/disconnect.ts @@ -0,0 +1,12 @@ +import { disconnect as disconnectSite } from "@shared/api/external"; +import { FreighterApiError } from "@shared/api/types"; +import { FreighterApiNodeError } from "@shared/api/helpers/extensionMessaging"; +import { isBrowser } from "."; + +export const disconnect = async (): Promise<{ error?: FreighterApiError }> => { + if (isBrowser) { + return disconnectSite(); + } + + return { error: FreighterApiNodeError }; +}; diff --git a/@stellar/freighter-api/src/index.ts b/@stellar/freighter-api/src/index.ts index 558095d190..f55cdb31c5 100644 --- a/@stellar/freighter-api/src/index.ts +++ b/@stellar/freighter-api/src/index.ts @@ -9,6 +9,7 @@ import { getNetworkDetails } from "./getNetworkDetails"; import { isAllowed } from "./isAllowed"; import { setAllowed } from "./setAllowed"; import { requestAccess } from "./requestAccess"; +import { disconnect } from "./disconnect"; import { WatchWalletChanges } from "./watchWalletChanges"; export const isBrowser = typeof window !== "undefined"; @@ -25,6 +26,7 @@ export { isAllowed, setAllowed, requestAccess, + disconnect, WatchWalletChanges, }; export default { @@ -39,5 +41,6 @@ export default { isAllowed, setAllowed, requestAccess, + disconnect, WatchWalletChanges, }; diff --git a/extension/src/background/messageListener/__tests__/disconnect.test.ts b/extension/src/background/messageListener/__tests__/disconnect.test.ts new file mode 100644 index 0000000000..2406caf500 --- /dev/null +++ b/extension/src/background/messageListener/__tests__/disconnect.test.ts @@ -0,0 +1,131 @@ +import browser from "webextension-polyfill"; +import { Store } from "redux"; + +import { EXTERNAL_SERVICE_TYPES } from "@shared/constants/services"; +import { ExternalRequest } from "@shared/api/types"; +import { + FreighterApiInternalError, + FreighterApiLockedError, +} from "@shared/api/helpers/extensionMessaging"; +import { ALLOWLIST_ID, NETWORK_ID } from "constants/localStorageTypes"; +import { DataStorageAccess } from "background/helpers/dataStorageAccess"; +import { freighterApiMessageListener } from "../freighterApiMessageListener"; + +const ACTIVE_PUBLIC_KEY = + "GBTYAFHGNZSTE4VBWZYAGB3SRGJEPTI5I4Y22KZ4JTVAN56LESB6JZOF"; +const OTHER_PUBLIC_KEY = + "GDF32CQINROD3E2LMCGZUDVMWTXCJFR5SBYVRJ7WAAIAS3P7DCVWZEFY"; + +const mockPublicKeySelector = jest.fn(); + +jest.mock("background/ducks/session", () => ({ + ...jest.requireActual("background/ducks/session"), + publicKeySelector: () => mockPublicKeySelector(), +})); + +jest.mock("@sentry/browser", () => ({ + captureException: jest.fn(), +})); + +const makeLocalStore = (initial: Record) => { + const data: Record = { ...initial }; + + return { + data, + store: { + getItem: jest.fn((key: string) => Promise.resolve(data[key])), + setItem: jest.fn((key: string, value: unknown) => { + data[key] = value; + return Promise.resolve(); + }), + } as unknown as DataStorageAccess, + }; +}; + +const disconnect = (localStore: DataStorageAccess, url: string) => + freighterApiMessageListener( + { type: EXTERNAL_SERVICE_TYPES.DISCONNECT } as ExternalRequest, + { url } as browser.Runtime.MessageSender, + { getState: () => ({}) } as unknown as Store, + localStore, + ); + +describe("freighterApiMessageListener DISCONNECT", () => { + const initialAllowList = { + Testnet: { + [ACTIVE_PUBLIC_KEY]: ["example.com", "other.com"], + [OTHER_PUBLIC_KEY]: ["example.com"], + }, + Mainnet: { + [ACTIVE_PUBLIC_KEY]: ["example.com"], + }, + }; + + beforeEach(() => { + jest.clearAllMocks(); + mockPublicKeySelector.mockReturnValue(ACTIVE_PUBLIC_KEY); + }); + + it("removes the calling domain for the active account and network only", async () => { + const { data, store } = makeLocalStore({ + [NETWORK_ID]: { networkName: "Testnet" }, + [ALLOWLIST_ID]: initialAllowList, + }); + + const response = await disconnect(store, "https://example.com/app"); + + expect(response).toEqual({}); + expect(data[ALLOWLIST_ID]).toEqual({ + Testnet: { + [ACTIVE_PUBLIC_KEY]: ["other.com"], + [OTHER_PUBLIC_KEY]: ["example.com"], + }, + Mainnet: { + [ACTIVE_PUBLIC_KEY]: ["example.com"], + }, + }); + }); + + it("succeeds without changes when the domain isn't connected", async () => { + const { data, store } = makeLocalStore({ + [NETWORK_ID]: { networkName: "Testnet" }, + [ALLOWLIST_ID]: initialAllowList, + }); + + const response = await disconnect(store, "https://not-connected.com"); + + expect(response).toEqual({}); + expect(data[ALLOWLIST_ID]).toEqual(initialAllowList); + expect(store.setItem).not.toHaveBeenCalled(); + }); + + it("returns a locked error and leaves the allowlist untouched when locked", async () => { + mockPublicKeySelector.mockReturnValue(""); + const { data, store } = makeLocalStore({ + [NETWORK_ID]: { networkName: "Testnet" }, + [ALLOWLIST_ID]: initialAllowList, + }); + + const response = await disconnect(store, "https://example.com"); + + expect(response).toEqual({ + apiError: FreighterApiLockedError, + error: FreighterApiLockedError.message, + }); + expect(data[ALLOWLIST_ID]).toEqual(initialAllowList); + }); + + it("returns an internal error when storage fails", async () => { + const store = { + getItem: jest.fn().mockRejectedValue(new Error("storage failure")), + setItem: jest.fn(), + } as unknown as DataStorageAccess; + + const response = await disconnect(store, "https://example.com"); + + expect(response).toEqual({ + apiError: FreighterApiInternalError, + error: FreighterApiInternalError.message, + }); + }); +}); diff --git a/extension/src/background/messageListener/freighterApiMessageListener.ts b/extension/src/background/messageListener/freighterApiMessageListener.ts index 67d024a100..2d77b8400c 100644 --- a/extension/src/background/messageListener/freighterApiMessageListener.ts +++ b/extension/src/background/messageListener/freighterApiMessageListener.ts @@ -23,6 +23,7 @@ import { stellarSdkServer } from "@shared/api/helpers/stellarSdkServer"; import { FreighterApiInternalError, FreighterApiDeclinedError, + FreighterApiLockedError, } from "@shared/api/helpers/extensionMessaging"; import { EXTERNAL_SERVICE_TYPES, @@ -42,6 +43,7 @@ import { getIsMemoValidationEnabled, getNetworkDetails, getAllowListSegment, + removeAllowListDomain, } from "background/helpers/account"; import { isSenderAllowed } from "background/helpers/allowListAuthorization"; import { cachedFetch } from "background/helpers/cachedFetch"; @@ -861,6 +863,38 @@ export const freighterApiMessageListener = ( }; }; + const disconnect = async () => { + try { + const publicKey = publicKeySelector(sessionStore.getState()); + + // the allowlist is keyed by public key, which we can't know while locked + if (!publicKey) { + return { + apiError: FreighterApiLockedError, + error: FreighterApiLockedError.message, + }; + } + + const { url: tabUrl = "" } = sender; + const domain = getPunycodedDomain(getUrlHostname(tabUrl)); + const { networkName } = await getNetworkDetails({ localStore }); + + await removeAllowListDomain({ + publicKey, + networkName, + domain, + localStore, + }); + + return {}; + } catch (e) { + return { + apiError: FreighterApiInternalError, + error: FreighterApiInternalError.message, + }; + } + }; + const messageResponder: MessageResponder = { [EXTERNAL_SERVICE_TYPES.REQUEST_ACCESS]: requestAccess, [EXTERNAL_SERVICE_TYPES.REQUEST_PUBLIC_KEY]: requestPublicKey, @@ -874,6 +908,7 @@ export const freighterApiMessageListener = ( [EXTERNAL_SERVICE_TYPES.REQUEST_ALLOWED_STATUS]: requestAllowedStatus, [EXTERNAL_SERVICE_TYPES.SET_ALLOWED_STATUS]: setAllowedStatus, [EXTERNAL_SERVICE_TYPES.REQUEST_USER_INFO]: requestUserInfo, + [EXTERNAL_SERVICE_TYPES.DISCONNECT]: disconnect, }; return messageResponder[request.type](); From 5f044acbf0fab12d8ed809f544d600f9294f1f06 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 15:55:17 +0000 Subject: [PATCH 2/3] Treat unreachable extension as a disconnect error The content script returns only `error` when it can't reach the background. Map that to FreighterApiInternalError so the dApp doesn't see a failed disconnect as a success. Also correct the locked-state comment: an idle lock keeps the session public key, so disconnect still removes the domain for that account. Only a session that hasn't been unlocked yet has no public key. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01M6pbgqeBtJ7cL5biPuxBJ1 --- @shared/api/external.ts | 9 +++-- .../src/__tests__/disconnect.test.js | 9 +++++ .../__tests__/disconnect.test.ts | 36 +++++++++++++++++-- .../freighterApiMessageListener.ts | 3 +- 4 files changed, 52 insertions(+), 5 deletions(-) diff --git a/@shared/api/external.ts b/@shared/api/external.ts index 061e928820..c41c9ad037 100644 --- a/@shared/api/external.ts +++ b/@shared/api/external.ts @@ -317,9 +317,14 @@ export const disconnect = async (): Promise<{ console.error(e); } - if (!response) { + if (response?.apiError) { + return { error: response.apiError }; + } + + // the content script returns only `error` when it can't reach the background + if (!response || response.error) { return { error: FreighterApiInternalError }; } - return response.apiError ? { error: response.apiError } : {}; + return {}; }; diff --git a/@stellar/freighter-api/src/__tests__/disconnect.test.js b/@stellar/freighter-api/src/__tests__/disconnect.test.js index 4bb5b47887..890e03a2be 100644 --- a/@stellar/freighter-api/src/__tests__/disconnect.test.js +++ b/@stellar/freighter-api/src/__tests__/disconnect.test.js @@ -16,4 +16,13 @@ describe("disconnect", () => { const res = await disconnect(); expect(res).toEqual({ error: "error" }); }); + it("returns an internal error when the extension can't be reached", async () => { + extensionMessaging.sendMessageToContentScript = jest + .fn() + .mockReturnValue({ error: "Unable to send message to extension" }); + const res = await disconnect(); + expect(res).toEqual({ + error: extensionMessaging.FreighterApiInternalError, + }); + }); }); diff --git a/extension/src/background/messageListener/__tests__/disconnect.test.ts b/extension/src/background/messageListener/__tests__/disconnect.test.ts index 2406caf500..50beba7bbf 100644 --- a/extension/src/background/messageListener/__tests__/disconnect.test.ts +++ b/extension/src/background/messageListener/__tests__/disconnect.test.ts @@ -11,6 +11,9 @@ import { ALLOWLIST_ID, NETWORK_ID } from "constants/localStorageTypes"; import { DataStorageAccess } from "background/helpers/dataStorageAccess"; import { freighterApiMessageListener } from "../freighterApiMessageListener"; +const { publicKeySelector, sessionSlice, timeoutAccountAccess } = + jest.requireActual("background/ducks/session"); + const ACTIVE_PUBLIC_KEY = "GBTYAFHGNZSTE4VBWZYAGB3SRGJEPTI5I4Y22KZ4JTVAN56LESB6JZOF"; const OTHER_PUBLIC_KEY = @@ -20,7 +23,7 @@ const mockPublicKeySelector = jest.fn(); jest.mock("background/ducks/session", () => ({ ...jest.requireActual("background/ducks/session"), - publicKeySelector: () => mockPublicKeySelector(), + publicKeySelector: (state: unknown) => mockPublicKeySelector(state), })); jest.mock("@sentry/browser", () => ({ @@ -99,7 +102,36 @@ describe("freighterApiMessageListener DISCONNECT", () => { expect(store.setItem).not.toHaveBeenCalled(); }); - it("returns a locked error and leaves the allowlist untouched when locked", async () => { + it("removes the domain after an idle lock, which keeps the public key", async () => { + mockPublicKeySelector.mockImplementation(publicKeySelector); + const { data, store } = makeLocalStore({ + [NETWORK_ID]: { networkName: "Testnet" }, + [ALLOWLIST_ID]: initialAllowList, + }); + + const response = await freighterApiMessageListener( + { type: EXTERNAL_SERVICE_TYPES.DISCONNECT } as ExternalRequest, + { url: "https://example.com" } as browser.Runtime.MessageSender, + { + getState: () => ({ + session: { + ...sessionSlice.reducer(undefined, timeoutAccountAccess()), + publicKey: ACTIVE_PUBLIC_KEY, + }, + }), + } as unknown as Store, + store, + ); + + expect(response).toEqual({}); + expect( + (data[ALLOWLIST_ID] as typeof initialAllowList).Testnet[ + ACTIVE_PUBLIC_KEY + ], + ).toEqual(["other.com"]); + }); + + it("returns a locked error and leaves the allowlist untouched before the first unlock", async () => { mockPublicKeySelector.mockReturnValue(""); const { data, store } = makeLocalStore({ [NETWORK_ID]: { networkName: "Testnet" }, diff --git a/extension/src/background/messageListener/freighterApiMessageListener.ts b/extension/src/background/messageListener/freighterApiMessageListener.ts index 2d77b8400c..b7b2d3ede5 100644 --- a/extension/src/background/messageListener/freighterApiMessageListener.ts +++ b/extension/src/background/messageListener/freighterApiMessageListener.ts @@ -867,7 +867,8 @@ export const freighterApiMessageListener = ( try { const publicKey = publicKeySelector(sessionStore.getState()); - // the allowlist is keyed by public key, which we can't know while locked + // The allowlist is keyed by public key. An idle lock keeps the public + // key in the session, but it's empty until the user first unlocks. if (!publicKey) { return { apiError: FreighterApiLockedError, From 79afe7eefa027d05ff40ac1c2a8ca1fa1bdb557e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 16:07:53 +0000 Subject: [PATCH 3/3] Serialize allowlist updates setAllowListDomain and removeAllowListDomain read the whole allowlist and write it back. Two updates that overlap can overwrite each other, so a disconnect can report success while the domain stays allowed. Run the updates one at a time through an in-memory queue. All allowlist writers run in the background, so one queue covers grants, popup removal, and disconnect. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01M6pbgqeBtJ7cL5biPuxBJ1 --- .../helpers/__tests__/account.test.ts | 111 +++++++++++++++++- extension/src/background/helpers/account.ts | 91 ++++++++------ 2 files changed, 162 insertions(+), 40 deletions(-) diff --git a/extension/src/background/helpers/__tests__/account.test.ts b/extension/src/background/helpers/__tests__/account.test.ts index 6ea18d790d..c9d01ee237 100644 --- a/extension/src/background/helpers/__tests__/account.test.ts +++ b/extension/src/background/helpers/__tests__/account.test.ts @@ -1,5 +1,9 @@ -import { getIsRpcHealthy } from "../account"; -import { NETWORK_ID } from "constants/localStorageTypes"; +import { + getIsRpcHealthy, + removeAllowListDomain, + setAllowListDomain, +} from "../account"; +import { ALLOWLIST_ID, NETWORK_ID } from "constants/localStorageTypes"; import { DEFAULT_NETWORKS } from "@shared/constants/stellar"; import type { DataStorageAccess } from "background/helpers/dataStorageAccess"; import { callBackendV2 } from "background/helpers/callBackendV2"; @@ -113,3 +117,106 @@ describe("getIsRpcHealthy", () => { ); }); }); + +describe("allowlist updates", () => { + const PUBLIC_KEY = "GBTYAFHGNZSTE4VBWZYAGB3SRGJEPTI5I4Y22KZ4JTVAN56LESB6JZOF"; + const networkDetails = { ...DEFAULT_NETWORKS[1], networkName: "Testnet" }; + + // Yields between each read and write, so concurrent updates interleave + // unless they're serialized. + const makeSlowStore = (allowList: Record) => { + const data: Record = { [ALLOWLIST_ID]: allowList }; + const tick = () => new Promise((resolve) => setTimeout(resolve, 0)); + + return { + data, + store: { + getItem: async (key: string) => { + await tick(); + return data[key]; + }, + setItem: async (key: string, value: unknown) => { + await tick(); + data[key] = value; + }, + } as unknown as DataStorageAccess, + }; + }; + + it("applies concurrent removals without losing either", async () => { + const { data, store } = makeSlowStore({ + Testnet: { [PUBLIC_KEY]: ["a.com", "b.com", "c.com"] }, + }); + + await Promise.all([ + removeAllowListDomain({ + publicKey: PUBLIC_KEY, + networkName: "Testnet", + domain: "a.com", + localStore: store, + }), + removeAllowListDomain({ + publicKey: PUBLIC_KEY, + networkName: "Testnet", + domain: "b.com", + localStore: store, + }), + ]); + + expect(data[ALLOWLIST_ID]).toEqual({ + Testnet: { [PUBLIC_KEY]: ["c.com"] }, + }); + }); + + it("applies a concurrent removal and grant without losing either", async () => { + const { data, store } = makeSlowStore({ + Testnet: { [PUBLIC_KEY]: ["a.com"] }, + }); + + await Promise.all([ + removeAllowListDomain({ + publicKey: PUBLIC_KEY, + networkName: "Testnet", + domain: "a.com", + localStore: store, + }), + setAllowListDomain({ + publicKey: PUBLIC_KEY, + networkDetails, + domain: "b.com", + localStore: store, + }), + ]); + + expect(data[ALLOWLIST_ID]).toEqual({ + Testnet: { [PUBLIC_KEY]: ["b.com"] }, + }); + }); + + it("runs the next update after a failed one", async () => { + const { data, store } = makeSlowStore({ + Testnet: { [PUBLIC_KEY]: ["a.com"] }, + }); + const failingStore = { + ...store, + getItem: () => Promise.reject(new Error("storage failure")), + } as unknown as DataStorageAccess; + + const failed = removeAllowListDomain({ + publicKey: PUBLIC_KEY, + networkName: "Testnet", + domain: "a.com", + localStore: failingStore, + }); + const succeeded = removeAllowListDomain({ + publicKey: PUBLIC_KEY, + networkName: "Testnet", + domain: "a.com", + localStore: store, + }); + + await expect(failed).rejects.toThrow("storage failure"); + await succeeded; + expect(data[ALLOWLIST_ID]).toEqual({ Testnet: { [PUBLIC_KEY]: [] } }); + }); +}); diff --git a/extension/src/background/helpers/account.ts b/extension/src/background/helpers/account.ts index fc51d65995..04fd737d0d 100644 --- a/extension/src/background/helpers/account.ts +++ b/extension/src/background/helpers/account.ts @@ -121,7 +121,18 @@ export const getAllowListSegment = async ({ return allowListByKey; }; -export const setAllowListDomain = async ({ +// Allowlist updates read the whole allowlist and write it back. Run them one +// at a time so that concurrent updates (for example, two dApps calling +// disconnect) don't overwrite each other. +let allowListQueue: Promise = Promise.resolve(); + +const withAllowListLock = (fn: () => Promise): Promise => { + const result = allowListQueue.then(fn); + allowListQueue = result.catch(() => undefined); + return result; +}; + +export const setAllowListDomain = ({ publicKey, networkDetails, domain, @@ -131,28 +142,30 @@ export const setAllowListDomain = async ({ networkDetails: NetworkDetails; domain: string; localStore: DataStorageAccess; -}) => { - const allowList = (await localStore.getItem(ALLOWLIST_ID)) || {}; - const allowListByNetwork = { ...allowList[networkDetails.networkName] }; - const allowListPublicKeyArray: string[] = allowListByNetwork[publicKey] || []; +}) => + withAllowListLock(async () => { + const allowList = (await localStore.getItem(ALLOWLIST_ID)) || {}; + const allowListByNetwork = { ...allowList[networkDetails.networkName] }; + const allowListPublicKeyArray: string[] = + allowListByNetwork[publicKey] || []; + + if (allowListPublicKeyArray.includes(domain)) { + return allowListPublicKeyArray; + } - if (allowListPublicKeyArray.includes(domain)) { - return allowListPublicKeyArray; - } + allowListPublicKeyArray.push(domain); + await localStore.setItem(ALLOWLIST_ID, { + ...allowList, + [networkDetails.networkName]: { + ...allowListByNetwork, + [publicKey]: allowListPublicKeyArray, + }, + }); - allowListPublicKeyArray.push(domain); - await localStore.setItem(ALLOWLIST_ID, { - ...allowList, - [networkDetails.networkName]: { - ...allowListByNetwork, - [publicKey]: allowListPublicKeyArray, - }, + return allowListPublicKeyArray; }); - return allowListPublicKeyArray; -}; - -export const removeAllowListDomain = async ({ +export const removeAllowListDomain = ({ publicKey, networkName, domain, @@ -162,30 +175,32 @@ export const removeAllowListDomain = async ({ networkName: string; domain: string; localStore: DataStorageAccess; -}) => { - const allowList = (await localStore.getItem(ALLOWLIST_ID)) || {}; - const allowListByNetwork = { ...allowList[networkName] }; - const allowListPublicKeyArray: string[] = allowListByNetwork[publicKey] || []; +}) => + withAllowListLock(async () => { + const allowList = (await localStore.getItem(ALLOWLIST_ID)) || {}; + const allowListByNetwork = { ...allowList[networkName] }; + const allowListPublicKeyArray: string[] = + allowListByNetwork[publicKey] || []; + + if (!allowListPublicKeyArray.includes(domain)) { + return allowListPublicKeyArray; + } - if (!allowListPublicKeyArray.includes(domain)) { - return allowListPublicKeyArray; - } + const editedAllowList = allowListPublicKeyArray.filter( + (item) => item !== domain, + ); - const editedAllowList = allowListPublicKeyArray.filter( - (item) => item !== domain, - ); + await localStore.setItem(ALLOWLIST_ID, { + ...allowList, + [networkName]: { + ...allowListByNetwork, + [publicKey]: editedAllowList, + }, + }); - await localStore.setItem(ALLOWLIST_ID, { - ...allowList, - [networkName]: { - ...allowListByNetwork, - [publicKey]: editedAllowList, - }, + return allowListPublicKeyArray; }); - return allowListPublicKeyArray; -}; - export const getIsMemoValidationEnabled = async ({ localStore, }: {