This is an automated email from the ASF dual-hosted git repository.
jmclean pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 375116a60a [#5570] improve(CLI): Add the ability to revoke all roles
or groups in the Gravitino CLI. (#6240)
375116a60a is described below
commit 375116a60a8a5537587c1902cfb3859759429846
Author: Lord of Abyss <103809695+abyss-l...@users.noreply.github.com>
AuthorDate: Thu Jan 16 11:05:49 2025 +0800
[#5570] improve(CLI): Add the ability to revoke all roles or groups in the
Gravitino CLI. (#6240)
### What changes were proposed in this pull request?
Add ability to revoke all roles from a user or group in the Gravitino
CLI.
1. Add logic to handle the `--all` flag in
`UserCommandHandler#handleRevokeCommand`;
2. Add logic to handle the `--all `flag in
`GroupCommandHandler#handleRevokeCommand`;
3. Add new `RemoveAllRoles` command to handle user revoke --all and
group revoke `--all`;
4. Add unit tests;
### Why are the changes needed?
Fix: #5570
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
UT + local test.
User test.
```bash
# step1: grant roles to testRole
gcli user grant -m demo_metalake --user testRole --role roleA roleB roleC -I
# roleA added to testRole
# roleB added to testRole
# roleC added to testRole
# Add roles roleA, roleB, roleC to user testRole
# step2: get details of testRole
gcli user details -m demo_metalake --user testRole -I
# roleA,roleC,roleB
# step3: revoke all roles from testRole
gcli user revoke -m demo_metalake --user testRole -i --all
# Remove all roles from user testRole
# step4: get details of testRole
gcli user details -m demo_metalake --user testRole -I
# The user has no roles.
# step5: list exists roles
gcli role list -m demo_metalake -I
# admin,roleA,roleB,roleC
```
Group test.
```bash
# step1: grant roles to test_group
gcli group grant -m demo_metalake --group test_group --role roleA roleB
roleC -I
# roleA added to test_group
# roleB added to test_group
# roleC added to test_group
# Grant roles roleA, roleB, roleC to group test_group
# step2: get details of test_group
gcli group details -m demo_metalake --group test_group -I
# admin,roleA,roleC,roleB
# step3: revoke all roles from test_group
gcli group revoke -m demo_metalake --group test_group -i --all
# Remove all roles from group test_group
# step4: get details of test_group
gcli group details -m demo_metalake --group test_group -I
# The group has no roles.
# step5: list exists roles
gcli role list -m demo_metalake -I
# admin,roleA,roleB,roleC
```
---------
Co-authored-by: Chun-Hao Liu <liuchun...@users.noreply.github.com>
Co-authored-by: yangyang zhong <35210666+hdyg...@users.noreply.github.com>
Co-authored-by: Xiaojian Sun <sunxiaojian...@163.com>
Co-authored-by: roryqi <ror...@apache.org>
Co-authored-by: TengYao Chi <kiting...@gmail.com>
Co-authored-by: Qi Yu <y...@datastrato.com>
---
.../org/apache/gravitino/cli/GravitinoOptions.java | 2 +-
.../apache/gravitino/cli/GroupCommandHandler.java | 17 +++-
.../apache/gravitino/cli/TestableCommandLine.java | 6 ++
.../apache/gravitino/cli/UserCommandHandler.java | 19 +++--
.../gravitino/cli/commands/RemoveAllRoles.java | 98 ++++++++++++++++++++++
.../cli/commands/RemoveRoleFromGroup.java | 4 +-
clients/cli/src/main/resources/group_help.txt | 6 ++
clients/cli/src/main/resources/user_help.txt | 5 ++
.../apache/gravitino/cli/TestGroupCommands.java | 27 ++++++
.../org/apache/gravitino/cli/TestUserCommands.java | 22 +++++
docs/cli.md | 12 +++
11 files changed, 206 insertions(+), 12 deletions(-)
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java
index aaeb8f0184..47f9914233 100644
--- a/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java
+++ b/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java
@@ -109,7 +109,7 @@ public class GravitinoOptions {
options.addOption(createArgOption(DEFAULT, "default column value"));
options.addOption(createSimpleOption("o", OWNER, "display entity owner"));
options.addOption(createArgOption(COLUMNFILE, "CSV file describing
columns"));
- options.addOption(createSimpleOption(null, ALL, "all operation for
--enable"));
+ options.addOption(createSimpleOption(null, ALL, "on all entities"));
// model options
options.addOption(createArgOption(null, URI, "model version artifact"));
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/GroupCommandHandler.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/GroupCommandHandler.java
index e336003e6b..7542b17974 100644
---
a/clients/cli/src/main/java/org/apache/gravitino/cli/GroupCommandHandler.java
+++
b/clients/cli/src/main/java/org/apache/gravitino/cli/GroupCommandHandler.java
@@ -130,14 +130,23 @@ public class GroupCommandHandler extends CommandHandler {
/** Handles the "REVOKE" command. */
private void handleRevokeCommand() {
- String[] revokeRoles = line.getOptionValues(GravitinoOptions.ROLE);
- for (String role : revokeRoles) {
+ boolean revokeAll = line.hasOption(GravitinoOptions.ALL);
+ if (revokeAll) {
gravitinoCommandLine
- .newRemoveRoleFromGroup(url, ignore, metalake, group, role)
+ .newRemoveAllRoles(url, ignore, metalake, group,
CommandEntities.GROUP)
.validate()
.handle();
+ System.out.printf("Removed all roles from group %s%n", group);
+ } else {
+ String[] revokeRoles = line.getOptionValues(GravitinoOptions.ROLE);
+ for (String role : revokeRoles) {
+ gravitinoCommandLine
+ .newRemoveRoleFromGroup(url, ignore, metalake, group, role)
+ .validate()
+ .handle();
+ }
+ System.out.printf("Removed roles %s from group %s%n",
COMMA_JOINER.join(revokeRoles), group);
}
- System.out.printf("Remove roles %s from group %s%n",
COMMA_JOINER.join(revokeRoles), group);
}
/** Handles the "GRANT" command. */
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java
index d9e3b9288e..498a0060cb 100644
---
a/clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java
+++
b/clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java
@@ -86,6 +86,7 @@ import org.apache.gravitino.cli.commands.ModelAudit;
import org.apache.gravitino.cli.commands.ModelDetails;
import org.apache.gravitino.cli.commands.OwnerDetails;
import org.apache.gravitino.cli.commands.RegisterModel;
+import org.apache.gravitino.cli.commands.RemoveAllRoles;
import org.apache.gravitino.cli.commands.RemoveAllTags;
import org.apache.gravitino.cli.commands.RemoveCatalogProperty;
import org.apache.gravitino.cli.commands.RemoveFilesetProperty;
@@ -457,6 +458,11 @@ public class TestableCommandLine {
return new RemoveRoleFromGroup(url, ignore, metalake, group, role);
}
+ protected RemoveAllRoles newRemoveAllRoles(
+ String url, boolean ignore, String metalake, String entity, String
entityType) {
+ return new RemoveAllRoles(url, ignore, metalake, entity, entityType);
+ }
+
protected AddRoleToGroup newAddRoleToGroup(
String url, boolean ignore, String metalake, String group, String role) {
return new AddRoleToGroup(url, ignore, metalake, group, role);
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/UserCommandHandler.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/UserCommandHandler.java
index 9a8374ec34..23698aa5cf 100644
--- a/clients/cli/src/main/java/org/apache/gravitino/cli/UserCommandHandler.java
+++ b/clients/cli/src/main/java/org/apache/gravitino/cli/UserCommandHandler.java
@@ -150,14 +150,23 @@ public class UserCommandHandler extends CommandHandler {
/** Handles the "REVOKE" command. */
private void handleRevokeCommand() {
- String[] revokeRoles = line.getOptionValues(GravitinoOptions.ROLE);
- for (String role : revokeRoles) {
- this.gravitinoCommandLine
- .newRemoveRoleFromUser(this.url, this.ignore, this.metalake, user,
role)
+ boolean removeAll = line.hasOption(GravitinoOptions.ALL);
+ if (removeAll) {
+ gravitinoCommandLine
+ .newRemoveAllRoles(url, ignore, metalake, user, CommandEntities.USER)
.validate()
.handle();
+ System.out.printf("Removed all roles from user %s%n", user);
+ } else {
+ String[] revokeRoles = line.getOptionValues(GravitinoOptions.ROLE);
+ for (String role : revokeRoles) {
+ this.gravitinoCommandLine
+ .newRemoveRoleFromUser(this.url, this.ignore, this.metalake, user,
role)
+ .validate()
+ .handle();
+ }
+ System.out.printf("Removed roles %s from user %s%n",
COMMA_JOINER.join(revokeRoles), user);
}
- System.out.printf("Remove roles %s from user %s%n",
COMMA_JOINER.join(revokeRoles), user);
}
/** Handles the "GRANT" command. */
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveAllRoles.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveAllRoles.java
new file mode 100644
index 0000000000..eb7b354223
--- /dev/null
+++
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveAllRoles.java
@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.gravitino.cli.commands;
+
+import java.util.List;
+import org.apache.gravitino.authorization.Group;
+import org.apache.gravitino.authorization.User;
+import org.apache.gravitino.cli.CommandEntities;
+import org.apache.gravitino.cli.ErrorMessages;
+import org.apache.gravitino.client.GravitinoClient;
+import org.apache.gravitino.exceptions.NoSuchGroupException;
+import org.apache.gravitino.exceptions.NoSuchMetalakeException;
+import org.apache.gravitino.exceptions.NoSuchUserException;
+
+/** Removes all roles from a group or user. */
+public class RemoveAllRoles extends Command {
+ protected final String metalake;
+ protected final String entity;
+ protected final String entityType;
+
+ /**
+ * Removes all roles from a group or user.
+ *
+ * @param url The URL of the Gravitino server.
+ * @param ignoreVersions If true don't check the client/server versions
match.
+ * @param metalake The name of the metalake.
+ * @param entity the name of the group or user.
+ * @param entityType The type of the entity (group or user).
+ */
+ public RemoveAllRoles(
+ String url, boolean ignoreVersions, String metalake, String entity,
String entityType) {
+ super(url, ignoreVersions);
+ this.metalake = metalake;
+ this.entity = entity;
+ this.entityType = entityType;
+ }
+
+ /** Removes all roles from a group or user. */
+ @Override
+ public void handle() {
+ if (CommandEntities.GROUP.equals(entityType)) {
+ revokeAllRolesFromGroup();
+ } else {
+ revokeAllRolesFromUser();
+ }
+ }
+
+ /** Removes all roles from a group. */
+ private void revokeAllRolesFromGroup() {
+ List<String> roles;
+ try {
+ GravitinoClient client = buildClient(metalake);
+ Group group = client.getGroup(entity);
+ roles = group.roles();
+ client.revokeRolesFromGroup(roles, entity);
+ } catch (NoSuchMetalakeException e) {
+ exitWithError(ErrorMessages.UNKNOWN_METALAKE);
+ } catch (NoSuchGroupException e) {
+ exitWithError(ErrorMessages.UNKNOWN_GROUP);
+ } catch (Exception e) {
+ exitWithError(e.getMessage());
+ }
+ }
+
+ /** Removes all roles from a user. */
+ private void revokeAllRolesFromUser() {
+ List<String> roles;
+ try {
+ GravitinoClient client = buildClient(metalake);
+ User user = client.getUser(entity);
+ roles = user.roles();
+ client.revokeRolesFromUser(roles, entity);
+ } catch (NoSuchMetalakeException e) {
+ exitWithError(ErrorMessages.UNKNOWN_METALAKE);
+ } catch (NoSuchUserException e) {
+ exitWithError(ErrorMessages.UNKNOWN_USER);
+ } catch (Exception e) {
+ exitWithError(e.getMessage());
+ }
+ }
+}
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveRoleFromGroup.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveRoleFromGroup.java
index dd7ccc0e79..7f2f7e2b2c 100644
---
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveRoleFromGroup.java
+++
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveRoleFromGroup.java
@@ -34,7 +34,7 @@ public class RemoveRoleFromGroup extends Command {
protected String role;
/**
- * Removes a role from a group.
+ * Remove a role from a group.
*
* @param url The URL of the Gravitino server.
* @param ignoreVersions If true don't check the client/server versions
match.
@@ -50,7 +50,7 @@ public class RemoveRoleFromGroup extends Command {
this.role = role;
}
- /** Adds a role to a group. */
+ /** Remove a role from a group. */
@Override
public void handle() {
try {
diff --git a/clients/cli/src/main/resources/group_help.txt
b/clients/cli/src/main/resources/group_help.txt
index cbac95747a..a5badc8226 100644
--- a/clients/cli/src/main/resources/group_help.txt
+++ b/clients/cli/src/main/resources/group_help.txt
@@ -18,3 +18,9 @@ gcli group details --group new_group --audit
Delete a group
gcli group delete --group new_group
+
+Revoke a role from a group
+gcli group revoke --group new_group --role admin
+
+Revoke all roles from a group
+gcli group revoke --group new_group --all
\ No newline at end of file
diff --git a/clients/cli/src/main/resources/user_help.txt
b/clients/cli/src/main/resources/user_help.txt
index bd08bea59f..00add299d2 100644
--- a/clients/cli/src/main/resources/user_help.txt
+++ b/clients/cli/src/main/resources/user_help.txt
@@ -19,3 +19,8 @@ gcli user details --user new_user --audit
Delete a user
gcli user delete --user new_user
+Revoke a user's role
+gcli user revoke --user new_user --role admin
+
+Revoke all roles from a user
+gcli user revoke --user new_user --all
\ No newline at end of file
diff --git
a/clients/cli/src/test/java/org/apache/gravitino/cli/TestGroupCommands.java
b/clients/cli/src/test/java/org/apache/gravitino/cli/TestGroupCommands.java
index ce7a895682..cae8402d8b 100644
--- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestGroupCommands.java
+++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestGroupCommands.java
@@ -39,6 +39,7 @@ import org.apache.gravitino.cli.commands.DeleteGroup;
import org.apache.gravitino.cli.commands.GroupAudit;
import org.apache.gravitino.cli.commands.GroupDetails;
import org.apache.gravitino.cli.commands.ListGroups;
+import org.apache.gravitino.cli.commands.RemoveAllRoles;
import org.apache.gravitino.cli.commands.RemoveRoleFromGroup;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
@@ -260,6 +261,32 @@ class TestGroupCommands {
verify(mockRemoveSecondRole).handle();
}
+ @Test
+ void testRemoveAllRolesFromGroupCommand() {
+ RemoveAllRoles mock = mock(RemoveAllRoles.class);
+
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
+ when(mockCommandLine.hasOption(GravitinoOptions.GROUP)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.GROUP)).thenReturn("groupA");
+ when(mockCommandLine.hasOption(GravitinoOptions.ALL)).thenReturn(true);
+ GravitinoCommandLine commandLine =
+ spy(
+ new GravitinoCommandLine(
+ mockCommandLine, mockOptions, CommandEntities.GROUP,
CommandActions.REVOKE));
+
+ doReturn(mock)
+ .when(commandLine)
+ .newRemoveAllRoles(
+ GravitinoCommandLine.DEFAULT_URL,
+ false,
+ "metalake_demo",
+ "groupA",
+ CommandEntities.GROUP);
+ doReturn(mock).when(mock).validate();
+ commandLine.handleCommandLine();
+ verify(mock).handle();
+ }
+
@Test
void testAddRolesToGroupCommand() {
AddRoleToGroup mockAddFirstRole = mock(AddRoleToGroup.class);
diff --git
a/clients/cli/src/test/java/org/apache/gravitino/cli/TestUserCommands.java
b/clients/cli/src/test/java/org/apache/gravitino/cli/TestUserCommands.java
index c7612f6c87..47f003c96b 100644
--- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestUserCommands.java
+++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestUserCommands.java
@@ -37,6 +37,7 @@ import org.apache.gravitino.cli.commands.AddRoleToUser;
import org.apache.gravitino.cli.commands.CreateUser;
import org.apache.gravitino.cli.commands.DeleteUser;
import org.apache.gravitino.cli.commands.ListUsers;
+import org.apache.gravitino.cli.commands.RemoveAllRoles;
import org.apache.gravitino.cli.commands.RemoveRoleFromUser;
import org.apache.gravitino.cli.commands.UserAudit;
import org.apache.gravitino.cli.commands.UserDetails;
@@ -261,6 +262,27 @@ class TestUserCommands {
verify(mockRemoveFirstRole).handle();
}
+ @Test
+ void removeAllRolesFromUserCommand() {
+ RemoveAllRoles mockRemove = mock(RemoveAllRoles.class);
+
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
+ when(mockCommandLine.hasOption(GravitinoOptions.USER)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.USER)).thenReturn("user");
+ when(mockCommandLine.hasOption(GravitinoOptions.ALL)).thenReturn(true);
+ GravitinoCommandLine commandLine =
+ spy(
+ new GravitinoCommandLine(
+ mockCommandLine, mockOptions, CommandEntities.USER,
CommandActions.REVOKE));
+ doReturn(mockRemove)
+ .when(commandLine)
+ .newRemoveAllRoles(
+ GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "user",
CommandEntities.USER);
+ doReturn(mockRemove).when(mockRemove).validate();
+ commandLine.handleCommandLine();
+ verify(mockRemove).handle();
+ }
+
@Test
void testAddRolesToUserCommand() {
AddRoleToUser mockAddFirstRole = mock(AddRoleToUser.class);
diff --git a/docs/cli.md b/docs/cli.md
index 0598a36e03..7f6f190dd1 100644
--- a/docs/cli.md
+++ b/docs/cli.md
@@ -762,6 +762,12 @@ gcli user grant --user new_user --role admin
gcli user revoke --user new_user --role admin
```
+#### Remove all roles from a user
+
+```bash
+gcli user revoke --user new_user --all
+```
+
#### Add a role to a group
```bash
@@ -774,6 +780,12 @@ gcli group grant --group groupA --role admin
gcli group revoke --group groupA --role admin
```
+#### Remove all roles from a group
+
+```bash
+gcli group revoke --group groupA --all
+```
+
### Grant a privilege
```bash
