From 6fb45d6a947f0de1399b1d9fc461fea81134944e Mon Sep 17 00:00:00 2001 From: Lord of Abyss <103809695+Abyss-lord@users.noreply.github.com> Date: Fri, 17 Jan 2025 08:30:43 +0800 Subject: [PATCH] [#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 --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 ``` --- .../gravitino/cli/RoleCommandHandler.java | 18 ++- .../gravitino/cli/TestableCommandLine.java | 6 + .../cli/commands/RevokeAllPrivileges.java | 129 ++++++++++++++++++ .../commands/RevokePrivilegesFromRole.java | 12 +- clients/cli/src/main/resources/role_help.txt | 3 + .../gravitino/cli/TestRoleCommands.java | 51 +++++++ docs/cli.md | 6 + 7 files changed, 212 insertions(+), 13 deletions(-) create mode 100644 clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokeAllPrivileges.java 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 cf48783b8dc..327ef21fdd8 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 @@ url, ignore, metalake, getOneRole(), new FullName(line), privileges) } 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 498a0060cbd..b229ef16aa3 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.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 @@ protected RevokePrivilegesFromRole newRevokePrivilegesFromRole( 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 00000000000..3167c4b6c37 --- /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 matchedObjects; + Map> revokedPrivileges = Maps.newHashMap(); + + try { + GravitinoClient client = buildClient(metalake); + matchedObjects = getMatchedObjects(client); + + for (SecurableObject securableObject : matchedObjects) { + String objectFullName = securableObject.fullName(); + Set 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 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> revokedPrivileges) { + List revokedInfoList = Lists.newArrayList(); + + for (Map.Entry> entry : revokedPrivileges.entrySet()) { + List 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 fbf273ce0d8..5907c494c05 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 void handle() { 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 d0838cfa403..6c51980d1c5 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 529979582ff..2b4ab474521 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.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 @@ 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 @@ void testRoleDetailsCommand() { 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 @@ void testRevokePrivilegesFromRole() { 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 7f6f190dd1b..cc710a20805 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