This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new 35e5f618df [Cherry-pick to branch-1.3] [#11581] fix: Return JSON
ErrorResponse for 401/403 auth failures in AuthenticationFilter (#11614)
(#11619)
35e5f618df is described below
commit 35e5f618dfdf800685a93b69dbe156258670dd76
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Fri Jun 12 09:14:38 2026 -0700
[Cherry-pick to branch-1.3] [#11581] fix: Return JSON ErrorResponse for
401/403 auth failures in AuthenticationFilter (#11614) (#11619)
**Cherry-pick Information:**
- Original commit: f7489c71953a96531a35ca167f0a488dbda556a2
- Target branch: `branch-1.3`
- Status: ✅ Clean cherry-pick (no conflicts)
Co-authored-by: Akshay Thorat <[email protected]>
---
.../gravitino/dto/responses/ErrorConstants.java | 3 +
.../gravitino/dto/responses/ErrorResponse.java | 13 +++
.../authentication/AuthenticationFilter.java | 36 +++++--
.../authentication/TestAuthenticationFilter.java | 115 ++++++++++++++++++++-
4 files changed, 154 insertions(+), 13 deletions(-)
diff --git
a/common/src/main/java/org/apache/gravitino/dto/responses/ErrorConstants.java
b/common/src/main/java/org/apache/gravitino/dto/responses/ErrorConstants.java
index 4953463d74..870addfba9 100644
---
a/common/src/main/java/org/apache/gravitino/dto/responses/ErrorConstants.java
+++
b/common/src/main/java/org/apache/gravitino/dto/responses/ErrorConstants.java
@@ -54,6 +54,9 @@ public class ErrorConstants {
/** Error codes for drop an in use entity. */
public static final int IN_USE_CODE = 1010;
+ /** Error codes for unauthorized access. */
+ public static final int UNAUTHORIZED_CODE = 1011;
+
/** Error codes for invalid state. */
public static final int UNKNOWN_ERROR_CODE = 1100;
diff --git
a/common/src/main/java/org/apache/gravitino/dto/responses/ErrorResponse.java
b/common/src/main/java/org/apache/gravitino/dto/responses/ErrorResponse.java
index 10b510a581..a47d4cbb80 100644
--- a/common/src/main/java/org/apache/gravitino/dto/responses/ErrorResponse.java
+++ b/common/src/main/java/org/apache/gravitino/dto/responses/ErrorResponse.java
@@ -348,6 +348,19 @@ public class ErrorResponse extends BaseResponse {
getStackTrace(throwable));
}
+ /**
+ * Create a new unauthorized error instance of {@link ErrorResponse}.
+ *
+ * @param type The type of the error.
+ * @param message The message of the error.
+ * @param throwable The throwable that caused the error.
+ * @return The new instance.
+ */
+ public static ErrorResponse unauthorized(String type, String message,
Throwable throwable) {
+ return new ErrorResponse(
+ ErrorConstants.UNAUTHORIZED_CODE, type, message,
getStackTrace(throwable));
+ }
+
private static List<String> getStackTrace(Throwable throwable) {
if (throwable == null) {
return null;
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 f796a107e0..e2a1ac9245 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
@@ -33,7 +33,10 @@ import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
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;
public class AuthenticationFilter implements Filter {
@@ -114,22 +117,35 @@ public class AuthenticationFilter implements Filter {
}
/**
- * Sends an error response when authentication fails. Subclasses can
override this to customize
- * the error response format (e.g., Iceberg REST server returns JSON error
bodies).
- *
- * <p>TODO: Gravitino server should override this method to return a correct
JSON response
- * following the Gravitino error response spec.
+ * Sends a JSON error response when authentication fails. Subclasses can
override this to
+ * customize the error response format (e.g., Iceberg REST server returns
Iceberg-specific JSON
+ * error bodies).
*
* @param response the HTTP servlet response
* @param exception the authentication exception
*/
protected void sendAuthErrorResponse(HttpServletResponse response, Exception
exception)
throws IOException {
- int status =
- exception instanceof UnauthorizedException
- ? HttpServletResponse.SC_UNAUTHORIZED
- : HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
- response.sendError(status, exception.getMessage());
+ int httpStatus;
+ ErrorResponse errorResponse;
+
+ if (exception instanceof UnauthorizedException) {
+ httpStatus = HttpServletResponse.SC_UNAUTHORIZED;
+ errorResponse =
+ ErrorResponse.unauthorized(
+ exception.getClass().getSimpleName(), exception.getMessage(),
exception);
+ } else if (exception instanceof ForbiddenException) {
+ httpStatus = HttpServletResponse.SC_FORBIDDEN;
+ errorResponse = ErrorResponse.forbidden(exception.getMessage(),
exception);
+ } else {
+ httpStatus = HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
+ errorResponse = ErrorResponse.internalError(exception.getMessage(),
exception);
+ }
+
+ response.setStatus(httpStatus);
+ response.setContentType("application/json");
+ response.setCharacterEncoding(StandardCharsets.UTF_8.name());
+ ObjectMapperProvider.objectMapper().writeValue(response.getWriter(),
errorResponse);
}
/**
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 8c227727b4..fa8a01bc40 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
@@ -27,8 +27,11 @@ import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
+import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.Lists;
import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
import java.util.Collections;
import java.util.Vector;
import javax.servlet.FilterChain;
@@ -37,7 +40,11 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.gravitino.UserPrincipal;
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.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
public class TestAuthenticationFilter {
@@ -66,6 +73,9 @@ public class TestAuthenticationFilter {
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());
when(authenticator.supportsToken(any())).thenReturn(true);
@@ -73,7 +83,17 @@ public class TestAuthenticationFilter {
when(authenticator.authenticateToken(any()))
.thenThrow(new UnauthorizedException("UNAUTHORIZED"));
filter.doFilter(mockRequest, mockResponse, mockChain);
- verify(mockResponse).sendError(HttpServletResponse.SC_UNAUTHORIZED,
"UNAUTHORIZED");
+ verify(mockResponse).setStatus(HttpServletResponse.SC_UNAUTHORIZED);
+ verify(mockResponse).setContentType("application/json");
+ verify(mockResponse).setCharacterEncoding("UTF-8");
+
+ printWriter.flush();
+ String json = stringWriter.toString();
+ ObjectMapper mapper = ObjectMapperProvider.objectMapper();
+ ErrorResponse errorResponse = mapper.readValue(json, ErrorResponse.class);
+ Assertions.assertEquals(1011, errorResponse.getCode());
+ Assertions.assertEquals("UnauthorizedException", errorResponse.getType());
+ Assertions.assertEquals("UNAUTHORIZED", errorResponse.getMessage());
}
@Test
@@ -109,6 +129,9 @@ public class TestAuthenticationFilter {
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());
when(authenticator1.supportsToken(any())).thenReturn(false);
@@ -120,7 +143,16 @@ public class TestAuthenticationFilter {
.thenThrow(new UnauthorizedException("UNAUTHORIZED"));
filter.doFilter(mockRequest, mockResponse, mockChain);
- verify(mockResponse).sendError(HttpServletResponse.SC_UNAUTHORIZED,
"UNAUTHORIZED");
+ verify(mockResponse).setStatus(HttpServletResponse.SC_UNAUTHORIZED);
+ verify(mockResponse).setContentType("application/json");
+
+ printWriter.flush();
+ String json = stringWriter.toString();
+ ObjectMapper mapper = ObjectMapperProvider.objectMapper();
+ ErrorResponse errorResponse = mapper.readValue(json, ErrorResponse.class);
+ Assertions.assertEquals(1011, errorResponse.getCode());
+ Assertions.assertEquals("UnauthorizedException", errorResponse.getType());
+ Assertions.assertEquals("UNAUTHORIZED", errorResponse.getMessage());
}
@Test
@@ -169,6 +201,9 @@ public class TestAuthenticationFilter {
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.getRequestURI()).thenReturn(path);
when(mockRequest.getHeaders(AuthConstants.HTTP_HEADER_AUTHORIZATION))
.thenReturn(new
Vector<>(Collections.singletonList("user")).elements());
@@ -180,7 +215,81 @@ public class TestAuthenticationFilter {
filter.doFilter(mockRequest, mockResponse, mockChain);
// Auth flow ran and rejected — proves these paths are not exempted.
- verify(mockResponse).sendError(HttpServletResponse.SC_UNAUTHORIZED,
"UNAUTHORIZED");
+ verify(mockResponse).setStatus(HttpServletResponse.SC_UNAUTHORIZED);
+ verify(mockResponse).setContentType("application/json");
}
}
+
+ @Test
+ public void testUnauthorizedErrorReturnsJsonBody() throws Exception {
+ AuthenticationFilter filter = new
AuthenticationFilter(Lists.newArrayList());
+
+ HttpServletResponse response = mock(HttpServletResponse.class);
+ StringWriter stringWriter = new StringWriter();
+ PrintWriter printWriter = new PrintWriter(stringWriter);
+ when(response.getWriter()).thenReturn(printWriter);
+
+ filter.sendAuthErrorResponse(
+ response, new UnauthorizedException("The provided credentials did not
support"));
+
+ verify(response).setStatus(HttpServletResponse.SC_UNAUTHORIZED);
+ verify(response).setContentType("application/json");
+ verify(response).setCharacterEncoding("UTF-8");
+
+ printWriter.flush();
+ String json = stringWriter.toString();
+ ObjectMapper mapper = ObjectMapperProvider.objectMapper();
+ ErrorResponse errorResponse = mapper.readValue(json, ErrorResponse.class);
+ Assertions.assertEquals(1011, errorResponse.getCode());
+ Assertions.assertEquals("UnauthorizedException", errorResponse.getType());
+ Assertions.assertEquals("The provided credentials did not support",
errorResponse.getMessage());
+ }
+
+ @Test
+ public void testForbiddenErrorReturnsJsonBody() throws Exception {
+ AuthenticationFilter filter = new
AuthenticationFilter(Lists.newArrayList());
+
+ HttpServletResponse response = mock(HttpServletResponse.class);
+ StringWriter stringWriter = new StringWriter();
+ PrintWriter printWriter = new PrintWriter(stringWriter);
+ when(response.getWriter()).thenReturn(printWriter);
+
+ filter.sendAuthErrorResponse(response, new ForbiddenException("Access
denied"));
+
+ verify(response).setStatus(HttpServletResponse.SC_FORBIDDEN);
+ verify(response).setContentType("application/json");
+ verify(response).setCharacterEncoding("UTF-8");
+
+ printWriter.flush();
+ String json = stringWriter.toString();
+ ObjectMapper mapper = ObjectMapperProvider.objectMapper();
+ ErrorResponse errorResponse = mapper.readValue(json, ErrorResponse.class);
+ Assertions.assertEquals(1008, errorResponse.getCode());
+ Assertions.assertEquals("ForbiddenException", errorResponse.getType());
+ Assertions.assertEquals("Access denied", errorResponse.getMessage());
+ }
+
+ @Test
+ public void testInternalServerErrorReturnsJsonBody() throws Exception {
+ AuthenticationFilter filter = new
AuthenticationFilter(Lists.newArrayList());
+
+ HttpServletResponse response = mock(HttpServletResponse.class);
+ StringWriter stringWriter = new StringWriter();
+ PrintWriter printWriter = new PrintWriter(stringWriter);
+ when(response.getWriter()).thenReturn(printWriter);
+
+ filter.sendAuthErrorResponse(response, new RuntimeException("Something
went wrong"));
+
+ verify(response).setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ verify(response).setContentType("application/json");
+ verify(response).setCharacterEncoding("UTF-8");
+
+ printWriter.flush();
+ String json = stringWriter.toString();
+ ObjectMapper mapper = ObjectMapperProvider.objectMapper();
+ ErrorResponse errorResponse = mapper.readValue(json, ErrorResponse.class);
+ Assertions.assertEquals(1002, errorResponse.getCode());
+ Assertions.assertEquals("RuntimeException", errorResponse.getType());
+ Assertions.assertEquals("Something went wrong",
errorResponse.getMessage());
+ }
}
