[jira] [Commented] (GEODE-3412) Implement a basic authentication mechanism for the new protocol

2017-08-15 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/GEODE-3412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16127854#comment-16127854
 ] 

ASF GitHub Bot commented on GEODE-3412:
---

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/714


> Implement a basic authentication mechanism for the new protocol
> ---
>
> Key: GEODE-3412
> URL: https://issues.apache.org/jira/browse/GEODE-3412
> Project: Geode
>  Issue Type: New Feature
>  Components: client/server
>Reporter: Brian Rowe
>Assignee: Brian Rowe
>
> Implement a simple username/password authentication for the new protocol.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (GEODE-3412) Implement a basic authentication mechanism for the new protocol

2017-08-15 Thread ASF subversion and git services (JIRA)

[ 
https://issues.apache.org/jira/browse/GEODE-3412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16127853#comment-16127853
 ] 

ASF subversion and git services commented on GEODE-3412:


Commit bc655eb9b3683282705e7449f0f7a720c1a6243d in geode's branch 
refs/heads/develop from [~WireBaron]
[ https://git-wip-us.apache.org/repos/asf?p=geode.git;h=bc655eb ]

GEODE-3412: adding files missing from last commit. This now closes #714


> Implement a basic authentication mechanism for the new protocol
> ---
>
> Key: GEODE-3412
> URL: https://issues.apache.org/jira/browse/GEODE-3412
> Project: Geode
>  Issue Type: New Feature
>  Components: client/server
>Reporter: Brian Rowe
>Assignee: Brian Rowe
>
> Implement a simple username/password authentication for the new protocol.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (GEODE-3412) Implement a basic authentication mechanism for the new protocol

2017-08-15 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/GEODE-3412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16127710#comment-16127710
 ] 

ASF GitHub Bot commented on GEODE-3412:
---

GitHub user WireBaron opened a pull request:

https://github.com/apache/geode/pull/714

GEODE-3412: adding files missing from last commit

This is adding some files which should have been in the last commit for 
this feature.

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced in 
the commit message?

- [x] Has your PR been rebased against the latest commit within the target 
branch (typically `develop`)?

- [x] Is your initial contribution a single, squashed commit?

- [ ] Does `gradlew build` run cleanly?

- [ ] Have you written or updated unit tests to verify your changes?

- [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)?

### Note:
Please ensure that once the PR is submitted, you check travis-ci for build 
issues and
submit an update to your PR as soon as possible. If you need help, please 
send an
email to d...@geode.apache.org.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/WireBaron/geode feature/GEODE3412

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/geode/pull/714.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #714


commit 1022f323a0061f43155bd5ea74879981b8bc8ffc
Author: Brian Rowe 
Date:   2017-08-15T18:21:51Z

GEODE-3412: adding files missing from last commit




> Implement a basic authentication mechanism for the new protocol
> ---
>
> Key: GEODE-3412
> URL: https://issues.apache.org/jira/browse/GEODE-3412
> Project: Geode
>  Issue Type: New Feature
>  Components: client/server
>Reporter: Brian Rowe
>Assignee: Brian Rowe
>
> Implement a simple username/password authentication for the new protocol.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (GEODE-3412) Implement a basic authentication mechanism for the new protocol

2017-08-15 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/GEODE-3412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16127626#comment-16127626
 ] 

ASF GitHub Bot commented on GEODE-3412:
---

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/707


> Implement a basic authentication mechanism for the new protocol
> ---
>
> Key: GEODE-3412
> URL: https://issues.apache.org/jira/browse/GEODE-3412
> Project: Geode
>  Issue Type: New Feature
>  Components: client/server
>Reporter: Brian Rowe
>
> Implement a simple username/password authentication for the new protocol.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (GEODE-3412) Implement a basic authentication mechanism for the new protocol

2017-08-15 Thread ASF subversion and git services (JIRA)

