You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@cloudstack.apache.org by saksham srivastava <sa...@citrix.com> on 2012/08/30 19:16:43 UTC

Review Request: CS-10219 Add option to set login attempts to portal.

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

Review request for cloudstack, Rajesh Battala and Alena Prokharchyk.


Description
-------

Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
Also after the maximum attempts are reached the user account is disabled.


This addresses bug CS-10219.


Diffs
-----

  core/src/com/cloud/user/UserAccountVO.java 5e7c018 
  server/src/com/cloud/configuration/Config.java ebcd070 
  server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
  setup/db/create-schema.sql fa933e3 
  setup/db/db/schema-302to40.sql aaf23e6 

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


Testing
-------

Verified locally.


Thanks,

saksham srivastava

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by saksham srivastava <sa...@citrix.com>.


> On Sept. 4, 2012, 4:05 a.m., Vijay Venkatachalam wrote:
> > server/src/com/cloud/user/AccountManagerImpl.java, line 1869
> > <https://reviews.apache.org/r/6858/diff/3/?file=149072#file149072line1869>
> >
> >     Your intentions are good to give a custom log on the last erroneous login attempt. But, the message says that you are disabling the account (it should be user), which is not true. Also, you might want to throw an Exception with a message similar to the special log message and not just log & return null. Returning null will give a generic message to user - 'please provide valid credentials'.

Changed the log message. Using user instead of account.


- saksham


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


