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:03 UTC
[1/3] hbase git commit: HBASE-18437 Revoke access permissions of a
user from a table does not work as expected
Repository: hbase
Updated Branches:
refs/heads/branch-1 d7c6a0bf4 -> a49d43bfb
refs/heads/branch-1.3 8a9005486 -> 7800fa152
refs/heads/branch-1.4 122012493 -> 5b27f6253
HBASE-18437 Revoke access permissions of a user from a table does not work as expected
Signed-off-by: Andrew Purtell <ap...@apache.org>
Conflicts:
hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java
hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/a49d43bf
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/a49d43bf
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/a49d43bf
Branch: refs/heads/branch-1
Commit: a49d43bfbf8f839ad897f36996bf75a1578d9397
Parents: d7c6a0b
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 18:59:21 2017 -0700
----------------------------------------------------------------------
.../hbase/security/access/Permission.java | 6 ++
.../security/access/AccessControlLists.java | 53 +++++++++--
.../security/access/TestAccessController.java | 96 ++++++++++++++------
3 files changed, 118 insertions(+), 37 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/a49d43bf/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 f4538a6..3a01ace 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
@@ -112,6 +112,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/a49d43bf/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 7197526..a2ac927 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
@@ -185,10 +185,18 @@ public class AccessControlLists {
LOG.debug("Writing permission with rowKey " + Bytes.toString(rowKey) + " "
+ Bytes.toString(key) + ": " + Bytes.toStringBinary(value));
}
- try {
- t.put(p);
- } finally {
- t.close();
+ if (t == null) {
+ try (Connection connection = ConnectionFactory.createConnection(conf)) {
+ try (Table table = connection.getTable(ACL_TABLE_NAME)) {
+ table.put(p);
+ }
+ }
+ } else {
+ try {
+ t.put(p);
+ } finally {
+ t.close();
+ }
}
}
@@ -220,13 +228,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));
if (t == null) {
try (Connection connection = ConnectionFactory.createConnection(conf)) {
try (Table table = connection.getTable(ACL_TABLE_NAME)) {
http://git-wip-us.apache.org/repos/asf/hbase/blob/a49d43bf/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 51aeff8..c5c0470 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
@@ -19,6 +19,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)
@@ -2546,28 +2549,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);
}
@@ -2925,4 +2929,40 @@ public class TestAccessController extends SecureTestUtil {
verifyDenied(replicateLogEntriesAction, USER_CREATE, USER_RW, USER_RO, USER_NONE, USER_OWNER,
USER_GROUP_READ, USER_GROUP_ADMIN, USER_GROUP_CREATE);
}
+
+ @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);
+ }
+ }
}
[2/3] hbase git commit: HBASE-18437 Revoke access permissions of a
user from a table does not work as expected
Posted by ap...@apache.org.
HBASE-18437 Revoke access permissions of a user from a table does not work as expected
Signed-off-by: Andrew Purtell <ap...@apache.org>
Conflicts:
hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java
hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/5b27f625
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/5b27f625
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/5b27f625
Branch: refs/heads/branch-1.4
Commit: 5b27f6253ad7e561ee4a3b3491ec647be7c726b0
Parents: 1220124
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 19:00:44 2017 -0700
----------------------------------------------------------------------
.../hbase/security/access/Permission.java | 6 ++
.../security/access/AccessControlLists.java | 53 +++++++++--
.../security/access/TestAccessController.java | 96 ++++++++++++++------
3 files changed, 118 insertions(+), 37 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/5b27f625/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 f4538a6..3a01ace 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
@@ -112,6 +112,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/5b27f625/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 7197526..a2ac927 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
@@ -185,10 +185,18 @@ public class AccessControlLists {
LOG.debug("Writing permission with rowKey " + Bytes.toString(rowKey) + " "
+ Bytes.toString(key) + ": " + Bytes.toStringBinary(value));
}
- try {
- t.put(p);
- } finally {
- t.close();
+ if (t == null) {
+ try (Connection connection = ConnectionFactory.createConnection(conf)) {
+ try (Table table = connection.getTable(ACL_TABLE_NAME)) {
+ table.put(p);
+ }
+ }
+ } else {
+ try {
+ t.put(p);
+ } finally {
+ t.close();
+ }
}
}
@@ -220,13 +228,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));
if (t == null) {
try (Connection connection = ConnectionFactory.createConnection(conf)) {
try (Table table = connection.getTable(ACL_TABLE_NAME)) {
http://git-wip-us.apache.org/repos/asf/hbase/blob/5b27f625/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 51aeff8..c5c0470 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
@@ -19,6 +19,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)
@@ -2546,28 +2549,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);
}
@@ -2925,4 +2929,40 @@ public class TestAccessController extends SecureTestUtil {
verifyDenied(replicateLogEntriesAction, USER_CREATE, USER_RW, USER_RO, USER_NONE, USER_OWNER,
USER_GROUP_READ, USER_GROUP_ADMIN, USER_GROUP_CREATE);
}
+
+ @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);
+ }
+ }
}
[3/3] hbase git commit: HBASE-18437 Revoke access permissions of a
user from a table does not work as expected
Posted by ap...@apache.org.
HBASE-18437 Revoke access permissions of a user from a table does not work as expected
Signed-off-by: Andrew Purtell <ap...@apache.org>
Conflicts:
hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java
hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/7800fa15
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/7800fa15
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/7800fa15
Branch: refs/heads/branch-1.3
Commit: 7800fa152e3f5ecdd43391f9f2bb162bd33046c5
Parents: 8a90054
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 19:00:47 2017 -0700
----------------------------------------------------------------------
.../hbase/security/access/Permission.java | 6 ++
.../security/access/AccessControlLists.java | 53 +++++++++--
.../security/access/TestAccessController.java | 96 ++++++++++++++------
3 files changed, 118 insertions(+), 37 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/7800fa15/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 f4538a6..3a01ace 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
@@ -112,6 +112,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/7800fa15/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 50d575e..ba81f3d 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
@@ -170,10 +170,18 @@ public class AccessControlLists {
Bytes.toString(key)+": "+Bytes.toStringBinary(value)
);
}
- try {
- t.put(p);
- } finally {
- t.close();
+ if (t == null) {
+ try (Connection connection = ConnectionFactory.createConnection(conf)) {
+ try (Table table = connection.getTable(ACL_TABLE_NAME)) {
+ table.put(p);
+ }
+ }
+ } else {
+ try {
+ t.put(p);
+ } finally {
+ t.close();
+ }
}
}
@@ -193,13 +201,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));
if (t == null) {
try (Connection connection = ConnectionFactory.createConnection(conf)) {
try (Table table = connection.getTable(ACL_TABLE_NAME)) {
http://git-wip-us.apache.org/repos/asf/hbase/blob/7800fa15/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 2e77c78..da2b42c 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
@@ -19,6 +19,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;
@@ -2384,28 +2385,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)
@@ -2428,28 +2431,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);
}
@@ -2807,4 +2811,40 @@ public class TestAccessController extends SecureTestUtil {
verifyDenied(replicateLogEntriesAction, USER_CREATE, USER_RW, USER_RO, USER_NONE, USER_OWNER,
USER_GROUP_READ, USER_GROUP_ADMIN, USER_GROUP_CREATE);
}
+
+ @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);
+ }
+ }
}