----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/73922/#review224313 -----------------------------------------------------------
security-admin/db/mysql/optimized/current/ranger_core_db_mysql.sql Line 1692 (original), 1694 (patched) <https://reviews.apache.org/r/73922/#comment313169> - consider leaving old_passwords and password_update_time to be null here (i.e. no changes to INSERT statements). Note that value in password column can be used in change-password flow - update change-password flow to verify that the new password doesn't match values in columns password and old_passwords - on successful password change, existing value in password column should be added to old_passwords column security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java Lines 121 (patched) <https://reviews.apache.org/r/73922/#comment313168> Consider using a static const for defaultPwdHistoryStore, like: private static final int DEFAULT_PASSWORD_HISTORY_COUNT = 4; private int passwordHistoryCount = PropertiesUtil.getIntProperty("ranger.passord.history.count", DEFAULT_PASSWORD_HISTORY_COUNT); security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java Lines 147 (patched) <https://reviews.apache.org/r/73922/#comment313171> pwdHistoryStore=0 should be allowed, to retain current behavior i.e., no restriction on reuse of password. if (passwordHistoryCount < 0) { passwordHistoryCount = 0; } security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java Lines 169 (patched) <https://reviews.apache.org/r/73922/#comment313172> Per earlier comment in ranger_core_db_mysql.sql, line #169 is not needed. security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java Line 386 (original), 394 (patched) <https://reviews.apache.org/r/73922/#comment313170> Consider replacing #394 - #409 with the following, per earlier comments in ranger_core_db_mysql.sql: String oldPasswordStr = gjUser.getOldPasswords(); List<String> oldPasswords; if (StringUtils.isNotEmpty(oldPasswordStr)) { oldPasswords = new ArrayList<>(oldPasswordStr.split(","); } else { oldPasswords = new ArrayList<>(); } oldPasswords.append(gjUser.getPassword()); while (oldPasswords.size() > this.passwordHistroyCount) { oldPasswords.remove(0); } boolean isNewPasswordDifferent = oldPasswords.isEmpty(); for (String oldPassword : oldPasswords) { if (this.isFipsEnabled) { isNewPasswordDifferent = isNewPasswordDifferent(pwdChange.getLoginId(), oldPassword, encryptedNewPwd); } else { isNewPasswordDifferent = !encryptedNewPwd.equals(oldPassword); } if (!isNewPasswordDifferent) { break; } } security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java Lines 436 (patched) <https://reviews.apache.org/r/73922/#comment313173> Per changes suggested in earlier comments, consider updating updateOldPasswords() to: private void updateOldPasswords(XXPortalUser gjUser, String encryptedNewPwd, List<String> oldPasswords) { String oldPasswordStr = StringUtils.isNotEmpty(oldPasswords) ? StringUtils.join(oldPasswords, ",") : null; gjUser.setOldPasswords(oldPasswordStr); gjUser.setPasswordUpdatedTime(DateUtil.getUTCDate()); } - Madhan Neethiraj On April 13, 2022, 2:57 a.m., bhavik patel wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/73922/ > ----------------------------------------------------------- > > (Updated April 13, 2022, 2:57 a.m.) > > > Review request for ranger, Dhaval Shah, Dineshkumar Yadav, Kirby Zhou, Abhay > Kulkarni, Madhan Neethiraj, Mateen Mansoori, Mehul Parikh, Pradeep Agrawal, > Ramesh Mani, and Velmurugan Periasamy. > > > Bugs: RANGER-3687 > https://issues.apache.org/jira/browse/RANGER-3687 > > > Repository: ranger > > > Description > ------- > > Password history should be configured to restrict users from reusing their > last 4 or 5 passwords. > > > Diffs > ----- > > security-admin/db/mysql/optimized/current/ranger_core_db_mysql.sql > 26282f770 > security-admin/db/mysql/patches/059-update-x-portal-user-table.sql > PRE-CREATION > security-admin/db/oracle/optimized/current/ranger_core_db_oracle.sql > e2475cfbd > security-admin/db/oracle/patches/059-update-x-portal-user-table..sql > PRE-CREATION > security-admin/db/postgres/optimized/current/ranger_core_db_postgres.sql > f5c6ed8f5 > security-admin/db/postgres/patches/059-update-x-portal-user-table.sql > PRE-CREATION > > security-admin/db/sqlanywhere/optimized/current/ranger_core_db_sqlanywhere.sql > 1887d6da9 > security-admin/db/sqlanywhere/patches/059-update-x-portal-user-table.sql > PRE-CREATION > security-admin/db/sqlserver/optimized/current/ranger_core_db_sqlserver.sql > 642e54cd5 > security-admin/db/sqlserver/patches/059-update-x-portal-user-table.sql > PRE-CREATION > security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java 0e61038d5 > security-admin/src/main/java/org/apache/ranger/entity/XXPortalUser.java > d0451b4d2 > security-admin/src/main/resources/conf.dist/ranger-admin-default-site.xml > e2bfc8fff > security-admin/src/test/java/org/apache/ranger/biz/TestUserMgr.java > f43b30196 > > > Diff: https://reviews.apache.org/r/73922/diff/7/ > > > Testing > ------- > > 1. Verified the basic functionality of "/passwordchange" api > 2. Verified "/secure/users" & "/secure/users/{id}" API’s > > 3. Once the basic review/discussion is done will fix the Test-cases > > > Thanks, > > bhavik patel > >
