From a9bf77ad3b17cd96ec9001576d61f874f58d7eba Mon Sep 17 00:00:00 2001 From: Jeffrey Manno Date: Wed, 9 Dec 2020 11:55:16 -0500 Subject: [PATCH 1/8] add security check for flush operation --- .../java/org/apache/accumulo/test/AuditMessageIT.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java b/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java index 69e294bdc1d..e642eaeae1c 100644 --- a/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java +++ b/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java @@ -35,6 +35,7 @@ import org.apache.accumulo.core.client.BatchWriter; import org.apache.accumulo.core.client.BatchWriterConfig; import org.apache.accumulo.core.client.Connector; +import org.apache.accumulo.core.client.NamespaceNotFoundException; import org.apache.accumulo.core.client.Scanner; import org.apache.accumulo.core.client.TableExistsException; import org.apache.accumulo.core.client.TableNotFoundException; @@ -482,6 +483,10 @@ public void testDeniedAudits() throws AccumuloSecurityException, AccumuloExcepti auditConnector.tableOperations().deleteRows(OLD_TEST_TABLE_NAME, new Text("myRow"), new Text("myRow~")); } catch (AccumuloSecurityException ex) {} + try { + auditConnector.tableOperations().flush(OLD_TEST_TABLE_NAME, new Text("start"), + new Text("end"), false); + } catch (AccumuloSecurityException ex) {} // ... that will do for now. // End of testing activities @@ -519,6 +524,11 @@ public void testDeniedAudits() throws AccumuloSecurityException, AccumuloExcepti "operation: denied;.*" + String.format(AuditedSecurityOperation.CAN_DELETE_RANGE_AUDIT_TEMPLATE, OLD_TEST_TABLE_NAME, "myRow", "myRow~")).size()); + assertEquals(1, + findAuditMessage(auditMessages, + "operation: denied;.*" + + String.format(AuditedSecurityOperation.CAN_FLUSH_TABLE_AUDIT_TEMPLATE, + "1", "\\+default")).size()); } @Test From f4f420656d130f72bd5a3791374b5031ada98537 Mon Sep 17 00:00:00 2001 From: Jeffrey Manno Date: Wed, 9 Dec 2020 12:01:27 -0500 Subject: [PATCH 2/8] remove unused import --- test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java b/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java index e642eaeae1c..96011ca1271 100644 --- a/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java +++ b/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java @@ -35,7 +35,6 @@ import org.apache.accumulo.core.client.BatchWriter; import org.apache.accumulo.core.client.BatchWriterConfig; import org.apache.accumulo.core.client.Connector; -import org.apache.accumulo.core.client.NamespaceNotFoundException; import org.apache.accumulo.core.client.Scanner; import org.apache.accumulo.core.client.TableExistsException; import org.apache.accumulo.core.client.TableNotFoundException; From d9209f5b27a4763717cfba65af62b872bd0fa77f Mon Sep 17 00:00:00 2001 From: Jeffrey Manno Date: Wed, 9 Dec 2020 13:19:49 -0500 Subject: [PATCH 3/8] make requested change --- .../main/java/org/apache/accumulo/test/AuditMessageIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java b/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java index 96011ca1271..f8681e04817 100644 --- a/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java +++ b/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java @@ -483,8 +483,8 @@ public void testDeniedAudits() throws AccumuloSecurityException, AccumuloExcepti new Text("myRow~")); } catch (AccumuloSecurityException ex) {} try { - auditConnector.tableOperations().flush(OLD_TEST_TABLE_NAME, new Text("start"), - new Text("end"), false); + auditConnector.tableOperations().flush(OLD_TEST_TABLE_NAME, new Text("myRow"), + new Text("myRow~"), false); } catch (AccumuloSecurityException ex) {} // ... that will do for now. From 489ec63f23dddda91b9a907fc4426fb1b4a294db Mon Sep 17 00:00:00 2001 From: Jeffrey Manno Date: Thu, 10 Dec 2020 08:51:43 -0500 Subject: [PATCH 4/8] add test to make sure we can flush --- .../apache/accumulo/test/functional/PermissionsIT.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java b/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java index 2876ef27599..dac44c4f73b 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java @@ -660,6 +660,14 @@ private void testMissingTablePermission(Connector test_user_conn, ClusterUser te if (e.getSecurityErrorCode() != SecurityErrorCode.PERMISSION_DENIED) throw e; } + // Now see if we can flush + try { + test_user_conn.tableOperations().flush(tableName, new Text("myrow"), new Text("myrow~"), false); + throw new IllegalStateException("Should NOT be able to flsuh a table"); + } catch (AccumuloSecurityException e){ + if (e.getSecurityErrorCode() != SecurityErrorCode.PERMISSION_DENIED) + throw e; + } break; case BULK_IMPORT: // test for bulk import permission would go here @@ -717,6 +725,7 @@ private void testGrantedTablePermission(Connector test_user_conn, ClusterUser no iter.next(); break; case WRITE: + test_user_conn.tableOperations().flush(tableName, new Text("myrow"), new Text("myrow~"), false); writer = test_user_conn.createBatchWriter(tableName, new BatchWriterConfig()); m = new Mutation(new Text("row")); m.put(new Text("a"), new Text("b"), new Value("c".getBytes())); From c95a31c4d2f273ffcc27de32ead4053458221f5b Mon Sep 17 00:00:00 2001 From: Jeffrey Manno Date: Thu, 10 Dec 2020 10:30:07 -0500 Subject: [PATCH 5/8] add basic test for System.SYSTEM perm --- .../test/functional/PermissionsIT.java | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java b/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java index dac44c4f73b..76a817b57fe 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Properties; import java.util.Set; import org.apache.accumulo.cluster.ClusterUser; @@ -40,6 +41,7 @@ import org.apache.accumulo.core.client.Scanner; import org.apache.accumulo.core.client.TableExistsException; import org.apache.accumulo.core.client.TableNotFoundException; +import org.apache.accumulo.core.client.admin.DelegationTokenConfig; import org.apache.accumulo.core.client.security.SecurityErrorCode; import org.apache.accumulo.core.client.security.tokens.AuthenticationToken; import org.apache.accumulo.core.client.security.tokens.PasswordToken; @@ -53,6 +55,7 @@ import org.apache.accumulo.core.security.TablePermission; import org.apache.accumulo.harness.AccumuloClusterHarness; import org.apache.accumulo.test.categories.MiniClusterOnlyTests; +import org.apache.commons.configuration.SystemConfiguration; import org.apache.hadoop.io.Text; import org.junit.Assume; import org.junit.Before; @@ -271,7 +274,19 @@ private void testMissingSystemPermission(String tableNamePrefix, Connector root_ } break; case SYSTEM: - // test for system permission would go here + user = "__TEMP_USER_WITHOUT_PERM_TEST__"; + loginAs(rootUser); + root_conn.securityOperations().createLocalUser(user, + (passwordBased ? new PasswordToken(password) : null)); + try{ + loginAs(testUser); + test_user_conn.securityOperations().getUserAuthorizations(user); + throw new IllegalStateException("Should NOT be able to get User Auths"); + } catch (AccumuloSecurityException e) { + loginAs(rootUser); + if (e.getSecurityErrorCode() != SecurityErrorCode.PERMISSION_DENIED) + throw e; + } break; case CREATE_NAMESPACE: namespace = "__CREATE_NAMESPACE_WITHOUT_PERM_TEST__"; @@ -458,7 +473,12 @@ private void testGrantedSystemPermission(String tableNamePrefix, Connector root_ throw new IllegalStateException("Should be able to alter a user"); break; case SYSTEM: - // test for system permission would go here + user = "__TEMP_USER_WITH_PERM_TEST__"; + loginAs(rootUser); + root_conn.securityOperations().createLocalUser(user, + (passwordBased ? new PasswordToken(password) : null)); + loginAs(testUser); + test_user_conn.securityOperations().getUserAuthorizations(user); break; case CREATE_NAMESPACE: namespace = "__CREATE_NAMESPACE_WITH_PERM_TEST__"; From fe1c24749a4de7d5847e12ece36f240a13d48f84 Mon Sep 17 00:00:00 2001 From: Jeffrey Manno Date: Tue, 15 Dec 2020 05:40:33 -0500 Subject: [PATCH 6/8] revise flush test and add set/remove prop test --- .../apache/accumulo/test/AuditMessageIT.java | 10 +-- .../test/functional/PermissionsIT.java | 73 ++++++++++++++----- 2 files changed, 59 insertions(+), 24 deletions(-) diff --git a/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java b/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java index f8681e04817..3a931c178a5 100644 --- a/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java +++ b/test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java @@ -484,7 +484,7 @@ public void testDeniedAudits() throws AccumuloSecurityException, AccumuloExcepti } catch (AccumuloSecurityException ex) {} try { auditConnector.tableOperations().flush(OLD_TEST_TABLE_NAME, new Text("myRow"), - new Text("myRow~"), false); + new Text("myRow~"), false); } catch (AccumuloSecurityException ex) {} // ... that will do for now. @@ -524,10 +524,10 @@ public void testDeniedAudits() throws AccumuloSecurityException, AccumuloExcepti + String.format(AuditedSecurityOperation.CAN_DELETE_RANGE_AUDIT_TEMPLATE, OLD_TEST_TABLE_NAME, "myRow", "myRow~")).size()); assertEquals(1, - findAuditMessage(auditMessages, - "operation: denied;.*" - + String.format(AuditedSecurityOperation.CAN_FLUSH_TABLE_AUDIT_TEMPLATE, - "1", "\\+default")).size()); + findAuditMessage(auditMessages, + "operation: denied;.*" + String + .format(AuditedSecurityOperation.CAN_FLUSH_TABLE_AUDIT_TEMPLATE, "1", "\\+default")) + .size()); } @Test diff --git a/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java b/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java index 76a817b57fe..33308e222a2 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java @@ -27,7 +27,6 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; -import java.util.Properties; import java.util.Set; import org.apache.accumulo.cluster.ClusterUser; @@ -41,7 +40,6 @@ import org.apache.accumulo.core.client.Scanner; import org.apache.accumulo.core.client.TableExistsException; import org.apache.accumulo.core.client.TableNotFoundException; -import org.apache.accumulo.core.client.admin.DelegationTokenConfig; import org.apache.accumulo.core.client.security.SecurityErrorCode; import org.apache.accumulo.core.client.security.tokens.AuthenticationToken; import org.apache.accumulo.core.client.security.tokens.PasswordToken; @@ -55,7 +53,6 @@ import org.apache.accumulo.core.security.TablePermission; import org.apache.accumulo.harness.AccumuloClusterHarness; import org.apache.accumulo.test.categories.MiniClusterOnlyTests; -import org.apache.commons.configuration.SystemConfiguration; import org.apache.hadoop.io.Text; import org.junit.Assume; import org.junit.Before; @@ -274,17 +271,33 @@ private void testMissingSystemPermission(String tableNamePrefix, Connector root_ } break; case SYSTEM: - user = "__TEMP_USER_WITHOUT_PERM_TEST__"; + try { + // Test setProperty + loginAs(testUser); + test_user_conn.instanceOperations() + .setProperty(Property.TSERV_TOTAL_MUTATION_QUEUE_MAX.getKey(), "10000"); + throw new IllegalStateException("Should NOT be able to set System Property"); + } catch (AccumuloSecurityException e) { + loginAs(rootUser); + if (e.getSecurityErrorCode() != SecurityErrorCode.PERMISSION_DENIED + || root_conn.instanceOperations().getSystemConfiguration() + .get(Property.TSERV_TOTAL_MUTATION_QUEUE_MAX.getKey()).equals("10000")) + throw e; + } + // Test removal of property loginAs(rootUser); - root_conn.securityOperations().createLocalUser(user, - (passwordBased ? new PasswordToken(password) : null)); - try{ + root_conn.instanceOperations().setProperty(Property.TSERV_TOTAL_MUTATION_QUEUE_MAX.getKey(), + "10000"); + try { loginAs(testUser); - test_user_conn.securityOperations().getUserAuthorizations(user); - throw new IllegalStateException("Should NOT be able to get User Auths"); + test_user_conn.instanceOperations() + .removeProperty(Property.TSERV_TOTAL_MUTATION_QUEUE_MAX.getKey()); + throw new IllegalStateException("Should NOT be able to remove Sysem Property"); } catch (AccumuloSecurityException e) { loginAs(rootUser); - if (e.getSecurityErrorCode() != SecurityErrorCode.PERMISSION_DENIED) + if (e.getSecurityErrorCode() != SecurityErrorCode.PERMISSION_DENIED + || !root_conn.instanceOperations().getSystemConfiguration() + .get(Property.TSERV_TOTAL_MUTATION_QUEUE_MAX.getKey()).equals("10000")) throw e; } break; @@ -473,12 +486,22 @@ private void testGrantedSystemPermission(String tableNamePrefix, Connector root_ throw new IllegalStateException("Should be able to alter a user"); break; case SYSTEM: - user = "__TEMP_USER_WITH_PERM_TEST__"; + // Test setProperty + loginAs(testUser); + test_user_conn.instanceOperations() + .setProperty(Property.TSERV_TOTAL_MUTATION_QUEUE_MAX.getKey(), "10000"); loginAs(rootUser); - root_conn.securityOperations().createLocalUser(user, - (passwordBased ? new PasswordToken(password) : null)); + if (!root_conn.instanceOperations().getSystemConfiguration() + .get(Property.TSERV_TOTAL_MUTATION_QUEUE_MAX.getKey()).equals("10000")) + throw new IllegalStateException("Should be able to set system property"); + // Test removal of property loginAs(testUser); - test_user_conn.securityOperations().getUserAuthorizations(user); + test_user_conn.instanceOperations() + .removeProperty(Property.TSERV_TOTAL_MUTATION_QUEUE_MAX.getKey()); + loginAs(rootUser); + if (root_conn.instanceOperations().getSystemConfiguration() + .get(Property.TSERV_TOTAL_MUTATION_QUEUE_MAX.getKey()).equals("10000")) + throw new IllegalStateException("Should be able remove systemproperty"); break; case CREATE_NAMESPACE: namespace = "__CREATE_NAMESPACE_WITH_PERM_TEST__"; @@ -682,11 +705,12 @@ private void testMissingTablePermission(Connector test_user_conn, ClusterUser te } // Now see if we can flush try { - test_user_conn.tableOperations().flush(tableName, new Text("myrow"), new Text("myrow~"), false); + test_user_conn.tableOperations().flush(tableName, new Text("myrow"), new Text("myrow~"), + false); throw new IllegalStateException("Should NOT be able to flsuh a table"); - } catch (AccumuloSecurityException e){ - if (e.getSecurityErrorCode() != SecurityErrorCode.PERMISSION_DENIED) - throw e; + } catch (AccumuloSecurityException e) { + if (e.getSecurityErrorCode() != SecurityErrorCode.PERMISSION_DENIED) + throw e; } break; case BULK_IMPORT: @@ -702,6 +726,14 @@ private void testMissingTablePermission(Connector test_user_conn, ClusterUser te if (e.getSecurityErrorCode() != SecurityErrorCode.PERMISSION_DENIED) throw e; } + try { + test_user_conn.tableOperations().flush(tableName, new Text("myrow"), new Text("myrow~"), + false); + throw new IllegalStateException("Should NOT be able to flsuh a table"); + } catch (AccumuloSecurityException e) { + if (e.getSecurityErrorCode() != SecurityErrorCode.PERMISSION_DENIED) + throw e; + } break; case DROP_TABLE: try { @@ -745,7 +777,8 @@ private void testGrantedTablePermission(Connector test_user_conn, ClusterUser no iter.next(); break; case WRITE: - test_user_conn.tableOperations().flush(tableName, new Text("myrow"), new Text("myrow~"), false); + test_user_conn.tableOperations().flush(tableName, new Text("myrow"), new Text("myrow~"), + false); writer = test_user_conn.createBatchWriter(tableName, new BatchWriterConfig()); m = new Mutation(new Text("row")); m.put(new Text("a"), new Text("b"), new Value("c".getBytes())); @@ -756,6 +789,8 @@ private void testGrantedTablePermission(Connector test_user_conn, ClusterUser no // test for bulk import permission would go here break; case ALTER_TABLE: + test_user_conn.tableOperations().flush(tableName, new Text("myrow"), new Text("myrow~"), + false); Map> groups = new HashMap<>(); groups.put("tgroup", new HashSet<>(Arrays.asList(new Text("t1"), new Text("t2")))); break; From ebef3fa4f2c4884259a515f104719f0982ded000 Mon Sep 17 00:00:00 2001 From: Jeffrey Manno Date: Tue, 15 Dec 2020 07:38:58 -0500 Subject: [PATCH 7/8] implement suggested idea --- .../test/functional/ManagerApiIT.java | 234 ++++++++++++++++++ 1 file changed, 234 insertions(+) create mode 100644 test/src/main/java/org/apache/accumulo/test/functional/ManagerApiIT.java diff --git a/test/src/main/java/org/apache/accumulo/test/functional/ManagerApiIT.java b/test/src/main/java/org/apache/accumulo/test/functional/ManagerApiIT.java new file mode 100644 index 00000000000..b77e9084667 --- /dev/null +++ b/test/src/main/java/org/apache/accumulo/test/functional/ManagerApiIT.java @@ -0,0 +1,234 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.test.functional; + +import org.apache.accumulo.core.client.AccumuloSecurityException; +import org.apache.accumulo.core.client.impl.ClientContext; +import org.apache.accumulo.core.client.impl.ClientExec; +import org.apache.accumulo.core.client.impl.Credentials; +import org.apache.accumulo.core.client.impl.MasterClient; +import org.apache.accumulo.core.client.security.SecurityErrorCode; +import org.apache.accumulo.core.client.security.tokens.PasswordToken; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.core.master.thrift.MasterClientService; +import org.apache.accumulo.core.master.thrift.MasterGoalState; +import org.apache.accumulo.core.security.SystemPermission; +import org.apache.accumulo.core.security.TablePermission; +import org.apache.accumulo.core.trace.Tracer; +import org.apache.accumulo.core.util.TextUtil; +import org.apache.accumulo.harness.AccumuloClusterHarness; +import org.apache.accumulo.test.categories.MiniClusterOnlyTests; +import org.apache.hadoop.io.Text; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@Category(MiniClusterOnlyTests.class) +public class ManagerApiIT extends AccumuloClusterHarness { + + private static final Logger LOG = LoggerFactory.getLogger(ManagerApiIT.class); + + private interface MasterApiMethodTest { + public void test() throws Exception; + } + + private class Flush implements MasterApiMethodTest { + @Override + public void test() throws Exception { + MasterClient.executeVoid(getContext(), new ClientExec() { + @Override + public void execute(MasterClientService.Client client) throws Exception { + String tableId = + getContext().getConnector().tableOperations().tableIdMap().get("ns1.NO_SUCH_TABLE"); + LOG.info("Initiating Flush"); + long flushID = client.initiateFlush(null, getContext().rpcCreds(), tableId); + client.waitForFlush(Tracer.traceInfo(), getContext().rpcCreds(), tableId, + TextUtil.getByteBuffer(new Text("myrow")), TextUtil.getByteBuffer(new Text("myrow~")), + flushID, 1); + } + }); + } + } + + private class ShutDown implements MasterApiMethodTest { + public void test() throws Exception { + MasterClient.executeVoid(getContext(), new ClientExec() { + @Override + public void execute(MasterClientService.Client client) throws Exception { + LOG.info("Sending ShutDown command via MasterClientService"); + client.shutdown(null, getContext().rpcCreds(), false); + } + }); + } + } + + private class ShutDownTServer implements MasterApiMethodTest { + public void test() throws Exception { + MasterClient.executeVoid(getContext(), new ClientExec() { + @Override + public void execute(MasterClientService.Client client) throws Exception { + LOG.info("Sending shutdown Tserver command to {} via MasterClientService", + "NO_SUCH_TSERVER"); + client.shutdownTabletServer(null, getContext().rpcCreds(), "NO_SUCH_TSERVER", false); + } + }); + } + } + + private class SetMasterGoalState implements MasterApiMethodTest { + public void test() throws Exception { + MasterClient.executeVoid(getContext(), new ClientExec() { + @Override + public void execute(MasterClientService.Client client) throws Exception { + LOG.info("Setting MasterGoalState to {} via MasterClientService", + MasterGoalState.CLEAN_STOP); + client.setMasterGoalState(null, getContext().rpcCreds(), MasterGoalState.CLEAN_STOP); + } + }); + } + } + + private class SetSystemProperty implements MasterApiMethodTest { + public void test() throws Exception { + MasterClient.executeVoid(getContext(), new ClientExec() { + @Override + public void execute(MasterClientService.Client client) throws Exception { + String prop = Property.TSERV_TOTAL_MUTATION_QUEUE_MAX.getKey(); + String value = "10000"; + LOG.info("Setting system property {} to {} via MasterClientService", prop, value); + client.setSystemProperty(null, getContext().rpcCreds(), prop, value); + } + }); + } + } + + private class RemoveSystemProperty implements MasterApiMethodTest { + public void test() throws Exception { + MasterClient.executeVoid(getContext(), new ClientExec() { + @Override + public void execute(MasterClientService.Client client) throws Exception { + String prop = Property.TSERV_TOTAL_MUTATION_QUEUE_MAX.getKey(); + LOG.info("Removing system property {} via MasterClientService", prop); + client.removeSystemProperty(null, getContext().rpcCreds(), prop); + } + }); + } + } + + @Override + public int defaultTimeoutSeconds() { + return 60; + } + + private ClientContext getContext() { + return new ClientContext(this.getConnector().getInstance(), user, + getClusterConfiguration().getClientConf()); + } + + private void runTest(MasterApiMethodTest test, boolean expectException, + boolean expectPermissionDenied) throws Exception { + try { + test.test(); + } catch (Exception ex) { + if (!expectException) { + throw ex; + } + if (ex instanceof AccumuloSecurityException && ((AccumuloSecurityException) ex) + .getSecurityErrorCode().equals(SecurityErrorCode.PERMISSION_DENIED)) { + if (!expectPermissionDenied) { + throw ex; + } + } + } + } + + private volatile Credentials user = null; + + @Test + public void testManagerApi() throws Exception { + // Set user to ADMIN user + Credentials admin = new Credentials(getAdminPrincipal(), getAdminToken()); + Credentials user1 = new Credentials("user1", new PasswordToken("user1")); + Credentials user2 = new Credentials("user2", new PasswordToken("user2")); + + user = admin; + getContext().getConnector().securityOperations().createLocalUser(user1.getPrincipal(), + (PasswordToken) user1.getToken()); + getContext().getConnector().securityOperations().createLocalUser(user2.getPrincipal(), + (PasswordToken) user2.getToken()); + getContext().getConnector().securityOperations().grantSystemPermission(user1.getPrincipal(), + SystemPermission.CREATE_NAMESPACE); + getContext().getConnector().securityOperations().grantSystemPermission(user2.getPrincipal(), + SystemPermission.SYSTEM); + // for Flush test. Create namespace and table and give user2 write permissions. + getContext().getConnector().namespaceOperations().create("ns1"); + getContext().getConnector().tableOperations().create("ns1.NO_SUCH_TABLE"); + getContext().getConnector().securityOperations().grantTablePermission(user2.getPrincipal(), + "ns1.NO_SUCH_TABLE", TablePermission.WRITE); + + // To setMasterGoalState, user needs System.system + user = admin; // admin has System.system + runTest(new SetMasterGoalState(), false, false); + user = user1; // user1 doesn't have System.system + runTest(new SetMasterGoalState(), true, true); + user = user2; // user2 has System.system + runTest(new SetMasterGoalState(), false, false); + + // To Flush, user needs WRITE or ALTER TABLE + user = admin; // admin created the table + runTest(new Flush(), false, false); + user = user1; // user1 doesn't have Write permissions + runTest(new Flush(), true, true); + user = user2; // user2 has write permissions + runTest(new Flush(), false, false); + + // To set system property, user needs System.system + user = admin; // admin has System.system + runTest(new SetSystemProperty(), false, false); + user = user1; // user1 doesn't have System.system + runTest(new SetSystemProperty(), true, true); + user = user2; // user2 has System.system + runTest(new SetSystemProperty(), false, false); + + // To remove system property, user needs System.system + user = admin; // admin has System.system + runTest(new RemoveSystemProperty(), false, false); + user = user1; // user1 doesn't have System.system + runTest(new RemoveSystemProperty(), true, true); + user = user2; // user2 has System.system + runTest(new RemoveSystemProperty(), true, false); + + // To shutdown Tserver, user needs System.system + user = admin; // admin has System.system + runTest(new ShutDownTServer(), true, false); + user = user1; // user1 doesn't have System.system + runTest(new ShutDownTServer(), true, true); + user = user2; // user2 has System.system + runTest(new ShutDownTServer(), true, false); + + // To shutdown, user needs System.system + user = admin; // admin has System.system + runTest(new ShutDown(), false, false); + user = user1; // user1 doesn't have System.system + runTest(new ShutDown(), true, true); + user = user2; // user2 has System.system + runTest(new ShutDown(), false, false); + + } + +} From fae24932d5b837da64e7009d4b1e362ff5e4a816 Mon Sep 17 00:00:00 2001 From: Christopher Tubbs Date: Tue, 15 Dec 2020 20:24:08 -0500 Subject: [PATCH 8/8] Update ManagerApiIT * Make tests independent with consistent naming scheme * Make use of assertThrows to simplify checks * Rename variables and inline the test operations to make it easier to follow the test intentions --- .../test/functional/ManagerApiIT.java | 332 +++++++++--------- 1 file changed, 157 insertions(+), 175 deletions(-) diff --git a/test/src/main/java/org/apache/accumulo/test/functional/ManagerApiIT.java b/test/src/main/java/org/apache/accumulo/test/functional/ManagerApiIT.java index b77e9084667..ad007c75ef3 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/ManagerApiIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/ManagerApiIT.java @@ -16,7 +16,15 @@ */ package org.apache.accumulo.test.functional; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; + +import java.util.Arrays; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Function; + import org.apache.accumulo.core.client.AccumuloSecurityException; +import org.apache.accumulo.core.client.admin.SecurityOperations; import org.apache.accumulo.core.client.impl.ClientContext; import org.apache.accumulo.core.client.impl.ClientExec; import org.apache.accumulo.core.client.impl.Credentials; @@ -28,207 +36,181 @@ import org.apache.accumulo.core.master.thrift.MasterGoalState; import org.apache.accumulo.core.security.SystemPermission; import org.apache.accumulo.core.security.TablePermission; -import org.apache.accumulo.core.trace.Tracer; +import org.apache.accumulo.core.security.thrift.TCredentials; import org.apache.accumulo.core.util.TextUtil; -import org.apache.accumulo.harness.AccumuloClusterHarness; -import org.apache.accumulo.test.categories.MiniClusterOnlyTests; +import org.apache.accumulo.harness.SharedMiniClusterBase; import org.apache.hadoop.io.Text; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.FixMethodOrder; import org.junit.Test; -import org.junit.experimental.categories.Category; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -@Category(MiniClusterOnlyTests.class) -public class ManagerApiIT extends AccumuloClusterHarness { +import org.junit.runners.MethodSorters; - private static final Logger LOG = LoggerFactory.getLogger(ManagerApiIT.class); +// the shutdown test should sort last, so other tests don't break +@FixMethodOrder(MethodSorters.NAME_ASCENDING) +public class ManagerApiIT extends SharedMiniClusterBase { - private interface MasterApiMethodTest { - public void test() throws Exception; + @Override + public int defaultTimeoutSeconds() { + return 60; } - private class Flush implements MasterApiMethodTest { - @Override - public void test() throws Exception { - MasterClient.executeVoid(getContext(), new ClientExec() { - @Override - public void execute(MasterClientService.Client client) throws Exception { - String tableId = - getContext().getConnector().tableOperations().tableIdMap().get("ns1.NO_SUCH_TABLE"); - LOG.info("Initiating Flush"); - long flushID = client.initiateFlush(null, getContext().rpcCreds(), tableId); - client.waitForFlush(Tracer.traceInfo(), getContext().rpcCreds(), tableId, - TextUtil.getByteBuffer(new Text("myrow")), TextUtil.getByteBuffer(new Text("myrow~")), - flushID, 1); - } - }); - } + private static Credentials rootUser; + private static Credentials regularUser; + private static Credentials privilegedUser; + + @BeforeClass + public static void setup() throws Exception { + SharedMiniClusterBase.startMiniCluster(); + rootUser = new Credentials(getPrincipal(), getToken()); + regularUser = new Credentials("regularUser", new PasswordToken("regularUser")); + privilegedUser = new Credentials("privilegedUser", new PasswordToken("privilegedUser")); + SecurityOperations rootSecOps = getConnector().securityOperations(); + for (Credentials user : Arrays.asList(regularUser, privilegedUser)) + rootSecOps.createLocalUser(user.getPrincipal(), (PasswordToken) user.getToken()); + rootSecOps.grantSystemPermission(privilegedUser.getPrincipal(), SystemPermission.SYSTEM); } - private class ShutDown implements MasterApiMethodTest { - public void test() throws Exception { - MasterClient.executeVoid(getContext(), new ClientExec() { - @Override - public void execute(MasterClientService.Client client) throws Exception { - LOG.info("Sending ShutDown command via MasterClientService"); - client.shutdown(null, getContext().rpcCreds(), false); - } - }); - } + @AfterClass + public static void teardown() throws Exception { + SharedMiniClusterBase.stopMiniCluster(); } - private class ShutDownTServer implements MasterApiMethodTest { - public void test() throws Exception { - MasterClient.executeVoid(getContext(), new ClientExec() { - @Override - public void execute(MasterClientService.Client client) throws Exception { - LOG.info("Sending shutdown Tserver command to {} via MasterClientService", - "NO_SUCH_TSERVER"); - client.shutdownTabletServer(null, getContext().rpcCreds(), "NO_SUCH_TSERVER", false); - } - }); - } - } + private Function> op; - private class SetMasterGoalState implements MasterApiMethodTest { - public void test() throws Exception { - MasterClient.executeVoid(getContext(), new ClientExec() { - @Override - public void execute(MasterClientService.Client client) throws Exception { - LOG.info("Setting MasterGoalState to {} via MasterClientService", - MasterGoalState.CLEAN_STOP); - client.setMasterGoalState(null, getContext().rpcCreds(), MasterGoalState.CLEAN_STOP); - } - }); - } + @Test + public void testPermissions_setMasterGoalState() throws Exception { + // To setMasterGoalState, user needs SystemPermission.SYSTEM + op = user -> client -> client.setMasterGoalState(null, user, MasterGoalState.NORMAL); + expectPermissionDenied(op, regularUser); + expectPermissionSuccess(op, rootUser); + expectPermissionSuccess(op, privilegedUser); } - private class SetSystemProperty implements MasterApiMethodTest { - public void test() throws Exception { - MasterClient.executeVoid(getContext(), new ClientExec() { - @Override - public void execute(MasterClientService.Client client) throws Exception { - String prop = Property.TSERV_TOTAL_MUTATION_QUEUE_MAX.getKey(); - String value = "10000"; - LOG.info("Setting system property {} to {} via MasterClientService", prop, value); - client.setSystemProperty(null, getContext().rpcCreds(), prop, value); - } - }); - } + @Test + public void testPermissions_initiateFlush() throws Exception { + // To initiateFlush, user needs TablePermission.WRITE or TablePermission.ALTER_TABLE + String[] uniqNames = getUniqueNames(3); + String tableName = uniqNames[0]; + Credentials regUserWithWrite = new Credentials(uniqNames[1], new PasswordToken(uniqNames[1])); + Credentials regUserWithAlter = new Credentials(uniqNames[2], new PasswordToken(uniqNames[2])); + SecurityOperations rootSecOps = getConnector().securityOperations(); + rootSecOps.createLocalUser(regUserWithWrite.getPrincipal(), + (PasswordToken) regUserWithWrite.getToken()); + rootSecOps.createLocalUser(regUserWithAlter.getPrincipal(), + (PasswordToken) regUserWithAlter.getToken()); + getConnector().tableOperations().create(tableName); + rootSecOps.grantTablePermission(regUserWithWrite.getPrincipal(), tableName, + TablePermission.WRITE); + rootSecOps.grantTablePermission(regUserWithAlter.getPrincipal(), tableName, + TablePermission.ALTER_TABLE); + String tableId = getConnector().tableOperations().tableIdMap().get(tableName); + op = user -> client -> client.initiateFlush(null, user, tableId); + expectPermissionDenied(op, regularUser); + // privileged users can grant themselves permission, but it's not default + expectPermissionDenied(op, privilegedUser); + expectPermissionSuccess(op, regUserWithWrite); + expectPermissionSuccess(op, regUserWithAlter); + // root user can because they created the table + expectPermissionSuccess(op, rootUser); } - private class RemoveSystemProperty implements MasterApiMethodTest { - public void test() throws Exception { - MasterClient.executeVoid(getContext(), new ClientExec() { - @Override - public void execute(MasterClientService.Client client) throws Exception { - String prop = Property.TSERV_TOTAL_MUTATION_QUEUE_MAX.getKey(); - LOG.info("Removing system property {} via MasterClientService", prop); - client.removeSystemProperty(null, getContext().rpcCreds(), prop); - } - }); - } + @Test + public void testPermissions_waitForFlush() throws Exception { + // To waitForFlush, user needs TablePermission.WRITE or TablePermission.ALTER_TABLE + String[] uniqNames = getUniqueNames(3); + String tableName = uniqNames[0]; + Credentials regUserWithWrite = new Credentials(uniqNames[1], new PasswordToken(uniqNames[1])); + Credentials regUserWithAlter = new Credentials(uniqNames[2], new PasswordToken(uniqNames[2])); + SecurityOperations rootSecOps = getConnector().securityOperations(); + rootSecOps.createLocalUser(regUserWithWrite.getPrincipal(), + (PasswordToken) regUserWithWrite.getToken()); + rootSecOps.createLocalUser(regUserWithAlter.getPrincipal(), + (PasswordToken) regUserWithAlter.getToken()); + getConnector().tableOperations().create(tableName); + rootSecOps.grantTablePermission(regUserWithWrite.getPrincipal(), tableName, + TablePermission.WRITE); + rootSecOps.grantTablePermission(regUserWithAlter.getPrincipal(), tableName, + TablePermission.ALTER_TABLE); + String tableId = getConnector().tableOperations().tableIdMap().get(tableName); + AtomicLong flushId = new AtomicLong(); + // initiateFlush as the root user to get the flushId, then test waitForFlush with other users + op = user -> client -> flushId.set(client.initiateFlush(null, user, tableId)); + expectPermissionSuccess(op, rootUser); + op = user -> client -> client.waitForFlush(null, user, tableId, + TextUtil.getByteBuffer(new Text("myrow")), TextUtil.getByteBuffer(new Text("myrow~")), + flushId.get(), 1); + expectPermissionDenied(op, regularUser); + // privileged users can grant themselves permission, but it's not default + expectPermissionDenied(op, privilegedUser); + expectPermissionSuccess(op, regUserWithWrite); + expectPermissionSuccess(op, regUserWithAlter); + // root user can because they created the table + expectPermissionSuccess(op, rootUser); } - @Override - public int defaultTimeoutSeconds() { - return 60; + @Test + public void testPermissions_setSystemProperty() throws Exception { + // To setSystemProperty, user needs SystemPermission.SYSTEM + String propKey = Property.TSERV_TOTAL_MUTATION_QUEUE_MAX.getKey(); + op = user -> client -> client.setSystemProperty(null, user, propKey, "10000"); + expectPermissionDenied(op, regularUser); + expectPermissionSuccess(op, rootUser); + expectPermissionSuccess(op, privilegedUser); + getConnector().instanceOperations().removeProperty(propKey); // clean up property } - private ClientContext getContext() { - return new ClientContext(this.getConnector().getInstance(), user, - getClusterConfiguration().getClientConf()); + @Test + public void testPermissions_removeSystemProperty() throws Exception { + // To removeSystemProperty, user needs SystemPermission.SYSTEM + String propKey1 = Property.GC_CYCLE_DELAY.getKey(); + String propKey2 = Property.GC_CYCLE_START.getKey(); + getConnector().instanceOperations().setProperty(propKey1, "10000"); // ensure it exists + getConnector().instanceOperations().setProperty(propKey2, "10000"); // ensure it exists + op = user -> client -> client.removeSystemProperty(null, user, propKey1); + expectPermissionDenied(op, regularUser); + expectPermissionSuccess(op, rootUser); + op = user -> client -> client.removeSystemProperty(null, user, propKey2); + expectPermissionSuccess(op, privilegedUser); } - private void runTest(MasterApiMethodTest test, boolean expectException, - boolean expectPermissionDenied) throws Exception { - try { - test.test(); - } catch (Exception ex) { - if (!expectException) { - throw ex; - } - if (ex instanceof AccumuloSecurityException && ((AccumuloSecurityException) ex) - .getSecurityErrorCode().equals(SecurityErrorCode.PERMISSION_DENIED)) { - if (!expectPermissionDenied) { - throw ex; - } - } - } + @Test + public void testPermissions_shutdownTabletServer() throws Exception { + // To shutdownTabletServer, user needs SystemPermission.SYSTEM + // this server won't exist, so shutting it down is a NOOP on success + String fakeHostAndPort = getUniqueNames(1)[0] + ":0"; + op = user -> client -> client.shutdownTabletServer(null, user, fakeHostAndPort, false); + expectPermissionDenied(op, regularUser); + expectPermissionSuccess(op, rootUser); + expectPermissionSuccess(op, privilegedUser); } - private volatile Credentials user = null; - + // this test should go last, because it shuts things down; + // see the junit annotation to control test ordering at the top of this class @Test - public void testManagerApi() throws Exception { - // Set user to ADMIN user - Credentials admin = new Credentials(getAdminPrincipal(), getAdminToken()); - Credentials user1 = new Credentials("user1", new PasswordToken("user1")); - Credentials user2 = new Credentials("user2", new PasswordToken("user2")); - - user = admin; - getContext().getConnector().securityOperations().createLocalUser(user1.getPrincipal(), - (PasswordToken) user1.getToken()); - getContext().getConnector().securityOperations().createLocalUser(user2.getPrincipal(), - (PasswordToken) user2.getToken()); - getContext().getConnector().securityOperations().grantSystemPermission(user1.getPrincipal(), - SystemPermission.CREATE_NAMESPACE); - getContext().getConnector().securityOperations().grantSystemPermission(user2.getPrincipal(), - SystemPermission.SYSTEM); - // for Flush test. Create namespace and table and give user2 write permissions. - getContext().getConnector().namespaceOperations().create("ns1"); - getContext().getConnector().tableOperations().create("ns1.NO_SUCH_TABLE"); - getContext().getConnector().securityOperations().grantTablePermission(user2.getPrincipal(), - "ns1.NO_SUCH_TABLE", TablePermission.WRITE); - - // To setMasterGoalState, user needs System.system - user = admin; // admin has System.system - runTest(new SetMasterGoalState(), false, false); - user = user1; // user1 doesn't have System.system - runTest(new SetMasterGoalState(), true, true); - user = user2; // user2 has System.system - runTest(new SetMasterGoalState(), false, false); - - // To Flush, user needs WRITE or ALTER TABLE - user = admin; // admin created the table - runTest(new Flush(), false, false); - user = user1; // user1 doesn't have Write permissions - runTest(new Flush(), true, true); - user = user2; // user2 has write permissions - runTest(new Flush(), false, false); - - // To set system property, user needs System.system - user = admin; // admin has System.system - runTest(new SetSystemProperty(), false, false); - user = user1; // user1 doesn't have System.system - runTest(new SetSystemProperty(), true, true); - user = user2; // user2 has System.system - runTest(new SetSystemProperty(), false, false); - - // To remove system property, user needs System.system - user = admin; // admin has System.system - runTest(new RemoveSystemProperty(), false, false); - user = user1; // user1 doesn't have System.system - runTest(new RemoveSystemProperty(), true, true); - user = user2; // user2 has System.system - runTest(new RemoveSystemProperty(), true, false); - - // To shutdown Tserver, user needs System.system - user = admin; // admin has System.system - runTest(new ShutDownTServer(), true, false); - user = user1; // user1 doesn't have System.system - runTest(new ShutDownTServer(), true, true); - user = user2; // user2 has System.system - runTest(new ShutDownTServer(), true, false); - - // To shutdown, user needs System.system - user = admin; // admin has System.system - runTest(new ShutDown(), false, false); - user = user1; // user1 doesn't have System.system - runTest(new ShutDown(), true, true); - user = user2; // user2 has System.system - runTest(new ShutDown(), false, false); + public void z99_testPermissions_shutdown() throws Exception { + // To shutdown, user needs SystemPermission.SYSTEM + op = user -> client -> client.shutdown(null, user, false); + expectPermissionDenied(op, regularUser); + // We should be able to do both of the following RPC calls before it actually shuts down + expectPermissionSuccess(op, rootUser); + expectPermissionSuccess(op, privilegedUser); + } + + private static void expectPermissionSuccess( + Function> op, Credentials user) + throws Exception { + ClientContext context = + new ClientContext(getConnector().getInstance(), user, getCluster().getClientConfig()); + MasterClient.executeVoid(context, op.apply(context.rpcCreds())); + } + private static void expectPermissionDenied( + Function> op, Credentials user) + throws Exception { + AccumuloSecurityException e = + assertThrows(AccumuloSecurityException.class, () -> expectPermissionSuccess(op, user)); + assertSame(SecurityErrorCode.PERMISSION_DENIED, e.getSecurityErrorCode()); } }