Skip to content

Commit

Permalink
Add shorthand for granting a user all permissions
Browse files Browse the repository at this point in the history
Add a shorthand for granting a user all permissions on a resource by
not specifying the `scope` parameter to the `permissions add` command,
similar to the way `permissions rm` works.
  • Loading branch information
simenheg committed Mar 5, 2024
1 parent 527371e commit 0f34544
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 26 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
## ?.?.? - Unreleased

* Added support for creating CSV->Delta dataset pipelines.
* Added a shorthand for granting a user all permissions on a resource by not
specifying the `scope` parameter to the `permissions add` command, similar to
the way `permissions rm` works.
* Fixed extraneous newline printing in table output.
* Improved error messages in the `datasets ls` and `datasets cp` commands when a
given dataset doesn't exist.
Expand Down
6 changes: 3 additions & 3 deletions doc/permissions.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ This will list every permission associated with the dataset `my-dataset`.
The commands for granting and revoking permissions to and from users, are:

```bash
okdata permissions add <resource_name> <user> <scope>
okdata permissions add <resource_name> <user> [<scope>]
okdata permissions rm <resource_name> <user> [<scope>]
```

Expand All @@ -105,8 +105,8 @@ And to revoke that same permission:
okdata permissions rm okdata:dataset:my-dataset janedoe okdata:dataset:read
```

The `scope` parameter is optional for `rm`. When omitted, all permissions for
the user on the given resource are revoked.
The `scope` parameter is optional in both commands. When omitted, all
permissions are granted or revoked for the user on the given resource.

Both commands support additional `--team` and `--client` flags, which are used
when the given user ID belongs to a team or a machine user, instead of a person
Expand Down
29 changes: 15 additions & 14 deletions okdata/cli/commands/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@ class PermissionsCommand(BaseCommand):
Usage:
okdata permissions ls [<resource_name>] [options]
okdata permissions add <resource_name> <user> <scope> [--team | --client] [options]
okdata permissions add <resource_name> <user> [<scope>] [--team | --client] [options]
okdata permissions rm <resource_name> <user> [<scope>] [--team | --client] [options]
Examples:
okdata permissions ls
okdata permissions ls okdata:dataset:my-dataset
okdata permissions add okdata:dataset:my-dataset janedoe
okdata permissions add okdata:dataset:my-dataset janedoe okdata:dataset:read
okdata permissions rm okdata:dataset:my-dataset janedoe
okdata permissions rm okdata:dataset:my-dataset janedoe okdata:dataset:write
Expand Down Expand Up @@ -57,10 +58,10 @@ def handler(self):
]
if self.cmd("add"):
self.add_user(resource_name, user_class(user), scope)
self.print("Granted {} on '{}' for {} '{}'".format(*fmt_args))
self.print("Granted {} on '{}' to {} '{}'".format(*fmt_args))
else:
self.remove_user(resource_name, user_class(user), scope)
self.print("Revoked {} on '{}' for {} '{}'".format(*fmt_args))
self.print("Revoked {} on '{}' from {} '{}'".format(*fmt_args))

def list_my_permissions(self):
"""Print all permissions for the current user."""
Expand Down Expand Up @@ -99,21 +100,21 @@ def list_permissions(self, resource_name):
out.add_rows(sorted(results, key=itemgetter("user_type", "user_name")))
self.print(f"Permissions for '{resource_name}'", out)

def add_user(self, resource_name, user, scope):
"""Grant `scope` on `resource_name` to `user`."""
self.client.update_permission(resource_name, scope, add_users=[user])
def add_user(self, resource_name, user, scope=None):
"""Grant `scope` on `resource_name` to `user`.
When `scope` is unspecified, grant every permission on `resource_name`.
"""
return self.client.update_permission(
resource_name, scope or "__all__", add_users=[user]
)

def remove_user(self, resource_name, user, scope=None):
"""Revoke `scope` on `resource_name` for `user`.
When `scope` is unspecified, revoke every permission on `resource_name`
for `user`.
"""
if scope:
self.client.update_permission(resource_name, scope, remove_users=[user])
else:
for permission in self.client.get_permissions(resource_name):
if user.user_id in permission[f"{user.user_type}s"]:
self.client.update_permission(
resource_name, permission["scope"], remove_users=[user]
)
return self.client.update_permission(
resource_name, scope or "__all__", remove_users=[user]
)
12 changes: 3 additions & 9 deletions tests/origocli/commands/permissions_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,19 +110,13 @@ def test_remove_user(mocker):
cmd = make_cmd(mocker, "rm", "okdata:dataset:my-dataset", "foo")
cmd.handler()
cmd.client.update_permission.assert_called_once_with(
"okdata:dataset:my-dataset", "okdata:dataset:read", remove_users=[ANY]
"okdata:dataset:my-dataset", "__all__", remove_users=[ANY]
)
user = cmd.client.update_permission.mock_calls[0][2]["remove_users"][0]
assert user.user_id == "foo"
assert user.user_type == "user"


def test_remove_user_nothing_to_remove(mocker):
cmd = make_cmd(mocker, "rm", "okdata:dataset:my-dataset", "foobar")
cmd.handler()
cmd.client.update_permission.assert_not_called()


def test_remove_user_scope(mocker):
cmd = make_cmd(
mocker, "rm", "okdata:dataset:my-dataset", "foo", "okdata:dataset:write"
Expand All @@ -140,7 +134,7 @@ def test_remove_client(mocker):
cmd = make_cmd(mocker, "rm", "okdata:dataset:my-dataset", "bar", "--client")
cmd.handler()
cmd.client.update_permission.assert_called_once_with(
"okdata:dataset:my-dataset", "okdata:dataset:read", remove_users=[ANY]
"okdata:dataset:my-dataset", "__all__", remove_users=[ANY]
)
user = cmd.client.update_permission.mock_calls[0][2]["remove_users"][0]
assert user.user_id == "bar"
Expand All @@ -151,7 +145,7 @@ def test_remove_team(mocker):
cmd = make_cmd(mocker, "rm", "okdata:dataset:my-dataset", "baz", "--team")
cmd.handler()
cmd.client.update_permission.assert_called_once_with(
"okdata:dataset:my-dataset", "okdata:dataset:read", remove_users=[ANY]
"okdata:dataset:my-dataset", "__all__", remove_users=[ANY]
)
user = cmd.client.update_permission.mock_calls[0][2]["remove_users"][0]
assert user.user_id == "baz"
Expand Down

0 comments on commit 0f34544

Please sign in to comment.