[#10966] feat(authn): Add built-in IdP model by lasdf1234 · Pull Request #11022 · apache/gravitino

lasdf1234 · 2026-05-10T10:34:04Z

What changes were proposed in this pull request?

This PR adds the built-in IdP API model classes under the plugins:idp-basic module, including DTOs, requests, responses, and their unit tests.

Why are the changes needed?

The built-in IdP management work needs its API model classes to live with the idp-basic plugin instead of introducing them in broader shared modules.

Fix: #10966

Does this PR introduce any user-facing change?

No.

How was this patch tested?

./gradlew :plugins:idp-basic:test \
  --tests org.apache.gravitino.idp.basic.dto.TestIdpUserDTO \
  --tests org.apache.gravitino.idp.basic.dto.TestIdpGroupDTO \
  --tests org.apache.gravitino.idp.basic.dto.requests.TestCreateUserRequest \
  --tests org.apache.gravitino.idp.basic.dto.requests.TestCreateGroupRequest \
  --tests org.apache.gravitino.idp.basic.dto.requests.TestResetPasswordRequest \
  --tests org.apache.gravitino.idp.basic.dto.requests.TestUpdateGroupUsersRequest \
  --tests org.apache.gravitino.idp.basic.dto.responses.TestIdpUserResponse \
  --tests org.apache.gravitino.idp.basic.dto.responses.TestIdpGroupResponse \
  -PskipWeb=true

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR introduces the API model layer for the built-in IdP feature within the plugins:idp-basic module, including DTOs, REST request/response objects, and focused unit tests to validate JSON (de)serialization and input validation behavior.

Changes:

Added built-in IdP DTOs for users and groups (IdpUserDTO, IdpGroupDTO).
Added request/response model classes for user/group operations (create user/group, reset password, update group users).
Added unit tests covering SerDe, validation, and ensuring password fields are not exposed via toString(); updated module dependencies accordingly.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
plugins/idp-basic/src/main/java/org/apache/gravitino/idp/basic/dto/IdpUserDTO.java	Adds user DTO with builder and JSON annotations.
plugins/idp-basic/src/main/java/org/apache/gravitino/idp/basic/dto/IdpGroupDTO.java	Adds group DTO with builder and JSON annotations.
plugins/idp-basic/src/main/java/org/apache/gravitino/idp/basic/dto/requests/CreateUserRequest.java	Adds create-user request model with validation and password redaction in `toString()`.
plugins/idp-basic/src/main/java/org/apache/gravitino/idp/basic/dto/requests/CreateGroupRequest.java	Adds create-group request model with validation.
plugins/idp-basic/src/main/java/org/apache/gravitino/idp/basic/dto/requests/ResetPasswordRequest.java	Adds reset-password request model with validation and password redaction in `toString()`.
plugins/idp-basic/src/main/java/org/apache/gravitino/idp/basic/dto/requests/UpdateGroupUsersRequest.java	Adds group-membership update request model with validation.
plugins/idp-basic/src/main/java/org/apache/gravitino/idp/basic/dto/responses/IdpUserResponse.java	Adds user response wrapper with validation.
plugins/idp-basic/src/main/java/org/apache/gravitino/idp/basic/dto/responses/IdpGroupResponse.java	Adds group response wrapper with validation.
plugins/idp-basic/src/test/java/org/apache/gravitino/idp/basic/dto/TestIdpUserDTO.java	Tests user DTO JSON SerDe and builder validation.
plugins/idp-basic/src/test/java/org/apache/gravitino/idp/basic/dto/TestIdpGroupDTO.java	Tests group DTO JSON SerDe and builder validation.
plugins/idp-basic/src/test/java/org/apache/gravitino/idp/basic/dto/requests/TestCreateUserRequest.java	Tests create-user request SerDe/validate and password non-exposure via `toString()`.
plugins/idp-basic/src/test/java/org/apache/gravitino/idp/basic/dto/requests/TestCreateGroupRequest.java	Tests create-group request SerDe and validation.
plugins/idp-basic/src/test/java/org/apache/gravitino/idp/basic/dto/requests/TestResetPasswordRequest.java	Tests reset-password request SerDe/validate and password non-exposure via `toString()`.
plugins/idp-basic/src/test/java/org/apache/gravitino/idp/basic/dto/requests/TestUpdateGroupUsersRequest.java	Tests update-group-users request SerDe and validation.
plugins/idp-basic/src/test/java/org/apache/gravitino/idp/basic/dto/responses/TestIdpUserResponse.java	Tests user response SerDe and validation error messaging.
plugins/idp-basic/src/test/java/org/apache/gravitino/idp/basic/dto/responses/TestIdpGroupResponse.java	Tests group response SerDe and validation error messaging.
plugins/idp-basic/build.gradle.kts	Adds required dependencies (common module, Jackson, Lombok) to support new API models and tests.

Copilot

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

github-actions · 2026-05-10T15:16:33Z

Code Coverage Report

Overall Project	66.03% `+0.05%`	🟢
Files changed	96.75%	🟢

Module	Coverage
aliyun	1.73%	🔴
api	47.13%	🟢
authorization-common	85.96%	🟢
aws	1.1%	🔴
azure	2.6%	🔴
catalog-common	10.2%	🔴
catalog-fileset	80.02%	🟢
catalog-glue	82.47%	🟢
catalog-hive	81.83%	🟢
catalog-jdbc-clickhouse	79.18%	🟢
catalog-jdbc-common	43.93%	🟢
catalog-jdbc-doris	80.28%	🟢
catalog-jdbc-hologres	54.03%	🟢
catalog-jdbc-mysql	79.23%	🟢
catalog-jdbc-oceanbase	78.38%	🟢
catalog-jdbc-postgresql	82.05%	🟢
catalog-jdbc-starrocks	78.27%	🟢
catalog-kafka	77.01%	🟢
catalog-lakehouse-generic	45.14%	🟢
catalog-lakehouse-hudi	79.1%	🟢
catalog-lakehouse-iceberg	87.08%	🟢
catalog-lakehouse-paimon	77.48%	🟢
catalog-model	77.72%	🟢
cli	44.51%	🟢
client-java	77.96%	🟢
common	50.01%	🟢
core	82.27%	🟢
filesystem-hadoop3	76.97%	🟢
flink	42.54%	🟢
flink-runtime	0.0%	🔴
gcp	14.2%	🔴
hadoop-common	10.39%	🔴
hive-metastore-common	46.83%	🟢
iceberg-common	55.24%	🟢
iceberg-rest-server	68.93%	🟢
idp-basic	95.85% `+1.17%`	🟢
integration-test-common	0.0%	🔴
jobs	66.17%	🟢
lance-common	19.95%	🔴
lance-rest-server	62.78%	🟢
lineage	53.02%	🟢
optimizer	82.95%	🟢
optimizer-api	21.95%	🔴
server	85.78%	🟢
server-common	71.21%	🟢
spark	32.79%	🔴
spark-common	39.09%	🔴
trino-connector	35.14%	🔴

Files

Module	File	Coverage
idp-basic	IdpGroupDTO.java	100.0%	🟢
	IdpUserDTO.java	100.0%	🟢
	CreateUserRequest.java	100.0%	🟢
	ResetPasswordRequest.java	100.0%	🟢
	UpdateGroupUsersRequest.java	93.33%	🟢
	IdpGroupResponse.java	92.86%	🟢
	IdpUserResponse.java	92.86%	🟢
	CreateGroupRequest.java	91.67%	🟢

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

lasdf1234 · 2026-05-11T02:12:18Z

@roryqi The CI process has been successfully completed. If you have time, please review it for me. Thank you.

roryqi · 2026-05-11T07:08:43Z

I will suggest reviewing storage pull request first.

roryqi · 2026-05-11T07:09:21Z

+@ToString
+@Builder
+@Jacksonized
+public class CreateGroupRequest implements RESTRequest {


Could u use AddGroupRequest?

Got I have modified

roryqi · 2026-05-11T07:09:43Z

+@ToString
+@Builder
+@Jacksonized
+public class CreateUserRequest implements RESTRequest {


Could u use AddUserRequest?

Got I have modified

roryqi · 2026-05-11T07:10:51Z

+@ToString
+@Builder
+@Jacksonized
+public class UpdateGroupUsersRequest implements RESTRequest {


Could u use GroupMembershipChangeRequest?

Got I have added GroupAddUsersRequest

roryqi · 2026-05-11T07:11:59Z

+import org.apache.commons.lang3.StringUtils;
+
+/** Represents a built-in IdP group Data Transfer Object (DTO). */
+@EqualsAndHashCode


Could u use builder annotation here?

Got It has been supplemented.

roryqi · 2026-05-11T07:13:00Z

+@ToString
+@Builder
+@Jacksonized
+public class UpdateGroupUsersRequest implements RESTRequest {


How could we remove users?

Got I have added GroupRemoveUsersRequest

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

lasdf1234 · 2026-05-11T08:08:20Z

@roryqi All the codes have been modified according to the suggestions. If you have the time, could you please review the code for me?

roryqi · 2026-05-11T08:15:16Z

+
+/** The interface of a built-in IdP user. */
+@Evolving
+public interface IdpUser {


Why do we need this interfaces?

It has no practical use. The project contains some DTO classes and uses interfaces. If it is not necessary, interface can be removed.

We shouldn't put plugin classes into api module. API module is visible to users. Users shouldn't know these concepts.

Got, I have deleted two interfaces. cc @roryqi

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

lasdf1234 · 2026-05-11T11:15:04Z

@roryqi All the codes have been revised. If you have time, please review them for me. Thank you.

roryqi · 2026-05-11T11:50:42Z

 }

 dependencies {
+  implementation(project(":common"))


Could u add a blank line here? Because next line belongs the different segment.

roryqi · 2026-05-11T11:51:02Z


 dependencies {
+  implementation(project(":common"))
  implementation(libs.bcprov.jdk18on)


Could u follow the alphabet order here?

roryqi · 2026-05-11T11:52:02Z

  implementation(libs.guava)
+  implementation(libs.jackson.annotations)
+  implementation(libs.jackson.databind)
+  annotationProcessor(libs.lombok)


Why does lombok repeat for many times? Is it required?

roryqi · 2026-05-11T11:55:34Z

+@ToString
+@Builder
+@Jacksonized
+public class GroupRemoveUsersRequest implements RESTRequest {


I have concern about this class. I don't we should have this request. I prefer using

GroupMembershipChangeRequest { String[] additions; String[] removals; }

But we can see the next pull request.

I also don't like the order you raised the pull requests.

You raise the storage pull request first.
Then logic layer pull request.
Finally API layer pull request.

Move IdP DTOs into idp-basic plugin

ebfa199

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings May 10, 2026 10:34

Copilot started reviewing on behalf of lasdf1234 May 10, 2026 10:34 View session

Copilot AI reviewed May 10, 2026

View reviewed changes

lasdf1234 requested a review from Copilot May 10, 2026 10:39

Copilot started reviewing on behalf of lasdf1234 May 10, 2026 10:40 View session

Copilot AI reviewed May 10, 2026

View reviewed changes

Comment thread plugins/idp-basic/src/main/java/org/apache/gravitino/dto/requests/CreateUserRequest.java Outdated

Comment thread plugins/idp-basic/src/main/java/org/apache/gravitino/dto/requests/CreateGroupRequest.java Outdated

Refactor idp-basic DTO packages

ab0a1f5

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

roryqi reviewed May 11, 2026

View reviewed changes

lasdf1234 and others added 4 commits May 11, 2026 15:44

Add built-in IdP user and group interfaces

e71af7f

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Rename built-in IdP add requests

256894e

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Split built-in IdP group user requests

a4f4a9d

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Align local auth wording

99532fb

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

lasdf1234 requested a review from roryqi May 11, 2026 08:08

roryqi reviewed May 11, 2026

View reviewed changes

lasdf1234 requested a review from roryqi May 11, 2026 09:38

lasdf1234 and others added 3 commits May 11, 2026 17:57

Remove built-in IdP API interfaces

77e3d36

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Merge branch 'main' into add-built-in-idp-model

23d328c

Merge branch 'main' into add-built-in-idp-model

e8745b9

roryqi reviewed May 11, 2026

View reviewed changes

Conversation

lasdf1234 commented May 10, 2026

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 10, 2026

Code Coverage Report

Uh oh!

lasdf1234 commented May 11, 2026

Uh oh!

roryqi commented May 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lasdf1234 commented May 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lasdf1234 commented May 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants