Skip to content

RANGER-5539: Add Authorisation Check for doAsUser Parameter#915

Merged
pradeepagrawal8184 merged 1 commit intoapache:masterfrom
ChinmayHegde24:RANGER-5539
Apr 22, 2026
Merged

RANGER-5539: Add Authorisation Check for doAsUser Parameter#915
pradeepagrawal8184 merged 1 commit intoapache:masterfrom
ChinmayHegde24:RANGER-5539

Conversation

@ChinmayHegde24
Copy link
Copy Markdown
Contributor

Currently RangerJwtAuthHandler accepts the doAsUser value directly from the incoming request and uses it to establish the authenticated user identity without performing any validation.
So the user should be validated for impersonation permission on doAsUser parameter.

Copy link
Copy Markdown
Contributor

@dineshkumar-yadav dineshkumar-yadav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pradeepagrawal8184 pradeepagrawal8184 merged commit 13dc2c1 into apache:master Apr 22, 2026
3 of 4 checks passed
@mneethiraj mneethiraj requested a review from Copilot April 22, 2026 15:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens Ranger’s JWT-based authentication flow by preventing unvalidated doAs impersonation and introducing an explicit trusted-proxy authorization check (via Hadoop ProxyUsers) before allowing an effective user switch.

Changes:

  • Remove doAs handling from the low-level JWT token authentication routine (always authenticate as the JWT subject first).
  • Add doAs impersonation gating in RangerDefaultJwtAuthHandler with overridable hooks (isProxyEnabled() / authorizeProxyUser()).
  • Implement proxy enablement + ProxyUsers.authorize() logic and proxyuser config refresh in RangerJwtAuthFilter, and add/extend unit tests around wrapper/filter behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerJwtAuthFilter.java Refreshes ProxyUsers config from ranger.proxyuser.* and enforces proxy-user authorization when trusted proxy is enabled.
ranger-authn/src/main/java/org/apache/ranger/authz/handler/jwt/RangerJwtAuthHandler.java Removes doAs from the core JWT authentication method so it can’t be used to override the token subject.
ranger-authn/src/main/java/org/apache/ranger/authz/handler/jwt/RangerDefaultJwtAuthHandler.java Adds doAs handling with trusted-proxy checks and an authorization hook before switching the effective user.
security-admin/src/test/java/org/apache/ranger/security/web/filter/TestRangerJwtAuthWrapper.java Adds tests for wrapper behavior across bearer header/cookie/authenticated/browser redirect paths.
security-admin/src/test/java/org/apache/ranger/security/web/filter/TestRangerJwtAuthFilter.java Adds tests for trusted-proxy enablement and proxyuser config copying (currently via reflection).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +100 to +109
LOG.debug("RangerDefaultJwtAuthHandler.authenticate(): Calling authorizeProxyUser: realUser=[{}], doAs=[{}], remoteAddr=[{}]",
realUser, doAsUser, httpServletRequest.getRemoteAddr());
// Check: is realUser authorized to impersonate doAsUser
if (!authorizeProxyUser(realUser, doAsUser, httpServletRequest.getRemoteAddr())) {
LOG.warn("RangerDefaultJwtAuthHandler.authenticate(): doAs=[{}] not authorized for realUser=[{}]. Rejecting.", doAsUser, realUser);
return null;
}
//Checks passed → switch to doAs user
effectiveUser = doAsUser.trim();
LOG.info("JWT doAs authorized: effectiveUser=[{}], realUser=[{}]", effectiveUser, realUser);
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doAsUser is trimmed only after the proxy authorization check, but the authorization call uses the untrimmed request parameter. This can cause legitimate impersonation to be rejected (or mismatched) when the parameter has leading/trailing whitespace. Trim (and ideally normalize) doAsUser once up-front and use the trimmed value consistently for authorizeProxyUser(...), logging, and effectiveUser.

