diff --git a/app/scripts/lib/rpc-method-middleware/handlers/request-accounts.test.ts b/app/scripts/lib/rpc-method-middleware/handlers/request-accounts.test.ts index d3a52aad230b..ad5fa9b345d9 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/request-accounts.test.ts +++ b/app/scripts/lib/rpc-method-middleware/handlers/request-accounts.test.ts @@ -39,7 +39,8 @@ const createMockedHandler = () => { '0x3': {}, }, }; - const requestCaip25PermissionForOrigin = jest.fn().mockResolvedValue({}); + const requestCaip25ApprovalForOrigin = jest.fn().mockResolvedValue({}); + const grantPermissionsForOrigin = jest.fn().mockReturnValue({}); const response: PendingJsonRpcResponse = { jsonrpc: '2.0' as const, id: 0, @@ -53,7 +54,8 @@ const createMockedHandler = () => { getUnlockPromise, sendMetrics, metamaskState, - requestCaip25PermissionForOrigin, + requestCaip25ApprovalForOrigin, + grantPermissionsForOrigin, }); return { @@ -64,7 +66,8 @@ const createMockedHandler = () => { getUnlockPromise, sendMetrics, metamaskState, - requestCaip25PermissionForOrigin, + requestCaip25ApprovalForOrigin, + grantPermissionsForOrigin, handler, }; }; @@ -127,18 +130,17 @@ describe('requestEthereumAccountsHandler', () => { }); describe('eip155 account permissions do not exist', () => { - it('requests the CAIP-25 permission', async () => { - const { handler, requestCaip25PermissionForOrigin } = - createMockedHandler(); + it('requests the CAIP-25 approval', async () => { + const { handler, requestCaip25ApprovalForOrigin } = createMockedHandler(); await handler({ ...baseRequest, origin: 'http://test.com' }); - expect(requestCaip25PermissionForOrigin).toHaveBeenCalledWith(); + expect(requestCaip25ApprovalForOrigin).toHaveBeenCalledWith(); }); - it('throws an error if the CAIP-25 permission approval is rejected', async () => { - const { handler, requestCaip25PermissionForOrigin, end } = + it('throws an error if the CAIP-25 approval is rejected', async () => { + const { handler, requestCaip25ApprovalForOrigin, end } = createMockedHandler(); - requestCaip25PermissionForOrigin.mockRejectedValue( + requestCaip25ApprovalForOrigin.mockRejectedValue( new Error('approval rejected'), ); @@ -146,6 +148,19 @@ describe('requestEthereumAccountsHandler', () => { expect(end).toHaveBeenCalledWith(new Error('approval rejected')); }); + it('grants the CAIP-25 approval', async () => { + const { + handler, + requestCaip25ApprovalForOrigin, + grantPermissionsForOrigin, + } = createMockedHandler(); + + requestCaip25ApprovalForOrigin.mockResolvedValue({ foo: 'bar' }); + + await handler({ ...baseRequest, origin: 'http://test.com' }); + expect(grantPermissionsForOrigin).toHaveBeenCalledWith({ foo: 'bar' }); + }); + it('returns the newly granted and properly ordered eth accounts', async () => { const { handler, getAccounts, response } = createMockedHandler(); getAccounts diff --git a/app/scripts/lib/rpc-method-middleware/handlers/request-accounts.ts b/app/scripts/lib/rpc-method-middleware/handlers/request-accounts.ts index bb1dfb566643..462916e1a8c1 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/request-accounts.ts +++ b/app/scripts/lib/rpc-method-middleware/handlers/request-accounts.ts @@ -35,7 +35,8 @@ const requestEthereumAccounts = { getUnlockPromise: true, sendMetrics: true, metamaskState: true, - requestCaip25PermissionForOrigin: true, + requestCaip25ApprovalForOrigin: true, + grantPermissionsForOrigin: true, }, }; export default requestEthereumAccounts; @@ -59,7 +60,8 @@ const locks = new Set(); * @param options.getUnlockPromise - A hook that resolves when the wallet is unlocked. * @param options.sendMetrics - A hook that helps track metric events. * @param options.metamaskState - The MetaMask app state. - * @param options.requestCaip25PermissionForOrigin - A hook that requests the CAIP-25 permission for the origin. + * @param options.requestCaip25ApprovalForOrigin - A hook that requests approval for the CAIP-25 permission for the origin. + * @param options.grantPermissionsForOrigin - A hook that grants permission for the approved permissions for the origin. * @param options.metamaskState.metaMetricsId - The MetaMetrics ID. * @param options.metamaskState.permissionHistory - The permission history keyed by origin. * @param options.metamaskState.accounts - The accounts available in the wallet keyed by address. @@ -75,7 +77,8 @@ async function requestEthereumAccountsHandler( getUnlockPromise, sendMetrics, metamaskState, - requestCaip25PermissionForOrigin, + requestCaip25ApprovalForOrigin, + grantPermissionsForOrigin, }: { getAccounts: (ignoreLock?: boolean) => string[]; getUnlockPromise: (shouldShowUnlockRequest: true) => Promise; @@ -88,14 +91,15 @@ async function requestEthereumAccountsHandler( permissionHistory: Record; accounts: Record; }; - requestCaip25PermissionForOrigin: ( + requestCaip25ApprovalForOrigin: ( requestedPermissions?: RequestedPermissions, - ) => Promise< - ValidPermission< + ) => Promise; + grantPermissionsForOrigin: (approvedPermissions: RequestedPermissions) => { + [Caip25EndowmentPermissionName]: ValidPermission< typeof Caip25EndowmentPermissionName, Caveat - > - >; + >; + }; }, ) { const { origin } = req; @@ -125,7 +129,8 @@ async function requestEthereumAccountsHandler( } try { - await requestCaip25PermissionForOrigin(); + const caip25Approval = await requestCaip25ApprovalForOrigin(); + await grantPermissionsForOrigin(caip25Approval); } catch (error) { return end(error as unknown as Error); } diff --git a/app/scripts/lib/rpc-method-middleware/handlers/wallet-requestPermissions.test.ts b/app/scripts/lib/rpc-method-middleware/handlers/wallet-requestPermissions.test.ts index 45f6ef72e5ac..6ce5c9a7264c 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/wallet-requestPermissions.test.ts +++ b/app/scripts/lib/rpc-method-middleware/handlers/wallet-requestPermissions.test.ts @@ -33,7 +33,20 @@ const createMockedHandler = () => { const end = jest.fn(); const requestPermissionsForOrigin = jest.fn().mockResolvedValue({}); const getAccounts = jest.fn().mockReturnValue([]); - const requestCaip25PermissionForOrigin = jest.fn().mockResolvedValue({}); + const requestCaip25ApprovalForOrigin = jest.fn().mockResolvedValue({}); + const grantPermissionsForOrigin = jest.fn().mockReturnValue({ + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: {}, + }, + }, + ], + }, + }); const response: PendingJsonRpcResponse = { jsonrpc: '2.0' as const, id: 0, @@ -47,7 +60,8 @@ const createMockedHandler = () => { { getAccounts, requestPermissionsForOrigin, - requestCaip25PermissionForOrigin, + requestCaip25ApprovalForOrigin, + grantPermissionsForOrigin, }, ); @@ -57,7 +71,8 @@ const createMockedHandler = () => { end, getAccounts, requestPermissionsForOrigin, - requestCaip25PermissionForOrigin, + requestCaip25ApprovalForOrigin, + grantPermissionsForOrigin, handler, }; }; @@ -82,7 +97,7 @@ describe('requestPermissionsHandler', () => { const { handler, requestPermissionsForOrigin, - requestCaip25PermissionForOrigin, + requestCaip25ApprovalForOrigin, } = createMockedHandler(); await handler( @@ -98,7 +113,7 @@ describe('requestPermissionsHandler', () => { expect(requestPermissionsForOrigin).toHaveBeenCalledWith({ [Caip25EndowmentPermissionName]: {}, }); - expect(requestCaip25PermissionForOrigin).not.toHaveBeenCalled(); + expect(requestCaip25ApprovalForOrigin).not.toHaveBeenCalled(); }); it('requests the permission for the other permissions', async () => { @@ -173,8 +188,7 @@ describe('requestPermissionsHandler', () => { describe('only CAIP-25 equivalent permissions ("eth_accounts" and/or "endowment:permittedChains") requested', () => { it('requests the CAIP-25 permission using eth_accounts when only eth_accounts is specified in params', async () => { - const { handler, requestCaip25PermissionForOrigin } = - createMockedHandler(); + const { handler, requestCaip25ApprovalForOrigin } = createMockedHandler(); await handler( getBaseRequest({ @@ -188,7 +202,7 @@ describe('requestPermissionsHandler', () => { }), ); - expect(requestCaip25PermissionForOrigin).toHaveBeenCalledWith({ + expect(requestCaip25ApprovalForOrigin).toHaveBeenCalledWith({ [RestrictedMethods.eth_accounts]: { foo: 'bar', }, @@ -196,8 +210,7 @@ describe('requestPermissionsHandler', () => { }); it('requests the CAIP-25 permission for permittedChains when only permittedChains is specified in params', async () => { - const { handler, requestCaip25PermissionForOrigin } = - createMockedHandler(); + const { handler, requestCaip25ApprovalForOrigin } = createMockedHandler(); await handler( getBaseRequest({ @@ -216,7 +229,7 @@ describe('requestPermissionsHandler', () => { }), ); - expect(requestCaip25PermissionForOrigin).toHaveBeenCalledWith({ + expect(requestCaip25ApprovalForOrigin).toHaveBeenCalledWith({ [PermissionNames.permittedChains]: { caveats: [ { @@ -229,8 +242,7 @@ describe('requestPermissionsHandler', () => { }); it('requests the CAIP-25 permission for eth_accounts and permittedChains when both are specified in params', async () => { - const { handler, requestCaip25PermissionForOrigin } = - createMockedHandler(); + const { handler, requestCaip25ApprovalForOrigin } = createMockedHandler(); await handler( getBaseRequest({ @@ -252,7 +264,7 @@ describe('requestPermissionsHandler', () => { }), ); - expect(requestCaip25PermissionForOrigin).toHaveBeenCalledWith({ + expect(requestCaip25ApprovalForOrigin).toHaveBeenCalledWith({ [RestrictedMethods.eth_accounts]: { foo: 'bar', }, @@ -267,11 +279,11 @@ describe('requestPermissionsHandler', () => { }); }); - it('returns an error if requesting the CAIP-25 permission fails', async () => { - const { handler, requestCaip25PermissionForOrigin, end } = + it('returns an error if requesting the CAIP-25 approval fails', async () => { + const { handler, requestCaip25ApprovalForOrigin, end } = createMockedHandler(); - requestCaip25PermissionForOrigin.mockRejectedValue( - new Error('failed to request caip25 permission'), + requestCaip25ApprovalForOrigin.mockRejectedValue( + new Error('failed to request caip25 approval'), ); await handler( @@ -295,37 +307,51 @@ describe('requestPermissionsHandler', () => { ); expect(end).toHaveBeenCalledWith( - new Error('failed to request caip25 permission'), + new Error('failed to request caip25 approval'), ); }); - it('returns both eth_accounts and permittedChains permissions that were granted if there are permitted chains', async () => { + it('grants the CAIP-25 approval', async () => { const { handler, getAccounts, - requestCaip25PermissionForOrigin, - response, + requestCaip25ApprovalForOrigin, + grantPermissionsForOrigin, } = createMockedHandler(); getAccounts.mockReturnValue(['0xdeadbeef']); - requestCaip25PermissionForOrigin.mockResolvedValue({ - id: 'new', - parentCapability: Caip25EndowmentPermissionName, - caveats: [ - { - type: Caip25CaveatType, - value: { - requiredScopes: {}, - optionalScopes: { - 'eip155:1': { - accounts: ['0xdeadbeef'], - }, - 'eip155:5': { - accounts: ['0xdeadbeef'], + requestCaip25ApprovalForOrigin.mockResolvedValue({ + foo: 'bar', + }); + + await handler(getBaseRequest()); + expect(grantPermissionsForOrigin).toHaveBeenCalledWith({ foo: 'bar' }); + }); + + it('returns both eth_accounts and permittedChains permissions that were granted if there are permitted chains', async () => { + const { handler, getAccounts, grantPermissionsForOrigin, response } = + createMockedHandler(); + getAccounts.mockReturnValue(['0xdeadbeef']); + grantPermissionsForOrigin.mockReturnValue({ + [Caip25EndowmentPermissionName]: { + id: 'new', + parentCapability: Caip25EndowmentPermissionName, + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'eip155:1': { + accounts: ['0xdeadbeef'], + }, + 'eip155:5': { + accounts: ['0xdeadbeef'], + }, }, }, }, - }, - ], + ], + }, }); await handler(getBaseRequest()); @@ -354,29 +380,27 @@ describe('requestPermissionsHandler', () => { }); it('returns only eth_accounts permission that was granted if there are no permitted chains', async () => { - const { - handler, - getAccounts, - requestCaip25PermissionForOrigin, - response, - } = createMockedHandler(); + const { handler, getAccounts, grantPermissionsForOrigin, response } = + createMockedHandler(); getAccounts.mockReturnValue(['0xdeadbeef']); - requestCaip25PermissionForOrigin.mockResolvedValue({ - id: 'new', - parentCapability: Caip25EndowmentPermissionName, - caveats: [ - { - type: Caip25CaveatType, - value: { - requiredScopes: {}, - optionalScopes: { - 'wallet:eip155': { - accounts: ['0xdeadbeef'], + grantPermissionsForOrigin.mockReturnValue({ + [Caip25EndowmentPermissionName]: { + id: 'new', + parentCapability: Caip25EndowmentPermissionName, + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: ['0xdeadbeef'], + }, }, }, }, - }, - ], + ], + }, }); await handler(getBaseRequest()); @@ -396,13 +420,13 @@ describe('requestPermissionsHandler', () => { }); describe('both CAIP-25 equivalent and other permissions requested', () => { - describe('both CAIP-25 equivalent permissions and other permissions are granted', () => { + describe('both CAIP-25 equivalent permissions and other permissions are approved', () => { it('returns eth_accounts, permittedChains, and other permissions that were granted', async () => { const { handler, getAccounts, requestPermissionsForOrigin, - requestCaip25PermissionForOrigin, + grantPermissionsForOrigin, response, } = createMockedHandler(); requestPermissionsForOrigin.mockResolvedValue([ @@ -412,25 +436,27 @@ describe('requestPermissionsHandler', () => { }, ]); getAccounts.mockReturnValue(['0xdeadbeef']); - requestCaip25PermissionForOrigin.mockResolvedValue({ - id: 'new', - parentCapability: Caip25EndowmentPermissionName, - caveats: [ - { - type: Caip25CaveatType, - value: { - requiredScopes: {}, - optionalScopes: { - 'eip155:1': { - accounts: ['0xdeadbeef'], - }, - 'eip155:5': { - accounts: ['0xdeadbeef'], + grantPermissionsForOrigin.mockReturnValue({ + [Caip25EndowmentPermissionName]: { + id: 'new', + parentCapability: Caip25EndowmentPermissionName, + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'eip155:1': { + accounts: ['0xdeadbeef'], + }, + 'eip155:5': { + accounts: ['0xdeadbeef'], + }, }, }, }, - }, - ], + ], + }, }); await handler( @@ -446,6 +472,8 @@ describe('requestPermissionsHandler', () => { }), ); expect(response.result).toStrictEqual([ + { foo: 'bar' }, + { hello: true }, { caveats: [ { @@ -466,45 +494,20 @@ describe('requestPermissionsHandler', () => { id: 'new', parentCapability: PermissionNames.permittedChains, }, - { foo: 'bar' }, - { hello: true }, ]); }); }); - describe('CAIP-25 equivalent permissions are granted, but other permissions are not granted', () => { - it('returns both eth_accounts and permittedChains permissions that were granted', async () => { + describe('CAIP-25 equivalent permissions are approved, but other permissions are not approved', () => { + it('does not grant the CAIP-25 permission', async () => { const { handler, - getAccounts, requestPermissionsForOrigin, - requestCaip25PermissionForOrigin, - response, + grantPermissionsForOrigin, } = createMockedHandler(); requestPermissionsForOrigin.mockRejectedValue( new Error('other permissions rejected'), ); - getAccounts.mockReturnValue(['0xdeadbeef']); - requestCaip25PermissionForOrigin.mockResolvedValue({ - id: 'new', - parentCapability: Caip25EndowmentPermissionName, - caveats: [ - { - type: Caip25CaveatType, - value: { - requiredScopes: {}, - optionalScopes: { - 'eip155:1': { - accounts: ['0xdeadbeef'], - }, - 'eip155:5': { - accounts: ['0xdeadbeef'], - }, - }, - }, - }, - ], - }); await handler( getBaseRequest({ @@ -518,47 +521,45 @@ describe('requestPermissionsHandler', () => { ], }), ); - expect(response.result).toStrictEqual([ - { - caveats: [ - { - type: CaveatTypes.restrictReturnedAccounts, - value: ['0xdeadbeef'], - }, - ], - id: 'new', - parentCapability: RestrictedMethods.eth_accounts, - }, - { - caveats: [ + + expect(grantPermissionsForOrigin).not.toHaveBeenCalled(); + }); + + it('returns an error that the other permissions were not approved', async () => { + const { handler, requestPermissionsForOrigin, end } = + createMockedHandler(); + requestPermissionsForOrigin.mockRejectedValue( + new Error('other permissions rejected'), + ); + + await handler( + getBaseRequest({ + params: [ { - type: CaveatTypes.restrictNetworkSwitching, - value: ['0x1', '0x5'], + eth_accounts: {}, + 'endowment:permitted-chains': {}, + otherPermissionA: {}, + otherPermissionB: {}, }, ], - id: 'new', - parentCapability: PermissionNames.permittedChains, - }, - ]); + }), + ); + + expect(end).toHaveBeenCalledWith( + new Error('other permissions rejected'), + ); }); }); - describe('CAIP-25 equivalent permissions are not granted, but other permissions are granted', () => { - it('returns the other permissions that are granted', async () => { + describe('CAIP-25 equivalent permissions are not approved', () => { + it('does not grant the CAIP-25 permission', async () => { const { handler, - requestPermissionsForOrigin, - requestCaip25PermissionForOrigin, - response, + requestCaip25ApprovalForOrigin, + grantPermissionsForOrigin, } = createMockedHandler(); - requestPermissionsForOrigin.mockResolvedValue([ - { - otherPermissionA: { foo: 'bar' }, - otherPermissionB: { hello: true }, - }, - ]); - requestCaip25PermissionForOrigin.mockRejectedValue( - new Error('caip25 permission rejected'), + requestCaip25ApprovalForOrigin.mockRejectedValue( + new Error('caip25 approval rejected'), ); await handler( @@ -574,26 +575,17 @@ describe('requestPermissionsHandler', () => { }), ); - expect(response.result).toStrictEqual([ - { foo: 'bar' }, - { hello: true }, - ]); + expect(grantPermissionsForOrigin).not.toHaveBeenCalled(); }); - }); - describe('both CAIP-25 equivalent permissions and other permissions are not granted', () => { - it('returns the error from the rejected other permissions grant', async () => { + it('does not request approval for the other permissions', async () => { const { handler, + requestCaip25ApprovalForOrigin, requestPermissionsForOrigin, - requestCaip25PermissionForOrigin, - end, } = createMockedHandler(); - requestPermissionsForOrigin.mockRejectedValue( - new Error('other permissions rejected'), - ); - requestCaip25PermissionForOrigin.mockRejectedValue( - new Error('caip25 permission rejected'), + requestCaip25ApprovalForOrigin.mockRejectedValue( + new Error('caip25 approval rejected'), ); await handler( @@ -609,9 +601,30 @@ describe('requestPermissionsHandler', () => { }), ); - expect(end).toHaveBeenCalledWith( - new Error('other permissions rejected'), + expect(requestPermissionsForOrigin).not.toHaveBeenCalled(); + }); + + it('returns an error that the CAIP-25 permissions were not approved', async () => { + const { handler, requestCaip25ApprovalForOrigin, end } = + createMockedHandler(); + requestCaip25ApprovalForOrigin.mockRejectedValue( + new Error('caip25 approval rejected'), ); + + await handler( + getBaseRequest({ + params: [ + { + eth_accounts: {}, + 'endowment:permitted-chains': {}, + otherPermissionA: {}, + otherPermissionB: {}, + }, + ], + }), + ); + + expect(end).toHaveBeenCalledWith(new Error('caip25 approval rejected')); }); }); }); diff --git a/app/scripts/lib/rpc-method-middleware/handlers/wallet-requestPermissions.ts b/app/scripts/lib/rpc-method-middleware/handlers/wallet-requestPermissions.ts index c62809835e75..4645327abc89 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/wallet-requestPermissions.ts +++ b/app/scripts/lib/rpc-method-middleware/handlers/wallet-requestPermissions.ts @@ -33,7 +33,8 @@ export const requestPermissionsHandler = { hookNames: { getAccounts: true, requestPermissionsForOrigin: true, - requestCaip25PermissionForOrigin: true, + requestCaip25ApprovalForOrigin: true, + grantPermissionsForOrigin: true, }, }; @@ -55,7 +56,8 @@ type GrantedPermissions = Awaited< * @param end - JsonRpcEngine end() callback * @param options - Method hooks passed to the method implementation * @param options.getAccounts - A hook that returns the permitted eth accounts for the origin sorted by lastSelected. - * @param options.requestCaip25PermissionForOrigin - A hook that requests the CAIP-25 permission for the origin. + * @param options.requestCaip25ApprovalForOrigin - A hook that requests approval for the CAIP-25 permission for the origin. + * @param options.grantPermissionsForOrigin - A hook that grants permission for the approved permissions for the origin. * @param options.requestPermissionsForOrigin - A hook that requests permissions for the origin. * @returns A promise that resolves to nothing */ @@ -67,20 +69,22 @@ async function requestPermissionsImplementation( { getAccounts, requestPermissionsForOrigin, - requestCaip25PermissionForOrigin, + requestCaip25ApprovalForOrigin, + grantPermissionsForOrigin, }: { getAccounts: () => string[]; requestPermissionsForOrigin: ( requestedPermissions: RequestedPermissions, ) => Promise<[GrantedPermissions]>; - requestCaip25PermissionForOrigin: ( + requestCaip25ApprovalForOrigin: ( requestedPermissions?: RequestedPermissions, - ) => Promise< - ValidPermission< + ) => Promise; + grantPermissionsForOrigin: (approvedPermissions: RequestedPermissions) => { + [Caip25EndowmentPermissionName]: ValidPermission< typeof Caip25EndowmentPermissionName, Caveat - > - >; + >; + }; }, ) { const { params } = req; @@ -106,55 +110,14 @@ async function requestPermissionsImplementation( let grantedPermissions: GrantedPermissions = {}; - let caip25Endowment; - let caip25CaveatValue; + let caip25Approval; if (hasCaip25EquivalentPermissions) { try { - caip25Endowment = await requestCaip25PermissionForOrigin( + caip25Approval = await requestCaip25ApprovalForOrigin( caip25EquivalentPermissions, ); - caip25CaveatValue = caip25Endowment?.caveats?.find( - ({ type }) => type === Caip25CaveatType, - )?.value as Caip25CaveatValue | undefined; - - if (!caip25CaveatValue) { - throw new Error( - `could not find ${Caip25CaveatType} in granted ${Caip25EndowmentPermissionName} permission.`, - ); - } - - // We cannot derive correct eth_accounts value directly from the CAIP-25 permission - // because the accounts will not be in order of lastSelected - const ethAccounts = getAccounts(); - - grantedPermissions[RestrictedMethods.eth_accounts] = { - ...caip25Endowment, - parentCapability: RestrictedMethods.eth_accounts, - caveats: [ - { - type: CaveatTypes.restrictReturnedAccounts, - value: ethAccounts, - }, - ], - }; - - const ethChainIds = getPermittedEthChainIds(caip25CaveatValue); - if (ethChainIds.length > 0) { - grantedPermissions[PermissionNames.permittedChains] = { - ...caip25Endowment, - parentCapability: PermissionNames.permittedChains, - caveats: [ - { - type: CaveatTypes.restrictNetworkSwitching, - value: ethChainIds, - }, - ], - }; - } } catch (error) { - if (!hasOtherRequestedPermissions) { - return end(error as unknown as Error); - } + return end(error as unknown as Error); } } @@ -163,14 +126,54 @@ async function requestPermissionsImplementation( const [frozenGrantedPermissions] = await requestPermissionsForOrigin( requestedPermissions, ); - grantedPermissions = { - ...grantedPermissions, - ...frozenGrantedPermissions, - }; + grantedPermissions = { ...frozenGrantedPermissions }; } catch (error) { - if (Object.keys(grantedPermissions).length === 0) { - return end(error as unknown as Error); - } + return end(error as unknown as Error); + } + } + + if (caip25Approval) { + const grantedCaip25Permissions = grantPermissionsForOrigin(caip25Approval); + const caip25Endowment = + grantedCaip25Permissions[Caip25EndowmentPermissionName]; + + const caip25CaveatValue = caip25Endowment?.caveats?.find( + ({ type }) => type === Caip25CaveatType, + )?.value as Caip25CaveatValue | undefined; + + if (!caip25CaveatValue) { + throw new Error( + `could not find ${Caip25CaveatType} in granted ${Caip25EndowmentPermissionName} permission.`, + ); + } + + // We cannot derive correct eth_accounts value directly from the CAIP-25 permission + // because the accounts will not be in order of lastSelected + const ethAccounts = getAccounts(); + + grantedPermissions[RestrictedMethods.eth_accounts] = { + ...caip25Endowment, + parentCapability: RestrictedMethods.eth_accounts, + caveats: [ + { + type: CaveatTypes.restrictReturnedAccounts, + value: ethAccounts, + }, + ], + }; + + const ethChainIds = getPermittedEthChainIds(caip25CaveatValue); + if (ethChainIds.length > 0) { + grantedPermissions[PermissionNames.permittedChains] = { + ...caip25Endowment, + parentCapability: PermissionNames.permittedChains, + caveats: [ + { + type: CaveatTypes.restrictNetworkSwitching, + value: ethChainIds, + }, + ], + }; } } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 15c0fea5a1f0..12ff0a455856 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -5499,14 +5499,15 @@ export default class MetamaskController extends EventEmitter { } /** - * Requests CAIP-25 for permissions for the specified origin - * and replaces any existing CAIP-25 permission with a new one. + * Requests user approval for the CAIP-25 permission for the specified origin + * and returns a permissions object that must be passed to + * PermissionController.grantPermissions() to complete the permission granting. * * @param {string} origin - The origin to request approval for. * @param requestedPermissions - The legacy permissions to request approval for. - * @returns the granted CAIP-25 Permission. + * @returns the approved permissions object that must then be granted by calling the PermissionController. */ - async requestCaip25Permission(origin, requestedPermissions = {}) { + async requestCaip25Approval(origin, requestedPermissions = {}) { const permissions = pick(requestedPermissions, [ RestrictedMethods.eth_accounts, PermissionNames.permittedChains, @@ -5564,21 +5565,16 @@ export default class MetamaskController extends EventEmitter { legacyApproval.approvedAccounts, ); - const grantedPermissions = this.permissionController.grantPermissions({ - subject: { origin }, - approvedPermissions: { - [Caip25EndowmentPermissionName]: { - caveats: [ - { - type: Caip25CaveatType, - value: caveatValueWithAccounts, - }, - ], - }, + return { + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: caveatValueWithAccounts, + }, + ], }, - }); - - return grantedPermissions[Caip25EndowmentPermissionName]; + }; } // --------------------------------------------------------------------------- @@ -6367,10 +6363,16 @@ export default class MetamaskController extends EventEmitter { ), // Permission-related getAccounts: this.getPermittedAccounts.bind(this, origin), - requestCaip25PermissionForOrigin: this.requestCaip25Permission.bind( + requestCaip25ApprovalForOrigin: this.requestCaip25Approval.bind( this, origin, ), + grantPermissionsForOrigin: (approvedPermissions) => { + return this.permissionController.grantPermissions({ + subject: { origin }, + approvedPermissions, + }); + }, getPermissionsForOrigin: this.permissionController.getPermissions.bind( this.permissionController, origin, diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index cbad1dead1b1..5dde0db61819 100644 --- a/app/scripts/metamask-controller.test.js +++ b/app/scripts/metamask-controller.test.js @@ -953,7 +953,7 @@ describe('MetaMaskController', () => { }); }); - describe('#requestCaip25Permission', () => { + describe('#requestCaip25ApprovalApproval', () => { it('requests approval with well formed id and origin', async () => { jest .spyOn( @@ -972,7 +972,7 @@ describe('MetaMaskController', () => { }, }); - await metamaskController.requestCaip25Permission('test.com', {}); + await metamaskController.requestCaip25Approval('test.com', {}); expect( metamaskController.approvalController.addAndShowApprovalRequest, @@ -1014,7 +1014,7 @@ describe('MetaMaskController', () => { }, }); - await metamaskController.requestCaip25Permission('test.com', { + await metamaskController.requestCaip25Approval('test.com', { [PermissionNames.eth_accounts]: { caveats: [ { @@ -1071,7 +1071,7 @@ describe('MetaMaskController', () => { }, }); - await metamaskController.requestCaip25Permission('test.com', { + await metamaskController.requestCaip25Approval('test.com', { [PermissionNames.permittedChains]: { caveats: [ { @@ -1128,7 +1128,7 @@ describe('MetaMaskController', () => { }, }); - await metamaskController.requestCaip25Permission('test.com', { + await metamaskController.requestCaip25Approval('test.com', { [PermissionNames.eth_accounts]: { caveats: [ { @@ -1200,7 +1200,7 @@ describe('MetaMaskController', () => { }, }); - await metamaskController.requestCaip25Permission('npm:snap', { + await metamaskController.requestCaip25Approval('npm:snap', { [PermissionNames.eth_accounts]: { caveats: [ { @@ -1256,7 +1256,7 @@ describe('MetaMaskController', () => { }, }); - await metamaskController.requestCaip25Permission('npm:snap', { + await metamaskController.requestCaip25Approval('npm:snap', { [PermissionNames.permittedChains]: { caveats: [ { @@ -1305,7 +1305,7 @@ describe('MetaMaskController', () => { }, }); - await metamaskController.requestCaip25Permission('npm:snap', { + await metamaskController.requestCaip25Approval('npm:snap', { [PermissionNames.eth_accounts]: { caveats: [ { @@ -1360,13 +1360,13 @@ describe('MetaMaskController', () => { .mockRejectedValue(new Error('approval rejected')); await expect(() => - metamaskController.requestCaip25Permission('test.com', { + metamaskController.requestCaip25Approval('test.com', { eth_accounts: {}, }), ).rejects.toThrow(new Error('approval rejected')); }); - it('grants the CAIP-25 permission with eth accounts, chainIds, and isMultichainOrigin: false if origin is not snapId', async () => { + it('returns the CAIP-25 approval with eth accounts, chainIds, and isMultichainOrigin: false if origin is not snapId', async () => { jest .spyOn( metamaskController.approvalController, @@ -1376,45 +1376,36 @@ describe('MetaMaskController', () => { approvedChainIds: ['0x1', '0x5'], approvedAccounts: ['0xdeadbeef'], }); - jest - .spyOn(metamaskController.permissionController, 'grantPermissions') - .mockReturnValue({ - [Caip25EndowmentPermissionName]: { - foo: 'bar', - }, - }); - await metamaskController.requestCaip25Permission('test.com', {}); + const result = await metamaskController.requestCaip25Approval( + 'test.com', + {}, + ); - expect( - metamaskController.permissionController.grantPermissions, - ).toHaveBeenCalledWith({ - subject: { origin: 'test.com' }, - approvedPermissions: { - [Caip25EndowmentPermissionName]: { - caveats: [ - { - type: Caip25CaveatType, - value: { - requiredScopes: {}, - optionalScopes: { - 'eip155:1': { - accounts: ['eip155:1:0xdeadbeef'], - }, - 'eip155:5': { - accounts: ['eip155:5:0xdeadbeef'], - }, + expect(result).toStrictEqual({ + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'eip155:1': { + accounts: ['eip155:1:0xdeadbeef'], + }, + 'eip155:5': { + accounts: ['eip155:5:0xdeadbeef'], }, - isMultichainOrigin: false, }, + isMultichainOrigin: false, }, - ], - }, + }, + ], }, }); }); - it('grants the CAIP-25 permission approved accounts for the `wallet:eip155` scope (and no approved chainIds) with isMultichainOrigin: false if origin is snapId', async () => { + it('returns the CAIP-25 approval with approved accounts for the `wallet:eip155` scope (and no approved chainIds) with isMultichainOrigin: false if origin is snapId', async () => { jest .spyOn( metamaskController.approvalController, @@ -1432,55 +1423,30 @@ describe('MetaMaskController', () => { }, }); - await metamaskController.requestCaip25Permission('npm:snap', {}); + const result = await metamaskController.requestCaip25Approval( + 'npm:snap', + {}, + ); - expect( - metamaskController.permissionController.grantPermissions, - ).toHaveBeenCalledWith({ - subject: { origin: 'npm:snap' }, - approvedPermissions: { - [Caip25EndowmentPermissionName]: { - caveats: [ - { - type: Caip25CaveatType, - value: { - requiredScopes: {}, - optionalScopes: { - 'wallet:eip155': { - accounts: ['wallet:eip155:0xdeadbeef'], - }, + expect(result).toStrictEqual({ + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: ['wallet:eip155:0xdeadbeef'], }, - isMultichainOrigin: false, }, + isMultichainOrigin: false, }, - ], - }, + }, + ], }, }); }); - - it('returns the result from the ApprovalController', async () => { - jest - .spyOn( - metamaskController.approvalController, - 'addAndShowApprovalRequest', - ) - .mockResolvedValue({ - approvedChainIds: ['0x1', '0x5'], - approvedAccounts: ['0xdeadbeef'], - }); - jest - .spyOn(metamaskController.permissionController, 'grantPermissions') - .mockReturnValue({ - [Caip25EndowmentPermissionName]: { - foo: 'bar', - }, - }); - - expect( - await metamaskController.requestCaip25Permission('test.com', {}), - ).toStrictEqual({ foo: 'bar' }); - }); }); describe('requestApprovalPermittedChainsPermission', () => {