Skip to content

refactor(permissions): define default permission set#5075

Merged
mversic merged 1 commit intohyperledger-iroha:mainfrom
mversic:default_permission_set
Sep 27, 2024
Merged

refactor(permissions): define default permission set#5075
mversic merged 1 commit intohyperledger-iroha:mainfrom
mversic:default_permission_set

Conversation

@mversic
Copy link
Copy Markdown
Contributor

@mversic mversic commented Sep 13, 2024 •
edited
Loading

Context

  • added CanManagePeers, CanRegisterDomain and CanManageRoles permissions
  • unified CanSetKeyValueXXX + CanRemoveKeyValueXXX = CanModifyXXXMetadata
  • unified CanMintAsset + CanBurnAsset + CanTransferAsset = CanModifyAsset
  • changed FindPermissions so it returns only inherent permissions, not from roles
  • fixed validation of grant/revoke for roles

closes #4206

Solution

  • Describe the approach taken to achieve the objective / resolve the issue.

Review notes (optional)

  • For complex PRs, try to provide some information on how to approach the review more effectively.
  • For example, is there a natural order in which the affected files should be reviewed?

Checklist

  • I've read CONTRIBUTING.md.
  • (optional) I've written unit tests for the code changes.
  • All review comments have been resolved.
  • All CI checks pass.

@mversic mversic force-pushed the default_permission_set branch 3 times, most recently from 8bc8495 to 208c2f0 Compare September 13, 2024 08:42
@github-actions github-actions bot added the api-changes Changes in the API for client libraries label Sep 13, 2024
nxsaken
nxsaken previously approved these changes Sep 13, 2024
View reviewed changes
@mversic mversic force-pushed the default_permission_set branch from 208c2f0 to 371e332 Compare September 13, 2024 11:31
@github-actions github-actions bot added the config-changes Changes in configuration and start up of the Iroha label Sep 13, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 13, 2024

@BAStos525

@mversic mversic force-pushed the default_permission_set branch from 371e332 to 7e9d7a2 Compare September 15, 2024 00:35
@mversic mversic force-pushed the default_permission_set branch 7 times, most recently from 906bb5b to 502a580 Compare September 15, 2024 05:43
@mversic mversic changed the title chore(permissions): define default permission set refactor(permissions): define default permission set Sep 15, 2024
@mversic mversic force-pushed the default_permission_set branch 2 times, most recently from d88ccc4 to cd94357 Compare September 15, 2024 06:07
@mversic mversic force-pushed the default_permission_set branch 4 times, most recently from 817ec0c to 45edafa Compare September 16, 2024 04:58
@mversic mversic assigned nxsaken and unassigned nxsaken and 0x009922 Sep 16, 2024
nxsaken
nxsaken previously approved these changes Sep 18, 2024
View reviewed changes
@nxsaken nxsaken force-pushed the default_permission_set branch from 45edafa to fd4dd86 Compare September 18, 2024 07:57
@Erigara Erigara self-assigned this Sep 19, 2024
Erigara
Erigara reviewed Sep 19, 2024
View reviewed changes
Comment thread crates/iroha/tests/integration/asset.rs Outdated
Comment thread crates/iroha_executor/src/permission.rs Outdated
@mversic mversic force-pushed the default_permission_set branch 5 times, most recently from 7efbc8c to b9445e8 Compare September 25, 2024 13:43
@mversic mversic requested review from Erigara and nxsaken September 25, 2024 13:45
@mversic mversic force-pushed the default_permission_set branch 3 times, most recently from c08f7a2 to 1649d89 Compare September 26, 2024 04:55
nxsaken
nxsaken approved these changes Sep 27, 2024
View reviewed changes
Comment thread crates/iroha_executor_data_model/src/permission.rs
0x009922
0x009922 approved these changes Sep 27, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@0x009922 0x009922 left a comment

Choose a reason for hiding this comment

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

LGTM in general, but I lack the feeling of how much it is/was ergonomic for end users.

And, I guess a proper name for this PR would be "re-define and simplify default permission set", because it was already defined.

@mversic mversic force-pushed the default_permission_set branch from 1649d89 to e8c79db Compare September 27, 2024 12:41
@mversic
refactor(permissions): define default permission set
4b87ad5 
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
@mversic mversic force-pushed the default_permission_set branch from e8c79db to 4b87ad5 Compare September 27, 2024 13:53
@mversic mversic mentioned this pull request Sep 27, 2024
Open
@mversic mversic merged commit 002803f into hyperledger-iroha:main Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@0x009922 0x009922 0x009922 approved these changes

@s8sato s8sato Awaiting requested review from s8sato s8sato is a code owner

@SamHSmith SamHSmith Awaiting requested review from SamHSmith

@outoftardis outoftardis Awaiting requested review from outoftardis outoftardis is a code owner

@DCNick3 DCNick3 Awaiting requested review from DCNick3 DCNick3 is a code owner

@dima74 dima74 Awaiting requested review from dima74 dima74 is a code owner

+2 more reviewers

@nxsaken nxsaken nxsaken approved these changes

@Erigara Erigara Erigara approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@nxsaken nxsaken

@Erigara Erigara

@mversic mversic

Labels

api-changes Changes in the API for client libraries config-changes Changes in configuration and start up of the Iroha

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Define a set of default permission tokens

5 participants

@mversic @DCNick3 @nxsaken @Erigara @0x009922