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/15 18:14:01 UTC

Review Request 51923: GEODE-1883: making AuthInit optional when starting a server/client

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

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


Repository: geode


Description
-------

GEODE-1883: making AuthInit optional when starting a server/client

When starting a server trying to connect to a locator, customer should not have to implement an AuthInit object, they can just put security-username/security-password in their properties file.


Diffs
-----

  geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java d843792168e91b39b686c84de66b0087c1ae65b4 
  geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java 68ec0c0041f204775541db396022a1df14c868fe 
  geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java 00372aebc8d72376ff6613c13637faeaac8d1523 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java 7380c9a0cb7382f8a2758acc1018fd793ce815df 
  geode-core/src/main/java/org/apache/geode/security/AuthInitialize.java 9123ec4ac00eb2fe5f7d980e78c06c6fe6bcd767 
  geode-core/src/test/java/org/apache/geode/security/PDXGfshPostProcessorOnRemoteServerTest.java 870ff91ab1a3d3537754268f6e2f2c33f24141b7 

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


Testing
-------

precheckin


Thanks,

Jinmei Liao


Re: Review Request 51923: GEODE-1883: making AuthInit optional when starting a server/client

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


Ship it!




Ship It!

- Kirk Lund


On Sept. 15, 2016, 9 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51923/
> -----------------------------------------------------------
> 
> (Updated Sept. 15, 2016, 9 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, Kirk Lund, and Swapnil Bawaskar.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-1883: making AuthInit optional when starting a server/client
> 
> When starting a server trying to connect to a locator, customer should not have to implement an AuthInit object, they can just put security-username/security-password in their properties file.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java 8a4446cc19e5cdc0685e5bd6b2708d7da4f0d101 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java 68ec0c0041f204775541db396022a1df14c868fe 
>   geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java 00372aebc8d72376ff6613c13637faeaac8d1523 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java 7380c9a0cb7382f8a2758acc1018fd793ce815df 
>   geode-core/src/main/java/org/apache/geode/security/AuthInitialize.java 9123ec4ac00eb2fe5f7d980e78c06c6fe6bcd767 
>   geode-core/src/test/java/org/apache/geode/security/PDXGfshPostProcessorOnRemoteServerTest.java 870ff91ab1a3d3537754268f6e2f2c33f24141b7 
> 
> Diff: https://reviews.apache.org/r/51923/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>


Re: Review Request 51923: GEODE-1883: making AuthInit optional when starting a server/client

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

(Updated Sept. 15, 2016, 9 p.m.)


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


Repository: geode


Description
-------

GEODE-1883: making AuthInit optional when starting a server/client

When starting a server trying to connect to a locator, customer should not have to implement an AuthInit object, they can just put security-username/security-password in their properties file.


Diffs (updated)
-----

  geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java 8a4446cc19e5cdc0685e5bd6b2708d7da4f0d101 
  geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java 68ec0c0041f204775541db396022a1df14c868fe 
  geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java 00372aebc8d72376ff6613c13637faeaac8d1523 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java 7380c9a0cb7382f8a2758acc1018fd793ce815df 
  geode-core/src/main/java/org/apache/geode/security/AuthInitialize.java 9123ec4ac00eb2fe5f7d980e78c06c6fe6bcd767 
  geode-core/src/test/java/org/apache/geode/security/PDXGfshPostProcessorOnRemoteServerTest.java 870ff91ab1a3d3537754268f6e2f2c33f24141b7 

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


Testing
-------

precheckin


Thanks,

Jinmei Liao


Re: Review Request 51923: GEODE-1883: making AuthInit optional when starting a server/client

Posted by Udo Kohlmeyer <uk...@gmail.com>.

> On Sept. 15, 2016, 6:40 p.m., Kirk Lund wrote:
> > geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java, line 1728
> > <https://reviews.apache.org/r/51923/diff/1/?file=1499036#file1499036line1728>
> >
> >     Why is this deleted? This is for ssl-enabled-components (Udo's work) and has nothing to do with security-enabled-components aka authentication&authorization (A&A). 
> >     
> >     After Udo's latest commit, we probably need to change his code to use SSLSecurableComponents or SSLSecurableChannels and then change our code to have its own separate SecurableComponents with a different list of constants (depending on what Swapnil's requirements are).
> 
> Kirk Lund wrote:
>     I just looked at Udo's code. I think this needs to change to be SecurableCommunicationChannels. Udo introduced that for SSL_ENABLED_COMPONENTS, but I think he missed this place in the code. So instead of deleting it, we probably just need to change the list to what he has in SecurableCommunicationChannels (http changes to web) and change the @link to SecurableCommunicationChannels.

I've just committed the updated javadoc.


- Udo


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


On Sept. 15, 2016, 6:14 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51923/
> -----------------------------------------------------------
> 
> (Updated Sept. 15, 2016, 6:14 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, Kirk Lund, and Swapnil Bawaskar.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-1883: making AuthInit optional when starting a server/client
> 
> When starting a server trying to connect to a locator, customer should not have to implement an AuthInit object, they can just put security-username/security-password in their properties file.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java d843792168e91b39b686c84de66b0087c1ae65b4 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java 68ec0c0041f204775541db396022a1df14c868fe 
>   geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java 00372aebc8d72376ff6613c13637faeaac8d1523 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java 7380c9a0cb7382f8a2758acc1018fd793ce815df 
>   geode-core/src/main/java/org/apache/geode/security/AuthInitialize.java 9123ec4ac00eb2fe5f7d980e78c06c6fe6bcd767 
>   geode-core/src/test/java/org/apache/geode/security/PDXGfshPostProcessorOnRemoteServerTest.java 870ff91ab1a3d3537754268f6e2f2c33f24141b7 
> 
> Diff: https://reviews.apache.org/r/51923/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>


Re: Review Request 51923: GEODE-1883: making AuthInit optional when starting a server/client

Posted by Udo Kohlmeyer <uk...@gmail.com>.

> On Sept. 15, 2016, 6:40 p.m., Kirk Lund wrote:
> >

The ssl-enabled-components uses SecurableCommunicationChannels. I'll be updating this javadoc to reflect that.


- Udo


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


On Sept. 15, 2016, 6:14 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51923/
> -----------------------------------------------------------
> 
> (Updated Sept. 15, 2016, 6:14 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, Kirk Lund, and Swapnil Bawaskar.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-1883: making AuthInit optional when starting a server/client
> 
> When starting a server trying to connect to a locator, customer should not have to implement an AuthInit object, they can just put security-username/security-password in their properties file.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java d843792168e91b39b686c84de66b0087c1ae65b4 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java 68ec0c0041f204775541db396022a1df14c868fe 
>   geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java 00372aebc8d72376ff6613c13637faeaac8d1523 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java 7380c9a0cb7382f8a2758acc1018fd793ce815df 
>   geode-core/src/main/java/org/apache/geode/security/AuthInitialize.java 9123ec4ac00eb2fe5f7d980e78c06c6fe6bcd767 
>   geode-core/src/test/java/org/apache/geode/security/PDXGfshPostProcessorOnRemoteServerTest.java 870ff91ab1a3d3537754268f6e2f2c33f24141b7 
> 
> Diff: https://reviews.apache.org/r/51923/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>


Re: Review Request 51923: GEODE-1883: making AuthInit optional when starting a server/client

Posted by Kirk Lund <ki...@gmail.com>.

> On Sept. 15, 2016, 6:40 p.m., Kirk Lund wrote:
> > geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java, line 1728
> > <https://reviews.apache.org/r/51923/diff/1/?file=1499036#file1499036line1728>
> >
> >     Why is this deleted? This is for ssl-enabled-components (Udo's work) and has nothing to do with security-enabled-components aka authentication&authorization (A&A). 
> >     
> >     After Udo's latest commit, we probably need to change his code to use SSLSecurableComponents or SSLSecurableChannels and then change our code to have its own separate SecurableComponents with a different list of constants (depending on what Swapnil's requirements are).

I just looked at Udo's code. I think this needs to change to be SecurableCommunicationChannels. Udo introduced that for SSL_ENABLED_COMPONENTS, but I think he missed this place in the code. So instead of deleting it, we probably just need to change the list to what he has in SecurableCommunicationChannels (http changes to web) and change the @link to SecurableCommunicationChannels.


- Kirk


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


On Sept. 15, 2016, 6:14 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51923/
> -----------------------------------------------------------
> 
> (Updated Sept. 15, 2016, 6:14 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, Kirk Lund, and Swapnil Bawaskar.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-1883: making AuthInit optional when starting a server/client
> 
> When starting a server trying to connect to a locator, customer should not have to implement an AuthInit object, they can just put security-username/security-password in their properties file.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java d843792168e91b39b686c84de66b0087c1ae65b4 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java 68ec0c0041f204775541db396022a1df14c868fe 
>   geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java 00372aebc8d72376ff6613c13637faeaac8d1523 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java 7380c9a0cb7382f8a2758acc1018fd793ce815df 
>   geode-core/src/main/java/org/apache/geode/security/AuthInitialize.java 9123ec4ac00eb2fe5f7d980e78c06c6fe6bcd767 
>   geode-core/src/test/java/org/apache/geode/security/PDXGfshPostProcessorOnRemoteServerTest.java 870ff91ab1a3d3537754268f6e2f2c33f24141b7 
> 
> Diff: https://reviews.apache.org/r/51923/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>


Re: Review Request 51923: GEODE-1883: making AuthInit optional when starting a server/client

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




geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java 
<https://reviews.apache.org/r/51923/#comment216618>

    Why is this deleted? This is for ssl-enabled-components (Udo's work) and has nothing to do with security-enabled-components aka authentication&authorization (A&A). 
    
    After Udo's latest commit, we probably need to change his code to use SSLSecurableComponents or SSLSecurableChannels and then change our code to have its own separate SecurableComponents with a different list of constants (depending on what Swapnil's requirements are).



geode-core/src/main/java/org/apache/geode/security/AuthInitialize.java (line 104)
<https://reviews.apache.org/r/51923/#comment216620>

    This is broken javadoc that generates warnings during gradle javadoc task. You should delete the @param and @return unless you want to actually write out real values for them.



geode-core/src/main/java/org/apache/geode/security/AuthInitialize.java (line 106)
<https://reviews.apache.org/r/51923/#comment216621>

    Typo: secuirty


- Kirk Lund


On Sept. 15, 2016, 6:14 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51923/
> -----------------------------------------------------------
> 
> (Updated Sept. 15, 2016, 6:14 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, Kirk Lund, and Swapnil Bawaskar.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-1883: making AuthInit optional when starting a server/client
> 
> When starting a server trying to connect to a locator, customer should not have to implement an AuthInit object, they can just put security-username/security-password in their properties file.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java d843792168e91b39b686c84de66b0087c1ae65b4 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java 68ec0c0041f204775541db396022a1df14c868fe 
>   geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java 00372aebc8d72376ff6613c13637faeaac8d1523 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java 7380c9a0cb7382f8a2758acc1018fd793ce815df 
>   geode-core/src/main/java/org/apache/geode/security/AuthInitialize.java 9123ec4ac00eb2fe5f7d980e78c06c6fe6bcd767 
>   geode-core/src/test/java/org/apache/geode/security/PDXGfshPostProcessorOnRemoteServerTest.java 870ff91ab1a3d3537754268f6e2f2c33f24141b7 
> 
> Diff: https://reviews.apache.org/r/51923/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>