----------------------------------------------------------- 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 > >
