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 b91f65c0e9 [Cherry-pick to branch-1.3] [#14571][#14579] docs/fix(idp):
Document 401/406; reject null IdP bodies (#14572) (#14591)
b91f65c0e9 is described below
commit b91f65c0e94e44183432ccfc90c453edd2c14a6c
Author: MaSai <[email protected]>
AuthorDate: Wed Nov 4 09:29:44 2026 +0800
[Cherry-pick to branch-1.3] [#14571][#14579] docs/fix(idp): Document
401/406; reject null IdP bodies (#14572) (#14591)
### What changes were proposed in this pull request?
Manual conflict-free backport of #14572 / `58ef9fd6ff` onto
`branch-1.3`:
- Document **401** and **406** on the built-in IdP operations that exist
on `branch-1.3`.
- Narrow `IdpForbiddenErrorResponse` so it does not claim
unauthenticated callers receive 403; align the Forbidden example with
`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**.
`branch-1.3` does not include `PUT /idp/users/{user}/enabled`, so that
operation from main is intentionally omitted (that is what conflicted in
#14590).
### Why are the changes needed?
Fixes #14571 and #14579 on the 1.3 line. Supersedes conflicted bot
cherry-pick #14590.
### 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 :plugins:idp-basic:test --tests
org.apache.gravitino.idp.web.rest.TestIdpOperations` (includes
`testNullRequestBodyReturnsBadRequest`)
Co-authored-by: Cursor <[email protected]>
---
docs/open-api/idp/idp.yaml | 75 +++++++++++++++++++++-
.../org/apache/gravitino/idp/web/IdpRESTUtils.java | 17 +++++
.../gravitino/idp/web/rest/IdpGroupOperations.java | 8 +++
.../gravitino/idp/web/rest/IdpUserOperations.java | 8 +++
.../gravitino/idp/web/rest/TestIdpOperations.java | 23 +++++++
5 files changed, 129 insertions(+), 2 deletions(-)
diff --git a/docs/open-api/idp/idp.yaml b/docs/open-api/idp/idp.yaml
index 74e64a395b..c84d61149d 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"
@@ -92,8 +96,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:
@@ -115,8 +123,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"
@@ -165,8 +177,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:
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"
@@ -248,8 +268,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:
@@ -273,8 +297,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:
@@ -325,8 +353,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:
@@ -476,12 +508,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:
@@ -490,6 +533,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: {
@@ -657,9 +714,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 35a2831b5a..8e1be73c67 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;
@@ -81,6 +82,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 f2ad5233d4..e98ffbd656 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,
() -> {
@@ -111,6 +115,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 0335b41cf5..78c6e5dd15 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
@@ -81,6 +81,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,
() -> {
@@ -108,6 +112,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,
() -> {
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 42580f0861..b89b897f4a 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
@@ -120,6 +120,14 @@ class TestIdpOperations extends JerseyTest {
return resourceConfig;
}
+ @Test
+ void testNullRequestBodyReturnsBadRequest() {
+ assertNullBody(postNull("/idp/users"));
+ assertNullBody(putNull("/idp/users/user1/password"));
+ assertNullBody(postNull("/idp/groups"));
+ assertNullBody(putNull("/idp/groups/group1/users"));
+ }
+
@Test
void testAddUser() throws Exception {
AddUserRequest req = new AddUserRequest("user1", VALID_PASSWORD);
@@ -252,10 +260,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();
}
@@ -269,6 +285,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());
}
