From 7e5afd8630ee16e5dc8a1440950cd1666ea85d2c Mon Sep 17 00:00:00 2001 From: meiyi Date: Mon, 11 Mar 2019 19:22:42 +0800 Subject: [PATCH] HBASE-22015 UserPermission should be annotated as InterfaceAudience.Public --- .../hbase/security/access/AccessControlUtil.java | 11 +- .../hbase/security/access/GlobalPermission.java | 17 +-- .../hbase/security/access/NamespacePermission.java | 21 +--- .../hadoop/hbase/security/access/Permission.java | 32 ++++- .../security/access/ShadedAccessControlUtil.java | 12 +- .../hbase/security/access/TablePermission.java | 67 +---------- .../hbase/security/access/UserPermission.java | 85 +------------ .../hbase/security/access/AccessControlLists.java | 10 +- .../hbase/security/access/AccessController.java | 7 +- .../hbase/snapshot/RestoreSnapshotHelper.java | 6 +- .../security/access/TestAccessController.java | 45 ++++--- .../security/access/TestPermissionBuilder.java | 11 +- .../security/access/TestTablePermissions.java | 134 +++++++++++++-------- .../access/TestWithDisabledAuthorization.java | 24 ++-- .../security/access/TestZKPermissionWatcher.java | 7 +- 15 files changed, 189 insertions(+), 300 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlUtil.java index 216b332..32b4001 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlUtil.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlUtil.java @@ -274,7 +274,8 @@ public class AccessControlUtil { if (proto.getType() == AccessControlProtos.Permission.Type.Global) { AccessControlProtos.GlobalPermission perm = proto.getGlobalPermission(); List actions = toPermissionActions(perm.getActionList()); - return new GlobalPermission(actions.toArray(new Permission.Action[actions.size()])); + return Permission.newBuilder() + .withActions(actions.toArray(new Permission.Action[actions.size()])).build(); } if (proto.getType() == AccessControlProtos.Permission.Type.Namespace) { AccessControlProtos.NamespacePermission perm = proto.getNamespacePermission(); @@ -282,8 +283,8 @@ public class AccessControlUtil { if (!proto.hasNamespacePermission()) { throw new IllegalStateException("Namespace must not be empty in NamespacePermission"); } - return new NamespacePermission(perm.getNamespaceName().toStringUtf8(), - actions.toArray(new Permission.Action[actions.size()])); + return Permission.newBuilder(perm.getNamespaceName().toStringUtf8()) + .withActions(actions.toArray(new Permission.Action[actions.size()])).build(); } if (proto.getType() == AccessControlProtos.Permission.Type.Table) { AccessControlProtos.TablePermission perm = proto.getTablePermission(); @@ -301,8 +302,8 @@ public class AccessControlUtil { if (perm.hasQualifier()) { qualifier = perm.getQualifier().toByteArray(); } - return new TablePermission(table, family, qualifier, - actions.toArray(new Permission.Action[actions.size()])); + return Permission.newBuilder(table).withFamily(family).withQualifier(qualifier) + .withActions(actions.toArray(new Permission.Action[actions.size()])).build(); } throw new IllegalStateException("Unrecognize Perm Type: " + proto.getType()); } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/GlobalPermission.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/GlobalPermission.java index b29317a..01d53eb 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/GlobalPermission.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/GlobalPermission.java @@ -23,15 +23,9 @@ import org.apache.yetus.audience.InterfaceAudience; /** * Represents an authorization for access whole cluster. */ -@InterfaceAudience.Private +@InterfaceAudience.Public public class GlobalPermission extends Permission { - /** Default constructor for Writable, do not use */ - public GlobalPermission() { - super(); - this.scope = Scope.EMPTY; - } - /** * Construct a global permission. * @param assigned assigned actions @@ -41,15 +35,6 @@ public class GlobalPermission extends Permission { this.scope = Scope.GLOBAL; } - /** - * Construct a global permission. - * @param actionCode assigned actions - */ - GlobalPermission(byte[] actionCode) { - super(actionCode); - this.scope = Scope.GLOBAL; - } - @Override public int hashCode() { return super.hashCode(); diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/NamespacePermission.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/NamespacePermission.java index c7ede96..7781d22 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/NamespacePermission.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/NamespacePermission.java @@ -30,39 +30,22 @@ import org.apache.yetus.audience.InterfaceAudience; /** * Represents an authorization for access for the given namespace. */ -@InterfaceAudience.Private +@InterfaceAudience.Public public class NamespacePermission extends Permission { private String namespace = NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR; - /** Default constructor for Writable, do not use */ - public NamespacePermission() { - super(); - this.scope = Scope.EMPTY; - } - /** * Construct a namespace permission. * @param namespace namespace's name * @param assigned assigned actions */ - public NamespacePermission(String namespace, Action... assigned) { + NamespacePermission(String namespace, Action... assigned) { super(assigned); this.namespace = namespace; this.scope = Scope.NAMESPACE; } - /** - * Construct a namespace permission. - * @param namespace namespace's name - * @param actionCode assigned actions - */ - public NamespacePermission(String namespace, byte[] actionCode) { - super(actionCode); - this.namespace = namespace; - this.scope = Scope.NAMESPACE; - } - public String getNamespace() { return namespace; } 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 e80cd33..bd5ff53 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 @@ -21,8 +21,10 @@ package org.apache.hadoop.hbase.security.access; import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.EnumSet; +import java.util.List; import java.util.Map; import java.util.Objects; @@ -275,7 +277,7 @@ public class Permission extends VersionedWritable { private TableName tableName; private byte[] family; private byte[] qualifier; - private Action[] actions; + private List actions = new ArrayList<>(); private Builder() { } @@ -301,17 +303,37 @@ public class Permission extends VersionedWritable { } public Builder withActions(Action... actions) { - this.actions = actions; + for (Action action : actions) { + if (action != null) { + this.actions.add(action); + } + } + return this; + } + + public Builder withActions(byte[] actionCodes) { + if (actionCodes != null) { + for (byte code : actionCodes) { + Action action = ACTION_BY_CODE.get(code); + if (action == null) { + LOG.error("Ignoring unknown action code '{}'", + Bytes.toStringBinary(new byte[] { code })); + continue; + } + this.actions.add(action); + } + } return this; } public Permission build() { + Action[] actionArray = actions.toArray(new Action[actions.size()]); if (namespace != null) { - return new NamespacePermission(namespace, actions); + return new NamespacePermission(namespace, actionArray); } else if (tableName != null) { - return new TablePermission(tableName, family, qualifier, actions); + return new TablePermission(tableName, family, qualifier, actionArray); } else { - return new GlobalPermission(actions); + return new GlobalPermission(actionArray); } } } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/ShadedAccessControlUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/ShadedAccessControlUtil.java index 67fdba3..f83d357 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/ShadedAccessControlUtil.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/ShadedAccessControlUtil.java @@ -122,8 +122,8 @@ public class ShadedAccessControlUtil { if (proto.getType() == AccessControlProtos.Permission.Type.Global) { AccessControlProtos.GlobalPermission perm = proto.getGlobalPermission(); List actions = toPermissionActions(perm.getActionList()); - - return new GlobalPermission(actions.toArray(new Permission.Action[actions.size()])); + return Permission.newBuilder() + .withActions(actions.toArray(new Permission.Action[actions.size()])).build(); } if (proto.getType() == AccessControlProtos.Permission.Type.Namespace) { AccessControlProtos.NamespacePermission perm = proto.getNamespacePermission(); @@ -133,7 +133,8 @@ public class ShadedAccessControlUtil { throw new IllegalStateException("Namespace must not be empty in NamespacePermission"); } String ns = perm.getNamespaceName().toStringUtf8(); - return new NamespacePermission(ns, actions.toArray(new Permission.Action[actions.size()])); + return Permission.newBuilder(ns) + .withActions(actions.toArray(new Permission.Action[actions.size()])).build(); } if (proto.getType() == AccessControlProtos.Permission.Type.Table) { AccessControlProtos.TablePermission perm = proto.getTablePermission(); @@ -149,9 +150,8 @@ public class ShadedAccessControlUtil { if (perm.hasFamily()) family = perm.getFamily().toByteArray(); if (perm.hasQualifier()) qualifier = perm.getQualifier().toByteArray(); - - return new TablePermission(table, family, qualifier, - actions.toArray(new Permission.Action[actions.size()])); + return Permission.newBuilder(table).withFamily(family).withQualifier(qualifier) + .withActions(actions.toArray(new Permission.Action[actions.size()])).build(); } throw new IllegalStateException("Unrecognize Perm Type: " + proto.getType()); } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/TablePermission.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/TablePermission.java index 36ed8e4..272363d 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/TablePermission.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/TablePermission.java @@ -34,38 +34,13 @@ import org.apache.hadoop.hbase.util.Bytes; * given table. If the family property is null, it implies * full table access. */ -@InterfaceAudience.Private +@InterfaceAudience.Public public class TablePermission extends Permission { private TableName table; private byte[] family; private byte[] qualifier; - /** Nullary constructor for Writable, do not use */ - public TablePermission() { - super(); - this.scope = Scope.EMPTY; - } - - /** - * Construct a table permission. - * @param table table name - * @param assigned assigned actions - */ - public TablePermission(TableName table, Action... assigned) { - this(table, null, null, assigned); - } - - /** - * Construct a table:family permission. - * @param table table name - * @param family family name - * @param assigned assigned actions - */ - public TablePermission(TableName table, byte[] family, Action... assigned) { - this(table, family, null, assigned); - } - /** * Construct a table:family:qualifier permission. * @param table table name @@ -73,7 +48,7 @@ public class TablePermission extends Permission { * @param qualifier qualifier name * @param assigned assigned actions */ - public TablePermission(TableName table, byte[] family, byte[] qualifier, Action... assigned) { + protected TablePermission(TableName table, byte[] family, byte[] qualifier, Action... assigned) { super(assigned); this.table = table; this.family = family; @@ -81,48 +56,10 @@ public class TablePermission extends Permission { this.scope = Scope.TABLE; } - /** - * Construct a table permission. - * @param table table name - * @param actionCodes assigned actions - */ - public TablePermission(TableName table, byte[] actionCodes) { - this(table, null, null, actionCodes); - } - - /** - * Construct a table:family permission. - * @param table table name - * @param family family name - * @param actionCodes assigned actions - */ - public TablePermission(TableName table, byte[] family, byte[] actionCodes) { - this(table, family, null, actionCodes); - } - - /** - * Construct a table:family:qualifier permission. - * @param table table name - * @param family family name - * @param qualifier qualifier name - * @param actionCodes assigned actions - */ - public TablePermission(TableName table, byte[] family, byte[] qualifier, byte[] actionCodes) { - super(actionCodes); - this.table = table; - this.family = family; - this.qualifier = qualifier; - this.scope = Scope.TABLE; - } - public TableName getTableName() { return table; } - public void setTableName(TableName table) { - this.table = table; - } - public boolean hasFamily() { return family != null; } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/UserPermission.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/UserPermission.java index 2a9a109..896ba52 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/UserPermission.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/UserPermission.java @@ -20,102 +20,19 @@ package org.apache.hadoop.hbase.security.access; import java.util.Objects; -import org.apache.hadoop.hbase.TableName; import org.apache.yetus.audience.InterfaceAudience; /** * UserPermission consists of a user name and a permission. * Permission can be one of [Global, Namespace, Table] permission. */ -@InterfaceAudience.Private +@InterfaceAudience.Public public class UserPermission { private String user; private Permission permission; /** - * Construct a global user permission. - * @param user user name - * @param assigned assigned actions - */ - public UserPermission(String user, Permission.Action... assigned) { - this.user = user; - this.permission = new GlobalPermission(assigned); - } - - /** - * Construct a global user permission. - * @param user user name - * @param actionCode action codes - */ - public UserPermission(String user, byte[] actionCode) { - this.user = user; - this.permission = new GlobalPermission(actionCode); - } - - /** - * Construct a namespace user permission. - * @param user user name - * @param namespace namespace - * @param assigned assigned actions - */ - public UserPermission(String user, String namespace, Permission.Action... assigned) { - this.user = user; - this.permission = new NamespacePermission(namespace, assigned); - } - - /** - * Construct a table user permission. - * @param user user name - * @param tableName table name - * @param assigned assigned actions - */ - public UserPermission(String user, TableName tableName, Permission.Action... assigned) { - this.user = user; - this.permission = new TablePermission(tableName, assigned); - } - - /** - * Construct a table:family user permission. - * @param user user name - * @param tableName table name - * @param family family name of table - * @param assigned assigned actions - */ - public UserPermission(String user, TableName tableName, byte[] family, - Permission.Action... assigned) { - this(user, tableName, family, null, assigned); - } - - /** - * Construct a table:family:qualifier user permission. - * @param user user name - * @param tableName table name - * @param family family name of table - * @param qualifier qualifier name of table - * @param assigned assigned actions - */ - public UserPermission(String user, TableName tableName, byte[] family, byte[] qualifier, - Permission.Action... assigned) { - this.user = user; - this.permission = new TablePermission(tableName, family, qualifier, assigned); - } - - /** - * Construct a table:family:qualifier user permission. - * @param user user name - * @param tableName table name - * @param family family name of table - * @param qualifier qualifier name of table - * @param actionCodes assigned actions - */ - public UserPermission(String user, TableName tableName, byte[] family, byte[] qualifier, - byte[] actionCodes) { - this.user = user; - this.permission = new TablePermission(tableName, family, qualifier, actionCodes); - } - - /** * Construct a user permission given permission. * @param user user name * @param permission one of [Global, Namespace, Table] permission 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 b5bf3a4..c404725 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 @@ -637,8 +637,8 @@ public class AccessControlLists { return null; } - return new Pair<>(username, - new NamespacePermission(Bytes.toString(fromNamespaceEntry(entryName)), value)); + return new Pair<>(username, Permission + .newBuilder(Bytes.toString(fromNamespaceEntry(entryName))).withActions(value).build()); } // Handle global entry @@ -648,7 +648,7 @@ public class AccessControlLists { return null; } - return new Pair<>(username, new GlobalPermission(value)); + return new Pair<>(username, Permission.newBuilder().withActions(value).build()); } // Handle table entry @@ -681,8 +681,8 @@ public class AccessControlLists { } } - return new Pair<>(username, - new TablePermission(TableName.valueOf(entryName), permFamily, permQualifier, value)); + return new Pair<>(username, Permission.newBuilder(TableName.valueOf(entryName)) + .withFamily(permFamily).withQualifier(permQualifier).withActions(value).build()); } /* diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java index 2898a71..47c8b6d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java @@ -887,7 +887,7 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, if (owner == null) owner = getActiveUser(c).getShortName(); final UserPermission userPermission = new UserPermission(owner, - desc.getTableName(), Action.values()); + Permission.newBuilder(desc.getTableName()).withActions(Action.values()).build()); // switch to the real hbase master user for doing the RPC on the ACL table User.runAsLoginUser(new PrivilegedExceptionAction() { @Override @@ -990,7 +990,7 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, @Override public Void run() throws Exception { UserPermission userperm = new UserPermission(owner, - currentDesc.getTableName(), Action.values()); + Permission.newBuilder(currentDesc.getTableName()).withActions(Action.values()).build()); try (Table table = c.getEnvironment().getConnection(). getTable(AccessControlLists.ACL_TABLE_NAME)) { AccessControlLists.addUserPermission(conf, userperm, table); @@ -2211,7 +2211,8 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, // them. Also using acl as table name to be inline with the results of global admin and // will help in avoiding any leakage of information about being superusers. for (String user : Superusers.getSuperUsers()) { - perms.add(new UserPermission(user, Action.values())); + perms.add(new UserPermission(user, + Permission.newBuilder().withActions(Action.values()).build())); } } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/RestoreSnapshotHelper.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/RestoreSnapshotHelper.java index 0acfb1a..ec251b8 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/RestoreSnapshotHelper.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/RestoreSnapshotHelper.java @@ -865,10 +865,8 @@ public class RestoreSnapshotHelper { for (Entry e : perms.entries()) { String user = e.getKey(); TablePermission tablePerm = (TablePermission) e.getValue(); - TablePermission newPerm = new TablePermission(newTableName, - tablePerm.getFamily(), tablePerm.getQualifier(), tablePerm.getActions()); - AccessControlClient.grant(conn, newPerm.getTableName(), user, newPerm.getFamily(), - newPerm.getQualifier(), newPerm.getActions()); + AccessControlClient.grant(conn, newTableName, user, tablePerm.getFamily(), + tablePerm.getQualifier(), tablePerm.getActions()); } } catch (Throwable e) { throw new IOException("Grant acl into newly creatd table failed. snapshot: " + snapshot 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 cee34af..9270614 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 @@ -1171,8 +1171,9 @@ public class TestAccessController extends SecureTestUtil { @Override public Object run() throws Exception { try (Connection conn = ConnectionFactory.createConnection(conf)) { - conn.getAdmin().grant(USER_RO.getShortName(), - new TablePermission(TEST_TABLE, TEST_FAMILY, Action.READ), false); + conn.getAdmin().grant(USER_RO.getShortName(), Permission.newBuilder(TEST_TABLE) + .withFamily(TEST_FAMILY).withActions(Action.READ).build(), + false); } return null; } @@ -1221,8 +1222,8 @@ public class TestAccessController extends SecureTestUtil { @Override public Object run() throws Exception { ACCESS_CONTROLLER.preGrant(ObserverContextImpl.createAndPrepare(CP_ENV), - new UserPermission(USER_RO.getShortName(), - new TablePermission(TEST_TABLE, TEST_FAMILY, Action.READ)), + new UserPermission(USER_RO.getShortName(), Permission.newBuilder(TEST_TABLE) + .withFamily(TEST_FAMILY).withActions(Action.READ).build()), false); return null; } @@ -1232,8 +1233,8 @@ public class TestAccessController extends SecureTestUtil { @Override public Object run() throws Exception { ACCESS_CONTROLLER.preRevoke(ObserverContextImpl.createAndPrepare(CP_ENV), - new UserPermission(USER_RO.getShortName(), - new TablePermission(TEST_TABLE, TEST_FAMILY, Action.READ))); + new UserPermission(USER_RO.getShortName(), Permission.newBuilder(TEST_TABLE) + .withFamily(TEST_FAMILY).withActions(Action.READ).build())); return null; } }; @@ -1692,8 +1693,8 @@ public class TestAccessController extends SecureTestUtil { acl.close(); } - UserPermission ownerperm = - new UserPermission(USER_OWNER.getName(), tableName, Action.values()); + UserPermission ownerperm = new UserPermission(USER_OWNER.getName(), + Permission.newBuilder(tableName).withActions(Action.values()).build()); assertTrue("Owner should have all permissions on table", hasFoundUserPermission(ownerperm, perms)); @@ -1701,7 +1702,8 @@ public class TestAccessController extends SecureTestUtil { String userName = user.getShortName(); UserPermission up = - new UserPermission(userName, tableName, family1, qualifier, Permission.Action.READ); + new UserPermission(userName, Permission.newBuilder(tableName).withFamily(family1) + .withQualifier(qualifier).withActions(Permission.Action.READ).build()); assertFalse("User should not be granted permission: " + up.toString(), hasFoundUserPermission(up, perms)); @@ -1720,12 +1722,13 @@ public class TestAccessController extends SecureTestUtil { } UserPermission upToVerify = - new UserPermission(userName, tableName, family1, qualifier, Permission.Action.READ); + new UserPermission(userName, Permission.newBuilder(tableName).withFamily(family1) + .withQualifier(qualifier).withActions(Permission.Action.READ).build()); assertTrue("User should be granted permission: " + upToVerify.toString(), hasFoundUserPermission(upToVerify, perms)); - upToVerify = - new UserPermission(userName, tableName, family1, qualifier, Permission.Action.WRITE); + upToVerify = new UserPermission(userName, Permission.newBuilder(tableName).withFamily(family1) + .withQualifier(qualifier).withActions(Permission.Action.WRITE).build()); assertFalse("User should not be granted permission: " + upToVerify.toString(), hasFoundUserPermission(upToVerify, perms)); @@ -1743,9 +1746,9 @@ public class TestAccessController extends SecureTestUtil { acl.close(); } - upToVerify = - new UserPermission(userName, tableName, family1, qualifier, Permission.Action.WRITE, - Permission.Action.READ); + upToVerify = new UserPermission(userName, + Permission.newBuilder(tableName).withFamily(family1).withQualifier(qualifier) + .withActions(Permission.Action.WRITE, Permission.Action.READ).build()); assertTrue("User should be granted permission: " + upToVerify.toString(), hasFoundUserPermission(upToVerify, perms)); @@ -1783,8 +1786,8 @@ public class TestAccessController extends SecureTestUtil { acl.close(); } - UserPermission newOwnerperm = - new UserPermission(newOwner.getName(), tableName, Action.values()); + UserPermission newOwnerperm = new UserPermission(newOwner.getName(), + Permission.newBuilder(tableName).withActions(Action.values()).build()); assertTrue("New owner should have all permissions on table", hasFoundUserPermission(newOwnerperm, perms)); } finally { @@ -1808,10 +1811,12 @@ public class TestAccessController extends SecureTestUtil { Collection superUsers = Superusers.getSuperUsers(); List adminPerms = new ArrayList<>(superUsers.size() + 1); - adminPerms.add(new UserPermission(USER_ADMIN.getShortName(), Bytes.toBytes("ACRW"))); - for(String user: superUsers) { + adminPerms.add(new UserPermission(USER_ADMIN.getShortName(), Permission.newBuilder() + .withActions(Action.ADMIN, Action.CREATE, Action.READ, Action.WRITE).build())); + for (String user : superUsers) { // Global permission - adminPerms.add(new UserPermission(user, Action.values())); + adminPerms.add( + new UserPermission(user, Permission.newBuilder().withActions(Action.values()).build())); } assertTrue("Only super users, global users and user admin has permission on table hbase:acl " + "per setup", perms.size() == 5 + superUsers.size() && diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestPermissionBuilder.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestPermissionBuilder.java index 74a0c62..73fb44d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestPermissionBuilder.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestPermissionBuilder.java @@ -46,7 +46,7 @@ public class TestPermissionBuilder { assertEquals(0, permission.getActions().length); // check global permission with ADMIN action - permission = Permission.newBuilder().withActions(Action.ADMIN).build(); + permission = Permission.newBuilder().withActions(Bytes.toBytes("A")).build(); assertTrue(permission instanceof GlobalPermission); assertEquals(1, permission.getActions().length); assertTrue(permission.getActions()[0] == Action.ADMIN); @@ -57,8 +57,15 @@ public class TestPermissionBuilder { .withActions(Action.CREATE, Action.READ).build(); fail("Should throw NPE"); } catch (NullPointerException e) { - // catch NPE because set family but table name is null + // catch NPE because set qualifier but table name is null } + + permission = Permission.newBuilder().withActions(Bytes.toBytes("ACP")) + .withActions(Action.READ, Action.ADMIN).build(); + assertEquals(3, permission.getActions().length); + assertEquals(Action.READ, permission.getActions()[0]); + assertEquals(Action.CREATE, permission.getActions()[1]); + assertEquals(Action.ADMIN, permission.getActions()[2]); } @Test diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestTablePermissions.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestTablePermissions.java index 1c478b2..8363665 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestTablePermissions.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestTablePermissions.java @@ -143,14 +143,18 @@ public class TestTablePermissions { try (Connection connection = ConnectionFactory.createConnection(conf)) { // add some permissions addUserPermission(conf, - new UserPermission("george", TEST_TABLE, Permission.Action.READ, Permission.Action.WRITE), + new UserPermission("george", + Permission.newBuilder(TEST_TABLE) + .withActions(Permission.Action.READ, Permission.Action.WRITE).build()), connection.getTable(AccessControlLists.ACL_TABLE_NAME)); addUserPermission(conf, - new UserPermission("hubert", TEST_TABLE, Permission.Action.READ), + new UserPermission("hubert", + Permission.newBuilder(TEST_TABLE).withActions(Permission.Action.READ).build()), connection.getTable(AccessControlLists.ACL_TABLE_NAME)); addUserPermission(conf, - new UserPermission("humphrey", TEST_TABLE, TEST_FAMILY, TEST_QUALIFIER, - Permission.Action.READ), + new UserPermission("humphrey", + Permission.newBuilder(TEST_TABLE).withFamily(TEST_FAMILY).withQualifier(TEST_QUALIFIER) + .withActions(Permission.Action.READ).build()), connection.getTable(AccessControlLists.ACL_TABLE_NAME)); } // retrieve the same @@ -211,7 +215,8 @@ public class TestTablePermissions { try (Connection connection = ConnectionFactory.createConnection(conf); Table table = connection.getTable(AccessControlLists.ACL_TABLE_NAME)) { AccessControlLists.addUserPermission(conf, - new UserPermission("hubert", TEST_TABLE2, Permission.Action.READ, Permission.Action.WRITE), + new UserPermission("hubert", Permission.newBuilder(TEST_TABLE2) + .withActions(Permission.Action.READ, Permission.Action.WRITE).build()), table); } // check full load @@ -246,17 +251,24 @@ public class TestTablePermissions { Configuration conf = UTIL.getConfiguration(); try (Connection connection = ConnectionFactory.createConnection(conf)) { addUserPermission(conf, - new UserPermission("albert", TEST_TABLE, Permission.Action.READ), - connection.getTable(AccessControlLists.ACL_TABLE_NAME)); + new UserPermission("albert", + Permission.newBuilder(TEST_TABLE).withActions(Permission.Action.READ).build()), + connection.getTable(AccessControlLists.ACL_TABLE_NAME)); addUserPermission(conf, - new UserPermission("betty", TEST_TABLE, Permission.Action.READ, Permission.Action.WRITE), - connection.getTable(AccessControlLists.ACL_TABLE_NAME)); + new UserPermission("betty", + Permission.newBuilder(TEST_TABLE) + .withActions(Permission.Action.READ, Permission.Action.WRITE).build()), + connection.getTable(AccessControlLists.ACL_TABLE_NAME)); addUserPermission(conf, - new UserPermission("clark", TEST_TABLE, TEST_FAMILY, Permission.Action.READ), - connection.getTable(AccessControlLists.ACL_TABLE_NAME)); + new UserPermission("clark", + Permission.newBuilder(TEST_TABLE).withFamily(TEST_FAMILY) + .withActions(Permission.Action.READ).build()), + connection.getTable(AccessControlLists.ACL_TABLE_NAME)); addUserPermission(conf, - new UserPermission("dwight", TEST_TABLE, TEST_FAMILY, TEST_QUALIFIER, - Permission.Action.WRITE), connection.getTable(AccessControlLists.ACL_TABLE_NAME)); + new UserPermission("dwight", + Permission.newBuilder(TEST_TABLE).withFamily(TEST_FAMILY).withQualifier(TEST_QUALIFIER) + .withActions(Permission.Action.WRITE).build()), + connection.getTable(AccessControlLists.ACL_TABLE_NAME)); } // verify permissions survive changes in table metadata ListMultimap preperms = @@ -302,17 +314,16 @@ public class TestTablePermissions { private ListMultimap createPermissions() { ListMultimap permissions = ArrayListMultimap.create(); - permissions.put("george", - new UserPermission("george", TEST_TABLE, Permission.Action.READ)); - permissions.put("george", - new UserPermission("george", TEST_TABLE, TEST_FAMILY, Permission.Action.WRITE)); - permissions.put("george", - new UserPermission("george", TEST_TABLE2, Permission.Action.READ)); - permissions.put("hubert", - new UserPermission("hubert", TEST_TABLE2, Permission.Action.READ, - Permission.Action.WRITE)); - permissions.put("bruce", - new UserPermission("bruce", TEST_NAMESPACE, Permission.Action.READ)); + permissions.put("george", new UserPermission("george", + Permission.newBuilder(TEST_TABLE).withActions(Permission.Action.READ).build())); + permissions.put("george", new UserPermission("george", Permission.newBuilder(TEST_TABLE) + .withFamily(TEST_FAMILY).withActions(Permission.Action.WRITE).build())); + permissions.put("george", new UserPermission("george", + Permission.newBuilder(TEST_TABLE2).withActions(Permission.Action.READ).build())); + permissions.put("hubert", new UserPermission("hubert", Permission.newBuilder(TEST_TABLE2) + .withActions(Permission.Action.READ, Permission.Action.WRITE).build())); + permissions.put("bruce", new UserPermission("bruce", + Permission.newBuilder(TEST_NAMESPACE).withActions(Permission.Action.READ).build())); return permissions; } @@ -334,50 +345,58 @@ public class TestTablePermissions { @Test public void testEquals() throws Exception { - Permission p1 = new TablePermission(TEST_TABLE, Permission.Action.READ); - Permission p2 = new TablePermission(TEST_TABLE, Permission.Action.READ); + Permission p1 = Permission.newBuilder(TEST_TABLE).withActions(Permission.Action.READ).build(); + Permission p2 = Permission.newBuilder(TEST_TABLE).withActions(Permission.Action.READ).build(); assertTrue(p1.equals(p2)); assertTrue(p2.equals(p1)); - p1 = new TablePermission(TEST_TABLE, TablePermission.Action.READ, TablePermission.Action.WRITE); - p2 = new TablePermission(TEST_TABLE, TablePermission.Action.WRITE, TablePermission.Action.READ); + p1 = Permission.newBuilder(TEST_TABLE) + .withActions(TablePermission.Action.READ, TablePermission.Action.WRITE).build(); + p2 = Permission.newBuilder(TEST_TABLE) + .withActions(TablePermission.Action.WRITE, TablePermission.Action.READ).build(); assertTrue(p1.equals(p2)); assertTrue(p2.equals(p1)); - p1 = new TablePermission(TEST_TABLE, TEST_FAMILY, TablePermission.Action.READ, TablePermission.Action.WRITE); - p2 = new TablePermission(TEST_TABLE, TEST_FAMILY, TablePermission.Action.WRITE, TablePermission.Action.READ); + p1 = Permission.newBuilder(TEST_TABLE).withFamily(TEST_FAMILY) + .withActions(TablePermission.Action.READ, TablePermission.Action.WRITE).build(); + p2 = Permission.newBuilder(TEST_TABLE).withFamily(TEST_FAMILY) + .withActions(TablePermission.Action.WRITE, TablePermission.Action.READ).build(); assertTrue(p1.equals(p2)); assertTrue(p2.equals(p1)); - p1 = new TablePermission(TEST_TABLE, TEST_FAMILY, TEST_QUALIFIER, TablePermission.Action.READ, TablePermission.Action.WRITE); - p2 = new TablePermission(TEST_TABLE, TEST_FAMILY, TEST_QUALIFIER, TablePermission.Action.WRITE, TablePermission.Action.READ); + p1 = Permission.newBuilder(TEST_TABLE).withFamily(TEST_FAMILY).withQualifier(TEST_QUALIFIER) + .withActions(TablePermission.Action.READ, TablePermission.Action.WRITE).build(); + p2 = Permission.newBuilder(TEST_TABLE).withFamily(TEST_FAMILY).withQualifier(TEST_QUALIFIER) + .withActions(TablePermission.Action.WRITE, TablePermission.Action.READ).build(); assertTrue(p1.equals(p2)); assertTrue(p2.equals(p1)); - p1 = new TablePermission(TEST_TABLE, TablePermission.Action.READ); - p2 = new TablePermission(TEST_TABLE, TEST_FAMILY, TablePermission.Action.READ); + p1 = Permission.newBuilder(TEST_TABLE).withActions(TablePermission.Action.READ).build(); + p2 = Permission.newBuilder(TEST_TABLE).withFamily(TEST_FAMILY) + .withActions(TablePermission.Action.READ).build(); assertFalse(p1.equals(p2)); assertFalse(p2.equals(p1)); - p1 = new TablePermission(TEST_TABLE, TablePermission.Action.READ); - p2 = new TablePermission(TEST_TABLE, TablePermission.Action.WRITE); + p1 = Permission.newBuilder(TEST_TABLE).withActions(TablePermission.Action.READ).build(); + p2 = Permission.newBuilder(TEST_TABLE).withActions(TablePermission.Action.WRITE).build(); assertFalse(p1.equals(p2)); assertFalse(p2.equals(p1)); - p2 = new TablePermission(TEST_TABLE, TablePermission.Action.READ, TablePermission.Action.WRITE); + p2 = Permission.newBuilder(TEST_TABLE) + .withActions(TablePermission.Action.READ, TablePermission.Action.WRITE).build(); assertFalse(p1.equals(p2)); assertFalse(p2.equals(p1)); - p1 = new TablePermission(TEST_TABLE, TablePermission.Action.READ); - p2 = new TablePermission(TEST_TABLE2, TablePermission.Action.READ); + p1 = Permission.newBuilder(TEST_TABLE).withActions(TablePermission.Action.READ).build(); + p2 = Permission.newBuilder(TEST_TABLE2).withActions(TablePermission.Action.READ).build(); assertFalse(p1.equals(p2)); assertFalse(p2.equals(p1)); - p1 = new NamespacePermission(TEST_NAMESPACE, TablePermission.Action.READ); - p2 = new NamespacePermission(TEST_NAMESPACE, TablePermission.Action.READ); + p1 = Permission.newBuilder(TEST_NAMESPACE).withActions(TablePermission.Action.READ).build(); + p2 = Permission.newBuilder(TEST_NAMESPACE).withActions(TablePermission.Action.READ).build(); assertEquals(p1, p2); - p1 = new NamespacePermission(TEST_NAMESPACE, TablePermission.Action.READ); - p2 = new NamespacePermission(TEST_NAMESPACE2, TablePermission.Action.READ); + p1 = Permission.newBuilder(TEST_NAMESPACE).withActions(TablePermission.Action.READ).build(); + p2 = Permission.newBuilder(TEST_NAMESPACE2).withActions(TablePermission.Action.READ).build(); assertFalse(p1.equals(p2)); assertFalse(p2.equals(p1)); } @@ -389,15 +408,20 @@ public class TestTablePermissions { // add some permissions try (Connection connection = ConnectionFactory.createConnection(conf)) { addUserPermission(conf, - new UserPermission("user1", - Permission.Action.READ, Permission.Action.WRITE), connection.getTable(AccessControlLists.ACL_TABLE_NAME)); + new UserPermission("user1", Permission.newBuilder() + .withActions(Permission.Action.READ, Permission.Action.WRITE).build()), + connection.getTable(AccessControlLists.ACL_TABLE_NAME)); addUserPermission(conf, - new UserPermission("user2", - Permission.Action.CREATE), connection.getTable(AccessControlLists.ACL_TABLE_NAME)); + new UserPermission("user2", + Permission.newBuilder().withActions(Permission.Action.CREATE).build()), + connection.getTable(AccessControlLists.ACL_TABLE_NAME)); addUserPermission(conf, - new UserPermission("user3", - Permission.Action.ADMIN, Permission.Action.READ, Permission.Action.CREATE), - connection.getTable(AccessControlLists.ACL_TABLE_NAME)); + new UserPermission("user3", + Permission.newBuilder() + .withActions(Permission.Action.ADMIN, Permission.Action.READ, + Permission.Action.CREATE) + .build()), + connection.getTable(AccessControlLists.ACL_TABLE_NAME)); } ListMultimap perms = AccessControlLists.getTablePermissions(conf, null); @@ -434,9 +458,13 @@ public class TestTablePermissions { User currentUser = User.getCurrent(); assertTrue(authManager.authorizeUserGlobal(currentUser, Permission.Action.ADMIN)); try (Connection connection = ConnectionFactory.createConnection(conf)) { - for (int i=1; i<=50; i++) { - addUserPermission(conf, new UserPermission("testauth"+i, - Permission.Action.ADMIN, Permission.Action.READ, Permission.Action.WRITE), + for (int i = 1; i <= 50; i++) { + addUserPermission(conf, + new UserPermission("testauth" + i, + Permission.newBuilder() + .withActions(Permission.Action.ADMIN, Permission.Action.READ, + Permission.Action.WRITE) + .build()), connection.getTable(AccessControlLists.ACL_TABLE_NAME)); // make sure the system user still shows as authorized assertTrue("Failed current user auth check on iter "+i, diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java index 67c43ee..bc59f70 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java @@ -381,11 +381,12 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { AccessTestAction checkMultiQualifierRead = new AccessTestAction() { @Override public Void run() throws Exception { - checkTablePerms(TEST_UTIL, TEST_TABLE.getTableName(), new Permission[] { - new TablePermission(TEST_TABLE.getTableName(), TEST_FAMILY, TEST_Q1, - Permission.Action.READ), - new TablePermission(TEST_TABLE.getTableName(), TEST_FAMILY, TEST_Q2, - Permission.Action.READ), }); + checkTablePerms(TEST_UTIL, TEST_TABLE.getTableName(), + new Permission[] { + Permission.newBuilder(TEST_TABLE.getTableName()).withFamily(TEST_FAMILY) + .withQualifier(TEST_Q1).withActions(Action.READ).build(), + Permission.newBuilder(TEST_TABLE.getTableName()).withFamily(TEST_FAMILY) + .withQualifier(TEST_Q2).withActions(Action.READ).build() }); return null; } }; @@ -397,11 +398,14 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { AccessTestAction checkMultiQualifierReadWrite = new AccessTestAction() { @Override public Void run() throws Exception { - checkTablePerms(TEST_UTIL, TEST_TABLE.getTableName(), new Permission[] { - new TablePermission(TEST_TABLE.getTableName(), TEST_FAMILY, TEST_Q1, - Permission.Action.READ, Permission.Action.WRITE), - new TablePermission(TEST_TABLE.getTableName(), TEST_FAMILY, TEST_Q2, - Permission.Action.READ, Permission.Action.WRITE), }); + checkTablePerms(TEST_UTIL, TEST_TABLE.getTableName(), + new Permission[] { + Permission.newBuilder(TEST_TABLE.getTableName()).withFamily(TEST_FAMILY) + .withQualifier(TEST_Q1) + .withActions(Permission.Action.READ, Permission.Action.WRITE).build(), + Permission.newBuilder(TEST_TABLE.getTableName()).withFamily(TEST_FAMILY) + .withQualifier(TEST_Q2) + .withActions(Permission.Action.READ, Permission.Action.WRITE).build() }); return null; } }; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestZKPermissionWatcher.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestZKPermissionWatcher.java index cfd6512..327c49d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestZKPermissionWatcher.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestZKPermissionWatcher.java @@ -113,8 +113,8 @@ public class TestZKPermissionWatcher { // update ACL: george RW List acl = new ArrayList<>(1); - acl.add(new UserPermission(george.getShortName(), TEST_TABLE, - Permission.Action.READ, Permission.Action.WRITE)); + acl.add(new UserPermission(george.getShortName(), Permission.newBuilder(TEST_TABLE) + .withActions(Permission.Action.READ, Permission.Action.WRITE).build())); ListMultimap multimap = ArrayListMultimap.create(); multimap.putAll(george.getShortName(), acl); byte[] serialized = AccessControlLists.writePermissionsAsBytes(multimap, conf); @@ -141,7 +141,8 @@ public class TestZKPermissionWatcher { // update ACL: hubert R List acl2 = new ArrayList<>(1); - acl2.add(new UserPermission(hubert.getShortName(), TEST_TABLE, TablePermission.Action.READ)); + acl2.add(new UserPermission(hubert.getShortName(), + Permission.newBuilder(TEST_TABLE).withActions(TablePermission.Action.READ).build())); final long mtimeA = AUTH_A.getMTime(); multimap.putAll(hubert.getShortName(), acl2); byte[] serialized2 = AccessControlLists.writePermissionsAsBytes(multimap, conf); -- 2.7.4