Add shorthand for granting a user all permissions

CHANGELOG.md

@@ -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.

doc/permissions.md

@@ -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>]
 ```
 
@@ -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

okdata/cli/commands/permissions.py

@@ -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
@@ -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."""
@@ -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]
+        )

tests/origocli/commands/permissions_test.py

@@ -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"
@@ -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"
@@ -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"