Skip to content

Commit

Permalink
Jl/caip25 permission migration/move request grant hooks into mmc (#29213
Browse files Browse the repository at this point in the history
)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

* Adds `requestPermittedChainsPermission`,
`requestPermittedChainsPermissionIncremental`, and
`requestCaip25Permission` methods to `MetamaskController`
* Uses these new hooks in `wallet_requestPermissions`,
`eth_requestAccounts`, `wallet_switchEthereumChain`,
`wallet_addEthereumChain`
* Removes `wallet_requestPermissions` update caveat flow
* Make `wallet_requestPermissions` no longer support anything but
`eth_requestAccounts` and `endowment:permittedChains`

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29213?quickstart=1)

## **Related issues**

See:
#27847 (comment)

## **Manual testing steps**

Only user facing change is that `wallet_requestPermissions` should
correctly replace instead update existing permissions again

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------

Co-authored-by: Alex Donesky <[email protected]>
  • Loading branch information
jiexi and adonesky1 authored Dec 17, 2024
1 parent d000725 commit ff3b272
Show file tree
Hide file tree
Showing 12 changed files with 1,388 additions and 1,145 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,8 @@ const addEthereumChain = {
endApprovalFlow: true,
getCurrentChainIdForDomain: true,
getCaveat: true,
requestPermissionApprovalForOrigin: true,
updateCaveat: true,
grantPermissions: true,
requestPermittedChainsPermissionForOrigin: true,
requestPermittedChainsPermissionIncrementalForOrigin: true,
},
};

Expand All @@ -45,9 +44,8 @@ async function addEthereumChainHandler(
endApprovalFlow,
getCurrentChainIdForDomain,
getCaveat,
requestPermissionApprovalForOrigin,
updateCaveat,
grantPermissions,
requestPermittedChainsPermissionForOrigin,
requestPermittedChainsPermissionIncrementalForOrigin,
},
) {
let validParams;
Expand Down Expand Up @@ -199,10 +197,9 @@ async function addEthereumChainHandler(
isAddFlow: true,
setActiveNetwork,
getCaveat,
requestPermissionApprovalForOrigin,
updateCaveat,
endApprovalFlow,
grantPermissions,
requestPermittedChainsPermissionForOrigin,
requestPermittedChainsPermissionIncrementalForOrigin,
});
} else if (approvalFlowId) {
endApprovalFlow({ id: approvalFlowId });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ const createMockedHandler = () => {
getNetworkConfigurationByChainId: jest.fn(),
setActiveNetwork: jest.fn(),
requestUserApproval: jest.fn().mockResolvedValue(123),
requestPermissionApprovalForOrigin: jest.fn(),
getCaveat: jest.fn(),
startApprovalFlow: () => ({ id: 'approvalFlowId' }),
endApprovalFlow: jest.fn(),
Expand All @@ -80,8 +79,8 @@ const createMockedHandler = () => {
defaultRpcEndpointIndex: 0,
rpcEndpoints: [{ networkClientId: 123 }],
}),
updateCaveat: jest.fn(),
grantPermissions: jest.fn(),
requestPermittedChainsPermissionForOrigin: jest.fn(),
requestPermittedChainsPermissionIncrementalForOrigin: jest.fn(),
};
const response = {};
const handler = (request) =>
Expand Down Expand Up @@ -191,11 +190,11 @@ describe('addEthereumChainHandler', () => {
isAddFlow: true,
endApprovalFlow: mocks.endApprovalFlow,
getCaveat: mocks.getCaveat,
requestPermissionApprovalForOrigin:
mocks.requestPermissionApprovalForOrigin,
setActiveNetwork: mocks.setActiveNetwork,
updateCaveat: mocks.updateCaveat,
grantPermissions: mocks.grantPermissions,
requestPermittedChainsPermissionForOrigin:
mocks.requestPermittedChainsPermissionForOrigin,
requestPermittedChainsPermissionIncrementalForOrigin:
mocks.requestPermittedChainsPermissionIncrementalForOrigin,
},
);
});
Expand Down Expand Up @@ -260,11 +259,11 @@ describe('addEthereumChainHandler', () => {
isAddFlow: true,
endApprovalFlow: mocks.endApprovalFlow,
getCaveat: mocks.getCaveat,
requestPermissionApprovalForOrigin:
mocks.requestPermissionApprovalForOrigin,
setActiveNetwork: mocks.setActiveNetwork,
updateCaveat: mocks.updateCaveat,
grantPermissions: mocks.grantPermissions,
requestPermittedChainsPermissionForOrigin:
mocks.requestPermittedChainsPermissionForOrigin,
requestPermittedChainsPermissionIncrementalForOrigin:
mocks.requestPermittedChainsPermissionIncrementalForOrigin,
},
);
});
Expand Down Expand Up @@ -309,11 +308,11 @@ describe('addEthereumChainHandler', () => {
isAddFlow: true,
endApprovalFlow: mocks.endApprovalFlow,
getCaveat: mocks.getCaveat,
requestPermissionApprovalForOrigin:
mocks.requestPermissionApprovalForOrigin,
setActiveNetwork: mocks.setActiveNetwork,
updateCaveat: mocks.updateCaveat,
grantPermissions: mocks.grantPermissions,
requestPermittedChainsPermissionForOrigin:
mocks.requestPermittedChainsPermissionForOrigin,
requestPermittedChainsPermissionIncrementalForOrigin:
mocks.requestPermittedChainsPermissionIncrementalForOrigin,
},
);
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
import { errorCodes, rpcErrors, providerErrors } from '@metamask/rpc-errors';
import { errorCodes, rpcErrors } from '@metamask/rpc-errors';
import {
Caip25CaveatType,
Caip25EndowmentPermissionName,
getPermittedEthChainIds,
addPermittedEthChainId,
} from '@metamask/multichain';
import {
isPrefixedFormattedHexString,
isSafeChainId,
} from '../../../../../shared/modules/network.utils';
import { UNKNOWN_TICKER_SYMBOL } from '../../../../../shared/constants/app';
import { getValidUrl } from '../../util';
import { CaveatTypes } from '../../../../../shared/constants/permissions';
import { PermissionNames } from '../../../controllers/permissions';

