Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic Permissions #111

Merged
merged 8 commits into from
Dec 6, 2023
Merged
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 86 additions & 0 deletions SIPS/sip-14.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
---
sip: 14
title: Dynamic Permissions
status: Draft
discussions-to:
author: Frederik Bolding (@frederikbolding)
created: 2023-10-19
---

## Abstract

This SIP proposes changes to the Snap manifest and new RPC methods that would allow Snap developers to request additional permissions dynamically at runtime. This proposal outlines some of the details around this feature.
FrederikBolding marked this conversation as resolved.
Show resolved Hide resolved

## 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the exception of eth_accounts?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is true. But eth_accounts is kind of an edge-case right now. We should probably require it being put into dynamicPermissions

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@Montoya Montoya Oct 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

FrederikBolding marked this conversation as resolved.
Show resolved Hide resolved

## Specification

> Formal specifications are written in Typescript. Usage of `CAIP-N` specifications, where `N` is a number, are references to [Chain Agnostic Improvement Proposals](https://github.com/ChainAgnostic/CAIPs).
FrederikBolding marked this conversation as resolved.
Show resolved Hide resolved

### Language

The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
"SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and
"OPTIONAL" written in uppercase in this document are to be interpreted as described in [RFC 2119](https://www.ietf.org/rfc/rfc2119.txt)

### Snap Manifest

This SIP adds a new field to the Snap manifest called `dynamicPermissions`.
This field can be used in tandem with the existing `initialPermissions`, but keep in mind that permissions in this field are not granted by installation: They MUST be requested when needed. The field follows the same format as `initialPermissions`.
FrederikBolding marked this conversation as resolved.
Show resolved Hide resolved

The new field can be specified as follows in a `snap.manifest.json` file:

```json
{
"initialPermissions": {
"endowment:transaction-insight": {}
},
"dynamicPermissions": {
FrederikBolding marked this conversation as resolved.
Show resolved Hide resolved
"snap_dialog": {},
"snap_getBip44Entropy": [
{
"coinType": 1
},
{
"coinType": 3
Mrtenz marked this conversation as resolved.
Show resolved Hide resolved
}
]
}
}
```

### Permission caveats and merging
FrederikBolding marked this conversation as resolved.
Show resolved Hide resolved

In this initial version, duplicated permissions in `initialPermissions` and `dynamicPermissions` MUST NOT be allowed. A permission MUST only be able to exist in one of the manifest fields.
FrederikBolding marked this conversation as resolved.
Show resolved Hide resolved

Furthermore, permissions specified in `dynamicPermissions` MUST contain the caveats that will be requested at runtime and the permission request MUST fully match the caveats specified in the manifest.
Mrtenz marked this conversation as resolved.
Show resolved Hide resolved

This MAY change in a future SIP.

### RPC Methods

This SIP proposes the following RPC methods to manage the dynamic permissions:

#### snap_requestPermissions
FrederikBolding marked this conversation as resolved.
Show resolved Hide resolved

This RPC method SHOULD function as a subset of the existing `wallet_requestPermissions` RPC method and take the same parameters and have the same return value.
FrederikBolding marked this conversation as resolved.
Show resolved Hide resolved

This RPC method MUST prompt the user to get consent for any requested permissions and MUST validate that the requested permissions are specified in the manifest before continuing its execution (including caveats matching).
FrederikBolding marked this conversation as resolved.
Show resolved Hide resolved

#### snap_getPermissions
FrederikBolding marked this conversation as resolved.
Show resolved Hide resolved

This RPC method SHOULD be an alias for `wallet_getPermissions`, MAY be used by the snap for verifying whether it already has the permissions needed for operating. The return value and parameters SHOULD match the existing specification.
FrederikBolding marked this conversation as resolved.
Show resolved Hide resolved

#### snap_revokePermissions
FrederikBolding marked this conversation as resolved.
Show resolved Hide resolved

The RPC method parameters and return value are TBD.

Note: This RPC method does not currently have a `wallet_` counterpart. Coordinate with dapp API team as they may be shipping one.
FrederikBolding marked this conversation as resolved.
Show resolved Hide resolved

This RPC method MUST validate that the permissions requested to be revoked does not contain or overlap with the `initialPermissions`.
FrederikBolding marked this conversation as resolved.
Show resolved Hide resolved

## Copyright

Copyright and related rights waived via [CC0](../LICENSE).
Loading