From 3686192b07040806d70fd54c20612d412bd15c85 Mon Sep 17 00:00:00 2001 From: Balazs Meszaros Date: Mon, 20 Nov 2017 14:35:15 +0100 Subject: [PATCH] HBASE-19093 Check Admin/Table to ensure all operations go via AccessControl --- .../hbase/security/access/AccessController.java | 6 + .../access/TestAccessControllerMethods.java | 138 +++++++++++++++++++++ 2 files changed, 144 insertions(+) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControllerMethods.java diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java index 05f919540a..23a4daef59 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java @@ -1309,6 +1309,12 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, } } } + + @Override + public void preMasterInitialization( + ObserverContext ctx) throws IOException { + } + /** * Create the ACL table * @throws IOException diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControllerMethods.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControllerMethods.java new file mode 100644 index 0000000000..f2ef415c3b --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControllerMethods.java @@ -0,0 +1,138 @@ +/* + * 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.hadoop.hbase.security.access; + +import static org.junit.Assert.fail; + +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.Comparator; +import java.util.HashSet; +import java.util.Iterator; +import java.util.Set; +import java.util.TreeSet; + +import org.apache.hadoop.hbase.coprocessor.BulkLoadObserver; +import org.apache.hadoop.hbase.coprocessor.EndpointObserver; +import org.apache.hadoop.hbase.coprocessor.MasterObserver; +import org.apache.hadoop.hbase.coprocessor.RegionObserver; +import org.apache.hadoop.hbase.coprocessor.RegionServerObserver; +import org.junit.Ignore; +import org.junit.Test; + +/** + * Ensures that AccessController implements the necessary methods. Since its + * interfaces has default methods it is easy to forget to implement them. + * Considerations: + * - Most events have pre and post callbacks. It is enough to implement only one + * of them. (E.g. it is not an error, if preCreateTable is implemented, but + * postCreateTable is not.) + * - Methods ending with "Action" are ignored, because they are async events and + * they have synchronous equivalent. + */ +public class TestAccessControllerMethods { + public TestAccessControllerMethods() { + } + + private static String getNameWithoutPreOrPost(Method method) { + String name = method.getName(); + + if (name.startsWith("pre")) { + return name.substring(3); + } else if (name.startsWith("post")) { + return name.substring(4); + } else { + return name; + } + } + + private static boolean hasMethod(Method[] methods, Method requiredMethod) { + for (Method method : methods) { + if (method.getName().equals(requiredMethod.getName()) && + Arrays.equals(method.getParameterTypes(), requiredMethod.getParameterTypes()) && + !method.isDefault()) { + return true; + } + } + + return false; + } + + private static void checkMethods(Class interfaceClass) { + Method[] methods = AccessController.class.getMethods(); + Set implementedMethodNames = new HashSet<>(); + Set missingMethods = new TreeSet<>((Comparator)(m1, m2) -> + m1.getName().compareTo(m2.getName()) + ); + + for (Method method : interfaceClass.getMethods()) { + String name = getNameWithoutPreOrPost(method); + + if (name.endsWith("Action")) { + continue; + } + + if (hasMethod(methods, method)) { + implementedMethodNames.add(name); + } else { + missingMethods.add(method); + } + } + + Iterator iterator = missingMethods.iterator(); + + while (iterator.hasNext()) { + Method method = iterator.next(); + String name = getNameWithoutPreOrPost(method); + + if (implementedMethodNames.contains(name)) { + iterator.remove(); + } + } + + if (!missingMethods.isEmpty()) { + fail("Missing methods: " + missingMethods); + } + } + + @Test + public void testMasterObserverMethods() { + checkMethods(MasterObserver.class); + } + + @Test + @Ignore("RegionObserver has lots of methods which are not interesting for us") + public void testRegionObserverMethods() { + checkMethods(RegionObserver.class); + } + + @Test + public void testRegionServerObserverMethods() { + checkMethods(RegionServerObserver.class); + } + + @Test + public void testEndpointObserverMethods() { + checkMethods(EndpointObserver.class); + } + + @Test + public void testBulkLoadObserverMethods() { + checkMethods(BulkLoadObserver.class); + } +} -- 2.13.6 (Apple Git-96)