You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jinmei Liao <ji...@pivotal.io> on 2016/09/19 21:13:24 UTC

Review Request 52067: GEODE-1909: add authorization in GMSAuthenticator

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52067/
-----------------------------------------------------------

Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Repository: geode


Description
-------

GEODE-1909: add authorization in GMSAuthenticator

The user who get authenticated in the cluster needs to have CLUSTER:MANAGE previlage as well. Note this is NOT the user who issued "start server" command in gfsh, but is the peer user whose credential is put in the server's security.properties file.


Diffs
-----

  geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberFactory.java 08178e92ad9b4db30534c80873c63e8b4c11808f 
  geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java 3f030c9af4ab0503ad201c9172fe61a256eb961a 
  geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java d15e8bfa09b3c24d8c2db10d524bfce0d4b61ad5 
  geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java b61201ef60b4dc3ea6d42e7ae30d305e2ebb7e35 
  geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithAuthenticatorTest.java 45a5881207179e950bb40a897660f63f583a4e19 
  geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithSecurityManagerTest.java ddb208d043a8503b52119e363ff23c54acd0a4f4 
  geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java PRE-CREATION 
  geode-core/src/test/resources/org/apache/geode/management/internal/security/cacheServer.json 3bb3e2f010da9e97af3ade60dbd4b4b45b9370b3 

Diff: https://reviews.apache.org/r/52067/diff/


Testing
-------

precheckin running


Thanks,

Jinmei Liao


Re: Review Request 52067: GEODE-1909: add authorization in GMSAuthenticator

Posted by Jinmei Liao <ji...@pivotal.io>.

> On Sept. 22, 2016, 5:41 p.m., Kirk Lund wrote:
> > geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java, line 59
> > <https://reviews.apache.org/r/52067/diff/3/?file=1505481#file1505481line59>
> >
> >     If you change this test to pass zero in for locatorPort, does the locator start up on a random port without having to pre-fetch a random port with AvailablePortHelper? If it does, then that would actually be better. You would then return the locator's final port back through the invoke into an int variable:
> >     
> >     this.locatorPort = locator.invoke(() -> {
> >     ...
> >     return port;
> >     });

Good point. I will make the change.


- Jinmei


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52067/#review150034
-----------------------------------------------------------


