----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69319/#review210477 -----------------------------------------------------------
Fix it, then Ship it! security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerKRBAuthenticationFilter.java Lines 240 (patched) <https://reviews.apache.org/r/69319/#comment295172> - consider changing log level here to debug - surround LOG.debug() with if (LOG.isDebugEnabled()) - to avoid overhead of string concat when debug level is not enabled. Please review other LOG statements introduced in this patch. security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerKRBAuthenticationFilter.java Lines 251 (patched) <https://reviews.apache.org/r/69319/#comment295173> var14? Perhaps from a decompiled code? security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerKRBAuthenticationFilter.java Lines 253 (patched) <https://reviews.apache.org/r/69319/#comment295174> why log this debug or warn depending on log level? Shouldn't this be logged at warn level always? security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerKRBAuthenticationFilter.java Lines 271 (patched) <https://reviews.apache.org/r/69319/#comment295175> It will help to print the authenticated username as well in the log, like: LOG.info("Logged into Ranger as user=" + doAsUser + ", by authenticatedUser=" + authToken.getUserName()); - Madhan Neethiraj On Nov. 12, 2018, 7 p.m., Sailaja Polavarapu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69319/ > ----------------------------------------------------------- > > (Updated Nov. 12, 2018, 7 p.m.) > > > Review request for ranger. > > > Bugs: RANGER-2049 > https://issues.apache.org/jira/browse/RANGER-2049 > > > Repository: ranger > > > Description > ------- > > Introduced new configuration to enable trusted proxy for ranger. Added > support for ranger admin to handle doAs in the request parameter and trusted > proxy configuration is enabled for kerberized mode. Used hadoop library to > validate proxy user configuraiton and autorize accordingly. > > > Diffs > ----- > > > security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerKRBAuthenticationFilter.java > d20a203ea > > > Diff: https://reviews.apache.org/r/69319/diff/1/ > > > Testing > ------- > > 1. Patched 2.0 cluster with the ranger admin changes and tested functionality > with trusted proxy configuration enabled. > 2. Also ran some basic regression tests with trusted proxy disabled. > > > Thanks, > > Sailaja Polavarapu > >
