Throw sercurity exceptions when permissions checks fail. Backport to …

…1.10 (#1830) * backport sercurity exceptions to 1.10 * fix AuditMessageIT
apache · Dec 9, 2020 · 56142a8 · 56142a8
1 parent b8fd349
commit 56142a8
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 8 deletions.
diff --git a/server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java b/server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java
@@ -118,7 +118,8 @@ protected MasterClientServiceHandler(Master master) {
   public long initiateFlush(TInfo tinfo, TCredentials c, String tableId)
       throws ThriftSecurityException, ThriftTableOperationException {
     String namespaceId = getNamespaceIdFromTableId(TableOperation.FLUSH, tableId);
-    master.security.canFlush(c, tableId, namespaceId);
+    if (!master.security.canFlush(c, tableId, namespaceId))
+      throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED);
 
     String zTablePath = Constants.ZROOT + "/" + master.getInstance().getInstanceID()
         + Constants.ZTABLES + "/" + tableId + Constants.ZTABLE_FLUSH_ID;
@@ -150,7 +151,8 @@ public void waitForFlush(TInfo tinfo, TCredentials c, String tableId, ByteBuffer
       ByteBuffer endRow, long flushID, long maxLoops)
       throws ThriftSecurityException, ThriftTableOperationException {
     String namespaceId = getNamespaceIdFromTableId(TableOperation.FLUSH, tableId);
-    master.security.canFlush(c, tableId, namespaceId);
+    if (!master.security.canFlush(c, tableId, namespaceId))
+      throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED);
 
     if (endRow != null && startRow != null
         && ByteBufferUtil.toText(startRow).compareTo(ByteBufferUtil.toText(endRow)) >= 0)
@@ -305,7 +307,8 @@ public void setTableProperty(TInfo info, TCredentials credentials, String tableN
   @Override
   public void shutdown(TInfo info, TCredentials c, boolean stopTabletServers)
       throws ThriftSecurityException {
-    master.security.canPerformSystemActions(c);
+    if (!master.security.canPerformSystemActions(c))
+      throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED);
     if (stopTabletServers) {
       master.setMasterGoalState(MasterGoalState.CLEAN_STOP);
       EventCoordinator.Listener eventListener = master.nextEvent.getListener();
@@ -319,7 +322,8 @@ public void shutdown(TInfo info, TCredentials c, boolean stopTabletServers)
   @Override
   public void shutdownTabletServer(TInfo info, TCredentials c, String tabletServer, boolean force)
       throws ThriftSecurityException {
-    master.security.canPerformSystemActions(c);
+    if (!master.security.canPerformSystemActions(c))
+      throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED);
 
     final TServerInstance doomed = master.tserverSet.find(tabletServer);
     if (!force) {
@@ -391,15 +395,17 @@ public void reportTabletStatus(TInfo info, TCredentials credentials, String serv
   @Override
   public void setMasterGoalState(TInfo info, TCredentials c, MasterGoalState state)
       throws ThriftSecurityException {
-    master.security.canPerformSystemActions(c);
+    if (!master.security.canPerformSystemActions(c))
+      throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED);
 
     master.setMasterGoalState(state);
   }
 
   @Override
   public void removeSystemProperty(TInfo info, TCredentials c, String property)
       throws ThriftSecurityException {
-    master.security.canPerformSystemActions(c);
+    if (!master.security.canPerformSystemActions(c))
+      throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED);
 
     try {
       SystemPropUtil.removeSystemProperty(property);
@@ -413,7 +419,8 @@ public void removeSystemProperty(TInfo info, TCredentials c, String property)
   @Override
   public void setSystemProperty(TInfo info, TCredentials c, String property, String value)
       throws ThriftSecurityException, TException {
-    master.security.canPerformSystemActions(c);
+    if (!master.security.canPerformSystemActions(c))
+      throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED);
 
     try {
       SystemPropUtil.setSystemProperty(property, value);

diff --git a/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java b/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java
@@ -465,7 +465,7 @@ public void testDeniedAudits() throws AccumuloSecurityException, AccumuloExcepti
       auditConnector.tableOperations().rename(OLD_TEST_TABLE_NAME, NEW_TEST_TABLE_NAME);
     } catch (AccumuloSecurityException ex) {}
     try {
-      auditConnector.tableOperations().clone(OLD_TEST_TABLE_NAME, NEW_TEST_TABLE_NAME, true,
+      auditConnector.tableOperations().clone(OLD_TEST_TABLE_NAME, NEW_TEST_TABLE_NAME, false,
           Collections.<String,String>emptyMap(), Collections.<String>emptySet());
     } catch (AccumuloSecurityException ex) {}
     try {