----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/73898/#review224174 -----------------------------------------------------------
security-admin/src/main/java/org/apache/ranger/db/XXAuthSessionDao.java Lines 71 (patched) <https://reviews.apache.org/r/73898/#comment313084> This will result in 2 queries for each login attempt: 1. to get the last successful login time 2. to get number of failed login since last successful login Consider following to use only one query where possible: public boolean shouldLockUserLogin(String loginId, int seconds, int maxFailedLoginsBeforeLock) { boolean ret = false; Date startTime = new Date(DateUtil.getUTCDate().getTime() - seconds * 1000); int numOfFailedLogins = getFailedLoginsCountForUserSince(loginId, startTime); if (numOfFailedLogins >= maxFailedLoginsBeforeLock) { Date lastSuccessLoginTime = getLastSuccessfulLoginTimeSince(loginId, startTime); if (lastSuccessLoginTime != null) { numOfFailedLogins = getFailedLoginsCountForUser(loginId, lastSuccessLoginTime); ret = numOfFailedLogins >= maxFailedLoginsBeforeLock; } } return ret; } security-admin/src/main/java/org/apache/ranger/db/XXAuthSessionDao.java Lines 72 (patched) <https://reviews.apache.org/r/73898/#comment313085> Please replae dynamic queries in #72 and #80 with named queries in security-admin/src/main/resources/META-INF/jpa_named_queries.xml - Madhan Neethiraj On March 18, 2022, 5:59 a.m., Kirby Zhou wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/73898/ > ----------------------------------------------------------- > > (Updated March 18, 2022, 5:59 a.m.) > > > Review request for ranger, Bhavik Bavishi, Abhay Kulkarni, Madhan Neethiraj, > and Pradeep Agrawal. > > > Bugs: RANGER-2362 > https://issues.apache.org/jira/browse/RANGER-2362 > > > Repository: ranger > > > Description > ------- > > RANGER-2362 > > > Here is a simple demo code for discussion. > > Hard-codeed: > we limit 3 failures per 30 minutes. A successful login will reset the counter. > > > BTW: I think the code of RangerAuthenticationProvider is a bit anti-pattern. > > 1. We new RangerAuthenticationProvider at each time user login. It is > unreasonable, it should be a bean. > > see RangerKRBAuthenticationFilter.java and RangerSSOAuthenticationFilter.java > > 2. We new Jdbc/AD/Ldap/Pam authentication provider in > RangerAuthenticationProvider at each time user login. > > 3. The member 'private LdapAuthenticator authenticator' seems useless > > 4. The RangerAuthenticationProvider seem should be replaced with > ProviderManager or something like spring configuration. > > > Diffs > ----- > > security-admin/src/main/java/org/apache/ranger/biz/SessionMgr.java > 6b002cff994dd431a83ef46f10ee839fb83dafbb > security-admin/src/main/java/org/apache/ranger/db/XXAuthSessionDao.java > b0270e9d45aa5b5543735318eea4e22683cbfece > > security-admin/src/main/java/org/apache/ranger/security/handler/RangerAuthenticationProvider.java > 8f7abbe7df3d0344c7b5b1af89f7322d82a0d238 > > security-admin/src/main/java/org/apache/ranger/security/listener/SpringEventListener.java > af5622a5f756db931a7173ad01d8c4162d5ee05a > > > Diff: https://reviews.apache.org/r/73898/diff/2/ > > > Testing > ------- > > Self tested > > > Thanks, > > Kirby Zhou > >