[ 
https://issues.apache.org/jira/browse/GEODE-3412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16127625#comment-16127625
 ] 

ASF subversion and git services commented on GEODE-3412:


Commit a7a197d633a20ee3a2161d47389581858745c1cc in geode's branch 
refs/heads/develop from [~WireBaron]
[ https://git-wip-us.apache.org/repos/asf?p=geode.git;h=a7a197d ]

GEODE-3412: Add simple authentication flow to protobuf protocol. This now 
closes #707

This change adds a simple username/password validation to the protobuf protocol.
It also adds a new configuration parameter to specify the type of 
authentication required.

Signed-off-by: Galen O'Sullivan 


> Implement a basic authentication mechanism for the new protocol
> ---
>
> Key: GEODE-3412
> URL: https://issues.apache.org/jira/browse/GEODE-3412
> Project: Geode
>  Issue Type: New Feature
>  Components: client/server
>Reporter: Brian Rowe
>
> Implement a simple username/password authentication for the new protocol.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (GEODE-3412) Implement a basic authentication mechanism for the new protocol

2017-08-15 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/GEODE-3412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16127571#comment-16127571
 ] 

ASF GitHub Bot commented on GEODE-3412:
---

Github user pivotal-amurmann commented on a diff in the pull request:

https://github.com/apache/geode/pull/707#discussion_r133246611
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
 ---
@@ -22,59 +22,89 @@
 
 import java.io.IOException;
 import java.net.Socket;
+import java.util.HashMap;
 import java.util.Iterator;
+import java.util.Map;
 import java.util.ServiceLoader;
