You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by gracemeilen <gi...@git.apache.org> on 2016/08/02 18:07:33 UTC

[GitHub] incubator-geode pull request #224: GEODE-1648: Introduce security-enabled-co...

GitHub user gracemeilen opened a pull request:

    https://github.com/apache/incubator-geode/pull/224

    GEODE-1648: Introduce security-enabled-components and tests

    

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

    $ git pull https://github.com/gracemeilen/incubator-geode feature/GEODE-1648

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

    https://github.com/apache/incubator-geode/pull/224.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 #224
    
----
commit 4d9796e57bfa26cb1207145106eccb78447292a0
Author: gmeilen <gr...@gmail.com>
Date:   2016-08-02T17:52:18Z

    GEODE-1648: Introduce security-enabled-components and tests

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode issue #224: GEODE-1648: Introduce security-enabled-component...

Posted by kirklund <gi...@git.apache.org>.
Github user kirklund commented on the issue:

    https://github.com/apache/incubator-geode/pull/224
  
    1) we need to fix the javadocs for security-enabled-components on DistributedSystem class
    2) move SecurableComponent enum into internal.security pkg
    3) consider collapsing SecurableComponents interface into SecurableComponent enum (we don't really need both
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #224: GEODE-1648: Introduce security-enabled-co...

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/224#discussion_r73212878
  
    --- Diff: geode-core/src/main/java/com/gemstone/gemfire/management/internal/ManagementAgent.java ---
    @@ -447,8 +449,7 @@ public synchronized void start() throws IOException {
           }
         };
     
    -    String shiroConfig = this.config.getShiroInit();
    -    if (! StringUtils.isBlank(shiroConfig) || isIntegratedSecurity()) {
    +    if (isIntegratedSecurity()) {
    --- End diff --
    
    probably be easier to read if it just read "if (GeodeSecurityUtil.isJmxSecurityEnabled())" here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #224: GEODE-1648: Introduce security-enabled-co...

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/224#discussion_r73211230
  
    --- Diff: geode-core/src/main/java/com/gemstone/gemfire/internal/security/GeodeSecurityUtil.java ---
    @@ -471,16 +499,27 @@ public static PostProcessor getPostProcessor() {
         return postProcessor;
       }
     
    -  public static boolean isClientSecurityRequired() {
    -    return isClientAuthenticator || isIntegratedSecurity;
    +  public static boolean isIntegratedSecurity(){
    --- End diff --
    
    We can get rid of this public method now. I believe now all the other components are just using component specific security enabled function.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #224: GEODE-1648: Introduce security-enabled-co...

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/224#discussion_r73207985
  
    --- Diff: geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/AbstractDistributionConfig.java ---
    @@ -498,6 +502,29 @@ protected String checkRedisBindAddress(String value) {
         return value;
       }
     
    +  /**
    +   * First check if sslComponents are in the list of valid components. If so, check that no other *-ssl-* properties other than cluster-ssl-* are set.
    --- End diff --
    
    comments need to be changed here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #224: GEODE-1648: Introduce security-enabled-co...

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/224#discussion_r73213204
  
    --- Diff: geode-core/src/main/java/com/gemstone/gemfire/management/internal/ManagementAgent.java ---
    @@ -509,8 +510,7 @@ private void registerAccessControlMBean() {
     
     
       private boolean isIntegratedSecurity() {
    --- End diff --
    
    We can get rid of this method. I believe this is called to check if we need to enabled pulse security in the same file as well. Yeah, what about pulse security? Do we need to add another enum or just say if jmx is enabled. pulse should be enabled as well?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #224: GEODE-1648: Introduce security-enabled-co...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-geode/pull/224


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---