You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Darren Shepherd <da...@gmail.com> on 2013/09/16 18:56:10 UTC
Review Request 14148: Cleanup DefaultUserAuthenticator and removed masking
_name variable
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14148/
-----------------------------------------------------------
Review request for cloudstack and Abhinandan Prateek.
Repository: cloudstack-git
Description
-------
DefaultUserAuthenticator maskes the _name varible in ComponentLifecycleBase making the setName() method not work as expected. This patch cleans up the code such that getName() will be getClass().getSimpleName() unless overridden in the Spring configuration.
Diffs
-----
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java e62a3d8
plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java e5b169f
plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java f102275
plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java 91be922
server/src/com/cloud/server/auth/DefaultUserAuthenticator.java 952f724
Diff: https://reviews.apache.org/r/14148/diff/
Testing
-------
Thanks,
Darren Shepherd
Re: Review Request 14148: Cleanup DefaultUserAuthenticator and removed
masking _name variable
Posted by Darren Shepherd <da...@gmail.com>.
> On Sept. 18, 2013, 5:37 a.m., Abhinandan Prateek wrote:
> > In client/tomcatconf/applicationContext.xml.in we are referring passing name properties to the authentication adapters e.g.:
> >
> > <bean id="LdapAuthenticator" class="org.apache.cloudstack.ldap.LdapAuthenticator">
> > <property name="name" value="LDAP"/>
> > </bean>
> >
> > There adapters never make use of this property. We should just remove the property from applicationContext.xml.in too ?
This change removes the code that hardcoded it to LDAP. So by also removing the XML the name will become LdapAuthenticator (class.getSimpleName()). Either way it doesn't really matter. Really shortly I'm going to submit code to modularize spring config which will introduce new XML. The new XML will set the name to LDAP, because the way in which I'm doing the modularization, the names of the adapters matter. This is so that you can reorder the Adapters (as the order of authenticators is really important). So there will be a configuration parameter user.authenticators.order="SHA256SALT,MD5,LDAP,PLAINTEXT". So after saying all that, I'd just leave the XML as is, not reason to change it.
- Darren
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14148/#review26209
-----------------------------------------------------------
On Sept. 18, 2013, 5:39 a.m., Darren Shepherd wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14148/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2013, 5:39 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and Kelven Yang.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> DefaultUserAuthenticator maskes the _name varible in ComponentLifecycleBase making the setName() method not work as expected. This patch cleans up the code such that getName() will be getClass().getSimpleName() unless overridden in the Spring configuration.
>
>
> Diffs
> -----
>
> plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java e62a3d8
> plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java e5b169f
> plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java f102275
> plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java 91be922
> server/src/com/cloud/server/auth/DefaultUserAuthenticator.java 952f724
>
> Diff: https://reviews.apache.org/r/14148/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Darren Shepherd
>
>
Re: Review Request 14148: Cleanup DefaultUserAuthenticator and removed
masking _name variable
Posted by Abhinandan Prateek <ap...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14148/#review26209
-----------------------------------------------------------
In client/tomcatconf/applicationContext.xml.in we are referring passing name properties to the authentication adapters e.g.:
<bean id="LdapAuthenticator" class="org.apache.cloudstack.ldap.LdapAuthenticator">
<property name="name" value="LDAP"/>
</bean>
There adapters never make use of this property. We should just remove the property from applicationContext.xml.in too ?
- Abhinandan Prateek
On Sept. 16, 2013, 4:56 p.m., Darren Shepherd wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14148/
> -----------------------------------------------------------
>
> (Updated Sept. 16, 2013, 4:56 p.m.)
>
>
> Review request for cloudstack and Abhinandan Prateek.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> DefaultUserAuthenticator maskes the _name varible in ComponentLifecycleBase making the setName() method not work as expected. This patch cleans up the code such that getName() will be getClass().getSimpleName() unless overridden in the Spring configuration.
>
>
> Diffs
> -----
>
> plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java e62a3d8
> plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java e5b169f
> plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java f102275
> plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java 91be922
> server/src/com/cloud/server/auth/DefaultUserAuthenticator.java 952f724
>
> Diff: https://reviews.apache.org/r/14148/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Darren Shepherd
>
>
Re: Review Request 14148: Cleanup DefaultUserAuthenticator and removed
masking _name variable
Posted by Abhinandan Prateek <ap...@apache.org>.
> On Sept. 19, 2013, 4:20 a.m., Abhinandan Prateek wrote:
> > Ship It!
when i try to apply the patch git am says "Patch format detection failed.".
- Abhinandan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14148/#review26261
-----------------------------------------------------------
On Sept. 18, 2013, 5:39 a.m., Darren Shepherd wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14148/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2013, 5:39 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and Kelven Yang.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> DefaultUserAuthenticator maskes the _name varible in ComponentLifecycleBase making the setName() method not work as expected. This patch cleans up the code such that getName() will be getClass().getSimpleName() unless overridden in the Spring configuration.
>
>
> Diffs
> -----
>
> plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java e62a3d8
> plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java e5b169f
> plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java f102275
> plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java 91be922
> server/src/com/cloud/server/auth/DefaultUserAuthenticator.java 952f724
>
> Diff: https://reviews.apache.org/r/14148/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Darren Shepherd
>
>
Re: Review Request 14148: Cleanup DefaultUserAuthenticator and removed
masking _name variable
Posted by Abhinandan Prateek <ap...@apache.org>.
> On Sept. 19, 2013, 4:20 a.m., Abhinandan Prateek wrote:
> > Ship It!
>
> Abhinandan Prateek wrote:
> when i try to apply the patch git am says "Patch format detection failed.".
>
> Abhinandan Prateek wrote:
> Committed !
>
> https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=commit;h=4d01ce8fc766423d795955bd5784b56969ce11a8
reverting it for now as I see build failure, do not have time right now to look for the cause. Will put it back tomorrow.
- Abhinandan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14148/#review26261
-----------------------------------------------------------
On Sept. 18, 2013, 5:39 a.m., Darren Shepherd wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14148/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2013, 5:39 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and Kelven Yang.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> DefaultUserAuthenticator maskes the _name varible in ComponentLifecycleBase making the setName() method not work as expected. This patch cleans up the code such that getName() will be getClass().getSimpleName() unless overridden in the Spring configuration.
>
>
> Diffs
> -----
>
> plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java e62a3d8
> plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java e5b169f
> plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java f102275
> plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java 91be922
> server/src/com/cloud/server/auth/DefaultUserAuthenticator.java 952f724
>
> Diff: https://reviews.apache.org/r/14148/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Darren Shepherd
>
>
Re: Review Request 14148: Cleanup DefaultUserAuthenticator and removed
masking _name variable
Posted by Abhinandan Prateek <ap...@apache.org>.
> On Sept. 19, 2013, 4:20 a.m., Abhinandan Prateek wrote:
> > Ship It!
>
> Abhinandan Prateek wrote:
> when i try to apply the patch git am says "Patch format detection failed.".
Committed !
https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=commit;h=4d01ce8fc766423d795955bd5784b56969ce11a8
- Abhinandan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14148/#review26261
-----------------------------------------------------------
On Sept. 18, 2013, 5:39 a.m., Darren Shepherd wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14148/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2013, 5:39 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and Kelven Yang.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> DefaultUserAuthenticator maskes the _name varible in ComponentLifecycleBase making the setName() method not work as expected. This patch cleans up the code such that getName() will be getClass().getSimpleName() unless overridden in the Spring configuration.
>
>
> Diffs
> -----
>
> plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java e62a3d8
> plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java e5b169f
> plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java f102275
> plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java 91be922
> server/src/com/cloud/server/auth/DefaultUserAuthenticator.java 952f724
>
> Diff: https://reviews.apache.org/r/14148/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Darren Shepherd
>
>
Re: Review Request 14148: Cleanup DefaultUserAuthenticator and removed
masking _name variable
Posted by Abhinandan Prateek <ap...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14148/#review26261
-----------------------------------------------------------
Ship it!
Ship It!
- Abhinandan Prateek
On Sept. 18, 2013, 5:39 a.m., Darren Shepherd wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14148/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2013, 5:39 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and Kelven Yang.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> DefaultUserAuthenticator maskes the _name varible in ComponentLifecycleBase making the setName() method not work as expected. This patch cleans up the code such that getName() will be getClass().getSimpleName() unless overridden in the Spring configuration.
>
>
> Diffs
> -----
>
> plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java e62a3d8
> plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java e5b169f
> plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java f102275
> plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java 91be922
> server/src/com/cloud/server/auth/DefaultUserAuthenticator.java 952f724
>
> Diff: https://reviews.apache.org/r/14148/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Darren Shepherd
>
>
Re: Review Request 14148: Cleanup DefaultUserAuthenticator and removed
masking _name variable
Posted by Prasanna Santhanam <ts...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14148/
-----------------------------------------------------------
(Updated Sept. 18, 2013, 5:39 a.m.)
Review request for cloudstack, Abhinandan Prateek and Kelven Yang.
Repository: cloudstack-git
Description
-------
DefaultUserAuthenticator maskes the _name varible in ComponentLifecycleBase making the setName() method not work as expected. This patch cleans up the code such that getName() will be getClass().getSimpleName() unless overridden in the Spring configuration.
Diffs
-----
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java e62a3d8
plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java e5b169f
plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java f102275
plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java 91be922
server/src/com/cloud/server/auth/DefaultUserAuthenticator.java 952f724
Diff: https://reviews.apache.org/r/14148/diff/
Testing
-------
Thanks,
Darren Shepherd
Re: Review Request 14148: Cleanup DefaultUserAuthenticator and removed
masking _name variable
Posted by Prasanna Santhanam <ts...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14148/
-----------------------------------------------------------
(Updated Sept. 18, 2013, 5:38 a.m.)
Review request for cloudstack and Abhinandan Prateek.
Changes
-------
adding kelven for spring changes.
Repository: cloudstack-git
Description
-------
DefaultUserAuthenticator maskes the _name varible in ComponentLifecycleBase making the setName() method not work as expected. This patch cleans up the code such that getName() will be getClass().getSimpleName() unless overridden in the Spring configuration.
Diffs
-----
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java e62a3d8
plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java e5b169f
plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java f102275
plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java 91be922
server/src/com/cloud/server/auth/DefaultUserAuthenticator.java 952f724
Diff: https://reviews.apache.org/r/14148/diff/
Testing
-------
Thanks,
Darren Shepherd