diff --git a/@shared/api/external.ts b/@shared/api/external.ts index fbeae6a43c..c41c9ad037 100644 --- a/@shared/api/external.ts +++ b/@shared/api/external.ts @@ -303,3 +303,28 @@ 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?.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 {}; +}; 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..890e03a2be --- /dev/null +++ b/@stellar/freighter-api/src/__tests__/disconnect.test.js @@ -0,0 +1,28 @@ +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" }); + }); + 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/@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/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, }: { 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..50beba7bbf --- /dev/null +++ b/extension/src/background/messageListener/__tests__/disconnect.test.ts @@ -0,0 +1,163 @@ +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 { publicKeySelector, sessionSlice, timeoutAccountAccess } = + jest.requireActual("background/ducks/session"); + +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: (state: unknown) => mockPublicKeySelector(state), +})); + +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("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" }, + [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..b7b2d3ede5 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,39 @@ export const freighterApiMessageListener = ( }; }; + const disconnect = async () => { + try { + const publicKey = publicKeySelector(sessionStore.getState()); + + // 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, + 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 +909,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]();