----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40343/#review106808 -----------------------------------------------------------
security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java (line 89) <https://reviews.apache.org/r/40343/#comment165601> Consider splitting the if/else blocks for better readabilty. After rearranging the code, it is clear some branches are not handled. Please review all branches and update. if(ssoEnabled && isWebUserAgent(userAgent)) { if (!isAuthenticated) { if(jwtProperties != null { // do SSO stuff } } else { if(((HttpServletRequest) servletRequest).getRequestURI().contains(LOCAL_LOGIN_URL)) { String url = ((HttpServletRequest) servletRequest).getRequestURI().replace(LOCAL_LOGIN_URL+"/", ""); url = url.replace(LOCAL_LOGIN_URL, ""); LOG.warn("There is an active session and if you want local login to ranger, try this on a separate browser"); ((HttpServletResponse)servletResponse).sendRedirect(url); } } } else { filterChain.doFilter(servletRequest, servletResponse); } security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java (line 90) <https://reviews.apache.org/r/40343/#comment165598> Shouldn't this be "! isAuthenticated()"? security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java (line 136) <https://reviews.apache.org/r/40343/#comment165599> Looks like "!isAuthenticated()" should be "isAuthenticated()" - please review. security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java (line 193) <https://reviews.apache.org/r/40343/#comment165602> The return value should be the reverse of what it was before renaming the method. Please review. security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java (line 113) <https://reviews.apache.org/r/40343/#comment165603> Where does the groups/roles for the user are set? For example, if admin login using SSO, would ROLE_SYS_ADMIN be set? or only the default role, ROLE_USER be set in the session? - Madhan Neethiraj On Nov. 17, 2015, 8:13 a.m., Gautam Borad wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40343/ > ----------------------------------------------------------- > > (Updated Nov. 17, 2015, 8:13 a.m.) > > > Review request for ranger, Alok Lal, Don Bosco Durai, Abhay Kulkarni, Larry > McCay, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan > Periasamy. > > > Bugs: RANGER-685 > https://issues.apache.org/jira/browse/RANGER-685 > > > Repository: ranger > > > Description > ------- > > Add Ability to Authenticate users with SSO option provided by Knox. > > > Diffs > ----- > > security-admin/pom.xml 3c26837efdedbd4deb95a791418251751f4f17e9 > security-admin/scripts/install.properties > f3af716fef39bf0ac5821466803e6fb56b54fd96 > security-admin/scripts/setup.sh 36696a036cf9f27fd6e106abb5e5bf94e79190ad > security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java > 9173d6e2f9e89b2de5b93227230afd2bd91c9edc > > security-admin/src/main/java/org/apache/ranger/security/handler/RangerAuthenticationProvider.java > 40b08c414caa65e8d3a995d7713a14b6147af1d3 > > security-admin/src/main/java/org/apache/ranger/security/web/authentication/RangerAuthenticationEntryPoint.java > 52228ddc2ea824e2f3fa6bea383f81fee6cf4d7d > > security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java > PRE-CREATION > > security-admin/src/main/java/org/apache/ranger/security/web/filter/SSOAuthentication.java > PRE-CREATION > > security-admin/src/main/java/org/apache/ranger/security/web/filter/SSOAuthenticationProperties.java > PRE-CREATION > security-admin/src/main/resources/conf.dist/ranger-admin-site.xml > fe7320c79ee9c337acd2fc41fd34f610ca98dcd5 > security-admin/src/main/resources/conf.dist/security-applicationContext.xml > 162afc621c61d14b9089b79b2d6e9bb2b1d19850 > security-admin/src/main/webapp/scripts/utils/XAUtils.js > 8cb90e36df9fc08907b7aca74d510a4485f21ca1 > security-admin/src/main/webapp/scripts/views/common/ErrorView.js > a9d5739ac0b541e9d713fd234ce9d9ceade2e9d1 > security-admin/src/main/webapp/scripts/views/common/ProfileBar.js > 0f872708f31202fc9b3422b7672d88eae107dc2f > > Diff: https://reviews.apache.org/r/40343/diff/ > > > Testing > ------- > > Tested with local Ranger admin authentication against KNOX SSO and tried > various scenarios. > > > Thanks, > > Gautam Borad > >
