incubator-blur-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tim Williams <william...@gmail.com>
Subject Re: git commit: Changing the server security feature to be driven by a factory pattern. It also supports a filter pattern for server security.
Date Tue, 03 Feb 2015 04:22:24 GMT
On Mon, Feb 2, 2015 at 8:36 PM,  <amccurry@apache.org> wrote:
> Repository: incubator-blur
> Updated Branches:
>   refs/heads/master 4468f6cc5 -> 9e9e8ed1b
>
>
> Changing the server security feature to be driven by a factory pattern.  It also supports
a filter pattern for server security.
>
>
> Project: http://git-wip-us.apache.org/repos/asf/incubator-blur/repo
> Commit: http://git-wip-us.apache.org/repos/asf/incubator-blur/commit/9e9e8ed1
> Tree: http://git-wip-us.apache.org/repos/asf/incubator-blur/tree/9e9e8ed1
> Diff: http://git-wip-us.apache.org/repos/asf/incubator-blur/diff/9e9e8ed1
>
> Branch: refs/heads/master
> Commit: 9e9e8ed1b21e4537f3a5bcffe4573f04a8a644f4
> Parents: 4468f6c
> Author: Aaron McCurry <amccurry@gmail.com>
> Authored: Mon Feb 2 20:36:03 2015 -0500
> Committer: Aaron McCurry <amccurry@gmail.com>
> Committed: Mon Feb 2 20:36:03 2015 -0500
>
> ----------------------------------------------------------------------
>  .../org/apache/blur/server/ServerSecurity.java  |  8 +---
>  .../blur/server/ServerSecurityFactory.java      | 33 +++++++++++++
>  .../apache/blur/server/ServerSecurityUtil.java  | 24 ++++++----
>  .../example/SimpleExampleServerSecurity.java    | 24 +++++-----
>  .../blur/thrift/ThriftBlurControllerServer.java |  4 +-
>  .../blur/thrift/ThriftBlurShardServer.java      |  4 +-
>  .../org/apache/blur/thrift/ThriftServer.java    | 50 +++++++++++++-------
>  .../org/apache/blur/utils/BlurConstants.java    |  3 +-
>  8 files changed, 104 insertions(+), 46 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/incubator-blur/blob/9e9e8ed1/blur-core/src/main/java/org/apache/blur/server/ServerSecurity.java
> ----------------------------------------------------------------------
> diff --git a/blur-core/src/main/java/org/apache/blur/server/ServerSecurity.java b/blur-core/src/main/java/org/apache/blur/server/ServerSecurity.java
> index 3ebf079..ee74559 100644
> --- a/blur-core/src/main/java/org/apache/blur/server/ServerSecurity.java
> +++ b/blur-core/src/main/java/org/apache/blur/server/ServerSecurity.java
> @@ -19,16 +19,12 @@ package org.apache.blur.server;
>  import java.lang.reflect.Method;
>  import java.net.InetAddress;
>
> -import org.apache.blur.BlurConfiguration;
>  import org.apache.blur.thrift.generated.BlurException;
>  import org.apache.blur.user.User;
>
>  public abstract class ServerSecurity {

I didn't mention earlier - this should be named ServerAccessFilter?

> -
> -  public ServerSecurity(BlurConfiguration configuration) {
> -
> -  }
>
> -  public abstract boolean canAccess(Method method, Object[] args, User user, InetAddress
address, int port) throws BlurException;
> +  public abstract boolean canAccess(Method method, Object[] args, User user, InetAddress
address, int port)
> +      throws BlurException;
>
>  }
>
> http://git-wip-us.apache.org/repos/asf/incubator-blur/blob/9e9e8ed1/blur-core/src/main/java/org/apache/blur/server/ServerSecurityFactory.java
> ----------------------------------------------------------------------
> diff --git a/blur-core/src/main/java/org/apache/blur/server/ServerSecurityFactory.java
b/blur-core/src/main/java/org/apache/blur/server/ServerSecurityFactory.java
> new file mode 100644
> index 0000000..759112e
> --- /dev/null
> +++ b/blur-core/src/main/java/org/apache/blur/server/ServerSecurityFactory.java
> @@ -0,0 +1,33 @@
> +/**
> + * 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.blur.server;
> +
> +import org.apache.blur.BlurConfiguration;
> +
> +public abstract class ServerSecurityFactory {

Again with naming, ServerAccessFilterFactory?

> +
> +  public enum ServerType {
> +    CONTROLLER, SHARD
> +  }
> +
> +  public ServerSecurityFactory() {
> +
> +  }
> +
> +  public abstract ServerSecurity getServerSecurity(ServerType server, BlurConfiguration
configuration);
> +
> +}
>
> http://git-wip-us.apache.org/repos/asf/incubator-blur/blob/9e9e8ed1/blur-core/src/main/java/org/apache/blur/server/ServerSecurityUtil.java
> ----------------------------------------------------------------------
> diff --git a/blur-core/src/main/java/org/apache/blur/server/ServerSecurityUtil.java b/blur-core/src/main/java/org/apache/blur/server/ServerSecurityUtil.java
> index 162fe35..cd7deca 100644
> --- a/blur-core/src/main/java/org/apache/blur/server/ServerSecurityUtil.java
> +++ b/blur-core/src/main/java/org/apache/blur/server/ServerSecurityUtil.java
> @@ -22,6 +22,7 @@ import java.lang.reflect.Method;
>  import java.lang.reflect.Proxy;
>  import java.net.InetAddress;
>  import java.net.InetSocketAddress;
> +import java.util.List;
>
>  import org.apache.blur.log.Log;
>  import org.apache.blur.log.LogFactory;
> @@ -34,12 +35,15 @@ public class ServerSecurityUtil {
>
>    private static final Log LOG = LogFactory.getLog(ServerSecurityUtil.class);
>
> -  public static Iface applySecurity(final Iface iface, final ServerSecurity serverSecurity,
final boolean shardServer) {
> -    if (serverSecurity == null) {
> +  public static Iface applySecurity(final Iface iface, final List<ServerSecurity>
serverSecurityList,
> +      final boolean shardServer) {
> +    if (serverSecurityList == null || serverSecurityList.isEmpty()) {
>        LOG.info("No server security configured.");
>        return iface;
>      }
> -    LOG.info("Server security configured with [{0}] class [{1}].", serverSecurity, serverSecurity.getClass());
> +    for (ServerSecurity serverSecurity : serverSecurityList) {
> +      LOG.info("Server security configured with [{0}] class [{1}].", serverSecurity,
serverSecurity.getClass());
> +    }
>      InvocationHandler handler = new InvocationHandler() {
>        @Override
>        public Object invoke(Object proxy, Method method, Object[] args) throws Throwable
{
> @@ -53,14 +57,16 @@ public class ServerSecurityUtil {
>          InetAddress address = remoteSocketAddress.getAddress();
>          int port = remoteSocketAddress.getPort();
>          User user = UserContext.getUser();
> -        if (serverSecurity.canAccess(method, args, user, address, port)) {
> -          try {
> -            return method.invoke(iface, args);
> -          } catch (InvocationTargetException e) {
> -            throw e.getTargetException();
> +        for (ServerSecurity serverSecurity : serverSecurityList) {
> +          if (!serverSecurity.canAccess(method, args, user, address, port)) {
> +            throw new BException("ACCESS DENIED for User [{0}] method [{1}].", user,
method.getName());
>            }
>          }
> -        throw new BException("ACCESS DENIED for User [{0}] method [{1}].", user, method.getName());
> +        try {
> +          return method.invoke(iface, args);
> +        } catch (InvocationTargetException e) {
> +          throw e.getTargetException();
> +        }
>        }
>      };
>      return (Iface) Proxy.newProxyInstance(Iface.class.getClassLoader(), new Class[]
{ Iface.class }, handler);
>
> http://git-wip-us.apache.org/repos/asf/incubator-blur/blob/9e9e8ed1/blur-core/src/main/java/org/apache/blur/server/example/SimpleExampleServerSecurity.java
> ----------------------------------------------------------------------
> diff --git a/blur-core/src/main/java/org/apache/blur/server/example/SimpleExampleServerSecurity.java
b/blur-core/src/main/java/org/apache/blur/server/example/SimpleExampleServerSecurity.java
> index 206c054..e6628ae 100644
> --- a/blur-core/src/main/java/org/apache/blur/server/example/SimpleExampleServerSecurity.java
> +++ b/blur-core/src/main/java/org/apache/blur/server/example/SimpleExampleServerSecurity.java
> @@ -21,24 +21,26 @@ import java.net.InetAddress;
>
>  import org.apache.blur.BlurConfiguration;
>  import org.apache.blur.server.ServerSecurity;
> +import org.apache.blur.server.ServerSecurityFactory;
>  import org.apache.blur.thrift.generated.BlurException;
>  import org.apache.blur.user.User;
>
> -public class SimpleExampleServerSecurity extends ServerSecurity {
> -
> -  public SimpleExampleServerSecurity(BlurConfiguration configuration) {
> -    super(configuration);
> -  }
> +public class SimpleExampleServerSecurity extends ServerSecurityFactory {
>
>    @Override
> -  public boolean canAccess(Method method, Object[] args, User user, InetAddress address,
int port) throws BlurException {
> -    if (method.getName().equals("createTable")) {
> -      if (user != null && user.getUsername().equals("admin")) {
> +  public ServerSecurity getServerSecurity(ServerType server, BlurConfiguration configuration)
{
> +    return new ServerSecurity() {
> +      @Override
> +      public boolean canAccess(Method method, Object[] args, User user, InetAddress
address, int port) throws BlurException {
> +        if (method.getName().equals("createTable")) {
> +          if (user != null && user.getUsername().equals("admin")) {
> +            return true;
> +          }
> +          return false;
> +        }
>          return true;
>        }
> -      return false;
> -    }
> -    return true;
> +    };
>    }
>
>  }
>
> http://git-wip-us.apache.org/repos/asf/incubator-blur/blob/9e9e8ed1/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurControllerServer.java
> ----------------------------------------------------------------------
> diff --git a/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurControllerServer.java
b/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurControllerServer.java
> index 5632b88..2e5bb20 100644
> --- a/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurControllerServer.java
> +++ b/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurControllerServer.java
> @@ -50,6 +50,7 @@ import static org.apache.blur.utils.BlurConstants.BLUR_ZOOKEEPER_TIMEOUT_DEFAULT
>  import static org.apache.blur.utils.BlurUtil.quietClose;
>
>  import java.io.File;
> +import java.util.List;
>
>  import org.apache.blur.BlurConfiguration;
>  import org.apache.blur.command.ControllerCommandManager;
> @@ -65,6 +66,7 @@ import org.apache.blur.manager.indexserver.BlurServerShutDown.BlurShutdown;
>  import org.apache.blur.metrics.ReporterSetup;
>  import org.apache.blur.server.ControllerServerEventHandler;
>  import org.apache.blur.server.ServerSecurity;
> +import org.apache.blur.server.ServerSecurityFactory;
>  import org.apache.blur.server.ServerSecurityUtil;
>  import org.apache.blur.thirdparty.thrift_0_9_0.protocol.TJSONProtocol;
>  import org.apache.blur.thirdparty.thrift_0_9_0.server.TServlet;
> @@ -184,7 +186,7 @@ public class ThriftBlurControllerServer extends ThriftServer {
>      Trace.setStorage(traceStorage);
>      Trace.setNodeName(nodeName);
>
> -    ServerSecurity serverSecurity = getServerSecurity(configuration, false);
> +    List<ServerSecurity> serverSecurity = getServerSecurityList(configuration,
ServerSecurityFactory.ServerType.CONTROLLER);
>
>      Iface iface = BlurUtil.wrapFilteredBlurServer(configuration, controllerServer, false);
>      iface = ServerSecurityUtil.applySecurity(iface, serverSecurity, false);
>
> http://git-wip-us.apache.org/repos/asf/incubator-blur/blob/9e9e8ed1/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurShardServer.java
> ----------------------------------------------------------------------
> diff --git a/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurShardServer.java
b/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurShardServer.java
> index 0d91bec..5a0549a 100644
> --- a/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurShardServer.java
> +++ b/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurShardServer.java
> @@ -59,6 +59,7 @@ import static org.apache.blur.utils.BlurUtil.quietClose;
>  import java.io.Closeable;
>  import java.io.File;
>  import java.lang.reflect.Constructor;
> +import java.util.List;
>  import java.util.Map.Entry;
>  import java.util.Set;
>  import java.util.Timer;
> @@ -85,6 +86,7 @@ import org.apache.blur.manager.indexserver.DistributedLayoutFactoryImpl;
>  import org.apache.blur.metrics.JSONReporter;
>  import org.apache.blur.metrics.ReporterSetup;
>  import org.apache.blur.server.ServerSecurity;
> +import org.apache.blur.server.ServerSecurityFactory;
>  import org.apache.blur.server.ServerSecurityUtil;
>  import org.apache.blur.server.ShardServerEventHandler;
>  import org.apache.blur.server.TableContext;
> @@ -264,7 +266,7 @@ public class ThriftBlurShardServer extends ThriftServer {
>      Trace.setStorage(traceStorage);
>      Trace.setNodeName(nodeName);
>
> -    ServerSecurity serverSecurity = getServerSecurity(configuration, true);
> +    List<ServerSecurity> serverSecurity = getServerSecurityList(configuration,
ServerSecurityFactory.ServerType.SHARD);
>
>      Iface iface = BlurUtil.wrapFilteredBlurServer(configuration, shardServer, true);
>      iface = ServerSecurityUtil.applySecurity(iface, serverSecurity, true);
>
> http://git-wip-us.apache.org/repos/asf/incubator-blur/blob/9e9e8ed1/blur-core/src/main/java/org/apache/blur/thrift/ThriftServer.java
> ----------------------------------------------------------------------
> diff --git a/blur-core/src/main/java/org/apache/blur/thrift/ThriftServer.java b/blur-core/src/main/java/org/apache/blur/thrift/ThriftServer.java
> index c7310ba..f1c3e94 100644
> --- a/blur-core/src/main/java/org/apache/blur/thrift/ThriftServer.java
> +++ b/blur-core/src/main/java/org/apache/blur/thrift/ThriftServer.java
> @@ -22,10 +22,9 @@ import static org.apache.blur.metrics.MetricsConstants.JVM;
>  import static org.apache.blur.metrics.MetricsConstants.LOAD_AVERAGE;
>  import static org.apache.blur.metrics.MetricsConstants.ORG_APACHE_BLUR;
>  import static org.apache.blur.metrics.MetricsConstants.SYSTEM;
> -import static org.apache.blur.utils.BlurConstants.BLUR_CONTROLLER_SERVER_SECURITY_CLASS;
>  import static org.apache.blur.utils.BlurConstants.BLUR_HDFS_TRACE_PATH;
>  import static org.apache.blur.utils.BlurConstants.BLUR_HOME;
> -import static org.apache.blur.utils.BlurConstants.BLUR_SHARD_SERVER_SECURITY_CLASS;
> +import static org.apache.blur.utils.BlurConstants.BLUR_SERVER_SECURITY_CLASS;
>  import static org.apache.blur.utils.BlurConstants.BLUR_ZOOKEEPER_TRACE_PATH;
>
>  import java.io.BufferedReader;
> @@ -37,11 +36,15 @@ import java.lang.management.ManagementFactory;
>  import java.lang.management.MemoryMXBean;
>  import java.lang.management.MemoryUsage;
>  import java.lang.management.OperatingSystemMXBean;
> -import java.lang.reflect.Constructor;
>  import java.lang.reflect.Method;
>  import java.net.InetAddress;
>  import java.net.InetSocketAddress;
>  import java.net.UnknownHostException;
> +import java.util.ArrayList;
> +import java.util.List;
> +import java.util.Map;
> +import java.util.Map.Entry;
> +import java.util.TreeMap;
>  import java.util.UUID;
>  import java.util.concurrent.ExecutorService;
>
> @@ -51,6 +54,7 @@ import org.apache.blur.log.Log;
>  import org.apache.blur.log.LogFactory;
>  import org.apache.blur.manager.indexserver.BlurServerShutDown.BlurShutdown;
>  import org.apache.blur.server.ServerSecurity;
> +import org.apache.blur.server.ServerSecurityFactory;
>  import org.apache.blur.thirdparty.thrift_0_9_0.protocol.TBinaryProtocol;
>  import org.apache.blur.thirdparty.thrift_0_9_0.protocol.TCompactProtocol;
>  import org.apache.blur.thirdparty.thrift_0_9_0.server.TServer;
> @@ -439,22 +443,36 @@ public class ThriftServer {
>    }
>
>    @SuppressWarnings("unchecked")
> -  public static ServerSecurity getServerSecurity(BlurConfiguration configuration, boolean
shardServer) {
> -    String className;
> -    if (shardServer) {
> -      className = configuration.get(BLUR_SHARD_SERVER_SECURITY_CLASS);
> -    } else {
> -      className = configuration.get(BLUR_CONTROLLER_SERVER_SECURITY_CLASS);
> +  public static List<ServerSecurity> getServerSecurityList(BlurConfiguration configuration,
> +      ServerSecurityFactory.ServerType type) {
> +    Map<String, String> properties = configuration.getProperties();
> +    Map<String, String> classMap = new TreeMap<String, String>();
> +    for (Entry<String, String> e : properties.entrySet()) {
> +      String property = e.getKey();
> +      String value = e.getValue();
> +      if (value == null || value.isEmpty()) {
> +        continue;
> +      }
> +      if (property.startsWith(BLUR_SERVER_SECURITY_CLASS)) {
> +        classMap.put(property, value);
> +      }
>      }
> -    if (className == null) {
> +    if (classMap.isEmpty()) {
>        return null;
>      }
> -    try {
> -      Class<? extends ServerSecurity> clazz = (Class<? extends ServerSecurity>)
Class.forName(className);
> -      Constructor<? extends ServerSecurity> constructor = clazz.getConstructor(new
Class[] { BlurConfiguration.class });
> -      return constructor.newInstance(configuration);
> -    } catch (Exception e) {
> -      throw new RuntimeException(e);
> +    List<ServerSecurity> result = new ArrayList<ServerSecurity>();
> +    for (Entry<String, String> entry : classMap.entrySet()) {
> +      String className = entry.getValue();
> +      try {
> +        LOG.info("Loading factory class [{0}]", className);
> +        Class<? extends ServerSecurityFactory> clazz = (Class<? extends ServerSecurityFactory>)
Class
> +            .forName(className);
> +        ServerSecurityFactory serverSecurityFactory = clazz.newInstance();
> +        result.add(serverSecurityFactory.getServerSecurity(type, configuration));

I was concerned that this would feel awkward and I think it does.  We
want the flexibility of chained filters which feels weird with a
factory pattern because we're really chaining factories here; but then
we want to vary by server type, so I dunno, maybe this is all that we
can do - just thinking aloud at this point...

--tim

Mime
View raw message