Return-Path: X-Original-To: apmail-accumulo-commits-archive@www.apache.org Delivered-To: apmail-accumulo-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id CF92010E9A for ; Thu, 30 Jan 2014 21:58:06 +0000 (UTC) Received: (qmail 44613 invoked by uid 500); 30 Jan 2014 21:57:57 -0000 Delivered-To: apmail-accumulo-commits-archive@accumulo.apache.org Received: (qmail 44507 invoked by uid 500); 30 Jan 2014 21:57:54 -0000 Mailing-List: contact commits-help@accumulo.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@accumulo.apache.org Delivered-To: mailing list commits@accumulo.apache.org Received: (qmail 44453 invoked by uid 99); 30 Jan 2014 21:57:53 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 30 Jan 2014 21:57:53 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 04B029167AC; Thu, 30 Jan 2014 21:57:53 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: vines@apache.org To: commits@accumulo.apache.org Date: Thu, 30 Jan 2014 21:57:55 -0000 Message-Id: <30d86ee2a1cf4110afe46a81662f3ae7@git.apache.org> In-Reply-To: <04ca4de1252542f7b54b78fd5a733bc3@git.apache.org> References: <04ca4de1252542f7b54b78fd5a733bc3@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [04/10] git commit: ACCUMULO-1479 Simplification of namespace support in SecurityOperation ACCUMULO-1479 Simplification of namespace support in SecurityOperation Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/b80e1a4f Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/b80e1a4f Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/b80e1a4f Branch: refs/heads/master Commit: b80e1a4f9c9b180c9862964a9042b9ad70e1d9e7 Parents: ca6c82b Author: John Vines Authored: Tue Jan 28 16:26:08 2014 -0500 Committer: John Vines Committed: Thu Jan 30 11:51:32 2014 -0500 ---------------------------------------------------------------------- .../core/security/NamespacePermission.java | 67 +++++-- .../security/AuditedSecurityOperation.java | 4 +- .../server/security/SecurityOperation.java | 177 ++++++++----------- .../accumulo/master/FateServiceHandler.java | 12 +- .../accumulo/master/tableOps/CreateTable.java | 8 +- .../test/randomwalk/security/CreateTable.java | 2 +- .../org/apache/accumulo/test/NamespacesIT.java | 3 +- 7 files changed, 149 insertions(+), 124 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/accumulo/blob/b80e1a4f/core/src/main/java/org/apache/accumulo/core/security/NamespacePermission.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/accumulo/core/security/NamespacePermission.java b/core/src/main/java/org/apache/accumulo/core/security/NamespacePermission.java index 1066bc4..f9f7564 100644 --- a/core/src/main/java/org/apache/accumulo/core/security/NamespacePermission.java +++ b/core/src/main/java/org/apache/accumulo/core/security/NamespacePermission.java @@ -23,21 +23,20 @@ import java.util.List; * Accumulo namespace permissions. Each permission has an associated byte ID. */ public enum NamespacePermission { - /* - * One may add new permissions, but new permissions must use new numbers. - * Current numbers in use must not be changed. - */ + // One may add new permissions, but new permissions must use new numbers. Current numbers in use must not be changed. READ((byte) 0), WRITE((byte) 1), ALTER_NAMESPACE((byte) 2), GRANT((byte) 3), ALTER_TABLE((byte) 4), CREATE_TABLE((byte) 5), - DROP_TABLE((byte) 6); + DROP_TABLE((byte) 6), + BULK_IMPORT((byte) 7), + DROP_NAMESPACE((byte) 8); final private byte permID; - final private static NamespacePermission mapping[] = new NamespacePermission[8]; + final private static NamespacePermission mapping[] = new NamespacePermission[9]; static { for (NamespacePermission perm : NamespacePermission.values()) mapping[perm.permID] = perm; @@ -49,7 +48,7 @@ public enum NamespacePermission { /** * Gets the byte ID of this permission. - * + * * @return byte ID */ public byte getId() { @@ -58,7 +57,7 @@ public enum NamespacePermission { /** * Returns a list of printable permission values. - * + * * @return list of namespace permission values, as "Namespace." + permission name */ public static List printableValues() { @@ -74,10 +73,12 @@ public enum NamespacePermission { /** * Gets the permission matching the given byte ID. - * - * @param id byte ID + * + * @param id + * byte ID * @return system permission - * @throws IndexOutOfBoundsException if the byte ID is invalid + * @throws IndexOutOfBoundsException + * if the byte ID is invalid */ public static NamespacePermission getPermissionById(byte id) { NamespacePermission result = mapping[id]; @@ -86,4 +87,48 @@ public enum NamespacePermission { throw new IndexOutOfBoundsException("No such permission"); } + public static NamespacePermission getEquivalent(TablePermission permission) { + switch (permission) { + case READ: + return NamespacePermission.READ; + case WRITE: + return NamespacePermission.WRITE; + case ALTER_TABLE: + return NamespacePermission.ALTER_TABLE; + case GRANT: + return NamespacePermission.GRANT; + case DROP_TABLE: + return NamespacePermission.DROP_TABLE; + case BULK_IMPORT: + return NamespacePermission.BULK_IMPORT; + default: + return null; + } + + } + + public static NamespacePermission getEquivalent(SystemPermission permission) { + switch (permission) { + case CREATE_TABLE: + return NamespacePermission.CREATE_TABLE; + case DROP_TABLE: + return NamespacePermission.DROP_TABLE; + case ALTER_TABLE: + return NamespacePermission.ALTER_TABLE; + case ALTER_NAMESPACE: + return NamespacePermission.ALTER_NAMESPACE; + case DROP_NAMESPACE: + return NamespacePermission.DROP_NAMESPACE; + case GRANT: + return NamespacePermission.ALTER_NAMESPACE; + case CREATE_NAMESPACE: + case CREATE_USER: + case DROP_USER: + case ALTER_USER: + case SYSTEM: + default: + return null; + } + } + } http://git-wip-us.apache.org/repos/asf/accumulo/blob/b80e1a4f/server/base/src/main/java/org/apache/accumulo/server/security/AuditedSecurityOperation.java ---------------------------------------------------------------------- diff --git a/server/base/src/main/java/org/apache/accumulo/server/security/AuditedSecurityOperation.java b/server/base/src/main/java/org/apache/accumulo/server/security/AuditedSecurityOperation.java index bbfa71b..07492c6 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/security/AuditedSecurityOperation.java +++ b/server/base/src/main/java/org/apache/accumulo/server/security/AuditedSecurityOperation.java @@ -233,9 +233,9 @@ public class AuditedSecurityOperation extends SecurityOperation { public static final String CAN_CREATE_TABLE_AUDIT_TEMPLATE = "action: createTable; targetTable: %s;"; @Override - public boolean canCreateTable(TCredentials c, String tableName) throws ThriftSecurityException { + public boolean canCreateTable(TCredentials c, String tableName, String namespaceId) throws ThriftSecurityException { try { - boolean result = super.canCreateTable(c, tableName); + boolean result = super.canCreateTable(c, tableName, namespaceId); audit(c, result, CAN_CREATE_TABLE_AUDIT_TEMPLATE, tableName); return result; } catch (ThriftSecurityException ex) { http://git-wip-us.apache.org/repos/asf/accumulo/blob/b80e1a4f/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java ---------------------------------------------------------------------- diff --git a/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java b/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java index 4b302f0..ad1fbc0 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java +++ b/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java @@ -242,15 +242,41 @@ public class SecurityOperation { } } + private boolean hasSystemPermission(TCredentials credentials, SystemPermission permission, boolean useCached) throws ThriftSecurityException { + return hasSystemPermissionWithNamespaceId(credentials, permission, null, useCached); + } + + private boolean hasSystemPermissionWithTableId(TCredentials credentials, SystemPermission permission, String tableId, boolean useCached) + throws ThriftSecurityException { + if (isSystemUser(credentials)) + return true; + String namespaceId = null; + try { + namespaceId = Namespaces.getNamespaceId(HdfsZooInstance.getInstance(), Tables.getNamespace(HdfsZooInstance.getInstance(), tableId)); + } catch (NamespaceNotFoundException nnfe) { + // Don't care, we won't pay any attention to namespace permissions + } + + return hasSystemPermissionWithNamespaceId(credentials, permission, namespaceId, useCached); + } + /** * Checks if a user has a system permission * * @return true if a user exists and has permission; false otherwise */ - private boolean hasSystemPermission(TCredentials credentials, SystemPermission permission, boolean useCached) throws ThriftSecurityException { + private boolean hasSystemPermissionWithNamespaceId(TCredentials credentials, SystemPermission permission, String namespaceId, boolean useCached) + throws ThriftSecurityException { if (isSystemUser(credentials)) return true; - return _hasSystemPermission(credentials.getPrincipal(), permission, useCached); + + if (_hasSystemPermission(credentials.getPrincipal(), permission, useCached)) + return true; + if (namespaceId != null) { + return _hasNamespacePermission(credentials.getPrincipal(), namespaceId, NamespacePermission.getEquivalent(permission), useCached); + } + + return false; } /** @@ -282,7 +308,9 @@ public class SecurityOperation { protected boolean hasTablePermission(TCredentials credentials, String table, TablePermission permission, boolean useCached) throws ThriftSecurityException { if (isSystemUser(credentials)) return true; - return _hasTablePermission(credentials.getPrincipal(), table, permission, useCached); + return _hasTablePermission(credentials.getPrincipal(), table, permission, useCached) + || _hasNamespacePermission(credentials.getPrincipal(), Tables.getNamespace(HdfsZooInstance.getInstance(), table), + NamespacePermission.getEquivalent(permission), useCached); } /** @@ -309,51 +337,15 @@ public class SecurityOperation { } /** - * Checks if a user has a namespace permission - * - * @return true if a user exists and has permission; false otherwise - */ - protected boolean hasNamespacePermission(TCredentials credentials, String namespace, NamespacePermission permission, boolean useCached) - throws ThriftSecurityException { - if (isSystemUser(credentials)) - return true; - return _hasNamespacePermission(credentials.getPrincipal(), namespace, permission, useCached); - } - - /** - * Checks if a user has a namespace permission given a tableId - * - * @return true if a user exists and has permission; false otherwise - */ - protected boolean hasNamespacePermissionForTableId(TCredentials credentials, String tableId, NamespacePermission permission, boolean useCached) - throws ThriftSecurityException { - String namespace = Tables.getNamespace(HdfsZooInstance.getInstance(), tableId); - return hasNamespacePermission(credentials, namespace, permission, useCached); - } - - /** - * Checks if a user has a namespace permission given a tableName - * - * @return true if a user exists and has permission; false otherwise - */ - protected boolean hasNamespacePermissionForTableName(TCredentials credentials, String tableName, NamespacePermission permission, boolean useCached) - throws ThriftSecurityException { - String namespace = Tables.qualify(tableName).getFirst(); - try { - String namespaceId = Namespaces.getNamespaceId(HdfsZooInstance.getInstance(), namespace); - return hasNamespacePermission(credentials, namespaceId, permission, useCached); - } catch (NamespaceNotFoundException e) { - throw new ThriftSecurityException(credentials.getPrincipal(), SecurityErrorCode.NAMESPACE_DOESNT_EXIST); - } - } - - /** * Checks if a user has a namespace permission
* This cannot check if a system user has permission. * * @return true if a user exists and has permission; false otherwise */ protected boolean _hasNamespacePermission(String user, String namespace, NamespacePermission permission, boolean useCached) throws ThriftSecurityException { + if (permission == null) + return false; + targetUserExists(user); if (namespace.equals(Namespaces.ACCUMULO_NAMESPACE_ID) && permission.equals(NamespacePermission.READ)) @@ -391,8 +383,7 @@ public class SecurityOperation { public boolean canScan(TCredentials credentials, String table) throws ThriftSecurityException { authenticate(credentials); - return hasTablePermission(credentials, table, TablePermission.READ, true) - || hasNamespacePermissionForTableId(credentials, table, NamespacePermission.READ, true); + return hasTablePermission(credentials, table, TablePermission.READ, true); } public boolean canScan(TCredentials credentials, String table, TRange range, List columns, List ssiList, @@ -407,25 +398,21 @@ public class SecurityOperation { public boolean canWrite(TCredentials credentials, String table) throws ThriftSecurityException { authenticate(credentials); - return hasTablePermission(credentials, table, TablePermission.WRITE, true) - || hasNamespacePermissionForTableId(credentials, table, NamespacePermission.WRITE, true); + return hasTablePermission(credentials, table, TablePermission.WRITE, true); } public boolean canConditionallyUpdate(TCredentials credentials, String tableID, List authorizations) throws ThriftSecurityException { authenticate(credentials); - return (hasTablePermission(credentials, tableID, TablePermission.WRITE, true) || hasNamespacePermissionForTableId(credentials, tableID, - NamespacePermission.WRITE, true)) - && (hasTablePermission(credentials, tableID, TablePermission.READ, true) || hasNamespacePermissionForTableId(credentials, tableID, - NamespacePermission.READ, true)); + return hasTablePermission(credentials, tableID, TablePermission.WRITE, true) && hasTablePermission(credentials, tableID, TablePermission.READ, true); } - public boolean canSplitTablet(TCredentials credentials, String table) throws ThriftSecurityException { + public boolean canSplitTablet(TCredentials credentials, String tableId) throws ThriftSecurityException { authenticate(credentials); - return hasSystemPermission(credentials, SystemPermission.ALTER_TABLE, false) || hasSystemPermission(credentials, SystemPermission.SYSTEM, false) - || hasTablePermission(credentials, table, TablePermission.ALTER_TABLE, false) - || hasNamespacePermissionForTableId(credentials, table, NamespacePermission.ALTER_TABLE, false); + return hasSystemPermissionWithTableId(credentials, SystemPermission.ALTER_TABLE, tableId, false) + || hasSystemPermissionWithTableId(credentials, SystemPermission.SYSTEM, tableId, false) + || hasTablePermission(credentials, tableId, TablePermission.ALTER_TABLE, false); } /** @@ -438,64 +425,53 @@ public class SecurityOperation { public boolean canFlush(TCredentials c, String tableId) throws ThriftSecurityException { authenticate(c); - return hasTablePermission(c, tableId, TablePermission.WRITE, false) || hasTablePermission(c, tableId, TablePermission.ALTER_TABLE, false) - || hasNamespacePermissionForTableId(c, tableId, NamespacePermission.ALTER_TABLE, false) - || hasNamespacePermissionForTableId(c, tableId, NamespacePermission.WRITE, false); + return hasTablePermission(c, tableId, TablePermission.WRITE, false) || hasTablePermission(c, tableId, TablePermission.ALTER_TABLE, false); } public boolean canAlterTable(TCredentials c, String tableId) throws ThriftSecurityException { authenticate(c); - return hasTablePermission(c, tableId, TablePermission.ALTER_TABLE, false) || hasSystemPermission(c, SystemPermission.ALTER_TABLE, false) - || hasNamespacePermissionForTableId(c, tableId, NamespacePermission.ALTER_TABLE, false); + return hasTablePermission(c, tableId, TablePermission.ALTER_TABLE, false) + || hasSystemPermissionWithTableId(c, SystemPermission.ALTER_TABLE, tableId, false); } - public boolean canCreateTable(TCredentials c, String tableName) throws ThriftSecurityException { + public boolean canCreateTable(TCredentials c, String table, String namespaceId) throws ThriftSecurityException { authenticate(c); - return hasNamespacePermissionForTableName(c, tableName, NamespacePermission.CREATE_TABLE, false) || canCreateTable(c); - } - - public boolean canCreateTable(TCredentials c) throws ThriftSecurityException { - authenticate(c); - return hasSystemPermission(c, SystemPermission.CREATE_TABLE, false); + return hasSystemPermissionWithNamespaceId(c, SystemPermission.CREATE_TABLE, namespaceId, false); } public boolean canRenameTable(TCredentials c, String tableId, String oldTableName, String newTableName) throws ThriftSecurityException { authenticate(c); - return hasSystemPermission(c, SystemPermission.ALTER_TABLE, false) || hasTablePermission(c, tableId, TablePermission.ALTER_TABLE, false) - || hasNamespacePermissionForTableId(c, tableId, NamespacePermission.ALTER_TABLE, false); + return hasSystemPermissionWithTableId(c, SystemPermission.ALTER_TABLE, tableId, false) + || hasTablePermission(c, tableId, TablePermission.ALTER_TABLE, false); } public boolean canCloneTable(TCredentials c, String tableId, String tableName) throws ThriftSecurityException { authenticate(c); - return (hasSystemPermission(c, SystemPermission.CREATE_TABLE, false) || hasNamespacePermissionForTableName(c, tableName, NamespacePermission.CREATE_TABLE, - false)) - && (hasTablePermission(c, tableId, TablePermission.READ, false) || hasNamespacePermissionForTableId(c, tableId, NamespacePermission.READ, false)); + return hasSystemPermissionWithTableId(c, SystemPermission.CREATE_TABLE, tableId, false) && hasTablePermission(c, tableId, TablePermission.READ, false); } public boolean canDeleteTable(TCredentials c, String tableId) throws ThriftSecurityException { authenticate(c); - return hasSystemPermission(c, SystemPermission.DROP_TABLE, false) || hasTablePermission(c, tableId, TablePermission.DROP_TABLE, false) - || hasNamespacePermissionForTableId(c, tableId, NamespacePermission.DROP_TABLE, false); + return hasSystemPermissionWithTableId(c, SystemPermission.DROP_TABLE, tableId, false) || hasTablePermission(c, tableId, TablePermission.DROP_TABLE, false); } public boolean canOnlineOfflineTable(TCredentials c, String tableId, FateOperation op) throws ThriftSecurityException { authenticate(c); - return hasSystemPermission(c, SystemPermission.SYSTEM, false) || hasSystemPermission(c, SystemPermission.ALTER_TABLE, false) - || hasTablePermission(c, tableId, TablePermission.ALTER_TABLE, false) - || hasNamespacePermissionForTableId(c, tableId, NamespacePermission.ALTER_TABLE, false); + return hasSystemPermissionWithTableId(c, SystemPermission.SYSTEM, tableId, false) + || hasSystemPermissionWithTableId(c, SystemPermission.ALTER_TABLE, tableId, false) + || hasTablePermission(c, tableId, TablePermission.ALTER_TABLE, false); } public boolean canMerge(TCredentials c, String tableId) throws ThriftSecurityException { authenticate(c); - return hasSystemPermission(c, SystemPermission.SYSTEM, false) || hasSystemPermission(c, SystemPermission.ALTER_TABLE, false) - || hasTablePermission(c, tableId, TablePermission.ALTER_TABLE, false) - || hasNamespacePermissionForTableId(c, tableId, NamespacePermission.ALTER_TABLE, false); + return hasSystemPermissionWithTableId(c, SystemPermission.SYSTEM, tableId, false) + || hasSystemPermissionWithTableId(c, SystemPermission.ALTER_TABLE, tableId, false) + || hasTablePermission(c, tableId, TablePermission.ALTER_TABLE, false); } public boolean canDeleteRange(TCredentials c, String tableId, String tableName, Text startRow, Text endRow) throws ThriftSecurityException { authenticate(c); - return hasSystemPermission(c, SystemPermission.SYSTEM, false) || hasTablePermission(c, tableId, TablePermission.WRITE, false) - || hasNamespacePermissionForTableId(c, tableId, NamespacePermission.WRITE, false); + return hasSystemPermissionWithTableId(c, SystemPermission.SYSTEM, tableId, false) || hasTablePermission(c, tableId, TablePermission.WRITE, false); } public boolean canBulkImport(TCredentials c, String tableId, String tableName, String dir, String failDir) throws ThriftSecurityException { @@ -509,9 +485,8 @@ public class SecurityOperation { public boolean canCompact(TCredentials c, String tableId) throws ThriftSecurityException { authenticate(c); - return hasSystemPermission(c, SystemPermission.ALTER_TABLE, false) || hasTablePermission(c, tableId, TablePermission.ALTER_TABLE, false) - || hasTablePermission(c, tableId, TablePermission.WRITE, false) || hasNamespacePermissionForTableId(c, tableId, NamespacePermission.ALTER_TABLE, false) - || hasNamespacePermissionForTableId(c, tableId, NamespacePermission.WRITE, false); + return hasSystemPermissionWithTableId(c, SystemPermission.ALTER_TABLE, tableId, false) + || hasTablePermission(c, tableId, TablePermission.ALTER_TABLE, false) || hasTablePermission(c, tableId, TablePermission.WRITE, false); } public boolean canChangeAuthorizations(TCredentials c, String user) throws ThriftSecurityException { @@ -546,13 +521,21 @@ public class SecurityOperation { public boolean canGrantTable(TCredentials c, String user, String table) throws ThriftSecurityException { authenticate(c); - return hasSystemPermission(c, SystemPermission.ALTER_TABLE, false) || hasTablePermission(c, table, TablePermission.GRANT, false) - || hasNamespacePermissionForTableId(c, table, NamespacePermission.ALTER_TABLE, false); + return hasSystemPermissionWithTableId(c, SystemPermission.ALTER_TABLE, table, false) || hasTablePermission(c, table, TablePermission.GRANT, false); } public boolean canGrantNamespace(TCredentials c, String user, String namespace) throws ThriftSecurityException { + return canModifyNamespacePermission(c, user, namespace); + } + + private boolean canModifyNamespacePermission(TCredentials c, String user, String namespace) throws ThriftSecurityException { authenticate(c); - return hasSystemPermission(c, SystemPermission.ALTER_NAMESPACE, false) || hasNamespacePermission(c, namespace, NamespacePermission.GRANT, false); + // The one case where Table/SystemPermission -> NamespacePermission breaks down. The alternative is to make SystemPermission.ALTER_NAMESPACE provide + // NamespacePermission.GRANT & ALTER_NAMESPACE, but then it would cause some permission checks to succeed with GRANT when they shouldn't + + // This is a bit hackier then I (vines) wanted, but I think this one hackiness makes the overall SecurityOperations more succinct. + return hasSystemPermissionWithNamespaceId(c, SystemPermission.ALTER_NAMESPACE, namespace, false) + || hasNamespacePermission(c, c.principal, namespace, NamespacePermission.GRANT); } public boolean canRevokeSystem(TCredentials c, String user, SystemPermission sysPerm) throws ThriftSecurityException { @@ -570,13 +553,11 @@ public class SecurityOperation { public boolean canRevokeTable(TCredentials c, String user, String table) throws ThriftSecurityException { authenticate(c); - return hasSystemPermission(c, SystemPermission.ALTER_TABLE, false) || hasTablePermission(c, table, TablePermission.GRANT, false) - || hasNamespacePermissionForTableId(c, table, NamespacePermission.ALTER_TABLE, false); + return hasSystemPermissionWithTableId(c, SystemPermission.ALTER_TABLE, table, false) || hasTablePermission(c, table, TablePermission.GRANT, false); } public boolean canRevokeNamespace(TCredentials c, String user, String namespace) throws ThriftSecurityException { - authenticate(c); - return hasSystemPermission(c, SystemPermission.ALTER_NAMESPACE, false) || hasNamespacePermission(c, namespace, NamespacePermission.GRANT, false); + return canModifyNamespacePermission(c, user, namespace); } public void changeAuthorizations(TCredentials credentials, String user, Authorizations authorizations) throws ThriftSecurityException { @@ -787,20 +768,17 @@ public class SecurityOperation { public boolean canExport(TCredentials credentials, String tableId, String tableName, String exportDir) throws ThriftSecurityException { authenticate(credentials); - return hasTablePermission(credentials, tableId, TablePermission.READ, false) - || hasNamespacePermissionForTableId(credentials, tableId, NamespacePermission.READ, false); + return hasTablePermission(credentials, tableId, TablePermission.READ, false); } public boolean canImport(TCredentials credentials, String tableName, String importDir) throws ThriftSecurityException { authenticate(credentials); - return hasSystemPermission(credentials, SystemPermission.CREATE_TABLE, false) - || hasNamespacePermissionForTableName(credentials, tableName, NamespacePermission.CREATE_TABLE, false); + return hasSystemPermissionWithNamespaceId(credentials, SystemPermission.CREATE_TABLE, Tables.qualify(tableName).getFirst(), false); } public boolean canAlterNamespace(TCredentials credentials, String namespaceId) throws ThriftSecurityException { authenticate(credentials); - return hasNamespacePermission(credentials, namespaceId, NamespacePermission.ALTER_NAMESPACE, false) - || hasSystemPermission(credentials, SystemPermission.ALTER_NAMESPACE, false); + return hasSystemPermissionWithNamespaceId(credentials, SystemPermission.ALTER_NAMESPACE, namespaceId, false); } public boolean canCreateNamespace(TCredentials credentials, String namespace) throws ThriftSecurityException { @@ -815,13 +793,12 @@ public class SecurityOperation { public boolean canDeleteNamespace(TCredentials credentials, String namespaceId) throws ThriftSecurityException { authenticate(credentials); - return hasSystemPermission(credentials, SystemPermission.DROP_NAMESPACE, false); + return hasSystemPermissionWithNamespaceId(credentials, SystemPermission.DROP_NAMESPACE, namespaceId, false); } public boolean canRenameNamespace(TCredentials credentials, String namespaceId, String oldName, String newName) throws ThriftSecurityException { authenticate(credentials); - return hasNamespacePermission(credentials, namespaceId, NamespacePermission.ALTER_NAMESPACE, false) - || hasSystemPermission(credentials, SystemPermission.ALTER_NAMESPACE, false); + return hasSystemPermissionWithNamespaceId(credentials, SystemPermission.ALTER_NAMESPACE, namespaceId, false); } } http://git-wip-us.apache.org/repos/asf/accumulo/blob/b80e1a4f/server/master/src/main/java/org/apache/accumulo/master/FateServiceHandler.java ---------------------------------------------------------------------- diff --git a/server/master/src/main/java/org/apache/accumulo/master/FateServiceHandler.java b/server/master/src/main/java/org/apache/accumulo/master/FateServiceHandler.java index 3a14ca2..21a35de 100644 --- a/server/master/src/main/java/org/apache/accumulo/master/FateServiceHandler.java +++ b/server/master/src/main/java/org/apache/accumulo/master/FateServiceHandler.java @@ -58,6 +58,7 @@ import org.apache.accumulo.master.tableOps.RenameTable; import org.apache.accumulo.master.tableOps.TableRangeOp; import org.apache.accumulo.master.tableOps.TraceRepo; import org.apache.accumulo.server.client.ClientServiceHandler; +import org.apache.accumulo.server.client.HdfsZooInstance; import org.apache.accumulo.server.master.state.MergeInfo; import org.apache.accumulo.server.util.TablePropUtil; import org.apache.accumulo.trace.thrift.TInfo; @@ -126,14 +127,19 @@ class FateServiceHandler implements FateService.Iface { String tableName = validateTableNameArgument(arguments.get(0), tableOp, Tables.NOT_SYSTEM); TimeType timeType = TimeType.valueOf(ByteBufferUtil.toString(arguments.get(1))); - if (!master.security.canCreateTable(c, tableName)) - throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED); + String namespaceId; try { - master.fate.seedTransaction(opid, new TraceRepo(new CreateTable(c.getPrincipal(), tableName, timeType, options)), autoCleanup); + namespaceId = Namespaces.getNamespaceId(HdfsZooInstance.getInstance(), Tables.qualify(tableName).getFirst()); } catch (NamespaceNotFoundException e) { throw new ThriftTableOperationException(null, tableName, tableOp, TableOperationExceptionType.NAMESPACE_NOTFOUND, ""); } + + if (!master.security.canCreateTable(c, tableName, namespaceId)) + throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED); + + master.fate.seedTransaction(opid, new TraceRepo(new CreateTable(c.getPrincipal(), tableName, timeType, options, namespaceId)), autoCleanup); + break; } case TABLE_RENAME: { http://git-wip-us.apache.org/repos/asf/accumulo/blob/b80e1a4f/server/master/src/main/java/org/apache/accumulo/master/tableOps/CreateTable.java ---------------------------------------------------------------------- diff --git a/server/master/src/main/java/org/apache/accumulo/master/tableOps/CreateTable.java b/server/master/src/main/java/org/apache/accumulo/master/tableOps/CreateTable.java index 9535781..33ee878 100644 --- a/server/master/src/main/java/org/apache/accumulo/master/tableOps/CreateTable.java +++ b/server/master/src/main/java/org/apache/accumulo/master/tableOps/CreateTable.java @@ -22,9 +22,7 @@ import java.util.Map.Entry; import org.apache.accumulo.core.Constants; import org.apache.accumulo.core.client.Instance; -import org.apache.accumulo.core.client.NamespaceNotFoundException; import org.apache.accumulo.core.client.admin.TimeType; -import org.apache.accumulo.core.client.impl.Namespaces; import org.apache.accumulo.core.client.impl.Tables; import org.apache.accumulo.core.client.impl.thrift.TableOperation; import org.apache.accumulo.core.client.impl.thrift.ThriftSecurityException; @@ -35,7 +33,6 @@ import org.apache.accumulo.fate.Repo; import org.apache.accumulo.fate.zookeeper.ZooUtil.NodeExistsPolicy; import org.apache.accumulo.master.Master; import org.apache.accumulo.server.ServerConstants; -import org.apache.accumulo.server.client.HdfsZooInstance; import org.apache.accumulo.server.fs.VolumeManager; import org.apache.accumulo.server.security.AuditedSecurityOperation; import org.apache.accumulo.server.security.SecurityOperation; @@ -279,14 +276,13 @@ public class CreateTable extends MasterRepo { private TableInfo tableInfo; - public CreateTable(String user, String tableName, TimeType timeType, Map props) throws NamespaceNotFoundException { + public CreateTable(String user, String tableName, TimeType timeType, Map props, String namespaceId) { tableInfo = new TableInfo(); tableInfo.tableName = tableName; tableInfo.timeType = TabletTime.getTimeID(timeType); tableInfo.user = user; tableInfo.props = props; - Instance inst = HdfsZooInstance.getInstance(); - tableInfo.namespaceId = Namespaces.getNamespaceId(inst, Tables.qualify(tableInfo.tableName).getFirst()); + tableInfo.namespaceId = namespaceId; } @Override http://git-wip-us.apache.org/repos/asf/accumulo/blob/b80e1a4f/test/src/main/java/org/apache/accumulo/test/randomwalk/security/CreateTable.java ---------------------------------------------------------------------- diff --git a/test/src/main/java/org/apache/accumulo/test/randomwalk/security/CreateTable.java b/test/src/main/java/org/apache/accumulo/test/randomwalk/security/CreateTable.java index 16310a5..4c10b13 100644 --- a/test/src/main/java/org/apache/accumulo/test/randomwalk/security/CreateTable.java +++ b/test/src/main/java/org/apache/accumulo/test/randomwalk/security/CreateTable.java @@ -36,7 +36,7 @@ public class CreateTable extends Test { String tableName = WalkingSecurity.get(state).getTableName(); boolean exists = WalkingSecurity.get(state).getTableExists(); - boolean hasPermission = WalkingSecurity.get(state).canCreateTable(WalkingSecurity.get(state).getSysCredentials()); + boolean hasPermission = WalkingSecurity.get(state).canCreateTable(WalkingSecurity.get(state).getSysCredentials(), null, null); try { conn.tableOperations().create(tableName); http://git-wip-us.apache.org/repos/asf/accumulo/blob/b80e1a4f/test/src/test/java/org/apache/accumulo/test/NamespacesIT.java ---------------------------------------------------------------------- diff --git a/test/src/test/java/org/apache/accumulo/test/NamespacesIT.java b/test/src/test/java/org/apache/accumulo/test/NamespacesIT.java index addb377..6915c96 100644 --- a/test/src/test/java/org/apache/accumulo/test/NamespacesIT.java +++ b/test/src/test/java/org/apache/accumulo/test/NamespacesIT.java @@ -560,7 +560,7 @@ public class NamespacesIT extends SimpleMacIT { c.securityOperations().createLocalUser(u1, pass); Connector user1Con = c.getInstance().getConnector(u1, pass); - + try { user1Con.tableOperations().create(t2); fail(); @@ -680,6 +680,7 @@ public class NamespacesIT extends SimpleMacIT { user1Con.namespaceOperations().create(n2); c.securityOperations().revokeSystemPermission(u1, SystemPermission.CREATE_NAMESPACE); + c.securityOperations().revokeNamespacePermission(u1, n2, NamespacePermission.DROP_NAMESPACE); try { user1Con.namespaceOperations().delete(n2); fail();