geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jinmeil...@apache.org
Subject [geode] branch develop updated: GEODE-4633: un-deprecate peer-auth-init and more javadoc (#1442)
Date Thu, 15 Feb 2018 18:45:00 GMT
This is an automated email from the ASF dual-hosted git repository.

jinmeiliao pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new 7c5cd35  GEODE-4633: un-deprecate peer-auth-init and more javadoc (#1442)
7c5cd35 is described below

commit 7c5cd35adf328ef2b665e6da522fc2028ab10631
Author: jinmeiliao <jiliao@pivotal.io>
AuthorDate: Thu Feb 15 10:44:55 2018 -0800

    GEODE-4633: un-deprecate peer-auth-init and more javadoc (#1442)
---
 .../java/org/apache/geode/cache/CacheCallback.java |   2 +-
 .../geode/distributed/ConfigurationProperties.java |   2 -
 .../security/shiro/GeodeAuthenticationToken.java   |  11 +-
 .../org/apache/geode/security/AuthInitialize.java  |  32 +++---
 .../org/apache/geode/security/SecurityManager.java |  11 +-
 .../IntegratedSecurityPeerAuthDUnitTest.java       | 123 +++++++++++++++++++++
 6 files changed, 157 insertions(+), 24 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/cache/CacheCallback.java b/geode-core/src/main/java/org/apache/geode/cache/CacheCallback.java
index 6eee1a1..2535350 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/CacheCallback.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/CacheCallback.java
@@ -40,5 +40,5 @@ public interface CacheCallback extends Declarable {
    * @see Region#destroyRegion()
    * @see AttributesMutator
    */
-  void close();
+  default void close() {};
 }
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
b/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
index 2344f0c..8cbe7c5 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
@@ -1516,8 +1516,6 @@ public interface ConfigurationProperties {
   String SECURITY_LOG_LEVEL = SECURITY_PREFIX + "log-level";
   /**
    * The static String definition of the <i>"security-peer-auth-init"</i> property
-   *
-   * @deprecated since Geode 1.0. use security-username and security-password
    */
   String SECURITY_PEER_AUTH_INIT = SECURITY_PREFIX + "peer-auth-init";
   /**
diff --git a/geode-core/src/main/java/org/apache/geode/internal/security/shiro/GeodeAuthenticationToken.java
b/geode-core/src/main/java/org/apache/geode/internal/security/shiro/GeodeAuthenticationToken.java
index 2b9c60e..5692b10 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/security/shiro/GeodeAuthenticationToken.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/security/shiro/GeodeAuthenticationToken.java
@@ -19,15 +19,20 @@ import java.util.Properties;
 
 import org.apache.shiro.authc.UsernamePasswordToken;
 
-import org.apache.geode.management.internal.security.ResourceConstants;
+import org.apache.geode.security.AuthInitialize;
 
 public class GeodeAuthenticationToken extends UsernamePasswordToken {
 
   Properties properties;
 
+  // in case security-username or security-password is not present in the properties, empty
string
+  // is used to avoid a NPE in shiro
   public GeodeAuthenticationToken(Properties properties) {
-    super(properties.getProperty(ResourceConstants.USER_NAME),
-        properties.getProperty(ResourceConstants.PASSWORD));
+    super(
+        properties.getProperty(AuthInitialize.SECURITY_USERNAME) == null ? ""
+            : properties.getProperty(AuthInitialize.SECURITY_USERNAME),
+        properties.getProperty(AuthInitialize.SECURITY_PASSWORD) == null ? ""
+            : properties.getProperty(AuthInitialize.SECURITY_PASSWORD));
     this.properties = properties;
   }
 
diff --git a/geode-core/src/main/java/org/apache/geode/security/AuthInitialize.java b/geode-core/src/main/java/org/apache/geode/security/AuthInitialize.java
index d8c874b..d3af515 100644
--- a/geode-core/src/main/java/org/apache/geode/security/AuthInitialize.java
+++ b/geode-core/src/main/java/org/apache/geode/security/AuthInitialize.java
@@ -21,13 +21,10 @@ import org.apache.geode.LogWriter;
 import org.apache.geode.cache.CacheCallback;
 import org.apache.geode.distributed.DistributedMember;
 import org.apache.geode.distributed.DistributedSystem;
-import org.apache.geode.internal.cache.GemFireCacheImpl;
-import org.apache.geode.internal.cache.InternalCache;
 
-// TODO Add example usage of this interface and configuration details
 /**
  * Specifies the mechanism to obtain credentials for a client or peer. It is mandatory for
clients
- * and peers when running in secure mode and an {@link Authenticator} has been configured
on the
+ * and peers when running in secure mode and a {@link SecurityManager} has been configured
on the
  * server/locator side respectively. Implementations should register name of the static creation
  * function (that returns an object of the class) as the <i>security-peer-auth-init</i>
system
  * property on peers and as the <i>security-client-auth-init</i> system property
on clients.
@@ -36,28 +33,31 @@ import org.apache.geode.internal.cache.InternalCache;
  */
 public interface AuthInitialize extends CacheCallback {
 
+  String SECURITY_USERNAME = "security-username";
+  String SECURITY_PASSWORD = "security-password";
+
   /**
    * Initialize the callback for a client/peer. This is invoked when a new connection from
a
    * client/peer is created with the host.
    *
+   * For future implementations, do not use these loggers, use log4j logger directly.
+   *
    * @param systemLogger {@link LogWriter} for system logs
    * @param securityLogger {@link LogWriter} for security logs
    *
    * @throws AuthenticationFailedException if some exception occurs during the initialization
    *
-   * @deprecated since Geode 1.0, use init()
    */
-  @Deprecated
-  void init(LogWriter systemLogger, LogWriter securityLogger) throws AuthenticationFailedException;
+  default void init(LogWriter systemLogger, LogWriter securityLogger)
+      throws AuthenticationFailedException {};
 
   /**
-   * @since Geode 1.0. implement this method instead of init with logwriters. Implementation
should
-   *        use log4j instead of these loggers.
+   *
+   * @since Geode 1.0.
+   * @deprecated in Geode 1.5. Never called by the product. Use {@link #init(LogWriter systemLogger,
+   *             LogWriter securityLogger)}
    */
-  default void init() {
-    InternalCache cache = GemFireCacheImpl.getInstance();
-    init(cache.getLogger(), cache.getSecurityLogger());
-  }
+  default void init() {}
 
   /**
    * Initialize with the given set of security properties and return the credentials for
the
@@ -80,7 +80,7 @@ public interface AuthInitialize extends CacheCallback {
    * @throws AuthenticationFailedException in case of failure to obtain the credentials
    *
    * @return the credentials to be used for the given <code>server</code>
-   *
+   *         It needs to contain "security-username" and "security-password"
    *         When using Integrated security, all members, peer/client will use the same credentials.
    *         but we still need to use these params to support the old authenticator
    */
@@ -92,8 +92,8 @@ public interface AuthInitialize extends CacheCallback {
    * @param securityProps
    * @return the credentials to be used. It needs to contain "security-username" and
    *         "security-password"
-   * @deprecated As of Geode 1.3, please implement getCredentials(Properties, DistributedMember,
-   *             boolean)
+   * @deprecated in Geode 1.3. Never called by the product. Use {@link #getCredentials(Properties
+   *             securityProps, DistributedMember server, boolean isPeer)}
    */
   default Properties getCredentials(Properties securityProps) {
     return getCredentials(securityProps, null, true);
diff --git a/geode-core/src/main/java/org/apache/geode/security/SecurityManager.java b/geode-core/src/main/java/org/apache/geode/security/SecurityManager.java
index b20da1a..4cb6237 100644
--- a/geode-core/src/main/java/org/apache/geode/security/SecurityManager.java
+++ b/geode-core/src/main/java/org/apache/geode/security/SecurityManager.java
@@ -17,7 +17,6 @@ package org.apache.geode.security;
 import java.util.Properties;
 
 import org.apache.geode.distributed.DistributedSystem;
-import org.apache.geode.security.AuthenticationFailedException;
 
 /**
  * User implementation of a authentication/authorization logic for Integrated Security. The
@@ -39,8 +38,16 @@ public interface SecurityManager {
   /**
    * Verify the credentials provided in the properties
    *
+   * Your security manager needs to validate credentials coming from all communication channels.
+   * If you use AuthInitialize to generate your client/peer credentials, then the input of
this
+   * method is the output of your AuthInitialize.getCredentials method. But remember that
this
+   * method will also need to validate credentials coming from gfsh/jmx/rest client, the
framework
+   * is putting the username/password under security-username and security-password keys
in the
+   * property, so your securityManager implementation needs to validate these kind of properties
+   * as well.
+   *
    * @param credentials it contains the security-username and security-password as keys of
the
-   *        properties
+   *        properties, also the properties generated by your AuthInitialize interface
    * @return a serializable principal object
    * @throws AuthenticationFailedException
    */
diff --git a/geode-core/src/test/java/org/apache/geode/security/IntegratedSecurityPeerAuthDUnitTest.java
b/geode-core/src/test/java/org/apache/geode/security/IntegratedSecurityPeerAuthDUnitTest.java
new file mode 100644
index 0000000..487926c
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/security/IntegratedSecurityPeerAuthDUnitTest.java
@@ -0,0 +1,123 @@
+/*
+ * 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.geode.security;
+
+import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
+import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_PEER_AUTH_INIT;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.io.IOException;
+import java.util.Properties;
+
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.test.dunit.rules.ClusterStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.categories.SecurityTest;
+import org.apache.geode.test.junit.rules.ServerStarterRule;
+
+/**
+ * this test specifically uses a peer-auth-init that doesn't produce credentials that has
+ * security-username or security-password. It produces a property that its SecurityManager
+ * knows how to authenticate/authorize. It's just to show that we can pass in properties
+ * other than security-username and security-password. But this will be doable only in peer/client
+ * case. For gfsh/rest, we still expected the credentials to be wrapped as expected.
+ */
+@Category({DistributedTest.class, SecurityTest.class})
+public class IntegratedSecurityPeerAuthDUnitTest {
+
+  @ClassRule
+  public static ClusterStartupRule cluster = new ClusterStartupRule();
+
+  private static MemberVM locator;
+
+  @BeforeClass
+  public static void beforeClass() throws Exception {
+    Properties properties = new Properties();
+    properties.put(SECURITY_MANAGER, MySecurityManager.class.getName());
+    locator = cluster.startLocatorVM(0, properties);
+  }
+
+  @Test
+  public void startServer1WithPeerAuthInit_success() throws IOException {
+    Properties props = new Properties();
+    props.setProperty(SECURITY_PEER_AUTH_INIT, MyAuthInit.class.getName());
+    props.setProperty("security-name", "server-1");
+    cluster.startServerVM(1, props, locator.getPort());
+  }
+
+  @Test
+  public void startServer2_not_authorized() {
+    Properties props = new Properties();
+    props.setProperty(SECURITY_PEER_AUTH_INIT, MyAuthInit.class.getName());
+    props.setProperty("security-name", "server-2");
+    int locatorPort = locator.getPort();
+    cluster.getVM(2).invoke(() -> {
+      ServerStarterRule server = new ServerStarterRule();
+      server.withProperties(props).withConnectionToLocator(locatorPort).withAutoStart();
+      assertThatThrownBy(() -> server.before()).isInstanceOf(GemFireSecurityException.class)
+          .hasMessageContaining("server-2 not authorized for CLUSTER:MANAGE");
+    });
+  }
+
+  @Test
+  public void startServer3_not_authenticated() {
+    Properties props = new Properties();
+    props.setProperty(SECURITY_PEER_AUTH_INIT, MyAuthInit.class.getName());
+    props.setProperty("security-name", "server-3");
+    int locatorPort = locator.getPort();
+    cluster.getVM(3).invoke(() -> {
+      ServerStarterRule server = new ServerStarterRule();
+      server.withProperties(props).withConnectionToLocator(locatorPort).withAutoStart();
+      assertThatThrownBy(() -> server.before()).isInstanceOf(GemFireSecurityException.class)
+          .hasMessageContaining("Authentication error");
+    });
+  }
+
+  public static class MyAuthInit implements AuthInitialize {
+    @Override
+    public Properties getCredentials(Properties securityProps, DistributedMember server,
+        boolean isPeer) throws AuthenticationFailedException {
+      Properties properties = new Properties();
+      properties.setProperty("name", securityProps.getProperty("security-name"));
+      return properties;
+    }
+  }
+
+  public static class MySecurityManager implements SecurityManager {
+    @Override
+    public Object authenticate(Properties credentials) throws AuthenticationFailedException
{
+      // server1 and server2 are authenticated, server3 is not
+      String name = credentials.getProperty("name");
+      if ("server-3".equals(name)) {
+        throw new AuthenticationFailedException("server-3 is not authenticated");
+      }
+      return name;
+    }
+
+    @Override
+    public boolean authorize(Object principal, ResourcePermission permission) {
+      // server-1 and server-2 are authenticated, but only server-1 is authorized
+      return ("server-1".equals(principal));
+    }
+  }
+}

-- 
To stop receiving notification emails like this one, please contact
jinmeiliao@apache.org.

Mime
View raw message