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 6fb45d6a94 [#6278] improve(CLI): Revoke all privileges from a role via
the Gravitino CLI. (#6293)
6fb45d6a94 is described below
commit 6fb45d6a947f0de1399b1d9fc461fea81134944e
Author: Lord of Abyss <103809695+abyss-l...@users.noreply.github.com>
AuthorDate: Fri Jan 17 08:30:43 2025 +0800
[#6278] improve(CLI): Revoke all privileges from a role via the Gravitino
CLI. (#6293)
### What changes were proposed in this pull request?
Revoke all privileges from a role via the Gravitino CLI. syntax: `gcli
role revoke --name <name> --role <role> --all`
The step as follows:
1. Retrieve the `SecurableObject` instance using `role` and `fullname`;
2. Obtain the privileges associated with the `SecurableObject`;
3. Revoke all identified privileges;
### Why are the changes needed?
Fix: #6278
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
local tests + ut
```bash
gcli role grant -m demo_metalake --name Hive.default --role roleA
--privilege create_table modify_table -i
# roleA granted create_table,modify_table on Hive.default
gcli role revoke -m demo_metalake --name Hive.default --all --role roleA -i
# Hive.default: ALLOW create table, ALLOW modify table
```
---
.../apache/gravitino/cli/RoleCommandHandler.java | 18 ++-
.../apache/gravitino/cli/TestableCommandLine.java | 6 +
.../cli/commands/RevokeAllPrivileges.java | 129 +++++++++++++++++++++
.../cli/commands/RevokePrivilegesFromRole.java | 12 +-
clients/cli/src/main/resources/role_help.txt | 3 +
.../org/apache/gravitino/cli/TestRoleCommands.java | 51 ++++++++
docs/cli.md | 6 +
7 files changed, 212 insertions(+), 13 deletions(-)
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/RoleCommandHandler.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/RoleCommandHandler.java
index cf48783b8d..327ef21fdd 100644
--- a/clients/cli/src/main/java/org/apache/gravitino/cli/RoleCommandHandler.java
+++ b/clients/cli/src/main/java/org/apache/gravitino/cli/RoleCommandHandler.java
@@ -137,11 +137,19 @@ public class RoleCommandHandler extends CommandHandler {
}
private void handleRevokeCommand() {
- gravitinoCommandLine
- .newRevokePrivilegesFromRole(
- url, ignore, metalake, getOneRole(), new FullName(line),
privileges)
- .validate()
- .handle();
+ boolean removeAll = line.hasOption(GravitinoOptions.ALL);
+ if (removeAll) {
+ gravitinoCommandLine
+ .newRevokeAllPrivileges(url, ignore, metalake, getOneRole(), new
FullName(line))
+ .validate()
+ .handle();
+ } else {
+ gravitinoCommandLine
+ .newRevokePrivilegesFromRole(
+ url, ignore, metalake, getOneRole(), new FullName(line),
privileges)
+ .validate()
+ .handle();
+ }
}
private String getOneRole() {
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 498a0060cb..b229ef16aa 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
@@ -97,6 +97,7 @@ import org.apache.gravitino.cli.commands.RemoveSchemaProperty;
import org.apache.gravitino.cli.commands.RemoveTableProperty;
import org.apache.gravitino.cli.commands.RemoveTagProperty;
import org.apache.gravitino.cli.commands.RemoveTopicProperty;
+import org.apache.gravitino.cli.commands.RevokeAllPrivileges;
import org.apache.gravitino.cli.commands.RevokePrivilegesFromRole;
import org.apache.gravitino.cli.commands.RoleAudit;
import org.apache.gravitino.cli.commands.RoleDetails;
@@ -901,6 +902,11 @@ public class TestableCommandLine {
return new RevokePrivilegesFromRole(url, ignore, metalake, role, entity,
privileges);
}
+ protected RevokeAllPrivileges newRevokeAllPrivileges(
+ String url, boolean ignore, String metalake, String role, FullName
entity) {
+ return new RevokeAllPrivileges(url, ignore, metalake, role, entity);
+ }
+
protected MetalakeEnable newMetalakeEnable(
String url, boolean ignore, String metalake, boolean enableAllCatalogs) {
return new MetalakeEnable(url, ignore, metalake, enableAllCatalogs);
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokeAllPrivileges.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokeAllPrivileges.java
new file mode 100644
index 0000000000..3167c4b6c3
--- /dev/null
+++
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokeAllPrivileges.java
@@ -0,0 +1,129 @@
+/*
+ * 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 com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.authorization.Privilege;
+import org.apache.gravitino.authorization.Role;
+import org.apache.gravitino.authorization.SecurableObject;
+import org.apache.gravitino.cli.ErrorMessages;
+import org.apache.gravitino.cli.FullName;
+import org.apache.gravitino.client.GravitinoClient;
+import org.apache.gravitino.exceptions.NoSuchMetalakeException;
+import org.apache.gravitino.exceptions.NoSuchRoleException;
+
+/** Revokes all privileges from a role to an entity or all entities. */
+public class RevokeAllPrivileges extends MetadataCommand {
+
+ protected final String metalake;
+ protected final String role;
+ protected final FullName entity;
+
+ /**
+ * Revokes all privileges from a role to an entity or all entities.
+ *
+ * @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 role The name of the role.
+ * @param entity The name of the entity.
+ */
+ public RevokeAllPrivileges(
+ String url, boolean ignoreVersions, String metalake, String role,
FullName entity) {
+ super(url, ignoreVersions);
+ this.metalake = metalake;
+ this.role = role;
+ this.entity = entity;
+ }
+
+ /** Revokes all privileges from a role to an entity or all entities. */
+ @Override
+ public void handle() {
+ List<SecurableObject> matchedObjects;
+ Map<String, Set<Privilege>> revokedPrivileges = Maps.newHashMap();
+
+ try {
+ GravitinoClient client = buildClient(metalake);
+ matchedObjects = getMatchedObjects(client);
+
+ for (SecurableObject securableObject : matchedObjects) {
+ String objectFullName = securableObject.fullName();
+ Set<Privilege> privileges = new
HashSet<>(securableObject.privileges());
+ MetadataObject metadataObject = constructMetadataObject(entity,
client);
+ client.revokePrivilegesFromRole(role, metadataObject, privileges);
+
+ revokedPrivileges.put(objectFullName, privileges);
+ }
+ } catch (NoSuchMetalakeException e) {
+ exitWithError(ErrorMessages.UNKNOWN_METALAKE);
+ } catch (NoSuchRoleException e) {
+ exitWithError(ErrorMessages.UNKNOWN_ROLE);
+ } catch (Exception e) {
+ exitWithError(e.getMessage());
+ }
+
+ if (revokedPrivileges.isEmpty()) outputRevokedPrivileges("No privileges
revoked.");
+ outputRevokedPrivileges(revokedPrivileges);
+ }
+
+ private List<SecurableObject> getMatchedObjects(GravitinoClient client) {
+ Role gRole = client.getRole(role);
+ return gRole.securableObjects().stream()
+ .filter(s -> s.fullName().equals(entity.getName()))
+ .collect(Collectors.toList());
+ }
+
+ private void outputRevokedPrivileges(Map<String, Set<Privilege>>
revokedPrivileges) {
+ List<String> revokedInfoList = Lists.newArrayList();
+
+ for (Map.Entry<String, Set<Privilege>> entry :
revokedPrivileges.entrySet()) {
+ List<String> revokedPrivilegesList =
+
entry.getValue().stream().map(Privilege::simpleString).collect(Collectors.toList());
+ revokedInfoList.add(entry.getKey() + ": " +
COMMA_JOINER.join(revokedPrivilegesList));
+ }
+
+ System.out.println("Revoked privileges:");
+ for (String info : revokedInfoList) {
+ System.out.println(info);
+ }
+ }
+
+ private void outputRevokedPrivileges(String message) {
+ System.out.println(message);
+ }
+
+ /**
+ * verify the arguments.
+ *
+ * @return Returns itself via argument validation, otherwise exits.
+ */
+ @Override
+ public Command validate() {
+ if (!entity.hasName()) exitWithError(ErrorMessages.MISSING_NAME);
+ return super.validate();
+ }
+}
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokePrivilegesFromRole.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokePrivilegesFromRole.java
index fbf273ce0d..5907c494c0 100644
---
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokePrivilegesFromRole.java
+++
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokePrivilegesFromRole.java
@@ -87,17 +87,13 @@ public class RevokePrivilegesFromRole extends
MetadataCommand {
MetadataObject metadataObject = constructMetadataObject(entity, client);
client.revokePrivilegesFromRole(role, metadataObject, privilegesSet);
} catch (NoSuchMetalakeException err) {
- System.err.println(ErrorMessages.UNKNOWN_METALAKE);
- return;
+ exitWithError(ErrorMessages.UNKNOWN_METALAKE);
} catch (NoSuchRoleException err) {
- System.err.println(ErrorMessages.UNKNOWN_ROLE);
- return;
+ exitWithError(ErrorMessages.UNKNOWN_ROLE);
} catch (NoSuchMetadataObjectException err) {
- System.err.println(ErrorMessages.UNKNOWN_USER);
- return;
+ exitWithError(ErrorMessages.UNKNOWN_USER);
} catch (Exception exp) {
- System.err.println(exp.getMessage());
- return;
+ exitWithError(exp.getMessage());
}
String all = String.join(",", privileges);
diff --git a/clients/cli/src/main/resources/role_help.txt
b/clients/cli/src/main/resources/role_help.txt
index d0838cfa40..6c51980d1c 100644
--- a/clients/cli/src/main/resources/role_help.txt
+++ b/clients/cli/src/main/resources/role_help.txt
@@ -42,3 +42,6 @@ gcli role grant --name catalog_postgres --role admin
--privilege create_table mo
Revoke a privilege
gcli role revoke --metalake metalake_demo --name catalog_postgres --role admin
--privilege create_table modify_table
+
+Revoke all privileges
+gcli role revoke --metalake metalake_demo --name catalog_postgres --role admin
--all
\ No newline at end of file
diff --git
a/clients/cli/src/test/java/org/apache/gravitino/cli/TestRoleCommands.java
b/clients/cli/src/test/java/org/apache/gravitino/cli/TestRoleCommands.java
index 529979582f..2b4ab47452 100644
--- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestRoleCommands.java
+++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestRoleCommands.java
@@ -34,11 +34,14 @@ import java.io.ByteArrayOutputStream;
import java.io.PrintStream;
import java.nio.charset.StandardCharsets;
import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.DefaultParser;
import org.apache.commons.cli.Options;
+import org.apache.commons.cli.ParseException;
import org.apache.gravitino.cli.commands.CreateRole;
import org.apache.gravitino.cli.commands.DeleteRole;
import org.apache.gravitino.cli.commands.GrantPrivilegesToRole;
import org.apache.gravitino.cli.commands.ListRoles;
+import org.apache.gravitino.cli.commands.RevokeAllPrivileges;
import org.apache.gravitino.cli.commands.RevokePrivilegesFromRole;
import org.apache.gravitino.cli.commands.RoleAudit;
import org.apache.gravitino.cli.commands.RoleDetails;
@@ -48,6 +51,7 @@ import org.junit.jupiter.api.Test;
class TestRoleCommands {
private CommandLine mockCommandLine;
+ private Options options;
private Options mockOptions;
private final ByteArrayOutputStream outContent = new ByteArrayOutputStream();
private final ByteArrayOutputStream errContent = new ByteArrayOutputStream();
@@ -58,6 +62,7 @@ class TestRoleCommands {
void setUp() {
mockCommandLine = mock(CommandLine.class);
mockOptions = mock(Options.class);
+ options = new GravitinoOptions().options();
System.setOut(new PrintStream(outContent));
System.setErr(new PrintStream(errContent));
}
@@ -99,6 +104,7 @@ class TestRoleCommands {
doReturn(mockDetails)
.when(commandLine)
.newRoleDetails(GravitinoCommandLine.DEFAULT_URL, false,
"metalake_demo", "admin");
+
doReturn(mockDetails).when(mockDetails).validate();
commandLine.handleCommandLine();
verify(mockDetails).handle();
@@ -333,6 +339,51 @@ class TestRoleCommands {
verify(mockRevoke).handle();
}
+ @Test
+ void testRevokeAllPrivilegesFromRole() {
+ RevokeAllPrivileges mockRevoke = mock(RevokeAllPrivileges.class);
+
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
+ when(mockCommandLine.hasOption(GravitinoOptions.NAME)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.NAME)).thenReturn("catalog");
+ when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true);
+
when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE)).thenReturn(new
String[] {"admin"});
+ when(mockCommandLine.hasOption(GravitinoOptions.ALL)).thenReturn(true);
+ GravitinoCommandLine commandLine =
+ spy(
+ new GravitinoCommandLine(
+ mockCommandLine, mockOptions, CommandEntities.ROLE,
CommandActions.REVOKE));
+ doReturn(mockRevoke)
+ .when(commandLine)
+ .newRevokeAllPrivileges(
+ eq(GravitinoCommandLine.DEFAULT_URL),
+ eq(false),
+ eq("metalake_demo"),
+ eq("admin"),
+ any());
+ doReturn(mockRevoke).when(mockRevoke).validate();
+ commandLine.handleCommandLine();
+ verify(mockRevoke).handle();
+ }
+
+ @Test
+ void testRevokeAllPrivilegesFromRoleWithoutName() throws ParseException {
+ Main.useExit = false;
+ String[] args = {"role", "revoke", "-m", "metalake_demo", "--role",
"admin", "--all"};
+ CommandLine commandLine = new DefaultParser().parse(options, args);
+ FullName fullName = new FullName(commandLine);
+
+ RevokeAllPrivileges spyRevokeAllPrivileges =
+ spy(
+ new RevokeAllPrivileges(
+ GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo",
"admin", fullName));
+
+ assertThrows(RuntimeException.class, spyRevokeAllPrivileges::validate);
+ verify(spyRevokeAllPrivileges, never()).handle();
+ String errOutput = new String(errContent.toByteArray(),
StandardCharsets.UTF_8).trim();
+ assertEquals(ErrorMessages.MISSING_NAME, errOutput);
+ }
+
@Test
void testRevokePrivilegesFromRoleWithoutPrivileges() {
Main.useExit = false;
diff --git a/docs/cli.md b/docs/cli.md
index 7f6f190dd1..cc710a2080 100644
--- a/docs/cli.md
+++ b/docs/cli.md
@@ -798,6 +798,12 @@ gcli role grant --name catalog_postgres --role admin
--privilege create_table mo
gcli role revoke --metalake metalake_demo --name catalog_postgres --role admin
--privilege create_table modify_table
```
+### Revoke all privileges
+
+```bash
+gcli role revoke --metalake metalake_demo --name catalog_postgres --role admin
--all
+```
+
### Topic commands
#### Display a topic's details
