This is an automated email from the ASF dual-hosted git repository.
bharos pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new f8d7523b0d [#12095] feat(auth): parse X-Gravitino-Active-Roles into
the request path (#12096)
f8d7523b0d is described below
commit f8d7523b0d9a7fabdef6ee65c683b3484f997eda
Author: Bharath Krishna <[email protected]>
AuthorDate: Wed Jul 22 22:23:23 2026 -0700
[#12095] feat(auth): parse X-Gravitino-Active-Roles into the request path
(#12096)
### What changes were proposed in this pull request?
Parse the `X-Gravitino-Active-Roles` header and carry the declared
active roles on the `UserPrincipal`, so the narrowing from #11967 takes
effect.
- `UserPrincipal`: carry an immutable `ActiveRoles` (defaults to `ALL`)
alongside the existing per-request `accessToken`; `withActiveRoles()`
attaches the parsed value.
- `AuthenticationFilter`: parse the header after authentication and
attach it to the principal. A malformed value maps to `400` (the Lance
filter maps the same).
- `AuthorizationRequestContext`: read `activeRoles` from the current
`UserPrincipal`, so every authorization decision uses the request's
declaration.
Membership validation (`403` for an unheld role) is metalake-scoped and
follows in a later PR. Part of #11965.
### Why are the changes needed?
#11967 added the narrowing enforcement, but nothing populated the active
roles, so it never activated. This wires the header through the request
path.
### Does this PR introduce any user-facing change?
Yes, additive and backward compatible. A request may send
`X-Gravitino-Active-Roles: <role>[,<role>] | ALL | NONE` to narrow the
roles used for authorization; a malformed value returns `400`. An absent
header behaves as before (`ALL`).
### How was this patch tested?
New unit tests for `UserPrincipal.withActiveRoles`, the filter's header
parsing (valid / absent / malformed → `400`), and
`AuthorizationRequestContext` reading the active roles from the
principal. Verified with the module `test` and `spotlessCheck` tasks for
`core`, `server-common`, and `lance-rest-server`.
---
.../java/org/apache/gravitino/UserPrincipal.java | 32 +++++++++
.../authorization/AuthorizationRequestContext.java | 16 ++++-
.../TestAuthorizationRequestContext.java | 17 +++++
.../iceberg/service/IcebergExceptionMapper.java | 1 +
.../service/TestIcebergAuthenticationFilter.java | 20 ++++++
.../lance/service/LanceAuthenticationFilter.java | 7 ++
.../authentication/AuthenticationFilter.java | 20 +++++-
.../authentication/TestAuthenticationFilter.java | 80 ++++++++++++++++++++++
8 files changed, 189 insertions(+), 4 deletions(-)
diff --git a/core/src/main/java/org/apache/gravitino/UserPrincipal.java
b/core/src/main/java/org/apache/gravitino/UserPrincipal.java
index e3980720eb..cb3885a017 100644
--- a/core/src/main/java/org/apache/gravitino/UserPrincipal.java
+++ b/core/src/main/java/org/apache/gravitino/UserPrincipal.java
@@ -27,6 +27,7 @@ import java.util.List;
import java.util.Objects;
import java.util.Optional;
import javax.annotation.Nullable;
+import org.apache.gravitino.auth.ActiveRoles;
/**
* A simple implementation of Principal that holds a username, optional group
membership, and
@@ -38,6 +39,7 @@ public class UserPrincipal implements Principal {
private final String username;
private final List<UserGroup> groups;
@Nullable private final String accessToken;
+ private final ActiveRoles activeRoles;
/**
* Constructs a UserPrincipal with the given username.
@@ -80,6 +82,14 @@ public class UserPrincipal implements Principal {
*/
public UserPrincipal(
final String username, final List<UserGroup> groups, @Nullable final
String accessToken) {
+ this(username, groups, accessToken, ActiveRoles.all());
+ }
+
+ private UserPrincipal(
+ final String username,
+ final List<UserGroup> groups,
+ @Nullable final String accessToken,
+ final ActiveRoles activeRoles) {
Preconditions.checkArgument(username != null, "UserPrincipal must have the
username");
this.username = username;
this.groups =
@@ -87,6 +97,7 @@ public class UserPrincipal implements Principal {
? Collections.unmodifiableList(new ArrayList<>(groups))
: Collections.emptyList();
this.accessToken = accessToken;
+ this.activeRoles = Objects.requireNonNull(activeRoles, "activeRoles must
not be null");
}
/**
@@ -113,6 +124,27 @@ public class UserPrincipal implements Principal {
return groups;
}
+ /**
+ * Returns the roles the caller declared active for this request (role
assumption); {@link
+ * ActiveRoles#all()} when none was declared.
+ *
+ * @return the active-role declaration
+ */
+ public ActiveRoles getActiveRoles() {
+ return activeRoles;
+ }
+
+ /**
+ * Returns a copy of this principal carrying the given active-role
declaration, keeping the same
+ * identity.
+ *
+ * @param activeRoles the active-role declaration to attach
+ * @return a new principal with the same identity and the given active roles
+ */
+ public UserPrincipal withActiveRoles(final ActiveRoles activeRoles) {
+ return new UserPrincipal(username, groups, accessToken, activeRoles);
+ }
+
@Override
public int hashCode() {
return Objects.hash(username, groups);
diff --git
a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationRequestContext.java
b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationRequestContext.java
index d676f101b5..dc30ae58d0 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationRequestContext.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationRequestContext.java
@@ -30,11 +30,13 @@ import lombok.AllArgsConstructor;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.UserPrincipal;
import org.apache.gravitino.auth.ActiveRoles;
import org.apache.gravitino.storage.relational.po.auth.GroupUpdatedAt;
import org.apache.gravitino.storage.relational.po.auth.OwnerInfo;
import org.apache.gravitino.storage.relational.po.auth.RoleUpdatedAt;
import org.apache.gravitino.storage.relational.po.auth.UserUpdatedAt;
+import org.apache.gravitino.utils.PrincipalUtils;
/**
* Per-HTTP-request scratchpad shared by {@link GravitinoAuthorizer} calls. A
fresh instance is
@@ -86,10 +88,18 @@ public class AuthorizationRequestContext {
private volatile String originalAuthorizationExpression;
/**
- * The roles the caller has declared active for this request (role
assumption). Defaults to {@link
- * ActiveRoles#all()}, which evaluates every role the caller holds (no
narrowing).
+ * The roles the caller has declared active for this request (role
assumption). Read from the
+ * current {@link UserPrincipal}; defaults to {@link ActiveRoles#all()} (no
narrowing) when the
+ * caller declared none.
*/
- private volatile ActiveRoles activeRoles = ActiveRoles.all();
+ private volatile ActiveRoles activeRoles = currentPrincipalActiveRoles();
+
+ private static ActiveRoles currentPrincipalActiveRoles() {
+ Principal principal = PrincipalUtils.getCurrentPrincipal();
+ return principal instanceof UserPrincipal
+ ? ((UserPrincipal) principal).getActiveRoles()
+ : ActiveRoles.all();
+ }
/**
* check allow
diff --git
a/core/src/test/java/org/apache/gravitino/authorization/TestAuthorizationRequestContext.java
b/core/src/test/java/org/apache/gravitino/authorization/TestAuthorizationRequestContext.java
index e4ef672bcc..313d490eac 100644
---
a/core/src/test/java/org/apache/gravitino/authorization/TestAuthorizationRequestContext.java
+++
b/core/src/test/java/org/apache/gravitino/authorization/TestAuthorizationRequestContext.java
@@ -23,13 +23,17 @@ import static
org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.util.Arrays;
import java.util.Optional;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.gravitino.UserPrincipal;
+import org.apache.gravitino.auth.ActiveRoles;
import org.apache.gravitino.storage.relational.po.auth.GroupUpdatedAt;
import org.apache.gravitino.storage.relational.po.auth.OwnerInfo;
import org.apache.gravitino.storage.relational.po.auth.UserUpdatedAt;
+import org.apache.gravitino.utils.PrincipalUtils;
import org.junit.jupiter.api.Test;
public class TestAuthorizationRequestContext {
@@ -327,4 +331,17 @@ public class TestAuthorizationRequestContext {
context.setOriginalAuthorizationExpression("OWNER && HAS_PRIVILEGE");
assertEquals("OWNER && HAS_PRIVILEGE",
context.getOriginalAuthorizationExpression());
}
+
+ @Test
+ public void testActiveRolesInitializedFromPrincipal() throws Exception {
+ // With no active roles on the current principal, a new context defaults
to ALL (no narrowing).
+ assertEquals(ActiveRoles.all(), new
AuthorizationRequestContext().getActiveRoles());
+
+ // A new context picks up the active roles carried by the current
UserPrincipal.
+ ActiveRoles named = ActiveRoles.of(Arrays.asList("analyst"));
+ UserPrincipal principal = new
UserPrincipal("tester").withActiveRoles(named);
+ ActiveRoles seen =
+ PrincipalUtils.doAs(principal, () -> new
AuthorizationRequestContext().getActiveRoles());
+ assertEquals(named, seen);
+ }
}
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergExceptionMapper.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergExceptionMapper.java
index daaf1db3aa..dd00b9c4b6 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergExceptionMapper.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergExceptionMapper.java
@@ -57,6 +57,7 @@ public class IcebergExceptionMapper implements
ExceptionMapper<Exception> {
private static final Map<Class<? extends Exception>, Integer>
EXCEPTION_ERROR_CODES =
ImmutableMap.<Class<? extends Exception>, Integer>builder()
.put(IllegalArgumentException.class, 400)
+ .put(BadRequestException.class, 400)
.put(ValidationException.class, 400)
.put(IllegalNameIdentifierException.class, 400)
.put(NamespaceNotEmptyException.class, 409)
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestIcebergAuthenticationFilter.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestIcebergAuthenticationFilter.java
index 8f73cdf4d0..4dbd5e1292 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestIcebergAuthenticationFilter.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestIcebergAuthenticationFilter.java
@@ -28,6 +28,7 @@ import java.io.StringWriter;
import javax.servlet.ServletRequest;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import org.apache.gravitino.auth.IllegalActiveRolesException;
import org.apache.gravitino.exceptions.TokenExpiredException;
import org.apache.gravitino.exceptions.UnauthorizedException;
import org.apache.iceberg.rest.responses.ErrorResponse;
@@ -107,6 +108,25 @@ public class TestIcebergAuthenticationFilter {
Assertions.assertEquals("The provided credentials did not support",
errorResponse.message());
}
+ @Test
+ public void testIllegalActiveRolesReturnsBadRequest() throws Exception {
+ IcebergAuthenticationFilter filter = new IcebergAuthenticationFilter();
+
+ HttpServletResponse response = mock(HttpServletResponse.class);
+ StringWriter stringWriter = new StringWriter();
+ PrintWriter printWriter = new PrintWriter(stringWriter);
+ when(response.getWriter()).thenReturn(printWriter);
+
+ filter.sendAuthErrorResponse(
+ response, new IllegalActiveRolesException("malformed active-roles
header"));
+
+ verify(response).setStatus(HttpServletResponse.SC_BAD_REQUEST);
+
+ printWriter.flush();
+ ErrorResponse errorResponse = MAPPER.readValue(stringWriter.toString(),
ErrorResponse.class);
+ Assertions.assertEquals(400, errorResponse.code());
+ }
+
@Test
public void testInternalServerErrorReturnsJson() throws Exception {
IcebergAuthenticationFilter filter = new IcebergAuthenticationFilter();
diff --git
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceAuthenticationFilter.java
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceAuthenticationFilter.java
index 51aea85524..45cebe3852 100644
---
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceAuthenticationFilter.java
+++
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceAuthenticationFilter.java
@@ -22,6 +22,7 @@ import com.fasterxml.jackson.databind.ObjectMapper;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import javax.servlet.http.HttpServletResponse;
+import org.apache.gravitino.auth.IllegalActiveRolesException;
import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.exceptions.UnauthorizedException;
import org.apache.gravitino.server.authentication.AuthenticationFilter;
@@ -66,6 +67,12 @@ public class LanceAuthenticationFilter extends
AuthenticationFilter {
if (message == null || message.isEmpty()) {
message = "Access denied";
}
+ } else if (exception instanceof IllegalActiveRolesException) {
+ status = HttpServletResponse.SC_BAD_REQUEST;
+ message = exception.getMessage();
+ if (message == null || message.isEmpty()) {
+ message = "Bad request";
+ }
} else {
status = HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
LOG.error("Authentication failure", exception);
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authentication/AuthenticationFilter.java
b/server-common/src/main/java/org/apache/gravitino/server/authentication/AuthenticationFilter.java
index e8b100af22..8fa379922b 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authentication/AuthenticationFilter.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authentication/AuthenticationFilter.java
@@ -32,7 +32,11 @@ import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import org.apache.gravitino.UserPrincipal;
+import org.apache.gravitino.auth.ActiveRoles;
+import org.apache.gravitino.auth.ActiveRolesParser;
import org.apache.gravitino.auth.AuthConstants;
+import org.apache.gravitino.auth.IllegalActiveRolesException;
import org.apache.gravitino.dto.responses.ErrorResponse;
import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.exceptions.UnauthorizedException;
@@ -92,7 +96,6 @@ public class AuthenticationFilter implements Filter {
if (authenticator.supportsToken(authData) &&
authenticator.isDataFromToken()) {
principal = authenticator.authenticateToken(authData);
if (principal != null) {
-
request.setAttribute(AuthConstants.AUTHENTICATED_PRINCIPAL_ATTRIBUTE_NAME,
principal);
break;
}
}
@@ -100,6 +103,16 @@ public class AuthenticationFilter implements Filter {
if (principal == null) {
throw new UnauthorizedException("The provided credentials did not
support");
}
+ // Role assumption: parse the header (syntactic only; malformed -> 400)
and, only when
+ // narrowed, attach the roles to the principal. Membership 403 is
checked later.
+ ActiveRoles activeRoles =
+
ActiveRolesParser.parse(req.getHeader(AuthConstants.X_GRAVITINO_ACTIVE_ROLES_HEADER));
+ if (!activeRoles.isAll() && principal instanceof UserPrincipal) {
+ principal = ((UserPrincipal) principal).withActiveRoles(activeRoles);
+ }
+ // Publish the finalized principal (already carrying any narrowed roles)
so downstream
+ // re-binds from the attribute (e.g. Utils.doAs) see the same identity
and roles.
+
request.setAttribute(AuthConstants.AUTHENTICATED_PRINCIPAL_ATTRIBUTE_NAME,
principal);
PrincipalUtils.doAs(
principal,
() -> {
@@ -146,6 +159,11 @@ public class AuthenticationFilter implements Filter {
} else if (exception instanceof ForbiddenException) {
httpStatus = HttpServletResponse.SC_FORBIDDEN;
errorResponse = ErrorResponse.forbidden(exception.getMessage(),
exception);
+ } else if (exception instanceof IllegalActiveRolesException) {
+ httpStatus = HttpServletResponse.SC_BAD_REQUEST;
+ errorResponse =
+ ErrorResponse.illegalArguments(
+ exception.getClass().getSimpleName(), exception.getMessage(),
exception);
} else {
httpStatus = HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
errorResponse = ErrorResponse.internalError(exception.getMessage(),
exception);
diff --git
a/server-common/src/test/java/org/apache/gravitino/server/authentication/TestAuthenticationFilter.java
b/server-common/src/test/java/org/apache/gravitino/server/authentication/TestAuthenticationFilter.java
index fa8a01bc40..8af1b58fed 100644
---
a/server-common/src/test/java/org/apache/gravitino/server/authentication/TestAuthenticationFilter.java
+++
b/server-common/src/test/java/org/apache/gravitino/server/authentication/TestAuthenticationFilter.java
@@ -32,18 +32,23 @@ import com.google.common.collect.Lists;
import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringWriter;
+import java.security.Principal;
+import java.util.Arrays;
import java.util.Collections;
import java.util.Vector;
+import java.util.concurrent.atomic.AtomicReference;
import javax.servlet.FilterChain;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.gravitino.UserPrincipal;
+import org.apache.gravitino.auth.ActiveRoles;
import org.apache.gravitino.auth.AuthConstants;
import org.apache.gravitino.dto.responses.ErrorResponse;
import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.exceptions.UnauthorizedException;
import org.apache.gravitino.server.web.ObjectMapperProvider;
+import org.apache.gravitino.utils.PrincipalUtils;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
@@ -66,6 +71,81 @@ public class TestAuthenticationFilter {
verify(mockResponse, never()).sendError(anyInt(), anyString());
}
+ @Test
+ public void testDoFilterSetsActiveRolesFromHeader() throws ServletException,
IOException {
+ Authenticator authenticator = mock(Authenticator.class);
+ AuthenticationFilter filter = new
AuthenticationFilter(Lists.newArrayList(authenticator));
+ HttpServletRequest mockRequest = mock(HttpServletRequest.class);
+ HttpServletResponse mockResponse = mock(HttpServletResponse.class);
+ when(mockRequest.getHeaders(AuthConstants.HTTP_HEADER_AUTHORIZATION))
+ .thenReturn(new
Vector<>(Collections.singletonList("user")).elements());
+ when(mockRequest.getHeader(AuthConstants.X_GRAVITINO_ACTIVE_ROLES_HEADER))
+ .thenReturn("analyst,reader");
+ when(authenticator.supportsToken(any())).thenReturn(true);
+ when(authenticator.isDataFromToken()).thenReturn(true);
+ when(authenticator.authenticateToken(any())).thenReturn(new
UserPrincipal("user"));
+
+ // The active roles must be visible on the principal to the downstream
chain (where
+ // authorization runs).
+ AtomicReference<Principal> seenDuringChain = new AtomicReference<>();
+ FilterChain capturingChain =
+ (req, resp) ->
seenDuringChain.set(PrincipalUtils.getCurrentPrincipal());
+ filter.doFilter(mockRequest, mockResponse, capturingChain);
+
+ Assertions.assertInstanceOf(UserPrincipal.class, seenDuringChain.get());
+ Assertions.assertEquals(
+ ActiveRoles.of(Arrays.asList("analyst", "reader")),
+ ((UserPrincipal) seenDuringChain.get()).getActiveRoles());
+ }
+
+ @Test
+ public void testDoFilterDefaultsToAllWhenHeaderAbsent() throws
ServletException, IOException {
+ Authenticator authenticator = mock(Authenticator.class);
+ AuthenticationFilter filter = new
AuthenticationFilter(Lists.newArrayList(authenticator));
+ HttpServletRequest mockRequest = mock(HttpServletRequest.class);
+ HttpServletResponse mockResponse = mock(HttpServletResponse.class);
+ when(mockRequest.getHeaders(AuthConstants.HTTP_HEADER_AUTHORIZATION))
+ .thenReturn(new
Vector<>(Collections.singletonList("user")).elements());
+ when(authenticator.supportsToken(any())).thenReturn(true);
+ when(authenticator.isDataFromToken()).thenReturn(true);
+ when(authenticator.authenticateToken(any())).thenReturn(new
UserPrincipal("user"));
+
+ AtomicReference<Principal> seenDuringChain = new AtomicReference<>();
+ FilterChain capturingChain =
+ (req, resp) ->
seenDuringChain.set(PrincipalUtils.getCurrentPrincipal());
+ filter.doFilter(mockRequest, mockResponse, capturingChain);
+
+ // No header means today's behavior: every role the caller holds is active.
+ Assertions.assertInstanceOf(UserPrincipal.class, seenDuringChain.get());
+ Assertions.assertEquals(
+ ActiveRoles.all(), ((UserPrincipal)
seenDuringChain.get()).getActiveRoles());
+ }
+
+ @Test
+ public void testDoFilterRejectsMalformedActiveRolesHeader() throws
ServletException, IOException {
+ Authenticator authenticator = mock(Authenticator.class);
+ AuthenticationFilter filter = new
AuthenticationFilter(Lists.newArrayList(authenticator));
+ FilterChain mockChain = mock(FilterChain.class);
+ HttpServletRequest mockRequest = mock(HttpServletRequest.class);
+ HttpServletResponse mockResponse = mock(HttpServletResponse.class);
+ StringWriter stringWriter = new StringWriter();
+ PrintWriter printWriter = new PrintWriter(stringWriter);
+ when(mockResponse.getWriter()).thenReturn(printWriter);
+ when(mockRequest.getHeaders(AuthConstants.HTTP_HEADER_AUTHORIZATION))
+ .thenReturn(new
Vector<>(Collections.singletonList("user")).elements());
+ // A reserved keyword combined with a role name is syntactically invalid.
+ when(mockRequest.getHeader(AuthConstants.X_GRAVITINO_ACTIVE_ROLES_HEADER))
+ .thenReturn("ALL,analyst");
+ when(authenticator.supportsToken(any())).thenReturn(true);
+ when(authenticator.isDataFromToken()).thenReturn(true);
+ when(authenticator.authenticateToken(any())).thenReturn(new
UserPrincipal("user"));
+
+ filter.doFilter(mockRequest, mockResponse, mockChain);
+
+ verify(mockResponse).setStatus(HttpServletResponse.SC_BAD_REQUEST);
+ verify(mockChain, never()).doFilter(any(), any());
+ }
+
@Test
public void testDoFilterWithException() throws ServletException, IOException
{
Authenticator authenticator = mock(Authenticator.class);
