Re: Review Request 60205: Add support for consecutive login failure accounting

2017-06-22 Thread Jonathan Hurley 

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


Ship it!




Ship It!

- Jonathan Hurley


On June 22, 2017, 12:46 p.m., Robert Levas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60205/
> ---
> 
> (Updated June 22, 2017, 12:46 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/audit/event/LoginAuditEvent.java
>  9583b84cdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
>  01920f86d6 
>   
> 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/audit/LoginAuditEventTest.java
>  ac79967ea2 
>   
> 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 
>   
>

Re: Review Request 60205: Add support for consecutive login failure accounting

2017-06-22 Thread Robert Levas 

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

(Updated June 22, 2017, 12:46 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 (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/audit/event/LoginAuditEvent.java
 9583b84cdb 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
 01920f86d6 
  
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/audit/LoginAuditEventTest.java
 ac79967ea2 
  
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/4/

Changes: https://reviews.apache.org/r/60205/diff/3-4/


Testing
---

Manually tested in cluster

#Local test results: 
```
INFO] 
[INFO] BUILD SUCCESS
[INFO] 
[INFO] Total time: 29:50 min
[INFO] Finished at: 2017-06-21T15:43:10-04:00
[INFO]

Re: Review Request 60205: Add support for consecutive login failure accounting

2017-06-22 Thread Jonathan Hurley 

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


Fix it, then Ship it!





ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java
Lines 75-78 (patched)


100 retries? That could be a lot :) ... typically, I'd say a handful is 
enough here. Especially since you're catching every exception type.



ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java
Lines 1261 (patched)


sp: failure



ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java
Lines 1274 (patched)


sp: failure



ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java
Lines 1285-1295 (patched)


The else isn't needed since you return null above in the if-statement.



ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java
Lines 1345 (patched)


I think this method does what you want; it re-runs the command to increment 
the value in the case of entity being updated behind the scenes. 

The question is whether it needs to be done for any merge or just for the 
login attempt value. I'm fine leaving it as-is, especially since user values 
don't get updated that often, right?



ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java
Lines 1355 (patched)


Out of curiousity, any reason you didn't catch the OptimisticLockException 
directly (or a Runtime exception directly)? Throwable covers everything, yes, 
but seems kind of broad.


- Jonathan Hurley


On June 22, 2017, 10:39 a.m., Robert Levas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60205/
> ---
> 
> (Updated June 22, 2017, 10:39 a.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/audit/event/LoginAuditEvent.java
>  9583b84cdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
>  01920f86d6 
>   
> 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 
>   
>

Re: Review Request 60205: Add support for consecutive login failure accounting

2017-06-22 Thread Sebastian Toader 

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


Ship it!




Ship It!

- Sebastian Toader


On June 22, 2017, 4:39 p.m., Robert Levas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60205/
> ---
> 
> (Updated June 22, 2017, 4:39 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/audit/event/LoginAuditEvent.java
>  9583b84cdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
>  01920f86d6 
>   
> 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/audit/LoginAuditEventTest.java
>  ac79967ea2 
>   
> 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 
>   
>

Re: Review Request 60205: Add support for consecutive login failure accounting

2017-06-22 Thread Robert Levas 

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

(Updated June 22, 2017, 10:39 a.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 (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/audit/event/LoginAuditEvent.java
 9583b84cdb 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
 01920f86d6 
  
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/audit/LoginAuditEventTest.java
 ac79967ea2 
  
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/3/

Changes: https://reviews.apache.org/r/60205/diff/2-3/


Testing
---

Manually tested in cluster

#Local test results: 
```
INFO] 
[INFO] BUILD SUCCESS
[INFO] 
[INFO] Total time: 29:50 min
[INFO] Finished at: 2017-06-21T15:43:10-04:00
[INFO]

Re: Review Request 60205: Add support for consecutive login failure accounting

2017-06-22 Thread Sebastian Toader 

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




ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationEventHandlerImpl.java
Lines 118 (patched)


I think it's better to leave it null in this case and let audit logger log 
```Consecutive failures(UNKNOW USER)``` to the audit log. What so you think?



ambari-server/src/main/java/org/apache/ambari/server/audit/event/LoginAuditEvent.java
Lines 83 (patched)


If consecutiveFailures is null means that the user doesn't exist in the 
database and we don't have data in number of attemps I'm thinking it's better 
to log ```Consecutive failures(UNKNOW USER)``` to the audit log. What so you 
think?


- Sebastian Toader


On June 21, 2017, 9:46 p.m., Robert Levas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60205/
> ---
> 
> (Updated June 21, 2017, 9:46 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/audit/event/LoginAuditEvent.java
>  9583b84cdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
>  01920f86d6 
>   
> 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/audit/LoginAuditEventTest.java
>  ac79967ea2 
>   
> ambari-server/src/test/java/org/apache/ambari/server/security/authentication/AmbariBasicAuthenticationFilterTest.java
>  18c4ccee77 
>   
>

Re: Review Request 60205: Add support for consecutive login failure accounting

2017-06-21 Thread Robert Levas 

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

(Updated June 21, 2017, 3:46 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/audit/event/LoginAuditEvent.java
 9583b84cdb 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
 01920f86d6 
  
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/audit/LoginAuditEventTest.java
 ac79967ea2 
  
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/2/


Testing (updated)
---

Manually tested in cluster

#Local test results: 
```
INFO] 
[INFO] BUILD SUCCESS
[INFO] 
[INFO] Total time: 29:50 min
[INFO] Finished at: 2017-06-21T15:43:10-04:00
[INFO] Final Memory: 86M/1822M
[INFO]

Re: Review Request 60205: Add support for consecutive login failure accounting

2017-06-21 Thread Robert Levas 

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

(Updated June 21, 2017, 3:45 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 (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/audit/event/LoginAuditEvent.java
 9583b84cdb 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
 01920f86d6 
  
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/audit/LoginAuditEventTest.java
 ac79967ea2 
  
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/2/

Changes: https://reviews.apache.org/r/60205/diff/1-2/


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]

Re: Review Request 60205: Add support for consecutive login failure accounting

2017-06-21 Thread Sebastian Toader 


> On June 20, 2017, 1:45 p.m., Sebastian Toader wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationEventHandlerImpl.java
> > Lines 109-117 (patched)
> > 
> >
> > Add to audit log the number of login attemps.
> 
> Robert Levas wrote:
> Is there a special `with` call for that or do you mean that this 
> information should be added to the reason string?

I'd extend the LoginAuditEvent with a withLoginAttempts method and modify it's 
```buildAuditMessage``` method to take into account this new user related 
information. Is there any new field that might be useful from audit point of 
view?


> On June 20, 2017, 1:45 p.m., Sebastian Toader wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java
> > Lines 1304 (patched)
> > 
> >
> > 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)
> 
> Robert Levas wrote:
> I needed this for my test since I was updating the DB directly, but I 
> didn't know if normal caching would cause an issue as well.  Should I remove 
> this and assume all will work ok?

Assuming that all updates to user entity goes through the EntityManager (and 
EclipseLink) and no direct db or JPQL is used to change the underlying db 
record than the EM cache contains up to date user entity records thus I think 
it can be removed.


- Sebastian


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


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

Re: Review Request 60205: Add support for consecutive login failure accounting

2017-06-21 Thread Robert Levas 


> On June 20, 2017, 7:45 a.m., Sebastian Toader wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationEventHandlerImpl.java
> > Lines 109-117 (patched)
> > 
> >
> > Add to audit log the number of login attemps.

Is there a special `with` call for that or do you mean that this information 
should be added to the reason string?


> On June 20, 2017, 7:45 a.m., Sebastian Toader wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariBasicAuthenticationFilter.java
> > Line 120 (original), 102 (patched)
> > 
> >
> > Is it a valid scenario to not have an event handler passed in?

No, so I guess I should throw and exception here or allow the NPE to be thrown.


> On June 20, 2017, 7:45 a.m., Sebastian Toader wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariBasicAuthenticationFilter.java
> > Line 121 (original), 103 (patched)
> > 
> >
> > 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.

correct... so we should ensure that the eventHander has been set.


> On June 20, 2017, 7:45 a.m., Sebastian Toader wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java
> > Lines 1275 (patched)
> > 
> >
> > Wouldn't it be simpler to handle this within a transaction instead of 
> > optimistic locking?

I am not sure if transactions are sufficient here.  After discussing with 
@jhurley, this seems like the way to go to ensure the counter is updated 
properly in a concurrent update scenario.


> On June 20, 2017, 7:45 a.m., Sebastian Toader wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java
> > Lines 1300 (patched)
> > 
> >
> > I think it's a valid scenario that a user gets deleted by an admin in 
> > parallel so it's not an unexpected situtation.

good point.


> On June 20, 2017, 7:45 a.m., Sebastian Toader wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java
> > Lines 1304 (patched)
> > 
> >
> > 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)

I needed this for my test since I was updating the DB directly, but I didn't 
know if normal caching would cause an issue as well.  Should I remove this and 
assume all will work ok?


> On June 20, 2017, 7:45 a.m., Sebastian Toader wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java
> > Lines 1349 (patched)
> > 
> >
> > Shouldn't use optimistic locking here as well?

Nice call.. thanks.  Exception should be handled here as well.  Thanks for 
catching that.


- Robert


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


On June 19, 2017, 4: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, 4: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 
>   
>

Re: Review Request 60205: Add support for consecutive login failure accounting

2017-06-20 Thread Sebastian Toader 

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


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)


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)


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)


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)


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)


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)


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)


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)


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

Re: Review Request 60205: Add support for consecutive login failure accounting

2017-06-19 Thread Alejandro Fernandez 

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


Ship it!




Ship It!

- Alejandro Fernandez


On June 19, 2017, 8: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, 8: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 
> 
> 
>

Re: Review Request 60205: Add support for consecutive login failure accounting

2017-06-19 Thread Robert Levas 

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




ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java
Lines 1286-1288 (patched)


The exception thrown may not directly be a 
`javax.persistence.OptimisticLockException`, so we need to look at the cause(s) 
to determine if an `javax.persistence.OptimisticLockException` is at play.



ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java
Lines 1295-1304 (patched)


The `UserEntity` object at this point may be detached, so we need to get an 
_attached_ one and ensure that it contains the latest data... so a _refresh_ is 
needed.


- Robert Levas


On June 19, 2017, 4: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, 4: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 
>   
>