You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ap...@apache.org on 2017/08/16 05:29:24 UTC
[2/2] hbase git commit: HBASE-18437 Revoke access permissions of a
user from a table does not work as expected
HBASE-18437 Revoke access permissions of a user from a table does not work as expected
Signed-off-by: Andrew Purtell <ap...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/b0878184
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/b0878184
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/b0878184
Branch: refs/heads/master
Commit: b0878184a31804a4bf061df7581964157b4849d5
Parents: 59ffb611
Author: Ashish Singhi <as...@apache.org>
Authored: Fri Aug 11 12:48:32 2017 +0530
Committer: Andrew Purtell <ap...@apache.org>
Committed: Tue Aug 15 22:29:16 2017 -0700
----------------------------------------------------------------------
.../hbase/security/access/Permission.java | 6 ++
.../security/access/AccessControlLists.java | 37 +++++++-
.../security/access/TestAccessController.java | 96 ++++++++++++++------
3 files changed, 106 insertions(+), 33 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/b0878184/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/Permission.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/Permission.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/Permission.java
index 8476f61..18096e1 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/Permission.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/Permission.java
@@ -110,6 +110,12 @@ public class Permission extends VersionedWritable {
return false;
}
+ public void setActions(Action[] assigned) {
+ if (assigned != null && assigned.length > 0) {
+ actions = Arrays.copyOf(assigned, assigned.length);
+ }
+ }
+
@Override
public boolean equals(Object obj) {
if (!(obj instanceof Permission)) {
http://git-wip-us.apache.org/repos/asf/hbase/blob/b0878184/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java
index 12bdc22..38e292c 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java
@@ -241,13 +241,40 @@ public class AccessControlLists {
*/
static void removeUserPermission(Configuration conf, UserPermission userPerm, Table t)
throws IOException {
- Delete d = new Delete(userPermissionRowKey(userPerm));
- byte[] key = userPermissionKey(userPerm);
-
+ if (null == userPerm.getActions()) {
+ removePermissionRecord(conf, userPerm, t);
+ } else {
+ // Get all the global user permissions from the acl table
+ List<UserPermission> permsList = getUserPermissions(conf, userPermissionRowKey(userPerm));
+ List<Permission.Action> remainingActions = new ArrayList<>();
+ List<Permission.Action> dropActions = Arrays.asList(userPerm.getActions());
+ for (UserPermission perm : permsList) {
+ // Find the user and remove only the requested permissions
+ if (Bytes.toString(perm.getUser()).equals(Bytes.toString(userPerm.getUser()))) {
+ for (Permission.Action oldAction : perm.getActions()) {
+ if (!dropActions.contains(oldAction)) {
+ remainingActions.add(oldAction);
+ }
+ }
+ if (!remainingActions.isEmpty()) {
+ perm.setActions(remainingActions.toArray(new Permission.Action[remainingActions.size()]));
+ addUserPermission(conf, perm, t);
+ } else {
+ removePermissionRecord(conf, userPerm, t);
+ }
+ break;
+ }
+ }
+ }
if (LOG.isDebugEnabled()) {
- LOG.debug("Removing permission "+ userPerm.toString());
+ LOG.debug("Removed permission "+ userPerm.toString());
}
- d.addColumns(ACL_LIST_FAMILY, key);
+ }
+
+ private static void removePermissionRecord(Configuration conf, UserPermission userPerm, Table t)
+ throws IOException {
+ Delete d = new Delete(userPermissionRowKey(userPerm));
+ d.addColumns(ACL_LIST_FAMILY, userPermissionKey(userPerm));
try {
t.delete(d);
} finally {
http://git-wip-us.apache.org/repos/asf/hbase/blob/b0878184/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
index c1fbb28..6583366 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
@@ -18,6 +18,7 @@
package org.apache.hadoop.hbase.security.access;
import static org.apache.hadoop.hbase.AuthUtil.toGroupEntry;
+import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
@@ -2380,28 +2381,30 @@ public class TestAccessController extends SecureTestUtil {
// Grant table READ permissions to testGlobalGrantRevoke.
String userName = testGlobalGrantRevoke.getShortName();
try {
- grantGlobalUsingAccessControlClient(TEST_UTIL, systemUserConnection,
- userName, Permission.Action.READ);
+ grantGlobalUsingAccessControlClient(TEST_UTIL, systemUserConnection, userName,
+ Permission.Action.READ);
} catch (Throwable e) {
LOG.error("error during call of AccessControlClient.grant. ", e);
}
try {
// Now testGlobalGrantRevoke should be able to read also
verifyAllowed(getAction, testGlobalGrantRevoke);
-
- // Revoke table READ permission to testGlobalGrantRevoke.
- try {
- revokeGlobalUsingAccessControlClient(TEST_UTIL, systemUserConnection,
- userName, Permission.Action.READ);
- } catch (Throwable e) {
- LOG.error("error during call of AccessControlClient.revoke ", e);
- }
-
- // Now testGlobalGrantRevoke shouldn't be able read
- verifyDenied(getAction, testGlobalGrantRevoke);
- } finally {
+ } catch (Exception e) {
revokeGlobal(TEST_UTIL, userName, Permission.Action.READ);
+ throw e;
}
+
+ // Revoke table READ permission to testGlobalGrantRevoke.
+ try {
+ revokeGlobalUsingAccessControlClient(TEST_UTIL, systemUserConnection, userName,
+ Permission.Action.READ);
+ } catch (Throwable e) {
+ LOG.error("error during call of AccessControlClient.revoke ", e);
+ }
+
+ // Now testGlobalGrantRevoke shouldn't be able read
+ verifyDenied(getAction, testGlobalGrantRevoke);
+
}
@Test(timeout = 180000)
@@ -2547,28 +2550,29 @@ public class TestAccessController extends SecureTestUtil {
String namespace = TEST_TABLE.getNamespaceAsString();
// Grant namespace READ to testNS, this should supersede any table permissions
try {
- grantOnNamespaceUsingAccessControlClient(TEST_UTIL, systemUserConnection, userName,
- namespace, Permission.Action.READ);
+ grantOnNamespaceUsingAccessControlClient(TEST_UTIL, systemUserConnection, userName, namespace,
+ Permission.Action.READ);
} catch (Throwable e) {
LOG.error("error during call of AccessControlClient.grant. ", e);
}
try {
// Now testNS should be able to read also
verifyAllowed(getAction, testNS);
-
- // Revoke namespace READ to testNS, this should supersede any table permissions
- try {
- revokeFromNamespaceUsingAccessControlClient(TEST_UTIL, systemUserConnection, userName,
- namespace, Permission.Action.READ);
- } catch (Throwable e) {
- LOG.error("error during call of AccessControlClient.revoke ", e);
- }
-
- // Now testNS shouldn't be able read
- verifyDenied(getAction, testNS);
- } finally {
+ } catch (Exception e) {
revokeFromNamespace(TEST_UTIL, userName, namespace, Permission.Action.READ);
+ throw e;
+ }
+
+ // Revoke namespace READ to testNS, this should supersede any table permissions
+ try {
+ revokeFromNamespaceUsingAccessControlClient(TEST_UTIL, systemUserConnection, userName,
+ namespace, Permission.Action.READ);
+ } catch (Throwable e) {
+ LOG.error("error during call of AccessControlClient.revoke ", e);
}
+
+ // Now testNS shouldn't be able read
+ verifyDenied(getAction, testNS);
}
@@ -3175,4 +3179,40 @@ public class TestAccessController extends SecureTestUtil {
verifyAllowed(regionLockHeartbeatAction, SUPERUSER, USER_ADMIN, namespaceUser, tableACUser);
verifyDenied(regionLockHeartbeatAction, globalRWXUser, tableRWXUser);
}
+
+ @Test
+ public void testAccessControlRevokeOnlyFewPermission() throws Throwable {
+ TableName tname = TableName.valueOf("revoke");
+ try {
+ TEST_UTIL.createTable(tname, TEST_FAMILY);
+ User testUserPerms = User.createUserForTesting(conf, "revokePerms", new String[0]);
+ Permission.Action[] actions = { Action.READ, Action.WRITE };
+ AccessControlClient.grant(TEST_UTIL.getConnection(), tname, testUserPerms.getShortName(),
+ null, null, actions);
+
+ List<UserPermission> userPermissions = AccessControlClient
+ .getUserPermissions(TEST_UTIL.getConnection(), tname.getNameAsString());
+ assertEquals(2, userPermissions.size());
+
+ AccessControlClient.revoke(TEST_UTIL.getConnection(), tname, testUserPerms.getShortName(),
+ null, null, Action.WRITE);
+
+ userPermissions = AccessControlClient.getUserPermissions(TEST_UTIL.getConnection(),
+ tname.getNameAsString());
+ assertEquals(2, userPermissions.size());
+
+ Permission.Action[] expectedAction = { Action.READ };
+ boolean userFound = false;
+ for (UserPermission p : userPermissions) {
+ if (testUserPerms.getShortName().equals(Bytes.toString(p.getUser()))) {
+ assertArrayEquals(expectedAction, p.getActions());
+ userFound = true;
+ break;
+ }
+ }
+ assertTrue(userFound);
+ } finally {
+ TEST_UTIL.deleteTable(tname);
+ }
+ }
}