export function validateChainId(chainId) {
const lowercasedChainId =
Expand Down Expand Up @@ -176,9 +173,8 @@ export function validateAddEthereumChainParams(params) {
* @param {Function} hooks.setActiveNetwork - The callback to change the current network for the origin.
* @param {Function} hooks.endApprovalFlow - The optional callback to end the approval flow when approvalFlowId is provided.
* @param {Function} hooks.getCaveat - The callback to get the CAIP-25 caveat for the origin.
* @param {Function} hooks.requestPermissionApprovalForOrigin - The callback to prompt the user for permission approval.
* @param {Function} hooks.updateCaveat - The callback to update the CAIP-25 caveat value.
* @param {Function} hooks.grantPermissions - The callback to grant a CAIP-25 permission when one does not already exist.
* @param {Function} hooks.requestPermittedChainsPermissionForOrigin - The callback to request a new permittedChains-equivalent CAIP-25 permission.
* @param {Function} hooks.requestPermittedChainsPermissionIncrementalForOrigin - The callback to add a new chain to the permittedChains-equivalent CAIP-25 permission.
* @returns a null response on success or an error if user rejects an approval when isAddFlow is false or on unexpected errors.
*/
export async function switchChain(
Expand All @@ -192,9 +188,8 @@ export async function switchChain(
setActiveNetwork,
endApprovalFlow,
getCaveat,
requestPermissionApprovalForOrigin,
updateCaveat,
grantPermissions,
requestPermittedChainsPermissionForOrigin,
requestPermittedChainsPermissionIncrementalForOrigin,
},
) {
try {
Expand All @@ -207,68 +202,15 @@ export async function switchChain(
const ethChainIds = getPermittedEthChainIds(caip25Caveat.value);

if (!ethChainIds.includes(chainId)) {
if (caip25Caveat.value.isMultichainOrigin) {
return end(
providerErrors.unauthorized(
`Cannot switch to or add permissions for chainId '${chainId}' because permissions were granted over the Multichain API.`,
),
);
}

if (!isAddFlow) {
await requestPermissionApprovalForOrigin({
[PermissionNames.permittedChains]: {
caveats: [
{
type: CaveatTypes.restrictNetworkSwitching,
value: [chainId],
},
],
},
});
}

const updatedCaveatValue = addPermittedEthChainId(
caip25Caveat.value,
await requestPermittedChainsPermissionIncrementalForOrigin({
chainId,
);

updateCaveat(
Caip25EndowmentPermissionName,
Caip25CaveatType,
updatedCaveatValue,
);
}
} else {
if (!isAddFlow) {
await requestPermissionApprovalForOrigin({
[PermissionNames.permittedChains]: {
caveats: [
{
type: CaveatTypes.restrictNetworkSwitching,
value: [chainId],
},
],
},
autoApprove: isAddFlow,
});
}

let caveatValue = {
requiredScopes: {},
optionalScopes: {},
isMultichainOrigin: false,
};
caveatValue = addPermittedEthChainId(caveatValue, chainId);

grantPermissions({
[Caip25EndowmentPermissionName]: {
caveats: [
{
type: Caip25CaveatType,
value: caveatValue,
},
],
},
} else {
await requestPermittedChainsPermissionForOrigin({
chainId,
autoApprove: isAddFlow,
});
}

Expand Down
Loading

0 comments on commit ff3b272

Please sign in to comment.