From 6394d9d8a58c8d4c9b699a45c6aaeb88096d623a Mon Sep 17 00:00:00 2001 From: remilu <1175334135@qq.com> Date: Mon, 8 Jun 2026 17:21:30 +0800 Subject: [PATCH 1/6] [#11409] fix(idp-basic): Fail fast on Simple auth with authorization enabled Reject incompatible Simple + authorization configuration when the built-in IdP plugin starts, including the default gravitino.authenticators value. Co-authored-by: Cursor --- .../idp/config/IdpConfigurationValidator.java | 51 +++++++++++++ .../idp/web/rest/feature/IdpRESTFeature.java | 2 + .../config/TestIdpConfigurationValidator.java | 72 +++++++++++++++++++ 3 files changed, 125 insertions(+) create mode 100644 plugins/idp-basic/src/main/java/org/apache/gravitino/idp/config/IdpConfigurationValidator.java create mode 100644 plugins/idp-basic/src/test/java/org/apache/gravitino/idp/config/TestIdpConfigurationValidator.java diff --git a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/config/IdpConfigurationValidator.java b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/config/IdpConfigurationValidator.java new file mode 100644 index 00000000000..65f70cbb661 --- /dev/null +++ b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/config/IdpConfigurationValidator.java @@ -0,0 +1,51 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.idp.config; + +import org.apache.gravitino.Config; +import org.apache.gravitino.Configs; +import org.apache.gravitino.auth.AuthenticatorType; + +/** Validates server configuration before the built-in IdP plugin starts. */ +public final class IdpConfigurationValidator { + + private IdpConfigurationValidator() {} + + /** + * Validates that the server configuration is compatible with the built-in IdP plugin. + * + * @param config The server configuration. + * @throws IllegalStateException if Simple authentication is enabled together with authorization. + */ + public static void validate(Config config) { + if (!config.get(Configs.ENABLE_AUTHORIZATION)) { + return; + } + boolean usesSimple = + config.get(Configs.AUTHENTICATORS).stream() + .anyMatch(name -> AuthenticatorType.SIMPLE.name().equalsIgnoreCase(name.trim())); + if (usesSimple) { + throw new IllegalStateException( + "Built-in IdP cannot be used with Simple authentication when authorization is enabled. " + + "Remove 'simple' from gravitino.authenticators (default is simple), " + + "or disable gravitino.authorization.enable."); + } + } +} diff --git a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/feature/IdpRESTFeature.java b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/feature/IdpRESTFeature.java index 939c4d8406c..f48ba9779c3 100644 --- a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/feature/IdpRESTFeature.java +++ b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/feature/IdpRESTFeature.java @@ -28,6 +28,7 @@ import org.apache.gravitino.GravitinoEnv; import org.apache.gravitino.idp.IdpUserGroupManager; import org.apache.gravitino.idp.auth.BasicAuthenticator; +import org.apache.gravitino.idp.config.IdpConfigurationValidator; import org.apache.gravitino.idp.web.rest.IdpAuthorizationFilter; import org.apache.gravitino.idp.web.rest.IdpBasicBinder; import org.apache.gravitino.idp.web.rest.IdpGroupOperations; @@ -59,6 +60,7 @@ public class IdpRESTFeature implements Feature { public boolean configure(FeatureContext context) { GravitinoEnv env = GravitinoEnv.getInstance(); Config config = env.config(); + IdpConfigurationValidator.validate(config); registerBasicAuthenticator(config); try { diff --git a/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/config/TestIdpConfigurationValidator.java b/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/config/TestIdpConfigurationValidator.java new file mode 100644 index 00000000000..f98526f75e1 --- /dev/null +++ b/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/config/TestIdpConfigurationValidator.java @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.idp.config; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.google.common.collect.Lists; +import org.apache.gravitino.Config; +import org.apache.gravitino.Configs; +import org.apache.gravitino.auth.AuthenticatorType; +import org.junit.jupiter.api.Test; + +class TestIdpConfigurationValidator { + + @Test + void testSimpleAndAuthFails() { + Config config = newConfig(true, AuthenticatorType.SIMPLE.name().toLowerCase()); + + IllegalStateException exception = + assertThrows(IllegalStateException.class, () -> IdpConfigurationValidator.validate(config)); + + assertTrue(exception.getMessage().contains("cannot be used with Simple authentication")); + } + + @Test + void testDefaultSimpleAndAuthFails() { + Config config = new Config(false) {}; + config.set(Configs.ENABLE_AUTHORIZATION, true); + + assertThrows(IllegalStateException.class, () -> IdpConfigurationValidator.validate(config)); + } + + @Test + void testSimpleNoAuthOk() { + Config config = newConfig(false, AuthenticatorType.SIMPLE.name().toLowerCase()); + + assertDoesNotThrow(() -> IdpConfigurationValidator.validate(config)); + } + + @Test + void testOAuthAndAuthOk() { + Config config = newConfig(true, AuthenticatorType.OAUTH.name().toLowerCase()); + + assertDoesNotThrow(() -> IdpConfigurationValidator.validate(config)); + } + + private static Config newConfig(boolean authorizationEnabled, String... authenticators) { + Config config = new Config(false) {}; + config.set(Configs.ENABLE_AUTHORIZATION, authorizationEnabled); + config.set(Configs.AUTHENTICATORS, Lists.newArrayList(authenticators)); + return config; + } +} From e8b11293d79269a084b1902508a12f37e83c1c49 Mon Sep 17 00:00:00 2001 From: remilu <1175334135@qq.com> Date: Mon, 8 Jun 2026 18:15:59 +0800 Subject: [PATCH 2/6] [#11409] fix(idp-basic): Exit JVM on incompatible Simple auth configuration 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 --- .../idp/config/IdpConfigurationValidator.java | 8 +++- .../config/TestIdpConfigurationValidator.java | 45 ++++++++++++++++--- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/config/IdpConfigurationValidator.java b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/config/IdpConfigurationValidator.java index 65f70cbb661..e37c2056b3d 100644 --- a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/config/IdpConfigurationValidator.java +++ b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/config/IdpConfigurationValidator.java @@ -22,17 +22,20 @@ import org.apache.gravitino.Config; import org.apache.gravitino.Configs; import org.apache.gravitino.auth.AuthenticatorType; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** Validates server configuration before the built-in IdP plugin starts. */ public final class IdpConfigurationValidator { + private static final Logger LOG = LoggerFactory.getLogger(IdpConfigurationValidator.class); + private IdpConfigurationValidator() {} /** * Validates that the server configuration is compatible with the built-in IdP plugin. * * @param config The server configuration. - * @throws IllegalStateException if Simple authentication is enabled together with authorization. */ public static void validate(Config config) { if (!config.get(Configs.ENABLE_AUTHORIZATION)) { @@ -42,10 +45,11 @@ public static void validate(Config config) { config.get(Configs.AUTHENTICATORS).stream() .anyMatch(name -> AuthenticatorType.SIMPLE.name().equalsIgnoreCase(name.trim())); if (usesSimple) { - throw new IllegalStateException( + LOG.error( "Built-in IdP cannot be used with Simple authentication when authorization is enabled. " + "Remove 'simple' from gravitino.authenticators (default is simple), " + "or disable gravitino.authorization.enable."); + System.exit(1); } } } diff --git a/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/config/TestIdpConfigurationValidator.java b/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/config/TestIdpConfigurationValidator.java index f98526f75e1..c49ce2be95a 100644 --- a/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/config/TestIdpConfigurationValidator.java +++ b/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/config/TestIdpConfigurationValidator.java @@ -20,8 +20,8 @@ package org.apache.gravitino.idp.config; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; import com.google.common.collect.Lists; import org.apache.gravitino.Config; @@ -35,10 +35,10 @@ class TestIdpConfigurationValidator { void testSimpleAndAuthFails() { Config config = newConfig(true, AuthenticatorType.SIMPLE.name().toLowerCase()); - IllegalStateException exception = - assertThrows(IllegalStateException.class, () -> IdpConfigurationValidator.validate(config)); + SystemExitException exception = + assertThrows(SystemExitException.class, () -> validateWithExitGuard(config)); - assertTrue(exception.getMessage().contains("cannot be used with Simple authentication")); + assertEquals(1, exception.status()); } @Test @@ -46,7 +46,7 @@ void testDefaultSimpleAndAuthFails() { Config config = new Config(false) {}; config.set(Configs.ENABLE_AUTHORIZATION, true); - assertThrows(IllegalStateException.class, () -> IdpConfigurationValidator.validate(config)); + assertThrows(SystemExitException.class, () -> validateWithExitGuard(config)); } @Test @@ -69,4 +69,39 @@ private static Config newConfig(boolean authorizationEnabled, String... authenti config.set(Configs.AUTHENTICATORS, Lists.newArrayList(authenticators)); return config; } + + @SuppressWarnings("removal") + private static void validateWithExitGuard(Config config) { + SecurityManager original = System.getSecurityManager(); + System.setSecurityManager( + new SecurityManager() { + @Override + public void checkExit(int status) { + throw new SystemExitException(status); + } + + @Override + public void checkPermission(java.security.Permission perm) { + // Allow test execution. + } + }); + try { + IdpConfigurationValidator.validate(config); + } finally { + System.setSecurityManager(original); + } + } + + private static final class SystemExitException extends SecurityException { + private final int status; + + private SystemExitException(int status) { + super("System.exit(" + status + ")"); + this.status = status; + } + + private int status() { + return status; + } + } } From 6b10be1e22334a665c87dbc5c7544561d68876d7 Mon Sep 17 00:00:00 2001 From: remilu <1175334135@qq.com> Date: Mon, 8 Jun 2026 19:53:24 +0800 Subject: [PATCH 3/6] [#11409] refactor(idp-basic): Inline IdP config validation into 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 --- .../idp/config/IdpConfigurationValidator.java | 55 ------------------- .../idp/web/rest/feature/IdpRESTFeature.java | 25 ++++++++- .../rest/feature/TestIdpRESTFeature.java} | 10 ++-- 3 files changed, 28 insertions(+), 62 deletions(-) delete mode 100644 plugins/idp-basic/src/main/java/org/apache/gravitino/idp/config/IdpConfigurationValidator.java rename plugins/idp-basic/src/test/java/org/apache/gravitino/idp/{config/TestIdpConfigurationValidator.java => web/rest/feature/TestIdpRESTFeature.java} (91%) diff --git a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/config/IdpConfigurationValidator.java b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/config/IdpConfigurationValidator.java deleted file mode 100644 index e37c2056b3d..00000000000 --- a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/config/IdpConfigurationValidator.java +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.gravitino.idp.config; - -import org.apache.gravitino.Config; -import org.apache.gravitino.Configs; -import org.apache.gravitino.auth.AuthenticatorType; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** Validates server configuration before the built-in IdP plugin starts. */ -public final class IdpConfigurationValidator { - - private static final Logger LOG = LoggerFactory.getLogger(IdpConfigurationValidator.class); - - private IdpConfigurationValidator() {} - - /** - * Validates that the server configuration is compatible with the built-in IdP plugin. - * - * @param config The server configuration. - */ - public static void validate(Config config) { - if (!config.get(Configs.ENABLE_AUTHORIZATION)) { - return; - } - boolean usesSimple = - config.get(Configs.AUTHENTICATORS).stream() - .anyMatch(name -> AuthenticatorType.SIMPLE.name().equalsIgnoreCase(name.trim())); - if (usesSimple) { - LOG.error( - "Built-in IdP cannot be used with Simple authentication when authorization is enabled. " - + "Remove 'simple' from gravitino.authenticators (default is simple), " - + "or disable gravitino.authorization.enable."); - System.exit(1); - } - } -} diff --git a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/feature/IdpRESTFeature.java b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/feature/IdpRESTFeature.java index f48ba9779c3..9c662349118 100644 --- a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/feature/IdpRESTFeature.java +++ b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/feature/IdpRESTFeature.java @@ -26,9 +26,9 @@ import org.apache.gravitino.Config; import org.apache.gravitino.Configs; import org.apache.gravitino.GravitinoEnv; +import org.apache.gravitino.auth.AuthenticatorType; import org.apache.gravitino.idp.IdpUserGroupManager; import org.apache.gravitino.idp.auth.BasicAuthenticator; -import org.apache.gravitino.idp.config.IdpConfigurationValidator; import org.apache.gravitino.idp.web.rest.IdpAuthorizationFilter; import org.apache.gravitino.idp.web.rest.IdpBasicBinder; import org.apache.gravitino.idp.web.rest.IdpGroupOperations; @@ -60,7 +60,7 @@ public class IdpRESTFeature implements Feature { public boolean configure(FeatureContext context) { GravitinoEnv env = GravitinoEnv.getInstance(); Config config = env.config(); - IdpConfigurationValidator.validate(config); + validateConfiguration(config); registerBasicAuthenticator(config); try { @@ -78,6 +78,27 @@ public boolean configure(FeatureContext context) { return true; } + /** + * Validates that the server configuration is compatible with the built-in IdP plugin. + * + * @param config The server configuration. + */ + static void validateConfiguration(Config config) { + if (!config.get(Configs.ENABLE_AUTHORIZATION)) { + return; + } + boolean usesSimple = + config.get(Configs.AUTHENTICATORS).stream() + .anyMatch(name -> AuthenticatorType.SIMPLE.name().equalsIgnoreCase(name.trim())); + if (usesSimple) { + LOG.error( + "Built-in IdP cannot be used with Simple authentication when authorization is enabled. " + + "Remove 'simple' from gravitino.authenticators (default is simple), " + + "or disable gravitino.authorization.enable."); + System.exit(1); + } + } + private static void registerBasicAuthenticator(Config config) { List authenticators = ServerAuthenticator.getInstance().authenticators(); if (authenticators == null) { diff --git a/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/config/TestIdpConfigurationValidator.java b/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/web/rest/feature/TestIdpRESTFeature.java similarity index 91% rename from plugins/idp-basic/src/test/java/org/apache/gravitino/idp/config/TestIdpConfigurationValidator.java rename to plugins/idp-basic/src/test/java/org/apache/gravitino/idp/web/rest/feature/TestIdpRESTFeature.java index c49ce2be95a..43fc45dd8a3 100644 --- a/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/config/TestIdpConfigurationValidator.java +++ b/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/web/rest/feature/TestIdpRESTFeature.java @@ -17,7 +17,7 @@ * under the License. */ -package org.apache.gravitino.idp.config; +package org.apache.gravitino.idp.web.rest.feature; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -29,7 +29,7 @@ import org.apache.gravitino.auth.AuthenticatorType; import org.junit.jupiter.api.Test; -class TestIdpConfigurationValidator { +class TestIdpRESTFeature { @Test void testSimpleAndAuthFails() { @@ -53,14 +53,14 @@ void testDefaultSimpleAndAuthFails() { void testSimpleNoAuthOk() { Config config = newConfig(false, AuthenticatorType.SIMPLE.name().toLowerCase()); - assertDoesNotThrow(() -> IdpConfigurationValidator.validate(config)); + assertDoesNotThrow(() -> IdpRESTFeature.validateConfiguration(config)); } @Test void testOAuthAndAuthOk() { Config config = newConfig(true, AuthenticatorType.OAUTH.name().toLowerCase()); - assertDoesNotThrow(() -> IdpConfigurationValidator.validate(config)); + assertDoesNotThrow(() -> IdpRESTFeature.validateConfiguration(config)); } private static Config newConfig(boolean authorizationEnabled, String... authenticators) { @@ -86,7 +86,7 @@ public void checkPermission(java.security.Permission perm) { } }); try { - IdpConfigurationValidator.validate(config); + IdpRESTFeature.validateConfiguration(config); } finally { System.setSecurityManager(original); } From 19a53804622489a7741dcd30d5ac689fe86c0dfd Mon Sep 17 00:00:00 2001 From: remilu <1175334135@qq.com> Date: Tue, 9 Jun 2026 11:28:07 +0800 Subject: [PATCH 4/6] [#11409] fix(idp-basic): Reject Simple auth whenever IdP plugin 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 --- .../idp/web/rest/feature/IdpRESTFeature.java | 8 ++----- .../web/rest/feature/TestIdpRESTFeature.java | 21 ++++++------------- 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/feature/IdpRESTFeature.java b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/feature/IdpRESTFeature.java index 9c662349118..668b5d84a3b 100644 --- a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/feature/IdpRESTFeature.java +++ b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/web/rest/feature/IdpRESTFeature.java @@ -84,17 +84,13 @@ public boolean configure(FeatureContext context) { * @param config The server configuration. */ static void validateConfiguration(Config config) { - if (!config.get(Configs.ENABLE_AUTHORIZATION)) { - return; - } boolean usesSimple = config.get(Configs.AUTHENTICATORS).stream() .anyMatch(name -> AuthenticatorType.SIMPLE.name().equalsIgnoreCase(name.trim())); if (usesSimple) { LOG.error( - "Built-in IdP cannot be used with Simple authentication when authorization is enabled. " - + "Remove 'simple' from gravitino.authenticators (default is simple), " - + "or disable gravitino.authorization.enable."); + "Built-in IdP is incompatible with Simple authentication. " + + "Remove 'simple' from gravitino.authenticators (default is simple)."); System.exit(1); } } diff --git a/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/web/rest/feature/TestIdpRESTFeature.java b/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/web/rest/feature/TestIdpRESTFeature.java index 43fc45dd8a3..72153617979 100644 --- a/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/web/rest/feature/TestIdpRESTFeature.java +++ b/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/web/rest/feature/TestIdpRESTFeature.java @@ -32,8 +32,8 @@ class TestIdpRESTFeature { @Test - void testSimpleAndAuthFails() { - Config config = newConfig(true, AuthenticatorType.SIMPLE.name().toLowerCase()); + void testSimpleFails() { + Config config = newConfig(AuthenticatorType.SIMPLE.name().toLowerCase()); SystemExitException exception = assertThrows(SystemExitException.class, () -> validateWithExitGuard(config)); @@ -42,30 +42,21 @@ void testSimpleAndAuthFails() { } @Test - void testDefaultSimpleAndAuthFails() { + void testDefaultSimpleFails() { Config config = new Config(false) {}; - config.set(Configs.ENABLE_AUTHORIZATION, true); assertThrows(SystemExitException.class, () -> validateWithExitGuard(config)); } @Test - void testSimpleNoAuthOk() { - Config config = newConfig(false, AuthenticatorType.SIMPLE.name().toLowerCase()); + void testOAuthOk() { + Config config = newConfig(AuthenticatorType.OAUTH.name().toLowerCase()); assertDoesNotThrow(() -> IdpRESTFeature.validateConfiguration(config)); } - @Test - void testOAuthAndAuthOk() { - Config config = newConfig(true, AuthenticatorType.OAUTH.name().toLowerCase()); - - assertDoesNotThrow(() -> IdpRESTFeature.validateConfiguration(config)); - } - - private static Config newConfig(boolean authorizationEnabled, String... authenticators) { + private static Config newConfig(String... authenticators) { Config config = new Config(false) {}; - config.set(Configs.ENABLE_AUTHORIZATION, authorizationEnabled); config.set(Configs.AUTHENTICATORS, Lists.newArrayList(authenticators)); return config; } From 03e6e1dae4c27bbfe00a53dcddd68d2f8daaf62c Mon Sep 17 00:00:00 2001 From: remilu <1175334135@qq.com> Date: Tue, 9 Jun 2026 12:07:23 +0800 Subject: [PATCH 5/6] [#11409] fix(idp-basic): Fix IT and document Simple auth incompatibility 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 --- design-docs/gravitino-local-authentication.md | 3 +++ docs/open-api/idp/idp.yaml | 3 ++- docs/open-api/idp/openapi.yaml | 5 ++++- docs/security/how-to-authenticate.md | 3 +++ docs/security/how-to-use-built-in-idp.md | 13 +++++++++---- .../idp/integration/test/IdpRESTApiIT.java | 15 +++++++++++++-- 6 files changed, 34 insertions(+), 8 deletions(-) diff --git a/design-docs/gravitino-local-authentication.md b/design-docs/gravitino-local-authentication.md index bc90a39ea96..bfb1d7ea3fc 100644 --- a/design-docs/gravitino-local-authentication.md +++ b/design-docs/gravitino-local-authentication.md @@ -319,6 +319,9 @@ fresh Gravitino deployment. gravitino.authorization.serviceAdmins=admin1,admin2 ``` + Built-in IdP is incompatible with the `simple` authenticator. When the `idp-basic` plugin is + enabled, `gravitino.authenticators` must not include `simple`. + 2. Export the initial service admin password before starting Gravitino: ```bash diff --git a/docs/open-api/idp/idp.yaml b/docs/open-api/idp/idp.yaml index bc5e211ea9b..0f6cdd6bf47 100644 --- a/docs/open-api/idp/idp.yaml +++ b/docs/open-api/idp/idp.yaml @@ -26,7 +26,8 @@ paths: summary: Add built-in IDP user description: > Creates a built-in IDP user with the given username and password. - Requires the `basic` authenticator and the `idp-basic` plugin to be enabled. + Requires the `idp-basic` plugin and built-in IdP Basic authentication. + `gravitino.authenticators` must not include `simple`. operationId: addIdpUser requestBody: required: true diff --git a/docs/open-api/idp/openapi.yaml b/docs/open-api/idp/openapi.yaml index 76e064083f0..23000ef4f9e 100644 --- a/docs/open-api/idp/openapi.yaml +++ b/docs/open-api/idp/openapi.yaml @@ -25,7 +25,10 @@ info: version: 1.3.0-SNAPSHOT description: | OpenAPI specification for built-in IDP user and group management APIs exposed - by the `idp-basic` plugin when the `basic` authenticator is enabled. + by the `idp-basic` plugin. Clients authenticate with Basic credentials + validated against built-in IdP user metadata. Enable the plugin via + `gravitino.server.rest.extensionPackages`; `gravitino.authenticators` must not + include `simple` when IdP is enabled. servers: - url: "{scheme}://{host}:{port}/{basePath}" diff --git a/docs/security/how-to-authenticate.md b/docs/security/how-to-authenticate.md index 053821fb070..5389d4a7346 100644 --- a/docs/security/how-to-authenticate.md +++ b/docs/security/how-to-authenticate.md @@ -366,6 +366,9 @@ This example shows how to enable built-in Basic authentication. - Gravitino distribution package (includes the idp-basic plugin on the server classpath) +Built-in IdP is **incompatible** with the `simple` authenticator (the default). When the +`idp-basic` plugin is enabled, `gravitino.authenticators` must not include `simple`. + **Configuration:** Append the following to `conf/gravitino.conf`: diff --git a/docs/security/how-to-use-built-in-idp.md b/docs/security/how-to-use-built-in-idp.md index ef18b44eaff..c9c00f05aa2 100644 --- a/docs/security/how-to-use-built-in-idp.md +++ b/docs/security/how-to-use-built-in-idp.md @@ -31,7 +31,11 @@ Before you call `/api/idp/*`, ensure the following: gravitino.server.rest.extensionPackages = org.apache.gravitino.idp.web.rest.feature ``` -2. **Service admin passwords** — Built-in IDP requires every username in +2. **Server authenticator** — Built-in IdP is **incompatible** with the `simple` authenticator + (the default). When the `idp-basic` plugin is enabled, `gravitino.authenticators` must not + include `simple`. + +3. **Service admin passwords** — Built-in IDP requires every username in `gravitino.authorization.serviceAdmins` to have a password stored in `idp_user_meta` before you can call management APIs. @@ -61,9 +65,10 @@ Before you call `/api/idp/*`, ensure the following: Set service admins in `gravitino.conf` (see also [Prerequisites](#prerequisites)): -| Configuration item | Description | Example | -|-----------------------------------------|-------------------------------------------------------------------------------------|---------| -| `gravitino.authorization.serviceAdmins` | Comma-separated service admin that can call built-in IDP management APIs | `admin` | +| Configuration item | Description | Example | +|-------------------------------------------|-------------------------------------------------------------------------------------|---------| +| `gravitino.server.rest.extensionPackages` | Registers built-in IdP REST APIs | `org.apache.gravitino.idp.web.rest.feature` | +| `gravitino.authorization.serviceAdmins` | Comma-separated service admin that can call built-in IDP management APIs | `admin` | Example: diff --git a/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/integration/test/IdpRESTApiIT.java b/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/integration/test/IdpRESTApiIT.java index 899c143e985..2d6083cde40 100644 --- a/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/integration/test/IdpRESTApiIT.java +++ b/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/integration/test/IdpRESTApiIT.java @@ -37,6 +37,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.gravitino.Configs; import org.apache.gravitino.auth.AuthConstants; +import org.apache.gravitino.auth.AuthenticatorType; import org.apache.gravitino.dto.responses.ErrorConstants; import org.apache.gravitino.idp.dto.requests.AddGroupRequest; import org.apache.gravitino.idp.dto.requests.AddUserRequest; @@ -48,6 +49,7 @@ import org.apache.gravitino.integration.test.util.BaseIT; import org.apache.gravitino.integration.test.util.ITUtils; import org.apache.gravitino.json.JsonUtils; +import org.apache.gravitino.server.authentication.OAuthConfig; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -60,6 +62,10 @@ */ 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 = + "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA02WlnekWmY5osrFoIsye9AjujNcsiycb7bBNxTi0ISv9p7tjP0SgAT5RJlmA5vMOck83tcJiCnzsRnfSFBED7sovstzQi5YvDdjucaa+im3AV/rhYe/27RXXy9LC0Q7yNnicJbX7Bg34n7V6dUNIBbh2hDVMPndPM5v4N0k/7ofUPghnGlQIpGx30/w3YbIwwYN9TxAMQau+Ffgcqp7ZIlSIDcHVGyx2VA24xzboRvN4X5AxPWkK7XI245bFqa2VCn1NqUXWRYbDvE5BSXK5gIQ3iuFlsi4swQMPurZmXPDCG9KBTNUa0CcHuS7cmQqdlOPxWAD9U5lyOXOg0IiotQIDAQAB"; + private static final String ACCEPT = "application/vnd.gravitino.v1+json"; private static final String ADMIN = "admin"; private static final String ADMIN_PASSWORD = "Passw0rd-For-Admin1"; @@ -84,6 +90,11 @@ public void startIntegrationTest() throws Exception { configs.put(Configs.CACHE_ENABLED.getKey(), String.valueOf(false)); configs.put(Configs.STORE_DELETE_AFTER_TIME.getKey(), String.valueOf(20 * 60 * 1000L)); configs.put(Configs.SERVICE_ADMINS.getKey(), ADMIN); + configs.put(Configs.AUTHENTICATORS.getKey(), AuthenticatorType.OAUTH.name().toLowerCase()); + configs.put(OAuthConfig.SERVICE_AUDIENCE.getKey(), "service1"); + configs.put(OAuthConfig.DEFAULT_SIGN_KEY.getKey(), OAUTH_PUBLIC_SIGN_KEY); + configs.put(OAuthConfig.DEFAULT_SERVER_URI.getKey(), "test"); + configs.put(OAuthConfig.DEFAULT_TOKEN_PATH.getKey(), "test"); configs.put( Configs.REST_API_EXTENSION_PACKAGES.getKey(), IdpRESTFeature.IDP_REST_EXTENSION_PACKAGE); registerCustomConfigs(configs); @@ -117,8 +128,8 @@ private static void ensureDeployInitialAdminPasswordInDistributionEnv() throws I @Test void testIdpAuthorization() throws Exception { Assertions.assertEquals(200, get("/version", ADMIN, ADMIN_PASSWORD).statusCode()); - // No Authorization: simple authenticator allows anonymous access; IdP filter rejects. - assertError(403, get("/idp/users/" + USER1, null, null), ErrorConstants.FORBIDDEN_CODE); + // No Authorization: OAuth rejects the request before the IdP filter runs. + Assertions.assertEquals(401, get("/idp/users/" + USER1, null, null).statusCode()); postUser(USER2, USER_PASSWORD); assertError( From 0cef4da3e266f6663995051e90753052f4e62605 Mon Sep 17 00:00:00 2001 From: remilu <1175334135@qq.com> Date: Tue, 9 Jun 2026 12:53:54 +0800 Subject: [PATCH 6/6] [#11409] test(idp-basic): Generate OAuth sign key in IdpRESTApiIT Replace the hard-coded RSA public key constant with runtime generation. Co-authored-by: Cursor --- .../idp/integration/test/IdpRESTApiIT.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/integration/test/IdpRESTApiIT.java b/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/integration/test/IdpRESTApiIT.java index 2d6083cde40..8c5c29bacaa 100644 --- a/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/integration/test/IdpRESTApiIT.java +++ b/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/integration/test/IdpRESTApiIT.java @@ -30,6 +30,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.security.KeyPairGenerator; import java.util.Base64; import java.util.List; import java.util.Map; @@ -62,10 +63,6 @@ */ 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 = - "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA02WlnekWmY5osrFoIsye9AjujNcsiycb7bBNxTi0ISv9p7tjP0SgAT5RJlmA5vMOck83tcJiCnzsRnfSFBED7sovstzQi5YvDdjucaa+im3AV/rhYe/27RXXy9LC0Q7yNnicJbX7Bg34n7V6dUNIBbh2hDVMPndPM5v4N0k/7ofUPghnGlQIpGx30/w3YbIwwYN9TxAMQau+Ffgcqp7ZIlSIDcHVGyx2VA24xzboRvN4X5AxPWkK7XI245bFqa2VCn1NqUXWRYbDvE5BSXK5gIQ3iuFlsi4swQMPurZmXPDCG9KBTNUa0CcHuS7cmQqdlOPxWAD9U5lyOXOg0IiotQIDAQAB"; - private static final String ACCEPT = "application/vnd.gravitino.v1+json"; private static final String ADMIN = "admin"; private static final String ADMIN_PASSWORD = "Passw0rd-For-Admin1"; @@ -92,7 +89,7 @@ public void startIntegrationTest() throws Exception { configs.put(Configs.SERVICE_ADMINS.getKey(), ADMIN); configs.put(Configs.AUTHENTICATORS.getKey(), AuthenticatorType.OAUTH.name().toLowerCase()); configs.put(OAuthConfig.SERVICE_AUDIENCE.getKey(), "service1"); - configs.put(OAuthConfig.DEFAULT_SIGN_KEY.getKey(), OAUTH_PUBLIC_SIGN_KEY); + configs.put(OAuthConfig.DEFAULT_SIGN_KEY.getKey(), oauthPublicSignKey()); configs.put(OAuthConfig.DEFAULT_SERVER_URI.getKey(), "test"); configs.put(OAuthConfig.DEFAULT_TOKEN_PATH.getKey(), "test"); configs.put( @@ -374,4 +371,10 @@ private static void assertError(int expectedStatus, HttpResponse respons private static int errorCode(HttpResponse response) throws Exception { return JsonUtils.objectMapper().readTree(response.body()).get("code").asInt(); } + + private static String oauthPublicSignKey() throws Exception { + KeyPairGenerator generator = KeyPairGenerator.getInstance("RSA"); + generator.initialize(2048); + return Base64.getEncoder().encodeToString(generator.generateKeyPair().getPublic().getEncoded()); + } }