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

BREAKING: Remove namespaced permissions #1337

Merged
merged 8 commits into from
May 17, 2023
Merged

Conversation

rekmarks
Copy link
Member

@rekmarks rekmarks commented May 7, 2023

Description

Removes namespaced permissions from the PermissionController for the reasons described in #1323. As a consequence, also removes the notion of "target keys", which only existed to support namespaced methods. Controller state is not impacted by these changes.

Changes

  • BREAKING: Remove namespaced permissions
    • Namespaced permissions are no longer supported. Consumers should replace namespaced permissions with equivalent caveat-based implementations.
  • BREAKING: Remove targetKey concept
    • The target key/name distinction only existed to support namespaced permissions, which are removed as of this release. Henceforth, permissions only have "names".
    • The targetKey property of permission specifications has been renamed to targetName.

References

Fixes #1323

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation for new or updated code as appropriate (note: this will usually be JSDoc)
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

@rekmarks rekmarks force-pushed the remove-namespaced-permissions branch from 9bf9690 to 1ad521c Compare May 7, 2023 05:22
@rekmarks rekmarks force-pushed the remove-namespaced-permissions branch from a5ef4f3 to 1faa530 Compare May 7, 2023 05:28
@rekmarks rekmarks marked this pull request as ready for review May 7, 2023 05:29
@rekmarks rekmarks requested a review from a team as a code owner May 7, 2023 05:29
Copy link
Member

@FrederikBolding FrederikBolding left a comment

Choose a reason for hiding this comment

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

Overall this looks great - just a few small comments.

@legobeat

This comment was marked as resolved.

@FrederikBolding
Copy link
Member

For the sake of asking the question: As this is breaking API, would it make sense to push a release of the changes of @metamask/permssion-controller since last release?

https://github.com/MetaMask/core/commits/main/packages/permission-controller

I don't see a good reason tbh, there hasn't been any meaningful changes since last release.

Copy link
Member

@FrederikBolding FrederikBolding left a comment

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 e91871a into main May 17, 2023
@rekmarks rekmarks deleted the remove-namespaced-permissions branch May 17, 2023 16:17
brad-decker pushed a commit to dragana8/core that referenced this pull request Jun 1, 2023
Removes namespaced permissions from the `PermissionController` for the reasons described in MetaMask#1323. As a consequence, also removes the notion of "target keys", which only existed to support namespaced methods. Controller state is not impacted by these changes.

---------

Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
Co-authored-by: Frederik Bolding <frederik.bolding@gmail.com>
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
Removes namespaced permissions from the `PermissionController` for the reasons described in #1323. As a consequence, also removes the notion of "target keys", which only existed to support namespaced methods. Controller state is not impacted by these changes.

---------

Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
Co-authored-by: Frederik Bolding <frederik.bolding@gmail.com>
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
Removes namespaced permissions from the `PermissionController` for the reasons described in #1323. As a consequence, also removes the notion of "target keys", which only existed to support namespaced methods. Controller state is not impacted by these changes.

---------

Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
Co-authored-by: Frederik Bolding <frederik.bolding@gmail.com>
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.

Remove namespaced permissions from the PermissionController
3 participants