Skip to content

Add "Endowment" permissions - #152

Merged
rekmarks merged 11 commits into
mainfrom
endowment-permission-type
Dec 14, 2021
Merged

rekmarks merged 11 commits into
mainfrom
endowment-permission-type

Conversation

@rekmarks

@rekmarks rekmarks commented Dec 9, 2021 •

Copy link
Copy Markdown
Contributor

Introduces the notion of permission types to the PermissionController, and adds "Endowment" permissions. Invoking these permissions required a new method and action, PermissionController.getEndowments and PermissionController:getEndowments, respectively. See the updated PermissionController README and linked issue for details.

Here is the description of endowment permissions, from said README:

The name "endowment" comes from the endowments that you may provide to a Secure EcmaScript (SES) Compartment when it is constructed.
SES endowments are simply names that appear in the compartment's global scope.
In the context of the PermissionController, endowments are simply "things" that subjects should not be able to access by default.
They could be the names of endowments that are to be made available to a particular SES Compartment, but they could also be any JavaScript value, and it is the host's responsibility to make sense of them.

At present, endowment permissions may not have any caveats, but caveat support may be added in the future.

Closes #137

@rekmarks rekmarks linked an issue Dec 9, 2021 that may be closed by this pull request
@rekmarks
rekmarks force-pushed the endowment-permission-type branch 2 times, most recently from 7ea42f4 to a5cc8f0 Compare December 10, 2021 01:04
@rekmarks
rekmarks marked this pull request as ready for review December 10, 2021 01:07
@rekmarks rekmarks closed this Dec 10, 2021
@rekmarks rekmarks reopened this Dec 10, 2021
shanejonas
shanejonas previously approved these changes Dec 10, 2021
shanejonas
shanejonas previously approved these changes Dec 10, 2021
@rekmarks
rekmarks force-pushed the endowment-permission-type branch from 84c7392 to 1e35fe9 Compare December 11, 2021 05:00
@rekmarks
rekmarks requested a review from shanejonas December 11, 2021 05:05
ritave
ritave previously approved these changes Dec 13, 2021

@ritave ritave left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@@ -126,7 +143,8 @@ const permissionSpecifications = {

// This is a namespaced restricted method.
wallet_getSecret_*: {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

wallet_getSecret_* => 'wallet_getSecret_*'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nice catch: 8651ae5

},

// This is an endowment.
secretEndowment: {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should it be possible for the endowment to be unrestricted? That would mean that all endowments ever used would be specified in one place only - in PermissionController.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, at the time of writing, I can't think of any unrestricted endowment that would have to be specified. In the context of SES, for instance, an unrestricted endowment would just be available by default.

Comment thread packages/controllers/src/permissions/Permission.ts
Comment thread packages/controllers/src/permissions/Permission.ts Outdated
Comment thread packages/controllers/src/permissions/Permission.ts
Comment thread packages/controllers/src/permissions/Permission.ts Outdated
Comment on lines +218 to +219
// Getting endowments internally is the only option, since the host has to apply
// them in some way external to the permission system.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: I found this comment a little confusing. I assume that "getting endowments internally" means getting them from within our codebase rather than via RPC method. It makes sense to me that endowments are internal, because... well... we only have a JSON-RPC API, and that doesn't include endowments as a concept (at least not directly).

But I don't know what "the host has to apply them in some way external to the permission system" means, or what its relevance is to whether endowments are exposed via RPC or not.

Comment thread packages/controllers/src/permissions/PermissionController.ts Outdated
* caveat specifications available to the controller.
*/
export type PermissionControllerOptions<
ControllerPermissionSpecification extends ValidPermissionSpecification<PermissionSpecificationConstraint>,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why was ValidPermissionSpecification removed here? Not disagreeing, just the reason wasn't clear to me.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, ValidPermissionSpecification<PermissionSpecificationConstraint> evaluates to never with the new types when used as the right-hand operand of an extends expression. The other uses of ValidPermissionSpecification still work, and it didn't really add anything here, so removing it seemed like best move.

@Gudahtt Gudahtt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@rekmarks
rekmarks merged commit 7889940 into main Dec 14, 2021
@rekmarks
rekmarks deleted the endowment-permission-type branch December 14, 2021 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PermissionController: Add "endowment" permission type

4 participants