-import javax.management.ServiceNotFoundException;
 
 /**
  * Creates instances of ServerConnection based on the connection mode 
provided.
  */
 public class ServerConnectionFactory {
-  private static ClientProtocolMessageHandler protobufProtocolHandler;
-  private static final Object protocolLoadLock = new Object();
+  private ClientProtocolMessageHandler protobufProtocolHandler;
+  private Map authenticators 
= null;
 
-  private static ClientProtocolMessageHandler 
findClientProtocolMessageHandler() {
+  public ServerConnectionFactory() {}
+
+  private synchronized void initializeAuthenticatorsMap() {
+if (authenticators != null) {
+  return;
+}
+authenticators = new HashMap<>();
+ServiceLoader loader = 
ServiceLoader.load(StreamAuthenticator.class);
+for (StreamAuthenticator streamAuthenticator : loader) {
+  authenticators.put(streamAuthenticator.implementationID(), 
streamAuthenticator.getClass());
+}
+  }
+
+  private synchronized ClientProtocolMessageHandler 
initializeMessageHandler() {
 if (protobufProtocolHandler != null) {
   return protobufProtocolHandler;
 }
+ServiceLoader loader =
+ServiceLoader.load(ClientProtocolMessageHandler.class);
+Iterator iterator = loader.iterator();
 
-synchronized (protocolLoadLock) {
-  if (protobufProtocolHandler != null) {
-return protobufProtocolHandler;
-  }
-
-  ServiceLoader loader =
-  ServiceLoader.load(ClientProtocolMessageHandler.class);
-  Iterator iterator = loader.iterator();
-
-  if (!iterator.hasNext()) {
-throw new ServiceLoadingFailureException(
-"ClientProtocolMessageHandler implementation not found in 
JVM");
-  }
+if (!iterator.hasNext()) {
+  throw new ServiceLoadingFailureException(
+  "There is no ClientProtocolMessageHandler implementation found 
in JVM");
+}
 
-  ClientProtocolMessageHandler returnValue = iterator.next();
+protobufProtocolHandler = iterator.next();
+return protobufProtocolHandler;
+  }
 
-  if (iterator.hasNext()) {
+  private StreamAuthenticator findStreamAuthenticator(String 
implementationID) {
+if (authenticators == null) {
+  initializeAuthenticatorsMap();
+}
+Class streamAuthenticatorClass =
+authenticators.get(implementationID);
+if (streamAuthenticatorClass == null) {
+  throw new ServiceLoadingFailureException(
+  "Could not find implementation for StreamAuthenticator with 
implementation ID "
+  + implementationID);
+} else {
+  try {
+return streamAuthenticatorClass.newInstance();
+  } catch (InstantiationException | IllegalAccessException e) {
 throw new ServiceLoadingFailureException(
-"Multiple service implementations found for 
ClientProtocolMessageHandler");
--- End diff --

Where are we now handling the case of having multiple implementations?


> Implement a basic authentication mechanism for the new protocol
> ---
>
> Key: GEODE-3412
> URL: https://issues.apache.org/jira/browse/GEODE-3412
> Project: Geode
>  Issue Type: New Feature
>  Components: client/server
>Reporter: Brian Rowe
>
> Implement a simple username/password authentication for the new protocol.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (GEODE-3412) Implement a basic authentication mechanism for the new protocol

2017-08-14 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/GEODE-3412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16125278#comment-16125278
 ] 

ASF GitHub Bot commented on GEODE-3412:
---

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/707#discussion_r132878512
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
 ---
@@ -72,9 +99,15 @@ public static ServerConnection 
makeServerConnection(Socket s, InternalCache c,
 throw new IOException("Acceptor received unknown communication 
mode: " + communicationMode);
   } else {
 protobufProtocolHandler = findClientProtocolMessageHandler();
-return new GenericProtocolServerConnection(s, c, helper, stats, 
hsTimeout, socketBufferSize,
-communicationModeStr, communicationMode, acceptor, 
protobufProtocolHandler,
-securityService);
+authenticatorClass = findStreamAuthenticator(
+
c.getInternalDistributedSystem().getConfig().getProtobufProtocolAuthenticationMode());
+try {
+  return new GenericProtocolServerConnection(s, c, helper, stats, 
hsTimeout,
+  socketBufferSize, communicationModeStr, communicationMode, 
acceptor,
+  protobufProtocolHandler, securityService, 
authenticatorClass.newInstance());
--- End diff --

We find the class exactly once, and create a new instance per protocol 
instance. We then create an instance per `GenericProtocolServerConnection`. 
This is because the authenticator (unlike the protocol handler) is stateful, so 
we can't share it between connections.


> Implement a basic authentication mechanism for the new protocol
> ---
>
> Key: GEODE-3412
> URL: https://issues.apache.org/jira/browse/GEODE-3412
> Project: Geode
>  Issue Type: New Feature
>  Components: client/server
>Reporter: Brian Rowe
>
> Implement a simple username/password authentication for the new protocol.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (GEODE-3412) Implement a basic authentication mechanism for the new protocol

2017-08-14 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/GEODE-3412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16125275#comment-16125275
 ] 

ASF GitHub Bot commented on GEODE-3412:
---

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/707#discussion_r132878378
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
 ---
@@ -63,6 +65,31 @@ private static ClientProtocolMessageHandler 
findClientProtocolMessageHandler() {
 }
   }
 
+  private static Class 
findStreamAuthenticator(
+  String implementationID) {
+if (authenticatorClass != null) {
+  return authenticatorClass;
+}
+
+synchronized (streamAuthenticatorLoadLock) {
--- End diff --

Once `authenticatorClass` is initialized, there will no longer be any 
synchronization necessary in this method. This means that each thread will need 
to synchronize at most once, which means no locking / cache misses.


> Implement a basic authentication mechanism for the new protocol
> ---
>
> Key: GEODE-3412
> URL: https://issues.apache.org/jira/browse/GEODE-3412
> Project: Geode
>  Issue Type: New Feature
>  Components: client/server
>Reporter: Brian Rowe
>
> Implement a simple username/password authentication for the new protocol.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (GEODE-3412) Implement a basic authentication mechanism for the new protocol

2017-08-14 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/GEODE-3412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16125274#comment-16125274
 ] 

ASF GitHub Bot commented on GEODE-3412:
---

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/707#discussion_r132878110
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java
 ---
@@ -2433,6 +2433,25 @@
   String getRedundancyZone();
 
   /**
+   * @since Geode 1.??? TODO FIXME
+   */
+  @ConfigAttribute(type = String.class)
+  String PROTOBUF_PROTOCOL_AUTHENTICATION_MODE_NAME = 
PROTOBUF_PROTOCOL_AUTHENTICATION_MODE;
+  String DEFAULT_PROTOBUF_PROTOCOL_AUTHENTICATION_MODE = "NOOP";
+
+  /**
+   * @since Geode 1.??? TODO FIXME
+   */
+  @ConfigAttributeSetter(name = PROTOBUF_PROTOCOL_AUTHENTICATION_MODE)
+  void setProtobufProtocolAuthenticationMode(String authenticationMode);
+
+  /**
+   * @since Geode 1.??? TODO FIXME
+   */
+  @ConfigAttributeGetter(name = PROTOBUF_PROTOCOL_AUTHENTICATION_MODE)
+  String getProtobufProtocolAuthenticationMode();
+
+  /**
--- End diff --

That's a reasonable choice. Do you think this should be another 
undocumented system property? It might be fine for an alpha, but I don't think 
we should do it for long after that.


> Implement a basic authentication mechanism for the new protocol
> ---
>
> Key: GEODE-3412
> URL: https://issues.apache.org/jira/browse/GEODE-3412
> Project: Geode
>  Issue Type: New Feature
>  Components: client/server
>Reporter: Brian Rowe
>
> Implement a simple username/password authentication for the new protocol.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (GEODE-3412) Implement a basic authentication mechanism for the new protocol

2017-08-13 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/GEODE-3412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16125273#comment-16125273
 ] 

ASF GitHub Bot commented on GEODE-3412:
---

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/707#discussion_r132877993
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
 ---
@@ -1378,6 +1379,18 @@
*/
   String NAME = "name";
   /**
+   * The authentication mode for the protobuf client-server protocol.
+   *
+   * 
+   * Description: Specifies the authentication mode used by the 
geode-protobuf module.
+   * 
+   * Default: "NOOP"
+   * 
+   * Allowed values: "NOOP" "SIMPLE"
+   */
+  @Experimental
+  String PROTOBUF_PROTOCOL_AUTHENTICATION_MODE = 
"protobuf-protocol-authentication-mode";
--- End diff --

The property is a String representing the authentication mode to be used by 
the protobuf protocol. It currently has two valid settings. The protobuf in the 
name refers to the new client protocol (which we're calling the protobuf 
protocol).

I don't think we're quite ready to take the feature flag out yet, but we 
should consider it as this feature nears alpha. Or moving the feature flag to a 
better config mechanism (DistributionConfig), so that this property and that 
one are in approximately the same place.


> Implement a basic authentication mechanism for the new protocol
> ---
>
> Key: GEODE-3412
> URL: https://issues.apache.org/jira/browse/GEODE-3412
> Project: Geode
>  Issue Type: New Feature
>  Components: client/server
>Reporter: Brian Rowe
>
> Implement a simple username/password authentication for the new protocol.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (GEODE-3412) Implement a basic authentication mechanism for the new protocol

2017-08-11 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/GEODE-3412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16123662#comment-16123662
 ] 

ASF GitHub Bot commented on GEODE-3412:
---

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/707#discussion_r132737475
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java
 ---
@@ -2433,6 +2433,25 @@
   String getRedundancyZone();
 
   /**
+   * @since Geode 1.??? TODO FIXME
+   */
+  @ConfigAttribute(type = String.class)
+  String PROTOBUF_PROTOCOL_AUTHENTICATION_MODE_NAME = 
PROTOBUF_PROTOCOL_AUTHENTICATION_MODE;
+  String DEFAULT_PROTOBUF_PROTOCOL_AUTHENTICATION_MODE = "NOOP";
+
+  /**
+   * @since Geode 1.??? TODO FIXME
+   */
+  @ConfigAttributeSetter(name = PROTOBUF_PROTOCOL_AUTHENTICATION_MODE)
+  void setProtobufProtocolAuthenticationMode(String authenticationMode);
+
+  /**
+   * @since Geode 1.??? TODO FIXME
+   */
+  @ConfigAttributeGetter(name = PROTOBUF_PROTOCOL_AUTHENTICATION_MODE)
+  String getProtobufProtocolAuthenticationMode();
+
+  /**
--- End diff --

Without an official release version I don't think we should have any of the 
properties/configurations public until release.


> Implement a basic authentication mechanism for the new protocol
> ---
>
> Key: GEODE-3412
> URL: https://issues.apache.org/jira/browse/GEODE-3412
> Project: Geode
>  Issue Type: New Feature
>  Components: client/server
>Reporter: Brian Rowe
>
> Implement a simple username/password authentication for the new protocol.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (GEODE-3412) Implement a basic authentication mechanism for the new protocol

2017-08-11 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/GEODE-3412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16123664#comment-16123664
 ] 

ASF GitHub Bot commented on GEODE-3412:
---

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/707#discussion_r132735527
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
 ---
@@ -63,6 +65,31 @@ private static ClientProtocolMessageHandler 
findClientProtocolMessageHandler() {
 }
   }
 
+  private static Class 
findStreamAuthenticator(
+  String implementationID) {
+if (authenticatorClass != null) {
+  return authenticatorClass;
+}
+
+synchronized (streamAuthenticatorLoadLock) {
+  if (authenticatorClass != null) {
+return authenticatorClass;
+  }
+
+  ServiceLoader loader = 
ServiceLoader.load(StreamAuthenticator.class);
+
+  for (StreamAuthenticator classInstance : loader) {
+if (implementationID.equals(classInstance.implementationID())) {
+  return classInstance.getClass();
--- End diff --

Why do we get an instance of the StreamAuthenticator, just to call 
`getClass` on it, and then later we create a new instance of class from this 
instance. 


> Implement a basic authentication mechanism for the new protocol
> ---
>
> Key: GEODE-3412
> URL: https://issues.apache.org/jira/browse/GEODE-3412
> Project: Geode
>  Issue Type: New Feature
>  Components: client/server
>Reporter: Brian Rowe
>
> Implement a simple username/password authentication for the new protocol.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (GEODE-3412) Implement a basic authentication mechanism for the new protocol

2017-08-11 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/GEODE-3412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16123663#comment-16123663
 ] 

ASF GitHub Bot commented on GEODE-3412:
---

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/707#discussion_r132738100
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/NoOpStreamAuthenticator.java
 ---
@@ -0,0 +1,43 @@
+/*
+ * 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.internal.cache.tier.sockets;
--- End diff --

I'm not sure this class should live in this package. Is it not more related 
to security?


> Implement a basic authentication mechanism for the new protocol
> ---
>
> Key: GEODE-3412
> URL: https://issues.apache.org/jira/browse/GEODE-3412
> Project: Geode
>  Issue Type: New Feature
>  Components: client/server
>Reporter: Brian Rowe
>
> Implement a simple username/password authentication for the new protocol.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (GEODE-3412) Implement a basic authentication mechanism for the new protocol

2017-08-11 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/GEODE-3412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16123667#comment-16123667
 ] 

ASF GitHub Bot commented on GEODE-3412:
---

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/707#discussion_r132737182
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
 ---
@@ -1378,6 +1379,18 @@
*/
   String NAME = "name";
   /**
+   * The authentication mode for the protobuf client-server protocol.
+   *
+   * 
+   * Description: Specifies the authentication mode used by the 
geode-protobuf module.
+   * 
+   * Default: "NOOP"
+   * 
+   * Allowed values: "NOOP" "SIMPLE"
+   */
+  @Experimental
+  String PROTOBUF_PROTOCOL_AUTHENTICATION_MODE = 
"protobuf-protocol-authentication-mode";
--- End diff --

This property is misleading. It is NOT a protobuf specific authentication 
mode. It is merely an authentication mechanism that uses protobuf underneath 
the covers.
1) A different property name is to be used
2) With this property, the feature toggle should also maybe be removed??!!? 
One cannot live without the other


> Implement a basic authentication mechanism for the new protocol
> ---
>
> Key: GEODE-3412
> URL: https://issues.apache.org/jira/browse/GEODE-3412
> Project: Geode
>  Issue Type: New Feature
>  Components: client/server
>Reporter: Brian Rowe
>
> Implement a simple username/password authentication for the new protocol.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (GEODE-3412) Implement a basic authentication mechanism for the new protocol

2017-08-11 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/GEODE-3412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16123665#comment-16123665
 ] 

ASF GitHub Bot commented on GEODE-3412:
---

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/707#discussion_r132738771
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
 ---
@@ -63,6 +65,31 @@ private static ClientProtocolMessageHandler 
findClientProtocolMessageHandler() {
 }
   }
 
+  private static Class 
findStreamAuthenticator(
+  String implementationID) {
+if (authenticatorClass != null) {
+  return authenticatorClass;
+}
+
+synchronized (streamAuthenticatorLoadLock) {
--- End diff --

why do we prefer this approach over a synchronized method?


> Implement a basic authentication mechanism for the new protocol
> ---
>
> Key: GEODE-3412
> URL: https://issues.apache.org/jira/browse/GEODE-3412
> Project: Geode
>  Issue Type: New Feature
>  Components: client/server
>Reporter: Brian Rowe
>
> Implement a simple username/password authentication for the new protocol.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (GEODE-3412) Implement a basic authentication mechanism for the new protocol

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/GEODE-3412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122032#comment-16122032
 ] 

ASF GitHub Bot commented on GEODE-3412:
---

GitHub user WireBaron opened a pull request:

https://github.com/apache/geode/pull/707

GEODE-3412: Add simple authentication flow to protobuf protocol.

@pivotal-amurmann @galen-pivotal @kohlmu-pivotal @bschuchardt @hiteshk25 

This change adds a simple username/password validation to the protobuf 
protocol.
It also adds a new configuration parameter to specify the type of 
authentication required.

Signed-off-by: Galen O'Sullivan 

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced in 
the commit message?

- [ ] Has your PR been rebased against the latest commit within the target 
branch (typically `develop`)?

- [x] Is your initial contribution a single, squashed commit?

- [x] Does `gradlew build` run cleanly?

- [x] Have you written or updated unit tests to verify your changes?

- [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)?

### Note:
Please ensure that once the PR is submitted, you check travis-ci for build 
issues and
submit an update to your PR as soon as possible. If you need help, please 
send an
email to d...@geode.apache.org.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/WireBaron/geode feature/GEODE-3412

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/geode/pull/707.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #707


commit daa140875c4f65838dde4ae0f627a9551bea7516
Author: Brian Rowe 
Date:   2017-08-10T18:16:25Z

GEODE-3412: Add simple authentication flow to protobuf protocol.

This change adds a simple username/password validation to the protobuf 
protocol.
It also adds a new configuration parameter to specify the type of 
authentication required.

Signed-off-by: Galen O'Sullivan 




> Implement a basic authentication mechanism for the new protocol
> ---
>
> Key: GEODE-3412
> URL: https://issues.apache.org/jira/browse/GEODE-3412
> Project: Geode
>  Issue Type: New Feature
>  Components: client/server
>Reporter: Brian Rowe
>
> Implement a simple username/password authentication for the new protocol.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)