[#11409] fix(idp): Validate incompatible simple mode at startup by lasdf1234 · Pull Request #11497 · apache/gravitino

lasdf1234 · 2026-06-08T09:21:48Z

What changes were proposed in this pull request?

Add IdpConfigurationValidator in org.apache.gravitino.idp.config to validate server configuration before the built-in IdP plugin starts.
Reject startup when gravitino.authorization.enable=true and gravitino.authenticators includes simple (including the default value).
Invoke validation from IdpRESTFeature.configure() before IdP registers HTTP Basic authentication.
Add unit tests covering explicit simple, default authenticators, and compatible OAuth configuration.

Why are the changes needed?

Enabling built-in IdP (gravitino.server.rest.extensionPackages = org.apache.gravitino.idp.web.rest.feature) together with Simple authentication and authorization leads to broken Web UI login: the IdP plugin injects HTTP Basic authentication ahead of Simple, while Web v2 sends username-only Basic credentials.

This change fails fast at startup with a clear error instead of leaving operators with a running server and a non-functional UI.

Fix: #11409

Does this PR introduce any user-facing change?

Server startup now fails with IllegalStateException when built-in IdP is enabled with authorization and Simple authentication (explicit or default).
Operators must remove simple from gravitino.authenticators or disable gravitino.authorization.enable.

How was this patch tested?

./gradlew spotlessApply
./gradlew :plugins:idp-basic:test -PskipITs -PskipDockerTests=true --tests org.apache.gravitino.idp.config.TestIdpConfigurationValidator

…ation enabled Reject incompatible Simple + authorization configuration when the built-in IdP plugin starts, including the default gravitino.authenticators value. Co-authored-by: Cursor <cursoragent@cursor.com>

lasdf1234 · 2026-06-08T09:24:55Z

@roryqi Could you please review this PR for me? Thank you.

…onfiguration Log the configuration error and call System.exit(1) from IdpConfigurationValidator when built-in IdP is enabled with Simple authentication and authorization. Co-authored-by: Cursor <cursoragent@cursor.com>

roryqi · 2026-06-08T11:24:48Z

+import org.slf4j.LoggerFactory;
+
+/** Validates server configuration before the built-in IdP plugin starts. */
+public final class IdpConfigurationValidator {


The logic seems simple. We don't need this class.

Got this class has been removed.

roryqi · 2026-06-08T11:25:34Z

+   * @param config The server configuration.
+   */
+  public static void validate(Config config) {
+    if (!config.get(Configs.ENABLE_AUTHORIZATION)) {


Why do we need this logic? Authentication is different from authorization.

Only when "Simple + authorization" is used will the basic header be sent. If only "simple" is configured but authorization is not enabled, the "basic" header will not be sent, it is still usable.

No, it will send basic header only if we use the simple mode.

Only when "Simple + authorization" is used and user login graivitno through WEB-UI, front end will send the basic header.
If only "simple" is configured but authorization is not enabled, front end will not send the basic header.

Only front end send the basic header ,the IDP's filter to perform the verification, resulting in an error.

Why does front behavior decide backend behavior?

It is not the front end that determines the behavior of the back end.

No matter which mode enables authentication, it is necessary to inform the backend which user invoked the interface. In the simple mode, if authentication is enabled, it will use the same format as the basic mode and send a header containing the basic information. This will cause the bug to occur.

Authorization isn't different from authentication.
Basic is an authentication mode. We can use authentication when we don't enable authorization.
I will suggest removing the authorization check here.

… IdpRESTFeature Remove the standalone validator class per review feedback and keep the same fail-fast behavior with tests moved to TestIdpRESTFeature. Co-authored-by: Cursor <cursoragent@cursor.com>

github-actions · 2026-06-08T14:37:13Z

Code Coverage Report

Overall Project	67.04% `-0.02%`	🟢
Files changed	22.22%	🔴

Module	Coverage
aliyun	1.72%	🔴
api	46.8%	🟢
authorization-common	85.96%	🟢
aws	3.66%	🔴
azure	2.47%	🔴
catalog-common	10.42%	🔴
catalog-fileset	80.35%	🟢
catalog-glue	67.41%	🟢
catalog-hive	79.35%	🟢
catalog-jdbc-clickhouse	80.02%	🟢
catalog-jdbc-common	45.31%	🟢
catalog-jdbc-doris	80.28%	🟢
catalog-jdbc-hologres	54.03%	🟢
catalog-jdbc-mysql	79.23%	🟢
catalog-jdbc-oceanbase	78.38%	🟢
catalog-jdbc-postgresql	82.29%	🟢
catalog-jdbc-starrocks	78.51%	🟢
catalog-kafka	77.01%	🟢
catalog-lakehouse-generic	58.53%	🟢
catalog-lakehouse-hudi	79.1%	🟢
catalog-lakehouse-iceberg	85.86%	🟢
catalog-lakehouse-paimon	79.29%	🟢
catalog-model	77.72%	🟢
cli	44.51%	🟢
client-java	77.91%	🟢
common	50.17%	🟢
core	82.51%	🟢
filesystem-hadoop3	76.97%	🟢
flink	0.0%	🔴
flink-common	46.3%	🟢
flink-runtime	0.0%	🔴
gcp	14.12%	🔴
hadoop-common	10.39%	🔴
hive-metastore-common	53.9%	🟢
iceberg-common	57.41%	🟢
iceberg-rest-server	73.8%	🟢
idp-basic	86.18% `-2.43%`	🟢
integration-test-common	0.0%	🔴
jobs	66.17%	🟢
lance-common	20.81%	🔴
lance-rest-server	60.54%	🟢
lineage	53.02%	🟢
optimizer	82.87%	🟢
optimizer-api	21.95%	🔴
server	85.73%	🟢
server-common	73.28%	🟢
spark	28.57%	🔴
spark-common	41.01%	🟢
trino-connector	39.97%	🔴

Files

Module	File	Coverage
idp-basic	IdpRESTFeature.java	22.22%	🔴

… is enabled Drop the authorization.enable guard so Simple and built-in IdP Basic cannot be combined regardless of authorization settings, per review feedback. Co-authored-by: Cursor <cursoragent@cursor.com>

…atibility Configure IdpRESTApiIT with OAuth so the server starts under the new validation, and document that built-in IdP cannot be used with simple. Co-authored-by: Cursor <cursoragent@cursor.com>

roryqi · 2026-06-09T04:36:52Z

 public class IdpRESTApiIT extends BaseIT {

+  /** RSA public key used only to satisfy OAuth authenticator initialization in this IT. */
+  private static final String OAUTH_PUBLIC_SIGN_KEY =


Got resolved.

Replace the hard-coded RSA public key constant with runtime generation. Co-authored-by: Cursor <cursoragent@cursor.com>

…idation Resolve idp.yaml description conflict by keeping the Simple auth incompatibility note from this PR. Co-authored-by: Cursor <cursoragent@cursor.com>

roryqi

LGTM.

…simple mode at startup (#11497) (#11515) **Cherry-pick Information:** - Original commit: 5c43ca8 - Target branch: `branch-1.3` - Status: ✅ Clean cherry-pick (no conflicts) Co-authored-by: MaSai <30949009+lasdf1234@users.noreply.github.com>

[apache#11409] fix(idp-basic): Fail fast on Simple auth with authoriz…

6394d9d

…ation enabled Reject incompatible Simple + authorization configuration when the built-in IdP plugin starts, including the default gravitino.authenticators value. Co-authored-by: Cursor <cursoragent@cursor.com>

lasdf1234 added the branch-1.3 Automatically cherry-pick commit to branch-1.3 label Jun 8, 2026

lasdf1234 changed the title ~~[#11409] fix(idp-basic): Fail fast on Simple auth with authorization enabled~~ [#11409] fix(idp): Fail fast on Simple auth with authorization enabled Jun 8, 2026

lasdf1234 changed the title ~~[#11409] fix(idp): Fail fast on Simple auth with authorization enabled~~ [#11409] fix(idp): Validate incompatible Simple and authorization config at startup Jun 8, 2026

lasdf1234 self-assigned this Jun 8, 2026

roryqi reviewed Jun 8, 2026

View reviewed changes

[apache#11409] refactor(idp-basic): Inline IdP config validation into…

6b10be1

… IdpRESTFeature Remove the standalone validator class per review feedback and keep the same fail-fast behavior with tests moved to TestIdpRESTFeature. Co-authored-by: Cursor <cursoragent@cursor.com>

roryqi changed the title ~~[#11409] fix(idp): Validate incompatible Simple and authorization config at startup~~ [#11409] fix(idp): Validate incompatible simple mode and authorization config at startup Jun 8, 2026

roryqi changed the title ~~[#11409] fix(idp): Validate incompatible simple mode and authorization config at startup~~ [#11409] fix(idp): Validate incompatible simple mode at startup Jun 8, 2026

lasdf1234 requested a review from roryqi June 8, 2026 12:33

lasdf1234 and others added 2 commits June 9, 2026 11:28

[apache#11409] fix(idp-basic): Reject Simple auth whenever IdP plugin…

19a5380

… is enabled Drop the authorization.enable guard so Simple and built-in IdP Basic cannot be combined regardless of authorization settings, per review feedback. Co-authored-by: Cursor <cursoragent@cursor.com>

[apache#11409] fix(idp-basic): Fix IT and document Simple auth incomp…

03e6e1d

…atibility Configure IdpRESTApiIT with OAuth so the server starts under the new validation, and document that built-in IdP cannot be used with simple. Co-authored-by: Cursor <cursoragent@cursor.com>

roryqi reviewed Jun 9, 2026

View reviewed changes

lasdf1234 and others added 2 commits June 9, 2026 12:53

[apache#11409] test(idp-basic): Generate OAuth sign key in IdpRESTApiIT

0cef4da

Replace the hard-coded RSA public key constant with runtime generation. Co-authored-by: Cursor <cursoragent@cursor.com>

[apache#11409] Merge branch 'main' into fix/11409-idp-simple-auth-val…

50036f1

…idation Resolve idp.yaml description conflict by keeping the Simple auth incompatibility note from this PR. Co-authored-by: Cursor <cursoragent@cursor.com>

lasdf1234 requested a review from roryqi June 9, 2026 06:15

roryqi approved these changes Jun 9, 2026

View reviewed changes

roryqi merged commit 5c43ca8 into apache:main Jun 9, 2026
30 checks passed

Uh oh!

Conversation

lasdf1234 commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

lasdf1234 commented Jun 8, 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 Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

github-actions Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roryqi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lasdf1234 commented Jun 8, 2026 •

edited

Loading

lasdf1234 Jun 8, 2026 •

edited

Loading

github-actions Bot commented Jun 8, 2026 •

edited

Loading