core icon indicating copy to clipboard operation
core copied to clipboard

Add caveat merging and "incremental" permission requests

Open rekmarks opened this issue 1 year ago • 0 comments

[!NOTE] The source of truth for how caveat merging should work has moved to #4222.

Due to the introduction of "dynamic" (i.e. "requested / modified at runtime") permissions in Snaps, we need to introduce a notion of "merging" to the PermissionController's caveat abstraction. Let's consider a simplified motivating use case:

A subject has a caveat with a value of { foo: 'bar' }. At runtime, the subject requests to change the caveat to a value of { foo: 'baz' }. Since this equates to a change in the subject's authority, we must display a confirmation to the user. Currently, we have two options for how to handle it:

  1. Overwrite the existing permission (i.e. treat it as a brand new request for the permission)
  2. Manually handle the merge in the client, and then either:
    1. call PermissionController:updateCaveat to update the caveat directly.
    2. or, call PermissionController:revokePermissions followed by PermissionController:grantPermissions in order to replace the existing permission entirely.

Unfortunately, neither of these options work:

  • 1 is currently the only option afforded by our API—via wallet_requestPermissions—which will overwrite all existing permissions of the requesting subject and replace them with the new ones. This is a non-starter for the dynamic permissions use case, because we need to represent an incremental change in authority, rather than a complete rewrite of the subject's authority. It also places the burden of merging existing authorities on the caller, which is not their job.
  • 2.i is what we do for eth_accounts today, while we employ 2.ii during e.g. snap updates when their permissions differ, but we do not expose this capability via our API. Moreover, the permissions diff is calculated manually in an ad hoc fashion, which is a brittle approach.

What we want is to let the subject call something like wallet_requestIncrementalPermissions, enabling consumers to make incremental permission requests without worrying about how to calculate the diff. MetaMask's job is to calculate the diff and present a confirmation to the user explaining what the change in authority is. Thus, in total, "caveat merging" consists of these steps:

  1. Calculating the permission diff
  2. Presenting the user with a confirmation that explains the diff
  3. Applying the diff to PermissionController state

If we are to ship something like the proposed RPC method, we should establish the pattern for how to do caveat merging in our codebase. We could decide to do caveat merging "manually" for each permission and just make use of updateCaveat, but ideally merge operations should receive the same level of scrutiny as the implementation of a caveat or permission specification. This is an argument for associating the merge operation with the caveat specifications themselves, perhaps by means of a property like merger?: CaveatMerger<unknown>.

Appendix

Here follows a draft specification of wallet_requestIncrementalPermissions in set theory notation:

Note that, in software engineering terms, we are performing a "right-biased union", which is undefinable in set theory. Consider that: A: { foo: 'bar' } B: { foo: 'baz' } Should yield: C: { foo: 'baz' } In set theory, you'd end up with C = [A, B], whereas we end up with C = [B].

Nevertheless, I find it helpful to consider the algorithm in the formalisms of set theory. Just recall that the below specification does not account for merging objects or non-primitive arrays.

  • The subject has a set of permissions A
  • The subject requests incremental permissions B, where A ∩ B = ∅ (i.e. A and B are disjoint)
    • If A ∩ B ≠ ∅, any permissions in the set A ∩ B are preserved unmodified in C.
    • If B = A, the request is a no-op and returns A.
  • If the user approves the request, the subject ends up with the set of permissions C, where C = A ∪ B and C ⊇ B

rekmarks avatar Apr 15 '24 18:04 rekmarks