> On Aug. 10, 2017, 6:26 p.m., Robert Levas wrote: > > 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?
First I wanted to put it there, but I didn't find a way to move to the failure case from there. The counter will keep increasing after it passed the threashold. This can lead to a possible overflow after ~2 billion failures, which will eventually reset the counter. So maybe we shouldn't increase it beyond a certain limit. I can add overflow detection to UserEntity>incrementConsecutiveFailures and maximize the counter to Integer.MAX_VALUE. Or do you have a better idea? - Attila ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61501/#review182616 ----------------------------------------------------------- On Aug. 10, 2017, 3:28 p.m., Attila Magyar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61501/ > ----------------------------------------------------------- > > (Updated Aug. 10, 2017, 3:28 p.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 > >
