You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/05/20 11:26:49 UTC
[GitHub] [geode] jvarenina opened a new pull request #5139: GEODE-8149: New parameter and property introduced
jvarenina opened a new pull request #5139:
URL: https://github.com/apache/geode/pull/5139
* new parameter security-cn-auth-enabled introduced
* new property in security-manager interface introduced
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?
- [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, check Concourse for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on pull request #5139: GEODE-8149: New parameter and property introduced
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on pull request #5139:
URL: https://github.com/apache/geode/pull/5139#issuecomment-646339798
@jvarenina How about you close this PR until that is resolved then.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] jvarenina closed pull request #5139: GEODE-8149: New parameter and property introduced
Posted by GitBox <gi...@apache.org>.
jvarenina closed pull request #5139:
URL: https://github.com/apache/geode/pull/5139
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] jvarenina commented on pull request #5139: GEODE-8149: New parameter and property introduced
Posted by GitBox <gi...@apache.org>.
jvarenina commented on pull request #5139:
URL: https://github.com/apache/geode/pull/5139#issuecomment-645803468
Hi,
Yes, property is introduced for future works.
The reason I didn't update this PR is because we faced some issues when developing this feature which could affect this PR greatly when resolved. We have problem with retrieving certificates from RMI connections(e.g. jmx connections) and I will send more info related to this issue in dev list. We have also put on hold all other task related to this feature until we find the solution for RMI Connections.
@davebarnes97 thanks again for comments I will try to implement them as soon as possible.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] jvarenina commented on a change in pull request #5139: GEODE-8149: New parameter and property introduced
Posted by GitBox <gi...@apache.org>.
jvarenina commented on a change in pull request #5139:
URL: https://github.com/apache/geode/pull/5139#discussion_r429084330
##########
File path: geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
##########
@@ -2032,6 +2032,23 @@
* Default: empty. All security components use basic (username/password) authentication
*/
String SECURITY_AUTH_TOKEN_ENABLED_COMPONENTS = SECURITY_PREFIX + "auth-token-enabled-components";
+
+ /**
+ * The static String definition of the <i>"security-cn-auth-enabled"</i> property
+ * </p>
+ *
+ * <U>Description</U> This parameter only works if SSL with two-way handshake is enabled on
+ * all components and security manager is enabled. This property will determine if common name
+ * from client certificate will be used for authentication and authorization.
Review comment:
Thanks for the comments. I will rephrase it as you suggested.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] jvarenina commented on pull request #5139: GEODE-8149: New parameter and property introduced
Posted by GitBox <gi...@apache.org>.
jvarenina commented on pull request #5139:
URL: https://github.com/apache/geode/pull/5139#issuecomment-632540478
> This PR provides documentation at the Javadoc level.
> Consider where in the User Guide this should be mentioned, then
>
> * add a User Guide explanation as part of this PR/JIRA combination, or
>
> * add a subticket to GEODE-8118, the parent ticket of this one (GEODE-8149) to handle the User Guide docs.
> Either of the above will satisfy my concern. Thanks.
OK, I will create sub-ticket to handle User Guide docs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] davebarnes97 commented on pull request #5139: GEODE-8149: New parameter and property introduced
Posted by GitBox <gi...@apache.org>.
davebarnes97 commented on pull request #5139:
URL: https://github.com/apache/geode/pull/5139#issuecomment-645594536
> OK, I will create sub-ticket to handle User Guide docs.
I'm not finding the sub-ticket for docs. Please provide the ticket number - thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] davebarnes97 commented on a change in pull request #5139: GEODE-8149: New parameter and property introduced
Posted by GitBox <gi...@apache.org>.
davebarnes97 commented on a change in pull request #5139:
URL: https://github.com/apache/geode/pull/5139#discussion_r428315433
##########
File path: geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
##########
@@ -2032,6 +2032,23 @@
* Default: empty. All security components use basic (username/password) authentication
*/
String SECURITY_AUTH_TOKEN_ENABLED_COMPONENTS = SECURITY_PREFIX + "auth-token-enabled-components";
+
+ /**
+ * The static String definition of the <i>"security-cn-auth-enabled"</i> property
+ * </p>
+ *
+ * <U>Description</U> This parameter only works if SSL with two-way handshake is enabled on
+ * all components and security manager is enabled. This property will determine if common name
+ * from client certificate will be used for authentication and authorization.
Review comment:
Present tense would be clearer:
This property allows the common name from the client certificate to be used for authentication and authorization.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org