On Sept. 20, 2016, 12:59 a.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52067/
> -----------------------------------------------------------
> 
> (Updated Sept. 20, 2016, 12:59 a.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-1909: add authorization in GMSAuthenticator
> 
> The user who get authenticated in the cluster needs to have CLUSTER:MANAGE previlage as well. Note this is NOT the user who issued "start server" command in gfsh, but is the peer user whose credential is put in the server's security.properties file.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberFactory.java 08178e92ad9b4db30534c80873c63e8b4c11808f 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java 3f030c9af4ab0503ad201c9172fe61a256eb961a 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java d15e8bfa09b3c24d8c2db10d524bfce0d4b61ad5 
>   geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java b61201ef60b4dc3ea6d42e7ae30d305e2ebb7e35 
>   geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithAuthenticatorTest.java 45a5881207179e950bb40a897660f63f583a4e19 
>   geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithSecurityManagerTest.java ddb208d043a8503b52119e363ff23c54acd0a4f4 
>   geode-core/src/test/java/org/apache/geode/security/IntegratedSecurityPeerAuthDistributedTest.java acc2b86e8ea3dfa4402e9bd6d54d80741c73c2b6 
>   geode-core/src/test/java/org/apache/geode/security/P2PAuthenticationDUnitTest.java 9fcf4cdde5ca74564372657e2c4e0c45528e878a 
>   geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java PRE-CREATION 
>   geode-core/src/test/resources/org/apache/geode/management/internal/security/cacheServer.json 3bb3e2f010da9e97af3ade60dbd4b4b45b9370b3 
>   geode-core/src/test/resources/org/apache/geode/security/peerAuth.json 9bd893612e9ee3a3d300653fa449ee19a03f37c6 
> 
> Diff: https://reviews.apache.org/r/52067/diff/
> 
> 
> Testing
> -------
> 
> precheckin successful
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>


Re: Review Request 52067: GEODE-1909: add authorization in GMSAuthenticator

Posted by Kirk Lund <ki...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52067/#review150034
-----------------------------------------------------------


Fix it, then Ship it!




One recommendation to consider. Ship It!


geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java (line 59)
<https://reviews.apache.org/r/52067/#comment217849>

    If you change this test to pass zero in for locatorPort, does the locator start up on a random port without having to pre-fetch a random port with AvailablePortHelper? If it does, then that would actually be better. You would then return the locator's final port back through the invoke into an int variable:
    
    this.locatorPort = locator.invoke(() -> {
    ...
    return port;
    });


- Kirk Lund


On Sept. 20, 2016, 12:59 a.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52067/
> -----------------------------------------------------------
> 
> (Updated Sept. 20, 2016, 12:59 a.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-1909: add authorization in GMSAuthenticator
> 
> The user who get authenticated in the cluster needs to have CLUSTER:MANAGE previlage as well. Note this is NOT the user who issued "start server" command in gfsh, but is the peer user whose credential is put in the server's security.properties file.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberFactory.java 08178e92ad9b4db30534c80873c63e8b4c11808f 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java 3f030c9af4ab0503ad201c9172fe61a256eb961a 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java d15e8bfa09b3c24d8c2db10d524bfce0d4b61ad5 
>   geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java b61201ef60b4dc3ea6d42e7ae30d305e2ebb7e35 
>   geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithAuthenticatorTest.java 45a5881207179e950bb40a897660f63f583a4e19 
>   geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithSecurityManagerTest.java ddb208d043a8503b52119e363ff23c54acd0a4f4 
>   geode-core/src/test/java/org/apache/geode/security/IntegratedSecurityPeerAuthDistributedTest.java acc2b86e8ea3dfa4402e9bd6d54d80741c73c2b6 
>   geode-core/src/test/java/org/apache/geode/security/P2PAuthenticationDUnitTest.java 9fcf4cdde5ca74564372657e2c4e0c45528e878a 
>   geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java PRE-CREATION 
>   geode-core/src/test/resources/org/apache/geode/management/internal/security/cacheServer.json 3bb3e2f010da9e97af3ade60dbd4b4b45b9370b3 
>   geode-core/src/test/resources/org/apache/geode/security/peerAuth.json 9bd893612e9ee3a3d300653fa449ee19a03f37c6 
> 
> Diff: https://reviews.apache.org/r/52067/diff/
> 
> 
> Testing
> -------
> 
> precheckin successful
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>


Re: Review Request 52067: GEODE-1909: add authorization in GMSAuthenticator

Posted by Kevin Duling <kd...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52067/#review150039
-----------------------------------------------------------


Ship it!




Ship It!

- Kevin Duling


On Sept. 19, 2016, 5:59 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52067/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2016, 5:59 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-1909: add authorization in GMSAuthenticator
> 
> The user who get authenticated in the cluster needs to have CLUSTER:MANAGE previlage as well. Note this is NOT the user who issued "start server" command in gfsh, but is the peer user whose credential is put in the server's security.properties file.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberFactory.java 08178e92ad9b4db30534c80873c63e8b4c11808f 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java 3f030c9af4ab0503ad201c9172fe61a256eb961a 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java d15e8bfa09b3c24d8c2db10d524bfce0d4b61ad5 
>   geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java b61201ef60b4dc3ea6d42e7ae30d305e2ebb7e35 
>   geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithAuthenticatorTest.java 45a5881207179e950bb40a897660f63f583a4e19 
>   geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithSecurityManagerTest.java ddb208d043a8503b52119e363ff23c54acd0a4f4 
>   geode-core/src/test/java/org/apache/geode/security/IntegratedSecurityPeerAuthDistributedTest.java acc2b86e8ea3dfa4402e9bd6d54d80741c73c2b6 
>   geode-core/src/test/java/org/apache/geode/security/P2PAuthenticationDUnitTest.java 9fcf4cdde5ca74564372657e2c4e0c45528e878a 
>   geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java PRE-CREATION 
>   geode-core/src/test/resources/org/apache/geode/management/internal/security/cacheServer.json 3bb3e2f010da9e97af3ade60dbd4b4b45b9370b3 
>   geode-core/src/test/resources/org/apache/geode/security/peerAuth.json 9bd893612e9ee3a3d300653fa449ee19a03f37c6 
> 
> Diff: https://reviews.apache.org/r/52067/diff/
> 
> 
> Testing
> -------
> 
> precheckin successful
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>


Re: Review Request 52067: GEODE-1909: add authorization in GMSAuthenticator

Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52067/
-----------------------------------------------------------

(Updated Sept. 20, 2016, 12:59 a.m.)


Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Changes
-------

test changes.


Repository: geode


Description
-------

GEODE-1909: add authorization in GMSAuthenticator

The user who get authenticated in the cluster needs to have CLUSTER:MANAGE previlage as well. Note this is NOT the user who issued "start server" command in gfsh, but is the peer user whose credential is put in the server's security.properties file.


Diffs (updated)
-----

  geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberFactory.java 08178e92ad9b4db30534c80873c63e8b4c11808f 
  geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java 3f030c9af4ab0503ad201c9172fe61a256eb961a 
  geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java d15e8bfa09b3c24d8c2db10d524bfce0d4b61ad5 
  geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java b61201ef60b4dc3ea6d42e7ae30d305e2ebb7e35 
  geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithAuthenticatorTest.java 45a5881207179e950bb40a897660f63f583a4e19 
  geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithSecurityManagerTest.java ddb208d043a8503b52119e363ff23c54acd0a4f4 
  geode-core/src/test/java/org/apache/geode/security/IntegratedSecurityPeerAuthDistributedTest.java acc2b86e8ea3dfa4402e9bd6d54d80741c73c2b6 
  geode-core/src/test/java/org/apache/geode/security/P2PAuthenticationDUnitTest.java 9fcf4cdde5ca74564372657e2c4e0c45528e878a 
  geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java PRE-CREATION 
  geode-core/src/test/resources/org/apache/geode/management/internal/security/cacheServer.json 3bb3e2f010da9e97af3ade60dbd4b4b45b9370b3 
  geode-core/src/test/resources/org/apache/geode/security/peerAuth.json 9bd893612e9ee3a3d300653fa449ee19a03f37c6 

Diff: https://reviews.apache.org/r/52067/diff/


Testing (updated)
-------

precheckin successful


Thanks,

Jinmei Liao


Re: Review Request 52067: GEODE-1909: add authorization in GMSAuthenticator

Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52067/
-----------------------------------------------------------

(Updated Sept. 19, 2016, 9:29 p.m.)


Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Repository: geode


Description
-------

GEODE-1909: add authorization in GMSAuthenticator

The user who get authenticated in the cluster needs to have CLUSTER:MANAGE previlage as well. Note this is NOT the user who issued "start server" command in gfsh, but is the peer user whose credential is put in the server's security.properties file.


Diffs (updated)
-----

  geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberFactory.java 08178e92ad9b4db30534c80873c63e8b4c11808f 
  geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java 3f030c9af4ab0503ad201c9172fe61a256eb961a 
  geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java d15e8bfa09b3c24d8c2db10d524bfce0d4b61ad5 
  geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java b61201ef60b4dc3ea6d42e7ae30d305e2ebb7e35 
  geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithAuthenticatorTest.java 45a5881207179e950bb40a897660f63f583a4e19 
  geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithSecurityManagerTest.java ddb208d043a8503b52119e363ff23c54acd0a4f4 
  geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java PRE-CREATION 
  geode-core/src/test/resources/org/apache/geode/management/internal/security/cacheServer.json 3bb3e2f010da9e97af3ade60dbd4b4b45b9370b3 

Diff: https://reviews.apache.org/r/52067/diff/


Testing
-------

precheckin running


Thanks,

Jinmei Liao