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



Would it have been possble to add the lockout logic in 
`org.apache.ambari.server.security.authentication.AmbariAuthenticationEventHandlerImpl#onSuccessfulAuthentication`?
  I am not sure if chaning the _success_ to a _failure_ is possible here, 
though.

If the code stays as is, does the consective failure count increase when the 
user fails to authenticate becuase the failure count was passed the threshhold? 
If so, do we care?


ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
Lines 2774 (patched)
<https://reviews.apache.org/r/61501/#comment258589>

    Being that this really only applies to the _local_ authentication 
mechanism, maybe `authentication.max.failures` should be renamed to something 
like `authentication.local.max.failures`?


- Robert Levas


On Aug. 10, 2017, 11:28 a.m., Attila Magyar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61501/
> -----------------------------------------------------------
> 
> (Updated Aug. 10, 2017, 11:28 a.m.)
> 
> 
> Review request for Ambari, Balázs Bence Sári, Laszlo Puskas, Robert Levas, 
> and Sebastian Toader.
> 
> 
> Bugs: AMBARI-21680
>     https://issues.apache.org/jira/browse/AMBARI-21680
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Prevent users from authenticating if they exceed a configured number of login 
> failures, which is set as a configuration in the ambari.properties file - 
> authentication.max.failures.
> 
> After a users successfully authenticates, check the value of 
> org.apache.ambari.server.orm.entities.UserEntity#getConsecutiveFailures. 
> 
> If it exceeds the value set in authentication.max.failures, then fail 
> authentication. Else allow authentication to proceed.
> If failing authentication due to being "locked out", do not indicate this to 
> the user; however an Ambari server log message will be useful. 
> The normal "authentication failed" message should be returned as to not give 
> away any information about a user's authentication. If a special "locked out" 
> message is shown, then a hacker will be able to attempt a brute force attack 
> on a user's account since the returned error message will be different if 
> they eventually succeed in guessing the password.
> 
> To "unlock" the user, a user administrator (a user with the 
> AMBARI.MANAGE_USERS authorization) needs to reset the user's consecutive 
> failure count to 0.
> By default the authentication.max.failures should be 10; however 0 should 
> indicate that no lockout is desired.
> Options
> 
> 
> Diffs
> -----
> 
>   
> ambari-admin/src/main/resources/ui/admin-web/app/scripts/controllers/users/UsersShowCtrl.js
>  200872e 
>   ambari-admin/src/main/resources/ui/admin-web/app/scripts/i18n.config.js 
> 43b32da 
>   ambari-admin/src/main/resources/ui/admin-web/app/scripts/services/User.js 
> ac50653 
>   ambari-admin/src/main/resources/ui/admin-web/app/views/users/show.html 
> f965c5d 
>   ambari-server/docs/configuration/index.md 9dbe9c4 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  4f787c6 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
>  b52e2b1 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/UserRequest.java
>  d0836a9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserResourceProvider.java
>  a2d9917 
>   
> ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationEventHandlerImpl.java
>  3a5a66b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/security/authentication/TooManyLoginFailuresException.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariLocalUserProvider.java
>  2c8bf12 
>   
> ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLocalUserProviderTest.java
>  133fc9f 
> 
> 
> Diff: https://reviews.apache.org/r/61501/diff/2/
> 
> 
> Testing
> -------
> 
> 1
> - tried to log in 10 times with an incorrect password
> - tried to log in with the correct password and received authentication 
> failure
> - reseted the counter
> - logged in successfully with the correct password
> 
> 2
> - tried to log in 3 times with incorrect password
> - checked that UI showed Login failures=3
> - logged in successfully with the correct password
> - checked the UI showed Login failures=0
> 
> 3
> - added authentication.max.failures=3 to ambari.properties
> - repeated the first test
> 
> 4
> - added authentication.max.failures=0 to ambari.properties
> - tried to log in 11 times with an incorrect password
> - successfully logged in with the correct password
> 
> Results :
> Tests run: 4841, Failures: 0, Errors: 0, Skipped: 33
> ----------------------------------------------------------------------
> Total run:1145
> Total errors:0
> Total failures:0
> OK
> Ran 470 tests in 108.068s
> 
> 
> Thanks,
> 
> Attila Magyar
> 
>

Reply via email to