----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60205/#review178344 -----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationEventHandlerImpl.java Lines 109-117 (patched) <https://reviews.apache.org/r/60205/#comment252249> Add to audit log the number of login attemps. ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariBasicAuthenticationFilter.java Line 120 (original), 102 (patched) <https://reviews.apache.org/r/60205/#comment252286> Is it a valid scenario to not have an event handler passed in? ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariBasicAuthenticationFilter.java Line 121 (original), 103 (patched) <https://reviews.apache.org/r/60205/#comment252287> If the audit logging is factored out into an event handler we have to ensure that event handler is always instantiaded otherwise the won't have audit logs related to logins. ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariBasicAuthenticationFilter.java Line 147 (original), 123 (patched) <https://reviews.apache.org/r/60205/#comment252288> We have to ensure that there is alway at least one event handler that provides the audit logging of successful logins. ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariBasicAuthenticationFilter.java Line 179 (original), 155 (patched) <https://reviews.apache.org/r/60205/#comment252289> We have to ensure that there is alway at least one event handler that provides the audit logging of login failures. ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java Lines 1275 (patched) <https://reviews.apache.org/r/60205/#comment252296> Wouldn't it be simpler to handle this within a transaction instead of optimistic locking? ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java Lines 1300 (patched) <https://reviews.apache.org/r/60205/#comment252297> I think it's a valid scenario that a user gets deleted by an admin in parallel so it's not an unexpected situtation. ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java Lines 1304 (patched) <https://reviews.apache.org/r/60205/#comment252294> Why is this needed? The ```userEntity = userDAO.findByPK(userID);``` provides an up to date entity. (Unless user was updated in the db directly or by some named JPQL query) ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java Lines 1349 (patched) <https://reviews.apache.org/r/60205/#comment252298> Shouldn't use optimistic locking here as well? - Sebastian Toader On June 19, 2017, 10:26 p.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60205/ > ----------------------------------------------------------- > > (Updated June 19, 2017, 10:26 p.m.) > > > Review request for Ambari, Attila Magyar, Balázs Bence Sári, Eugene > Chekanskiy, Jonathan Hurley, Laszlo Puskas, and Sebastian Toader. > > > Bugs: AMBARI-21216 > https://issues.apache.org/jira/browse/AMBARI-21216 > > > Repository: ambari > > > Description > ------- > > Add support for consecutive login failure accounting where as log-in failures > should increment the {{users.consecutive_failures}} and successful > authentication attempts should reset the value to 0. > > Concurrency should be kept in mind to handle simultaneous authentication > attempts for the same user. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java > 01920f86d6 > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UserDAO.java > 0e28e50709 > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UserEntity.java > 66e9003873 > > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationEventHandler.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationEventHandlerImpl.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationException.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariBasicAuthenticationFilter.java > ac3e15fa0f > > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariJWTAuthenticationFilter.java > fca8b29fe2 > > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/UserNotFoundException.java > f6c4bcf2a7 > > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/kerberos/AmbariKerberosAuthenticationFilter.java > 1b001ec020 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariLdapAuthenticationProvider.java > 6137b68e99 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariLdapAuthoritiesPopulator.java > 5c482a1a13 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariLocalUserProvider.java > 517efe49b8 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/InvalidUsernamePasswordCombinationException.java > db8238184b > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java > 35eb255fcb > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/internal/AmbariInternalAuthenticationProvider.java > c57bdf1a99 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/jwt/JwtAuthenticationFilter.java > 3c3a446a61 > ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql 0c86591010 > ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql 83b1f48049 > ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql 215c01d7b7 > ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql 40ba709d1d > ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql > 35951f142b > ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql b7244abee2 > ambari-server/src/main/resources/webapp/WEB-INF/spring-security.xml > bdbf0de3fa > > ambari-server/src/test/java/org/apache/ambari/server/security/authentication/AmbariBasicAuthenticationFilterTest.java > 18c4ccee77 > > ambari-server/src/test/java/org/apache/ambari/server/security/authentication/AmbariJWTAuthenticationFilterTest.java > 961e65dfbb > > ambari-server/src/test/java/org/apache/ambari/server/security/authentication/kerberos/AmbariKerberosAuthenticationFilterTest.java > 5503fac6ac > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationProviderDisableUserTest.java > 33100dd33b > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLdapAuthenticationProviderForDNWithSpaceTest.java > 1bf122e0a1 > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLdapAuthenticationProviderTest.java > d9eb3350fe > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLocalUserProviderTest.java > 65a5400dc7 > > > Diff: https://reviews.apache.org/r/60205/diff/1/ > > > Testing > ------- > > Manually tested in cluster > > #Local test results: > ``` > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 30:58 min > [INFO] Finished at: 2017-06-19T15:16:18-04:00 > [INFO] Final Memory: 108M/1760M > [INFO] > ------------------------------------------------------------------------ > ``` > > #Jasper test results: PENDING > > > Thanks, > > Robert Levas > >
