> On Nov. 17, 2015, 8:37 a.m., Madhan Neethiraj wrote: > > security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java, > > line 89 > > <https://reviews.apache.org/r/40343/diff/1-2/?file=1126094#file1126094line89> > > > > 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); > > }
Adding comment for each if/else for time being for better readability. Will refactor it later as currently changing it will require thorough testing again. > On Nov. 17, 2015, 8:37 a.m., Madhan Neethiraj wrote: > > security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java, > > line 113 > > <https://reviews.apache.org/r/40343/diff/2/?file=1127987#file1127987line113> > > > > 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? By this the groups/role are not set as that user will already be there in ranger db and will pick up the role which is assigned to him from the db. Here am setting the minimum role because the method requires some grant authorities. Similar implementation has being done for LDAP/AD/JDBC in RangerAuthenticationProvider.java. - Gautam ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40343/#review106808 ----------------------------------------------------------- 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 > >
