Remove `RestrictedMethod` permission type
- Dominant language
- TypeScript
- Stars
- 413
- Forks
- 308
- Avg merge
- 1d 4h
- Merged PRs (30d)
- 253
Description
Requires: #4239
As elaborated in the permission controller's [architecture documentation](https://github.com/MetaMask/core/blob/3227bd51bc4eca0531a53f4be6468404e6a5f032/packages/permission-controller/ARCHITECTURE.md), its original purpose was to control access to RPC methods. To this end, we enshrined this responsibility within the current implementation of the permission controller in the form of the `RestrictedMethod` permission type. In the almost 2.5 years since [we began using this implementation](https://github.com/MetaMask/metamask-extension/pull/12243), we (special credit to @Gudahtt) have determined that the `RestrictedMethod` permission is misguided, and should be removed.
First, the `RestrictedMethod` permission type adds significant complexity to the implementation of the permission controller. `/restrictedmethod/i` is currently mentioned 46 times in the permission controller, and methods like `executeRestrictedMethod` and imports like `decorateWithCaveats` exist solely to support this permission type.
Second, and more importantly, although the `RestrictedMethod` permission type is supposed to encapsulate the enforcement of RPC method permissions within the permission controller, this goal has never been, and can never be, accomplished. For one thing, RPC method implementations still need to ask the permission controller if an action is permitted or not. Yet, the controller cannot and should not assume responsibility of the entire RPC pipeline. For another thing, we have been making ad hoc mutations of permissions since the days of `rpc-cap`, namely by editing the `eth_accounts` array according to our needs.
In other words, the `RestrictedMethod` permission type exists to uphold a lie at the cost of convoluting one of our most security-critical abstractions. The permission controller should have a single responsibility: at all times maintaining a valid permissions state. To that end, we should remove the `RestrictedMethod` permission type, replacing it entirely with the use-case agnostic `Endowment` permission type, and ultimately get rid of the notion of "permission types" entirely.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with the permission controller architecture documentation and inspect the permission controller references to `/restrictedmethod/i`, including `executeRestrictedMethod` and `decorateWithCaveats`. Review the prerequisite issue #4239 before changing anything. Done means the RestrictedMethod permission type and its supporting concepts are removed and replaced with the use-case-agnostic Endowment type.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- authorization, security
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100