On Sept. 5, 2012, 2:56 p.m., saksham srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6858/
> -----------------------------------------------------------
> 
> (Updated Sept. 5, 2012, 2:56 p.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.
> 
> 
> Description
> -------
> 
> Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
> Also after the maximum attempts are reached the user account is disabled.
> 
> 
> This addresses bug CS-10219.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/user/UserAccountVO.java 5e7c018 
>   server/src/com/cloud/configuration/Config.java ebcd070 
>   server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-302to40.sql aaf23e6 
> 
> Diff: https://reviews.apache.org/r/6858/diff/
> 
> 
> Testing
> -------
> 
> Verified locally.
> 
> 
> Thanks,
> 
> saksham srivastava
> 
>

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by Vijay Venkatachalam <vi...@citrix.com>.

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



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23608>

    Your intentions are good to give a custom log on the last erroneous login attempt. But, the message says that you are disabling the account (it should be user), which is not true. Also, you might want to throw an Exception with a message similar to the special log message and not just log & return null. Returning null will give a generic message to user - 'please provide valid credentials'. 


- Vijay Venkatachalam


On Sept. 3, 2012, 2 p.m., saksham srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6858/
> -----------------------------------------------------------
> 
> (Updated Sept. 3, 2012, 2 p.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Rajesh Battala, and Alena Prokharchyk.
> 
> 
> Description
> -------
> 
> Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
> Also after the maximum attempts are reached the user account is disabled.
> 
> 
> This addresses bug CS-10219.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/user/UserAccountVO.java 5e7c018 
>   server/src/com/cloud/configuration/Config.java ebcd070 
>   server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-302to40.sql aaf23e6 
> 
> Diff: https://reviews.apache.org/r/6858/diff/
> 
> 
> Testing
> -------
> 
> Verified locally.
> 
> 
> Thanks,
> 
> saksham srivastava
> 
>

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by saksham srivastava <sa...@citrix.com>.


> On Sept. 7, 2012, 3:46 a.m., Koushik Das wrote:
> > server/src/com/cloud/user/AccountManagerImpl.java, line 1879
> > <https://reviews.apache.org/r/6858/diff/5/?file=150231#file150231line1879>
> >
> >     I understand that ROOT/domain admin can only unlock but I don't see that in the list of changes. Is there another commit for implementing that?

ROOT Admin can enable any account, this functionality is already present in API (enableUser) and UI. For Domain Admin the API is present.


- saksham


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


On Sept. 7, 2012, 10:07 a.m., saksham srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6858/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2012, 10:07 a.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.
> 
> 
> Description
> -------
> 
> Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
> Also after the maximum attempts are reached the user account is disabled.
> 
> 
> This addresses bug CS-10219.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/user/UserAccountVO.java 5e7c018 
>   server/src/com/cloud/configuration/Config.java ebcd070 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java f9da08d 
>   server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-302to40.sql aaf23e6 
> 
> Diff: https://reviews.apache.org/r/6858/diff/
> 
> 
> Testing
> -------
> 
> Verified locally.
> 
> 
> Thanks,
> 
> saksham srivastava
> 
>

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by Koushik Das <ko...@citrix.com>.

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



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23870>

    I understand that ROOT/domain admin can only unlock but I don't see that in the list of changes. Is there another commit for implementing that?


- Koushik Das


On Sept. 6, 2012, 11:01 a.m., saksham srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6858/
> -----------------------------------------------------------
> 
> (Updated Sept. 6, 2012, 11:01 a.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.
> 
> 
> Description
> -------
> 
> Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
> Also after the maximum attempts are reached the user account is disabled.
> 
> 
> This addresses bug CS-10219.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/user/UserAccountVO.java 5e7c018 
>   server/src/com/cloud/configuration/Config.java ebcd070 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java f9da08d 
>   server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-302to40.sql aaf23e6 
> 
> Diff: https://reviews.apache.org/r/6858/diff/
> 
> 
> Testing
> -------
> 
> Verified locally.
> 
> 
> Thanks,
> 
> saksham srivastava
> 
>

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by Devdeep Singh <de...@citrix.com>.

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

Ship it!


Ship It!

- Devdeep Singh


On Sept. 6, 2012, 11:01 a.m., saksham srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6858/
> -----------------------------------------------------------
> 
> (Updated Sept. 6, 2012, 11:01 a.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.
> 
> 
> Description
> -------
> 
> Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
> Also after the maximum attempts are reached the user account is disabled.
> 
> 
> This addresses bug CS-10219.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/user/UserAccountVO.java 5e7c018 
>   server/src/com/cloud/configuration/Config.java ebcd070 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java f9da08d 
>   server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-302to40.sql aaf23e6 
> 
> Diff: https://reviews.apache.org/r/6858/diff/
> 
> 
> Testing
> -------
> 
> Verified locally.
> 
> 
> Thanks,
> 
> saksham srivastava
> 
>

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by Pranav Saxena <pr...@citrix.com>.


> On Sept. 6, 2012, 7:04 p.m., Koushik Das wrote:
> > server/src/com/cloud/user/AccountManagerImpl.java, line 1879
> > <https://reviews.apache.org/r/6858/diff/5/?file=150231#file150231line1879>
> >
> >     Once the user account gets locked, what are the options to unlock. I don't see any provision for that. So based on the current logic the user account becomes unusable. Should that be the desired behavior

Root Admin has the rights to unlock the "locked account" of any user. The user can't unlock his account on his own .So once his account his locked after n number of incorrect logins , only the root admin can unlock his account Not very sure , but that is what I think is the desired behaviour. 
 


- Pranav


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


On Sept. 6, 2012, 11:01 a.m., saksham srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6858/
> -----------------------------------------------------------
> 
> (Updated Sept. 6, 2012, 11:01 a.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.
> 
> 
> Description
> -------
> 
> Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
> Also after the maximum attempts are reached the user account is disabled.
> 
> 
> This addresses bug CS-10219.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/user/UserAccountVO.java 5e7c018 
>   server/src/com/cloud/configuration/Config.java ebcd070 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java f9da08d 
>   server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-302to40.sql aaf23e6 
> 
> Diff: https://reviews.apache.org/r/6858/diff/
> 
> 
> Testing
> -------
> 
> Verified locally.
> 
> 
> Thanks,
> 
> saksham srivastava
> 
>

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by saksham srivastava <sa...@citrix.com>.


> On Sept. 6, 2012, 7:04 p.m., Koushik Das wrote:
> > server/src/com/cloud/user/AccountManagerImpl.java, line 311
> > <https://reviews.apache.org/r/6858/diff/5/?file=150231#file150231line311>
> >
> >     Is this a valid assumption? If possible try to avoid the id and use some other data.

Added in new patch.


- saksham


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


On Sept. 7, 2012, 10:07 a.m., saksham srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6858/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2012, 10:07 a.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.
> 
> 
> Description
> -------
> 
> Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
> Also after the maximum attempts are reached the user account is disabled.
> 
> 
> This addresses bug CS-10219.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/user/UserAccountVO.java 5e7c018 
>   server/src/com/cloud/configuration/Config.java ebcd070 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java f9da08d 
>   server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-302to40.sql aaf23e6 
> 
> Diff: https://reviews.apache.org/r/6858/diff/
> 
> 
> Testing
> -------
> 
> Verified locally.
> 
> 
> Thanks,
> 
> saksham srivastava
> 
>

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by Koushik Das <ko...@citrix.com>.

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



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23845>

    Is this a valid assumption? If possible try to avoid the id and use some other data.



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23848>

    Once the user account gets locked, what are the options to unlock. I don't see any provision for that. So based on the current logic the user account becomes unusable. Should that be the desired behavior


- Koushik Das


On Sept. 6, 2012, 11:01 a.m., saksham srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6858/
> -----------------------------------------------------------
> 
> (Updated Sept. 6, 2012, 11:01 a.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.
> 
> 
> Description
> -------
> 
> Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
> Also after the maximum attempts are reached the user account is disabled.
> 
> 
> This addresses bug CS-10219.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/user/UserAccountVO.java 5e7c018 
>   server/src/com/cloud/configuration/Config.java ebcd070 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java f9da08d 
>   server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-302to40.sql aaf23e6 
> 
> Diff: https://reviews.apache.org/r/6858/diff/
> 
> 
> Testing
> -------
> 
> Verified locally.
> 
> 
> Thanks,
> 
> saksham srivastava
> 
>

RE: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by Nitin Mehta <Ni...@citrix.com>.

This was committed and tested last week with commit - a56631bc66713faaa99759eb374e11afd6f533d3
Saksham – Please close it

From: saksham srivastava [mailto:noreply@reviews.apache.org] On Behalf Of saksham srivastava
Sent: Monday, September 10, 2012 3:26 PM
To: Alena Prokharchyk; Nitin Mehta; Devdeep Singh
Cc: Saksham Srivastava; cloudstack
Subject: Re: Review Request: CS-10219 Add option to set login attempts to portal.

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

On September 7th, 2012, 10:29 a.m., Nitin Mehta wrote:
server/src/com/cloud/user/AccountManagerImpl.java<https://reviews.apache.org/r/6858/diff/6/?file=150620#file150620line1876> (Diff revision 6)

private UserAccount getUserAccount(String username, String password, Long domainId, Map<String, Object[]> requestParameters) {

1876

                            s_logger.warn("User " + acct.getUsername() + " has been disabled due to multiple failed login attempts");

Can you suggest him corrective action ? Like contacting him an admin or resetting his password. What options does he have.

I think there should be an alert generated to alert the admin that the user got locked. Also there should be an option to try infinitely like we have today.(may be setting it to -1). Its not fair to ask u to implement it right away but please file a bug for this.

On September 7th, 2012, 11:52 a.m., saksham srivastava wrote:

Improved the message, will create a related bug and also update it on Wiki page for Login attempts

On September 10th, 2012, 8:05 a.m., Nitin Mehta wrote:

If you dont mind can you please create the bug right away :) and let me know

Created issue : http://bugs.cloudstack.org/browse/CS-16314

- saksham

On September 7th, 2012, 11:46 a.m., saksham srivastava wrote:
Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.
By saksham srivastava.

Updated Sept. 7, 2012, 11:46 a.m.

Description

Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.

Also after the maximum attempts are reached the user account is disabled.

Testing

Verified locally.

Bugs: CS-10219
Diffs

 *   core/src/com/cloud/user/UserAccountVO.java (5e7c018)
 *   server/src/com/cloud/configuration/Config.java (ebcd070)
 *   server/src/com/cloud/configuration/ConfigurationManagerImpl.java (f9da08d)
 *   server/src/com/cloud/user/AccountManagerImpl.java (38153f3)
 *   setup/db/create-schema.sql (fa933e3)
 *   setup/db/db/schema-302to40.sql (aaf23e6)

View Diff<https://reviews.apache.org/r/6858/diff/>

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by saksham srivastava <sa...@citrix.com>.


> On Sept. 7, 2012, 10:29 a.m., Nitin Mehta wrote:
> > server/src/com/cloud/user/AccountManagerImpl.java, line 1876
> > <https://reviews.apache.org/r/6858/diff/6/?file=150620#file150620line1876>
> >
> >     Can you suggest him corrective action ? Like contacting him an admin or resetting his password. What options does he have.
> >     I think there should be an alert generated to alert the admin that the user got locked. Also there should be an option to try infinitely like we have today.(may be setting it to -1). Its not fair to ask u to implement it right away but please file a bug for this.
> >
> 
> saksham srivastava wrote:
>     Improved the message, will create a related bug and also update it on Wiki page for Login attempts
> 
> Nitin Mehta wrote:
>     If you dont mind can you please create the bug right away :) and let me know

Created issue : http://bugs.cloudstack.org/browse/CS-16314


- saksham


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


On Sept. 7, 2012, 11:46 a.m., saksham srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6858/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2012, 11:46 a.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.
> 
> 
> Description
> -------
> 
> Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
> Also after the maximum attempts are reached the user account is disabled.
> 
> 
> This addresses bug CS-10219.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/user/UserAccountVO.java 5e7c018 
>   server/src/com/cloud/configuration/Config.java ebcd070 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java f9da08d 
>   server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-302to40.sql aaf23e6 
> 
> Diff: https://reviews.apache.org/r/6858/diff/
> 
> 
> Testing
> -------
> 
> Verified locally.
> 
> 
> Thanks,
> 
> saksham srivastava
> 
>

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by saksham srivastava <sa...@citrix.com>.


> On Sept. 7, 2012, 10:29 a.m., Nitin Mehta wrote:
> > server/src/com/cloud/user/AccountManagerImpl.java, line 311
> > <https://reviews.apache.org/r/6858/diff/6/?file=150620#file150620line311>
> >
> >     suggestion - create a function is systemAccount.

Checking for both RootAdmin and System, so still keeping isInternalAccount.


> On Sept. 7, 2012, 10:29 a.m., Nitin Mehta wrote:
> > server/src/com/cloud/user/AccountManagerImpl.java, line 1876
> > <https://reviews.apache.org/r/6858/diff/6/?file=150620#file150620line1876>
> >
> >     Can you suggest him corrective action ? Like contacting him an admin or resetting his password. What options does he have.
> >     I think there should be an alert generated to alert the admin that the user got locked. Also there should be an option to try infinitely like we have today.(may be setting it to -1). Its not fair to ask u to implement it right away but please file a bug for this.
> >

Improved the message, will create a related bug and also update it on Wiki page for Login attempts


- saksham


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


On Sept. 7, 2012, 11:46 a.m., saksham srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6858/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2012, 11:46 a.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.
> 
> 
> Description
> -------
> 
> Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
> Also after the maximum attempts are reached the user account is disabled.
> 
> 
> This addresses bug CS-10219.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/user/UserAccountVO.java 5e7c018 
>   server/src/com/cloud/configuration/Config.java ebcd070 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java f9da08d 
>   server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-302to40.sql aaf23e6 
> 
> Diff: https://reviews.apache.org/r/6858/diff/
> 
> 
> Testing
> -------
> 
> Verified locally.
> 
> 
> Thanks,
> 
> saksham srivastava
> 
>

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by Nitin Mehta <ni...@citrix.com>.


> On Sept. 7, 2012, 10:29 a.m., Nitin Mehta wrote:
> > server/src/com/cloud/user/AccountManagerImpl.java, line 1876
> > <https://reviews.apache.org/r/6858/diff/6/?file=150620#file150620line1876>
> >
> >     Can you suggest him corrective action ? Like contacting him an admin or resetting his password. What options does he have.
> >     I think there should be an alert generated to alert the admin that the user got locked. Also there should be an option to try infinitely like we have today.(may be setting it to -1). Its not fair to ask u to implement it right away but please file a bug for this.
> >
> 
> saksham srivastava wrote:
>     Improved the message, will create a related bug and also update it on Wiki page for Login attempts

If you dont mind can you please create the bug right away :) and let me know


- Nitin


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


On Sept. 7, 2012, 11:46 a.m., saksham srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6858/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2012, 11:46 a.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.
> 
> 
> Description
> -------
> 
> Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
> Also after the maximum attempts are reached the user account is disabled.
> 
> 
> This addresses bug CS-10219.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/user/UserAccountVO.java 5e7c018 
>   server/src/com/cloud/configuration/Config.java ebcd070 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java f9da08d 
>   server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-302to40.sql aaf23e6 
> 
> Diff: https://reviews.apache.org/r/6858/diff/
> 
> 
> Testing
> -------
> 
> Verified locally.
> 
> 
> Thanks,
> 
> saksham srivastava
> 
>

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by Nitin Mehta <ni...@citrix.com>.

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



core/src/com/cloud/user/UserAccountVO.java
<https://reviews.apache.org/r/6858/#comment23873>

    Change to incorrect_login_attempts in DB and here



server/src/com/cloud/configuration/Config.java
<https://reviews.apache.org/r/6858/#comment23874>

     LoginAttemptsAllowed -->  InccorectLoginAttemptsAllowed
    
    Also is this setting for user or account - please be clear ?



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23875>

    suggestion - create a function is systemAccount.



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23878>

    it shouldn't be acct. it should be user



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23877>

    Can you suggest him corrective action ? Like contacting him an admin or resetting his password. What options does he have.
    I think there should be an alert generated to alert the admin that the user got locked. Also there should be an option to try infinitely like we have today.(may be setting it to -1). Its not fair to ask u to implement it right away but please file a bug for this.
    



setup/db/db/schema-302to40.sql
<https://reviews.apache.org/r/6858/#comment23876>

    The string here should be same as for the fresh install
      "Incorrect login attempts allowed before the account is disabled"


- Nitin Mehta


On Sept. 7, 2012, 10:07 a.m., saksham srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6858/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2012, 10:07 a.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.
> 
> 
> Description
> -------
> 
> Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
> Also after the maximum attempts are reached the user account is disabled.
> 
> 
> This addresses bug CS-10219.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/user/UserAccountVO.java 5e7c018 
>   server/src/com/cloud/configuration/Config.java ebcd070 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java f9da08d 
>   server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-302to40.sql aaf23e6 
> 
> Diff: https://reviews.apache.org/r/6858/diff/
> 
> 
> Testing
> -------
> 
> Verified locally.
> 
> 
> Thanks,
> 
> saksham srivastava
> 
>

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by saksham srivastava <sa...@citrix.com>.


> On Sept. 10, 2012, 8:01 a.m., Nitin Mehta wrote:
> > Ship It!

Commit id - a56631bc66713faaa99759eb374e11afd6f533d3


- saksham


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


On Sept. 7, 2012, 11:46 a.m., saksham srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6858/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2012, 11:46 a.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.
> 
> 
> Description
> -------
> 
> Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
> Also after the maximum attempts are reached the user account is disabled.
> 
> 
> This addresses bug CS-10219.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/user/UserAccountVO.java 5e7c018 
>   server/src/com/cloud/configuration/Config.java ebcd070 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java f9da08d 
>   server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-302to40.sql aaf23e6 
> 
> Diff: https://reviews.apache.org/r/6858/diff/
> 
> 
> Testing
> -------
> 
> Verified locally.
> 
> 
> Thanks,
> 
> saksham srivastava
> 
>

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by Nitin Mehta <ni...@citrix.com>.

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

Ship it!


Ship It!

- Nitin Mehta


On Sept. 7, 2012, 11:46 a.m., saksham srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6858/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2012, 11:46 a.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.
> 
> 
> Description
> -------
> 
> Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
> Also after the maximum attempts are reached the user account is disabled.
> 
> 
> This addresses bug CS-10219.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/user/UserAccountVO.java 5e7c018 
>   server/src/com/cloud/configuration/Config.java ebcd070 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java f9da08d 
>   server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-302to40.sql aaf23e6 
> 
> Diff: https://reviews.apache.org/r/6858/diff/
> 
> 
> Testing
> -------
> 
> Verified locally.
> 
> 
> Thanks,
> 
> saksham srivastava
> 
>

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by saksham srivastava <sa...@citrix.com>.

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

(Updated Sept. 7, 2012, 11:46 a.m.)


Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.


Changes
-------

Updated with review comments.


Description
-------

Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
Also after the maximum attempts are reached the user account is disabled.


This addresses bug CS-10219.


Diffs (updated)
-----

  core/src/com/cloud/user/UserAccountVO.java 5e7c018 
  server/src/com/cloud/configuration/Config.java ebcd070 
  server/src/com/cloud/configuration/ConfigurationManagerImpl.java f9da08d 
  server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
  setup/db/create-schema.sql fa933e3 
  setup/db/db/schema-302to40.sql aaf23e6 

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


Testing
-------

Verified locally.


Thanks,

saksham srivastava

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by saksham srivastava <sa...@citrix.com>.

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

(Updated Sept. 7, 2012, 10:07 a.m.)


Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.


Changes
-------

Adding a wiki page : http://wiki.cloudstack.org/display/COMM/Login+Attempts+for+Users


Description
-------

Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
Also after the maximum attempts are reached the user account is disabled.


This addresses bug CS-10219.


Diffs (updated)
-----

  core/src/com/cloud/user/UserAccountVO.java 5e7c018 
  server/src/com/cloud/configuration/Config.java ebcd070 
  server/src/com/cloud/configuration/ConfigurationManagerImpl.java f9da08d 
  server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
  setup/db/create-schema.sql fa933e3 
  setup/db/db/schema-302to40.sql aaf23e6 

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


Testing
-------

Verified locally.


Thanks,

saksham srivastava

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by saksham srivastava <sa...@citrix.com>.

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

(Updated Sept. 6, 2012, 11:01 a.m.)


Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.


Changes
-------

Added check for global configuration.


Description
-------

Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
Also after the maximum attempts are reached the user account is disabled.


This addresses bug CS-10219.


Diffs (updated)
-----

  core/src/com/cloud/user/UserAccountVO.java 5e7c018 
  server/src/com/cloud/configuration/Config.java ebcd070 
  server/src/com/cloud/configuration/ConfigurationManagerImpl.java f9da08d 
  server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
  setup/db/create-schema.sql fa933e3 
  setup/db/db/schema-302to40.sql aaf23e6 

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


Testing
-------

Verified locally.


Thanks,

saksham srivastava

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by Devdeep Singh <de...@citrix.com>.

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



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23818>

    The same check for whether the account is enabled or not is being done twice. May we can remove the second check and merge the two code blocks.


- Devdeep Singh


On Sept. 5, 2012, 2:56 p.m., saksham srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6858/
> -----------------------------------------------------------
> 
> (Updated Sept. 5, 2012, 2:56 p.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.
> 
> 
> Description
> -------
> 
> Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
> Also after the maximum attempts are reached the user account is disabled.
> 
> 
> This addresses bug CS-10219.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/user/UserAccountVO.java 5e7c018 
>   server/src/com/cloud/configuration/Config.java ebcd070 
>   server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-302to40.sql aaf23e6 
> 
> Diff: https://reviews.apache.org/r/6858/diff/
> 
> 
> Testing
> -------
> 
> Verified locally.
> 
> 
> Thanks,
> 
> saksham srivastava
> 
>

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by saksham srivastava <sa...@citrix.com>.

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

(Updated Sept. 5, 2012, 2:56 p.m.)


Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.


Changes
-------

Changes done in accordance with review comments.


Description
-------

Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
Also after the maximum attempts are reached the user account is disabled.


This addresses bug CS-10219.


Diffs (updated)
-----

  core/src/com/cloud/user/UserAccountVO.java 5e7c018 
  server/src/com/cloud/configuration/Config.java ebcd070 
  server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
  setup/db/create-schema.sql fa933e3 
  setup/db/db/schema-302to40.sql aaf23e6 

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


Testing
-------

Verified locally.


Thanks,

saksham srivastava

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by John Kinsella <jl...@stratosec.co>.

Just a note on this for the future - most of the penetration testing teams[1] out there now will attempt a dictionary attack not by trying different passwords with the same username, but by trying the same password across the 1000 most common usernames. Would be nice if we could track failed logins across multiple usernames (maybe for 4.1 ;) )

John
1: Malicious users are doing it as well, but depending on the CS user, they may (unfortunately) care more about the pen test report than the hacker…

On Sep 5, 2012, at 7:56 AM, saksham srivastava <sa...@citrix.com>>
wrote:

On Sept. 3, 2012, 5:28 p.m., Nitin Mehta wrote:
server/src/com/cloud/user/AccountManagerImpl.java, line 427
<https://reviews.apache.org/r/6858/diff/3/?file=149072#file149072line427>

The function signature should be something like this :-
updateLoginAttempts(Long accountId, int failedLoginAttempts , boolean isDisableAllowed)

Changed the name of variables to attempts and toDisable.

On Sept. 3, 2012, 5:28 p.m., Nitin Mehta wrote:
server/src/com/cloud/user/AccountManagerImpl.java, line 1029
<https://reviews.apache.org/r/6858/diff/3/?file=149072#file149072line1029>

Dont you have to do this for all the users belonging to this account ?

No, the aim is only to enable the login attempts of that particular user. All users have independent login_attempts irrespective of their account.

On Sept. 3, 2012, 5:28 p.m., Nitin Mehta wrote:
server/src/com/cloud/user/AccountManagerImpl.java, line 1859
<https://reviews.apache.org/r/6858/diff/3/?file=149072#file149072line1859>

The value can be 0 meaning for the 1st incorrect login u disable the account

0 means login in not allowed.1 means a single attempt is provided.

- saksham

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

On Sept. 5, 2012, 2:56 p.m., saksham srivastava wrote:

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

(Updated Sept. 5, 2012, 2:56 p.m.)

Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.

Description
-------

Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
Also after the maximum attempts are reached the user account is disabled.

This addresses bug CS-10219.

Diffs
-----

core/src/com/cloud/user/UserAccountVO.java 5e7c018
server/src/com/cloud/configuration/Config.java ebcd070
server/src/com/cloud/user/AccountManagerImpl.java 38153f3
setup/db/create-schema.sql fa933e3
setup/db/db/schema-302to40.sql aaf23e6

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

Testing
-------

Verified locally.

Thanks,

saksham srivastava

Stratosec<http://stratosec.co> - Secure Infrastructure as a Service
o: 415.315.9385
@johnlkinsella<http://twitter.com/johnlkinsella>

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by saksham srivastava <sa...@citrix.com>.


> On Sept. 3, 2012, 5:28 p.m., Nitin Mehta wrote:
> > server/src/com/cloud/user/AccountManagerImpl.java, line 427
> > <https://reviews.apache.org/r/6858/diff/3/?file=149072#file149072line427>
> >
> >     The function signature should be something like this :-
> >     updateLoginAttempts(Long accountId, int failedLoginAttempts , boolean isDisableAllowed)

Changed the name of variables to attempts and toDisable.


> On Sept. 3, 2012, 5:28 p.m., Nitin Mehta wrote:
> > server/src/com/cloud/user/AccountManagerImpl.java, line 1029
> > <https://reviews.apache.org/r/6858/diff/3/?file=149072#file149072line1029>
> >
> >     Dont you have to do this for all the users belonging to this account ?

No, the aim is only to enable the login attempts of that particular user. All users have independent login_attempts irrespective of their account.


> On Sept. 3, 2012, 5:28 p.m., Nitin Mehta wrote:
> > server/src/com/cloud/user/AccountManagerImpl.java, line 1859
> > <https://reviews.apache.org/r/6858/diff/3/?file=149072#file149072line1859>
> >
> >     The value can be 0 meaning for the 1st incorrect login u disable the account

0 means login in not allowed.1 means a single attempt is provided.


- saksham


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


On Sept. 5, 2012, 2:56 p.m., saksham srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6858/
> -----------------------------------------------------------
> 
> (Updated Sept. 5, 2012, 2:56 p.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Alena Prokharchyk.
> 
> 
> Description
> -------
> 
> Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
> Also after the maximum attempts are reached the user account is disabled.
> 
> 
> This addresses bug CS-10219.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/user/UserAccountVO.java 5e7c018 
>   server/src/com/cloud/configuration/Config.java ebcd070 
>   server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-302to40.sql aaf23e6 
> 
> Diff: https://reviews.apache.org/r/6858/diff/
> 
> 
> Testing
> -------
> 
> Verified locally.
> 
> 
> Thanks,
> 
> saksham srivastava
> 
>

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by Nitin Mehta <ni...@citrix.com>.

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



server/src/com/cloud/configuration/Config.java
<https://reviews.apache.org/r/6858/#comment23575>

    The global config should read as incorrect.login.attempts.allowed



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23576>

    Should be _allowedLoginAttempts instead
    
    
    Refer http://docs.cloudstack.org/CloudStack_Documentation/Design_Documents/Coding_Conventions



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23577>

    Use NumbersUtil to set a default value. See example below
    
            String value = configs.get(Config.AccountCleanupInterval.key());
            _cleanupInterval = NumbersUtil.parseInt(value, 60 * 60 * 24); // 1 day.



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23578>

    The function signature should be something like this :-
    updateLoginAttempts(Long accountId, int failedLoginAttempts , boolean isDisableAllowed)



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23586>

    Can you please refer to other dao functions to see how to use transactions and how to lock rows



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23585>

    Dont you have to do this for all the users belonging to this account ?



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23580>

    hardcoding to 2 scares me. I would say create a function boolean isInternalAccount(Long accountId)



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23583>

    Can you assume that you will always get acct object here ? It can also be null correct ?



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23579>

    Move this to configure function



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23581>

    The value can be 0 meaning for the 1st incorrect login u disable the account



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23582>

    There shouldnt be - 1 here.



server/src/com/cloud/user/AccountManagerImpl.java
<https://reviews.apache.org/r/6858/#comment23584>

    Please say - Account is disabled.


- Nitin Mehta


On Sept. 3, 2012, 2 p.m., saksham srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6858/
> -----------------------------------------------------------
> 
> (Updated Sept. 3, 2012, 2 p.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Rajesh Battala, and Alena Prokharchyk.
> 
> 
> Description
> -------
> 
> Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
> Also after the maximum attempts are reached the user account is disabled.
> 
> 
> This addresses bug CS-10219.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/user/UserAccountVO.java 5e7c018 
>   server/src/com/cloud/configuration/Config.java ebcd070 
>   server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-302to40.sql aaf23e6 
> 
> Diff: https://reviews.apache.org/r/6858/diff/
> 
> 
> Testing
> -------
> 
> Verified locally.
> 
> 
> Thanks,
> 
> saksham srivastava
> 
>

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by saksham srivastava <sa...@citrix.com>.

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

(Updated Sept. 3, 2012, 2 p.m.)


Review request for cloudstack, Devdeep Singh, Rajesh Battala, and Alena Prokharchyk.


Changes
-------

Added db lock.


Description
-------

Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
Also after the maximum attempts are reached the user account is disabled.


This addresses bug CS-10219.


Diffs (updated)
-----

  core/src/com/cloud/user/UserAccountVO.java 5e7c018 
  server/src/com/cloud/configuration/Config.java ebcd070 
  server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
  setup/db/create-schema.sql fa933e3 
  setup/db/db/schema-302to40.sql aaf23e6 

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


Testing
-------

Verified locally.


Thanks,

saksham srivastava

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by saksham srivastava <sa...@citrix.com>.

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

(Updated Aug. 31, 2012, 6:06 p.m.)


Review request for cloudstack, Rajesh Battala and Alena Prokharchyk.


Changes
-------

Changes made according to Alena's reviews.


Description
-------

Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
Also after the maximum attempts are reached the user account is disabled.


This addresses bug CS-10219.


Diffs (updated)
-----

  core/src/com/cloud/user/UserAccountVO.java 5e7c018 
  server/src/com/cloud/configuration/Config.java ebcd070 
  server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
  setup/db/create-schema.sql fa933e3 
  setup/db/db/schema-302to40.sql aaf23e6 

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


Testing
-------

Verified locally.


Thanks,

saksham srivastava

Re: Review Request: CS-10219 Add option to set login attempts to portal.

Posted by Alena Prokharchyk <al...@citrix.com>.

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

Couple of things to be fixed.

1) You should load global configuration variable just once, on the management server start. Make it static, and configure in configure() method. Right now you are doing it on every authentication attempt, which is incorrect:
int allowedAttempts = Integer.parseInt(conf.getValue());

2) When update the attempt count for the particular account, make sure you do lock the account db entry to make the modification synchronized. Look at the acquireInLockTable Dao method method. With the current implementation its possible to do logins from multiple sessions at the same time, and the update will be done incorrectly.

3) Reset login attempts - when do you make it? When user logs in?  Can you write up small doc explaining the functionality? 

- Alena Prokharchyk

On Aug. 30, 2012, 5:16 p.m., saksham srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6858/
> -----------------------------------------------------------
> 
> (Updated Aug. 30, 2012, 5:16 p.m.)
> 
> 
> Review request for cloudstack, Rajesh Battala and Alena Prokharchyk.
> 
> 
> Description
> -------
> 
> Added global setting login.attempts.allowed which defines the maximum incorrect password attempts allowed.
> Also after the maximum attempts are reached the user account is disabled.
> 
> 
> This addresses bug CS-10219.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/user/UserAccountVO.java 5e7c018 
>   server/src/com/cloud/configuration/Config.java ebcd070 
>   server/src/com/cloud/user/AccountManagerImpl.java 38153f3 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-302to40.sql aaf23e6 
> 
> Diff: https://reviews.apache.org/r/6858/diff/
> 
> 
> Testing
> -------
> 
> Verified locally.
> 
> 
> Thanks,
> 
> saksham srivastava
> 
>