Copilot uses AI. Check for mistakes.
Comment on lines +134 to +142
try {
UserGroupInformation ugi = UserGroupInformation.createRemoteUser(realUser);
ugi = UserGroupInformation.createProxyUser(doAsUser, ugi);
ProxyUsers.authorize(ugi, remoteAddr);
LOG.debug("RangerJwtAuthFilter.authorizeProxyUser(): ProxyUsers.authorize SUCCEEDED for realUser=[{}], doAs=[{}]",
realUser, doAsUser);
return true;
} catch (AuthorizationException ex) {
LOG.warn("JWT ProxyUsers.authorize failed for doAs=[{}], realUser=[{}]: {}", doAsUser, realUser, ex.getMessage());
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

authorizeProxyUser() passes doAsUser directly into UserGroupInformation.createProxyUser(...). Since the effective user in RangerDefaultJwtAuthHandler is trimmed, the authorization check should also use a trimmed doAsUser to avoid authorization mismatches for values with surrounding whitespace.

Suggested change
try {
UserGroupInformation ugi = UserGroupInformation.createRemoteUser(realUser);
ugi = UserGroupInformation.createProxyUser(doAsUser, ugi);
ProxyUsers.authorize(ugi, remoteAddr);
LOG.debug("RangerJwtAuthFilter.authorizeProxyUser(): ProxyUsers.authorize SUCCEEDED for realUser=[{}], doAs=[{}]",
realUser, doAsUser);
return true;
} catch (AuthorizationException ex) {
LOG.warn("JWT ProxyUsers.authorize failed for doAs=[{}], realUser=[{}]: {}", doAsUser, realUser, ex.getMessage());
String trimmedDoAsUser = doAsUser == null ? null : doAsUser.trim();
try {
UserGroupInformation ugi = UserGroupInformation.createRemoteUser(realUser);
ugi = UserGroupInformation.createProxyUser(trimmedDoAsUser, ugi);
ProxyUsers.authorize(ugi, remoteAddr);
LOG.debug("RangerJwtAuthFilter.authorizeProxyUser(): ProxyUsers.authorize SUCCEEDED for realUser=[{}], doAs=[{}]",
realUser, trimmedDoAsUser);
return true;
} catch (AuthorizationException ex) {
LOG.warn("JWT ProxyUsers.authorize failed for doAs=[{}], realUser=[{}]: {}", trimmedDoAsUser, realUser, ex.getMessage());

Copilot uses AI. Check for mistakes.
Comment on lines +147 to +151
private Configuration getProxyuserConfiguration() {
Configuration conf = new Configuration(false);
PropertiesUtil.getPropertiesMap().forEach((k, v) -> {
if (k.startsWith("ranger.proxyuser.")) {
conf.set(k, v);
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getProxyuserConfiguration() is introduced as private, which forces tests to use reflection and prevents reuse/overrides. Consider making this method protected (similar to RangerKRBAuthenticationFilter#getProxyuserConfiguration) or package-private so it can be unit-tested and extended without reflection.

Copilot uses AI. Check for mistakes.
Comment on lines +158 to +162
// Call private getProxyuserConfiguration() via reflection
Method m = RangerJwtAuthFilter.class.getDeclaredMethod("getProxyuserConfiguration");
m.setAccessible(true);

Configuration conf = (Configuration) m.invoke(filter);
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test uses reflection to access getProxyuserConfiguration(), which is brittle and tightly couples the test to private implementation details. If getProxyuserConfiguration() is made protected/package-private (as in RangerKRBAuthenticationFilter), the test can call it directly and avoid reflection.

Copilot uses AI. Check for mistakes.
Comment on lines +225 to +244
System.setProperty("ranger.default.browser-useragents", "Mozilla");

HttpServletRequest req = Mockito.mock(HttpServletRequest.class);
HttpServletResponse res = Mockito.mock(HttpServletResponse.class);
FilterChain chain = Mockito.mock(FilterChain.class);

Mockito.when(req.getHeader("Authorization")).thenReturn("Bearer token");
Mockito.when(req.getHeader("User-Agent")).thenReturn("Mozilla/5.0");

RangerJwtAuthFilter jwt = Mockito.mock(RangerJwtAuthFilter.class);
Mockito.doNothing().when(jwt).doFilter(req, res, chain);

RangerJwtAuthWrapper wrapper = new RangerJwtAuthWrapper();
wrapper.initialize(); // loads browser agents from properties/system property
setField(wrapper, "rangerJwtAuthFilter", jwt);

wrapper.doFilter(req, res, chain);

verify(res, times(1)).sendRedirect("/login.jsp");
verify(chain, times(1)).doFilter(req, res);
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

System.setProperty("ranger.default.browser-useragents", ...) is set in this test but never restored/cleared, which can leak into other tests in the JVM. Save the previous value and restore it in a finally block, or clear/restore it in @AfterEach.

Suggested change
System.setProperty("ranger.default.browser-useragents", "Mozilla");
HttpServletRequest req = Mockito.mock(HttpServletRequest.class);
HttpServletResponse res = Mockito.mock(HttpServletResponse.class);
FilterChain chain = Mockito.mock(FilterChain.class);
Mockito.when(req.getHeader("Authorization")).thenReturn("Bearer token");
Mockito.when(req.getHeader("User-Agent")).thenReturn("Mozilla/5.0");
RangerJwtAuthFilter jwt = Mockito.mock(RangerJwtAuthFilter.class);
Mockito.doNothing().when(jwt).doFilter(req, res, chain);
RangerJwtAuthWrapper wrapper = new RangerJwtAuthWrapper();
wrapper.initialize(); // loads browser agents from properties/system property
setField(wrapper, "rangerJwtAuthFilter", jwt);
wrapper.doFilter(req, res, chain);
verify(res, times(1)).sendRedirect("/login.jsp");
verify(chain, times(1)).doFilter(req, res);
String previousBrowserUserAgents = System.getProperty("ranger.default.browser-useragents");
System.setProperty("ranger.default.browser-useragents", "Mozilla");
try {
HttpServletRequest req = Mockito.mock(HttpServletRequest.class);
HttpServletResponse res = Mockito.mock(HttpServletResponse.class);
FilterChain chain = Mockito.mock(FilterChain.class);
Mockito.when(req.getHeader("Authorization")).thenReturn("Bearer token");
Mockito.when(req.getHeader("User-Agent")).thenReturn("Mozilla/5.0");
RangerJwtAuthFilter jwt = Mockito.mock(RangerJwtAuthFilter.class);
Mockito.doNothing().when(jwt).doFilter(req, res, chain);
RangerJwtAuthWrapper wrapper = new RangerJwtAuthWrapper();
wrapper.initialize(); // loads browser agents from properties/system property
setField(wrapper, "rangerJwtAuthFilter", jwt);
wrapper.doFilter(req, res, chain);
verify(res, times(1)).sendRedirect("/login.jsp");
verify(chain, times(1)).doFilter(req, res);
} finally {
if (previousBrowserUserAgents == null) {
System.clearProperty("ranger.default.browser-useragents");
} else {
System.setProperty("ranger.default.browser-useragents", previousBrowserUserAgents);
}
}

Copilot uses AI. Check for mistakes.
Comment on lines +142 to +147
@Test
void testAuthorizeProxyUser_returnsFalseWhenNoProxyConfigLoaded() {
RangerJwtAuthFilter filter = new RangerJwtAuthFilter();
// no proxyuser config loaded into ProxyUsers -> should fail safely
assertFalse(filter.authorizeProxyUser("knoxui", "admin", "10.0.0.1"));
}
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion depends on the global static state inside Hadoop ProxyUsers (and/or any prior ProxyUsers.refresh... calls) but the test doesn't reset that state. To make the test deterministic, explicitly refresh ProxyUsers with an empty Configuration(false) for the ranger.proxyuser. prefix (or call filter.initialize() after clearing ranger.proxyuser.* properties) before calling authorizeProxyUser(...).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants