accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From vi...@apache.org
Subject [04/10] git commit: ACCUMULO-1479 Simplification of namespace support in SecurityOperation
Date Thu, 30 Jan 2014 21:57:55 GMT
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 <vines@apache.org>
Authored: Tue Jan 28 16:26:08 2014 -0500
Committer: John Vines <vines@apache.org>
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<String> 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<br/>
    * 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<TColumn>
columns, List<IterInfo> 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<ByteBuffer>
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<Master>(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<Master>(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<String,String>
props) throws NamespaceNotFoundException {
+  public CreateTable(String user, String tableName, TimeType timeType, Map<String,String>
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();


Mime
View raw message