Dynamic Permissions #111
Dynamic Permissions #111
Conversation
Co-authored-by: Maarten Zuidhoorn <[email protected]>
SIPS/sip-14.md
Outdated
|
|
||
| ## Motivation | ||
|
|
||
| Snaps currently have to request all permissions that they plan to use at install-time. This becomes a problem when a Snap wants to use many permissions as the installation experience suffers and the user has to either accept all permissions requested or deny the installation. This proposal provides an improvement to the experience by letting Snaps request permissions at run-time as long as those permissions are statically defined in the manifest at build-time. |
There was a problem hiding this comment.
With the exception of eth_accounts?
There was a problem hiding this comment.
Yes, that is true. But eth_accounts is kind of an edge-case right now. We should probably require it being put into dynamicPermissions
There was a problem hiding this comment.
That would be a new permission? Right now we just require that they request endowment:ethereum-provider, so it's like a dynamic permission on top of a static one.
There was a problem hiding this comment.
eth_accounts is an edge-case because it is technically a permission that is granted when using certain RPC methods with endowment:ethereum-provider. It is not inherently granted just by having the endowment.
We could choose to enforce that you need to have it as part of dynamicPermissions to grant it via endowment:ethereum-provider. We could also choose to keep it as the edge-case that it is.
There was a problem hiding this comment.
I do wonder if we have created a strange design pattern with the current approach. What if a Snap just wants eth_accounts and is requesting endowment:ethereum-provider just for that, and not using anything else that the endowment offers? Maybe it should be a separate endowment that must be defined in dynamicPermissions and is never in initialPermissions. Assuming this change would be in manifest version 0.2 and would not be backwards compatible.
To be clear I am comfortable punting on this topic, leaving eth_accounts as a special edge case and solving this in a separate effort.
| } | ||
| ``` | ||
|
|
||
| The caveat information passed SHOULD be ignored in the initial implementation of this. Instead of processing the caveats, the implementation SHOULD revoke the entire permission key. We will revisit this at a later time to make it more granular. |
There was a problem hiding this comment.
Does it make sense to specify the caveats if it's ignored? Or is it for future compatibility?
There was a problem hiding this comment.
The idea was that it would be future proof this way. We can make it more granular as we see fit
There was a problem hiding this comment.
I would probably just remove the caveat part. It just makes it more complicated for Snaps developers to use this method, doesn't it? In the future when we revisit this, Snaps developers can just change it to only revoke certain caveats, and we revoke everything if the object is empty or something.
Co-authored-by: Maarten Zuidhoorn <[email protected]>
Adds a SIP that specifies how snaps can utilize dynamic permissions.
Closes MetaMask/snaps#1820