This is an automated email from the ASF dual-hosted git repository.
jerryshao 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 58ef9fd6ff [#14571][#14579] docs/fix(idp): Document 401/406; reject
null IdP bodies (#14572)
58ef9fd6ff is described below
commit 58ef9fd6ff7570af933486f5302098f10e36632b
Author: MaSai <[email protected]>
AuthorDate: Tue Nov 3 22:25:59 2026 +0800
[#14571][#14579] docs/fix(idp): Document 401/406; reject null IdP bodies
(#14572)
### What changes were proposed in this pull request?
- Document **401** and **406** on all nine built-in IdP operations in
`docs/open-api/idp/idp.yaml`.
- Narrow `IdpForbiddenErrorResponse` so it no longer claims
unauthenticated callers receive 403.
- Add Unauthorized / unsupported-API-version (406) response examples
aligned with `AuthenticationFilter` and `VersioningFilter`.
- Align the Forbidden example message with
`IdpAuthorizationFilter.SERVICE_ADMIN_REQUIRED_MESSAGE`.
- Return **400** (`Request body cannot be null`) for null JSON bodies on
IdP user/group write APIs instead of NPE **500**.
### Why are the changes needed?
Missing or invalid Basic credentials return 401. An **unsupported**
Gravitino API version in `Accept` (for example
`application/vnd.gravitino.v99+json`) returns 406 with `code` 1001 /
`IllegalArgumentException`. Missing Accept, `*/*`, or `application/json`
alone are rewritten to the default v1 media type and do not return 406.
The IdP OpenAPI only listed 403 and said unauthenticated requests
receive Forbidden, which does not match the server.
Null / empty JSON bodies on IdP write endpoints previously caused
`NullPointerException` (HTTP 500); metalake user APIs already return 400
for the same case.
Fixes #14571
Fixes #14579
### Does this PR introduce _any_ user-facing change?
- Docs / OpenAPI contract updates for 401/406/Forbidden wording.
- IdP write APIs now return 400 instead of 500 when the request body is
null.
### How was this patch tested?
- `./gradlew :docs:build`
- `./gradlew :plugins:idp-basic:test --tests
org.apache.gravitino.idp.web.rest.TestIdpOperations` (12 tests,
including `testNullRequestBodyReturnsBadRequest`)
- Checked `docs/open-api/idp/idp.yaml` against `VersioningFilter` /
`JsonErrorHandlerIT` and `IdpAuthorizationFilter`
Made with [Cursor](https://cursor.com)
---------
Co-authored-by: Cursor <[email protected]>
---
docs/open-api/idp/idp.yaml | 79 +++++++++++++++++++++-
.../org/apache/gravitino/idp/web/IdpRESTUtils.java | 17 +++++
.../gravitino/idp/web/rest/IdpGroupOperations.java | 8 +++
.../gravitino/idp/web/rest/IdpUserOperations.java | 12 ++++
.../gravitino/idp/web/rest/TestIdpOperations.java | 24 +++++++
5 files changed, 138 insertions(+), 2 deletions(-)
diff --git a/docs/open-api/idp/idp.yaml b/docs/open-api/idp/idp.yaml
index a3db6a9514..9941b58c19 100644
--- a/docs/open-api/idp/idp.yaml
+++ b/docs/open-api/idp/idp.yaml
@@ -67,8 +67,12 @@ paths:
examples:
AlreadyExistsException:
$ref: "#/components/examples/IdpAlreadyExistsException"
+ "401":
+ $ref: "#/components/responses/IdpUnauthorizedErrorResponse"
"403":
$ref: "#/components/responses/IdpForbiddenErrorResponse"
+ "406":
+ $ref: "#/components/responses/IdpNotAcceptableErrorResponse"
"5xx":
$ref: "../openapi.yaml#/components/responses/ServerErrorResponse"
@@ -95,8 +99,12 @@ paths:
examples:
IdpUserGetResponse:
$ref: "#/components/examples/IdpUserGetResponse"
+ "401":
+ $ref: "#/components/responses/IdpUnauthorizedErrorResponse"
"403":
$ref: "#/components/responses/IdpForbiddenErrorResponse"
+ "406":
+ $ref: "#/components/responses/IdpNotAcceptableErrorResponse"
"404":
description: Not Found - The specified local user store user does
not exist
content:
@@ -118,8 +126,12 @@ paths:
responses:
"200":
$ref: "../openapi.yaml#/components/responses/RemoveResponse"
+ "401":
+ $ref: "#/components/responses/IdpUnauthorizedErrorResponse"
"403":
$ref: "#/components/responses/IdpForbiddenErrorResponse"
+ "406":
+ $ref: "#/components/responses/IdpNotAcceptableErrorResponse"
"5xx":
$ref: "../openapi.yaml#/components/responses/ServerErrorResponse"
@@ -168,8 +180,12 @@ paths:
examples:
IllegalArgumentException:
$ref:
"#/components/examples/IdpChangePasswordIllegalArgumentException"
+ "401":
+ $ref: "#/components/responses/IdpUnauthorizedErrorResponse"
"403":
$ref: "#/components/responses/IdpForbiddenErrorResponse"
+ "406":
+ $ref: "#/components/responses/IdpNotAcceptableErrorResponse"
"404":
description: Not Found - The specified local user store user does
not exist
content:
@@ -223,8 +239,12 @@ paths:
examples:
IllegalArgumentException:
$ref:
"#/components/examples/IdpUpdateEnabledIllegalArgumentException"
+ "401":
+ $ref: "#/components/responses/IdpUnauthorizedErrorResponse"
"403":
$ref: "#/components/responses/IdpForbiddenErrorResponse"
+ "406":
+ $ref: "#/components/responses/IdpNotAcceptableErrorResponse"
"404":
description: Not Found - The specified local user store user does
not exist
content:
@@ -281,8 +301,12 @@ paths:
examples:
AlreadyExistsException:
$ref: "#/components/examples/IdpGroupAlreadyExistsException"
+ "401":
+ $ref: "#/components/responses/IdpUnauthorizedErrorResponse"
"403":
$ref: "#/components/responses/IdpForbiddenErrorResponse"
+ "406":
+ $ref: "#/components/responses/IdpNotAcceptableErrorResponse"
"5xx":
$ref: "../openapi.yaml#/components/responses/ServerErrorResponse"
@@ -306,8 +330,12 @@ paths:
examples:
IdpGroupGetResponse:
$ref: "#/components/examples/IdpGroupGetResponse"
+ "401":
+ $ref: "#/components/responses/IdpUnauthorizedErrorResponse"
"403":
$ref: "#/components/responses/IdpForbiddenErrorResponse"
+ "406":
+ $ref: "#/components/responses/IdpNotAcceptableErrorResponse"
"404":
description: Not Found - The specified local user store group does
not exist
content:
@@ -331,8 +359,12 @@ paths:
responses:
"200":
$ref: "../openapi.yaml#/components/responses/RemoveResponse"
+ "401":
+ $ref: "#/components/responses/IdpUnauthorizedErrorResponse"
"403":
$ref: "#/components/responses/IdpForbiddenErrorResponse"
+ "406":
+ $ref: "#/components/responses/IdpNotAcceptableErrorResponse"
"409":
description: Conflict - The group is not empty and force is false
content:
@@ -383,8 +415,12 @@ paths:
examples:
IllegalArgumentException:
$ref:
"#/components/examples/IdpGroupMembershipIllegalArgumentException"
+ "401":
+ $ref: "#/components/responses/IdpUnauthorizedErrorResponse"
"403":
$ref: "#/components/responses/IdpForbiddenErrorResponse"
+ "406":
+ $ref: "#/components/responses/IdpNotAcceptableErrorResponse"
"404":
description: Not Found - The specified local user store group or
user does not exist
content:
@@ -563,12 +599,23 @@ components:
$ref: "#/components/schemas/IdpGroup"
responses:
+ IdpUnauthorizedErrorResponse:
+ description: Unauthorized - Authentication credentials are missing or
invalid
+ content:
+ application/vnd.gravitino.v1+json:
+ schema:
+ $ref: "../openapi.yaml#/components/schemas/ErrorModel"
+ examples:
+ UnauthorizedException:
+ $ref: "#/components/examples/IdpUnauthorizedException"
+
IdpForbiddenErrorResponse:
description:
Forbidden - Callers listed in `gravitino.authorization.serviceAdmins`
may manage local
user store users and groups. Authenticated non-admin users may only
`GET`
`/idp/users/{user}` or `PUT` `/idp/users/{user}/password` for their
own username.
- Unauthenticated requests and other non-admin IdP management calls
receive this response.
+ Other authenticated non-admin IdP management calls receive this
response. Missing or
+ invalid credentials return 401 instead.
content:
application/vnd.gravitino.v1+json:
schema:
@@ -577,6 +624,20 @@ components:
ForbiddenException:
$ref: "#/components/examples/IdpForbiddenException"
+ IdpNotAcceptableErrorResponse:
+ description: >
+ Not Acceptable - Returned when `Accept` (or a versioned
`Content-Type`) declares an
+ unsupported Gravitino API version, for example
`application/vnd.gravitino.v99+json`.
+ Missing Accept, `*/*`, or `application/json` alone are rewritten to
the default
+ `application/vnd.gravitino.v1+json` and do not produce this response.
+ content:
+ application/vnd.gravitino.v1+json:
+ schema:
+ $ref: "../openapi.yaml#/components/schemas/ErrorModel"
+ examples:
+ UnsupportedApiVersion:
+ $ref: "#/components/examples/IdpUnsupportedApiVersionException"
+
examples:
AddUserRequest:
value: {
@@ -789,9 +850,23 @@ components:
"message": "Failed to operate local user store group [engineers]
operation [REMOVE], reason [IdP group engineers is not empty, use force=true to
delete it]"
}
+ IdpUnauthorizedException:
+ value: {
+ "code": 1011,
+ "type": "UnauthorizedException",
+ "message": "The provided credentials did not support"
+ }
+
IdpForbiddenException:
value: {
"code": 1008,
"type": "ForbiddenException",
- "message": "Only service admins can manage local user store users and
groups."
+ "message": "Only service admins can manage built-in IdP users and
groups."
+ }
+
+ IdpUnsupportedApiVersionException:
+ value: {
+ "code": 1001,
+ "type": "IllegalArgumentException",
+ "message": "Unsupported version v99 in request header"
}
diff --git
a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/IdpRESTUtils.java
b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/IdpRESTUtils.java
index 324bbf2f46..a0672f8492 100644
---
a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/IdpRESTUtils.java
+++
b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/IdpRESTUtils.java
@@ -19,6 +19,7 @@
package org.apache.gravitino.idp.web;
import java.security.PrivilegedExceptionAction;
+import javax.annotation.Nullable;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
@@ -82,6 +83,22 @@ public final class IdpRESTUtils {
return toErrorResponse(errorMsg, e);
}
+ /**
+ * Returns a 400 response when {@code request} is null; otherwise {@code
null}.
+ *
+ * @param request the deserialized request body, which may be null
+ * @return a bad-request response if {@code request} is null, otherwise
{@code null}
+ */
+ @Nullable
+ public static Response rejectNullRequestBody(Object request) {
+ if (request != null) {
+ return null;
+ }
+ LOG.warn("Received IdP request with null request body");
+ return illegalArguments(
+ "Request body cannot be null", new IllegalArgumentException("Request
body cannot be null"));
+ }
+
public static Response illegalArguments(String message, Throwable throwable)
{
return json(
Response.Status.BAD_REQUEST,
diff --git
a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/IdpGroupOperations.java
b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/IdpGroupOperations.java
index 968fc83071..7d94cffea1 100644
---
a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/IdpGroupOperations.java
+++
b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/IdpGroupOperations.java
@@ -77,6 +77,10 @@ public class IdpGroupOperations {
@Timed(name = "add-idp-group." + MetricNames.HTTP_PROCESS_DURATION, absolute
= true)
@ResponseMetered(name = "add-idp-group", absolute = true)
public Response addGroup(AddGroupRequest request) {
+ Response rejection = IdpRESTUtils.rejectNullRequestBody(request);
+ if (rejection != null) {
+ return rejection;
+ }
return IdpRESTUtils.doAs(
httpRequest,
() -> {
@@ -112,6 +116,10 @@ public class IdpGroupOperations {
@ResponseMetered(name = "change-idp-group-users", absolute = true)
public Response changeGroupMembership(
@PathParam("group") String group, GroupMembershipChangeRequest request) {
+ Response rejection = IdpRESTUtils.rejectNullRequestBody(request);
+ if (rejection != null) {
+ return rejection;
+ }
return IdpRESTUtils.doAs(
httpRequest,
() -> {
diff --git
a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/IdpUserOperations.java
b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/IdpUserOperations.java
index 35d994d896..11fd3d84ef 100644
---
a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/IdpUserOperations.java
+++
b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/IdpUserOperations.java
@@ -82,6 +82,10 @@ public class IdpUserOperations {
@Timed(name = "add-idp-user." + MetricNames.HTTP_PROCESS_DURATION, absolute
= true)
@ResponseMetered(name = "add-idp-user", absolute = true)
public Response addUser(AddUserRequest request) {
+ Response rejection = IdpRESTUtils.rejectNullRequestBody(request);
+ if (rejection != null) {
+ return rejection;
+ }
return IdpRESTUtils.doAs(
httpRequest,
() -> {
@@ -111,6 +115,10 @@ public class IdpUserOperations {
@Timed(name = "change-idp-user-password." +
MetricNames.HTTP_PROCESS_DURATION, absolute = true)
@ResponseMetered(name = "change-idp-user-password", absolute = true)
public Response changePassword(@PathParam("user") String user,
ChangePasswordRequest request) {
+ Response rejection = IdpRESTUtils.rejectNullRequestBody(request);
+ if (rejection != null) {
+ return rejection;
+ }
return IdpRESTUtils.doAs(
httpRequest,
() -> {
@@ -156,6 +164,10 @@ public class IdpUserOperations {
@Timed(name = "update-idp-user-enabled." +
MetricNames.HTTP_PROCESS_DURATION, absolute = true)
@ResponseMetered(name = "update-idp-user-enabled", absolute = true)
public Response updateEnabled(@PathParam("user") String user,
UpdateUserEnabledRequest request) {
+ Response rejection = IdpRESTUtils.rejectNullRequestBody(request);
+ if (rejection != null) {
+ return rejection;
+ }
return IdpRESTUtils.doAs(
httpRequest,
() -> {
diff --git
a/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/web/rest/TestIdpOperations.java
b/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/web/rest/TestIdpOperations.java
index 68c56b5c47..267726aaec 100644
---
a/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/web/rest/TestIdpOperations.java
+++
b/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/web/rest/TestIdpOperations.java
@@ -124,6 +124,15 @@ class TestIdpOperations extends JerseyTest {
return resourceConfig;
}
+ @Test
+ void testNullRequestBodyReturnsBadRequest() {
+ assertNullBody(postNull("/idp/users"));
+ assertNullBody(putNull("/idp/users/user1/password"));
+ assertNullBody(putNull("/idp/users/user1/enabled"));
+ assertNullBody(postNull("/idp/groups"));
+ assertNullBody(putNull("/idp/groups/group1/users"));
+ }
+
@Test
void testAddUser() throws Exception {
AddUserRequest req = new AddUserRequest("user1", VALID_PASSWORD);
@@ -295,10 +304,18 @@ class TestIdpOperations extends JerseyTest {
return request(path).post(Entity.entity(body,
MediaType.APPLICATION_JSON_TYPE));
}
+ private Response postNull(String path) {
+ return request(path).post(Entity.entity("null",
MediaType.APPLICATION_JSON_TYPE));
+ }
+
private Response put(String path, Object body) {
return request(path).put(Entity.entity(body,
MediaType.APPLICATION_JSON_TYPE));
}
+ private Response putNull(String path) {
+ return request(path).put(Entity.entity("null",
MediaType.APPLICATION_JSON_TYPE));
+ }
+
private Response delete(String path) {
return request(path).delete();
}
@@ -312,6 +329,13 @@ class TestIdpOperations extends JerseyTest {
Assertions.assertEquals(code,
response.readEntity(ErrorResponse.class).getCode());
}
+ private void assertNullBody(Response response) {
+ assertStatus(Response.Status.BAD_REQUEST, response);
+ ErrorResponse error = response.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
error.getCode());
+ Assertions.assertTrue(error.getMessage().contains("Request body cannot be
null"));
+ }
+
private IdpUser buildUser(String user) {
return new IdpUser(user, Collections.emptyList());
}
