Lui 83 #99
Lui 83 #99
Conversation
|
Can you include the ticket id in your commit message? |
|
ok let me do that |
| @@ -178,7 +180,14 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) | |||
| catch (ContextAuthenticationException e) { | |||
| // set the error message for the user telling them | |||
| // to try again | |||
| httpSession.setAttribute(WebConstants.OPENMRS_ERROR_ATTR, "auth.password.invalid"); | |||
| Integer maximumAlowedAttempts = 7; | |||
There was a problem hiding this comment.
Why did you make the maximum allowed attempts 7?
There was a problem hiding this comment.
because thats the default maximum allowed attemptts
There was a problem hiding this comment.
i have realised its a configurable figure , so let just try to acces it using a global property
mozzy11
force-pushed
the
LUI-83
branch
2 times, most recently
from
f4f774a
to
b2edebf
Compare
|
i have replaced the hard coded figure "7" with the Global property |
| @@ -97,7 +97,7 @@ public void shouldLockUserOutAfterFiveFailedLoginAttempts() throws Exception { | |||
| loginServlet.service(request, response); | |||
| } | |||
|
|
|||
| // now attempting to log in the fifth time should fail | |||
| // now attempting to log in the fifthth time should fail | |||
There was a problem hiding this comment.
Did you intend to make the above change?
There was a problem hiding this comment.
no that was accidenta, coz at some point , the tests were failing so i was cross checkingthem , but that was un intended. thx
| String ipAddress = request.getRemoteAddr(); | ||
| Integer loginAttempts = loginAttemptsByIP.get(ipAddress); | ||
| if (loginAttempts == null) { | ||
| loginAttempts = 1; | ||
| } | ||
|
|
||
| loginAttempts++; | ||
|
|
||
| loginAttemptsByUser = loginAttempts - 1; |
There was a problem hiding this comment.
What's the difference btn loginAttemptsByUser and loginAttempts? Are there attempts that aren't made by a User?
There was a problem hiding this comment.
i intended to differentiate between loginAttempts by User name , and loginAttempts by IP address
There was a problem hiding this comment.
Can you improve the naming to reflect that?
Displaying meaningful infowen user is locked made the variable local displaying use ful info when user is locked out LUI-83 : modified the method to use global property LUI-83 : modified the method to use global property LUI-83: Correcting the Variable Naming
|
hi @dkayiwa and @samuelmale . i corrected the Naming |
|
|
||
| } | ||
|
|
||
| if (loginAttemptsByUserName > maximumAlowedAttempts) { |
There was a problem hiding this comment.
I think use of if else here will be much better because we are to check for only one of the condition and not both separately.
|
@dkayiwa is this ready for merging? Can we wrap up this old work? |
https://issues.openmrs.org/browse/LUI-83