Skip to content

Commit

Permalink
[#5570] improve(CLI): Add the ability to revoke all roles or groups i…
Browse files Browse the repository at this point in the history
…n 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 <[email protected]>
Co-authored-by: yangyang zhong <[email protected]>
Co-authored-by: Xiaojian Sun <[email protected]>
Co-authored-by: roryqi <[email protected]>
Co-authored-by: TengYao Chi <[email protected]>
Co-authored-by: Qi Yu <[email protected]>
  • Loading branch information
7 people authored Jan 16, 2025
1 parent eb3fb31 commit 375116a
Show file tree
Hide file tree
Showing 11 changed files with 206 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public Options options() {
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"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,23 @@ private void handleDeleteCommand() {

/** 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. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
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;
Expand Down Expand Up @@ -457,6 +458,11 @@ protected RemoveRoleFromGroup newRemoveRoleFromGroup(
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,14 +150,23 @@ private void handleDeleteCommand() {

/** 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. */
Expand Down
Original file line number Diff line number Diff line change
@@ -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());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -50,7 +50,7 @@ public RemoveRoleFromGroup(
this.role = role;
}

/** Adds a role to a group. */
/** Remove a role from a group. */
@Override
public void handle() {
try {
Expand Down
6 changes: 6 additions & 0 deletions clients/cli/src/main/resources/group_help.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 5 additions & 0 deletions clients/cli/src/main/resources/user_help.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
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;
Expand Down Expand Up @@ -260,6 +261,32 @@ void testRemoveRolesFromGroupCommand() {
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
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;
Expand Down Expand Up @@ -261,6 +262,27 @@ void testRemoveRolesFromUserCommand() {
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);
Expand Down
12 changes: 12 additions & 0 deletions docs/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 375116a

Please sign in to comment.