[#10959] feat(idp-basic): Add Basic authentication for built-in IdP#11226
Conversation
…dP auth Implement HTTP Basic authentication in the idp-basic plugin, register the basic authenticator type, and resolve user groups from IdP metadata. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Wire idp-basic on the server-common test classpath and verify BasicAuthenticator behavior with AuthenticationFilter. Co-authored-by: Cursor <cursoragent@cursor.com>
…tests Trim Base64 credentials before decoding, treat blank passwords as invalid credentials, and add unit tests for malformed Basic auth headers. Co-authored-by: Cursor <cursoragent@cursor.com>
…tartup Add ServiceAdminInitializer in idp-basic to provision missing built-in IdP accounts from GRAVITINO_INITIAL_ADMIN_PASSWORD when basic auth is enabled. Wire startup through IdpStorageBootstrap and reflectively load the plugin from server without a compile-time dependency on idp-basic. Co-authored-by: Cursor <cursoragent@cursor.com>
Copy the plugin and bcprov jars into the Iceberg REST server package and use implementation dependencies for api and server-common in idp-basic. Co-authored-by: Cursor <cursoragent@cursor.com>
Document Basic mode alongside simple, OAuth, and Kerberos, including server setup, initial admin password, curl usage, and a full example. Co-authored-by: Cursor <cursoragent@cursor.com>
Introduce BasicTokenProvider and GravitinoClient builder support, plus document Basic mode client usage in how-to-authenticate.md. Co-authored-by: Cursor <cursoragent@cursor.com>
Sync with apache/gravitino main via origin/main. Resolve IdpStorageBootstrap conflict by keeping service-admin initialization only; relational storage and garbage collection are handled by IdpRelationalStorage on main. Co-authored-by: Cursor <cursoragent@cursor.com>
…to initializer Remove ServiceAdminManager and wire ServiceAdminInitializer directly to IdpUserMetaService, aligning user PO fields with IdpUserGroupManager. Co-authored-by: Cursor <cursoragent@cursor.com>
…inBootstrap SPI Replace reflective BuiltInIdpPluginLauncher with ServiceLoader-based ServerPluginBootstrapper and register idp-basic through META-INF/services. Co-authored-by: Cursor <cursoragent@cursor.com>
…e admin init Merge IdpStorageBootstrap into IdpServerPluginBootstrap, make ServiceAdminInitializer a static utility, and consolidate unit tests. Co-authored-by: Cursor <cursoragent@cursor.com>
…AdminInitializer Remove the private newUserPO helper and build the user PO at the insert call site. Co-authored-by: Cursor <cursoragent@cursor.com>
Remove duplicate curl example and info admonition from Basic mode section; Simple mode already documents Authorization header usage. Co-authored-by: Cursor <cursoragent@cursor.com>
…ic-authentication-flow
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds built-in HTTP Basic authentication backed by the IdP relational store, including server-side plugin bootstrapping and Java client support.
Changes:
- Introduces a ServiceLoader-based server plugin bootstrap mechanism and wires it into server startup.
- Adds IdP Basic authentication (authenticator + service-admin initialization) plus tests and distribution packaging updates.
- Updates Java client builder/token provider and documentation to support/configure Basic auth.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/main/java/org/apache/gravitino/server/GravitinoServer.java | Calls plugin bootstrapper during server initialization. |
| server-common/src/main/java/org/apache/gravitino/server/plugin/ServerPluginBootstrap.java | Defines the ServiceLoader-discovered plugin bootstrap SPI. |
| server-common/src/main/java/org/apache/gravitino/server/plugin/ServerPluginBootstrapper.java | Loads and initializes plugin bootstraps from the classpath. |
| server-common/src/main/java/org/apache/gravitino/server/authentication/AuthenticatorFactory.java | Registers BASIC authenticator type mapping. |
| common/src/main/java/org/apache/gravitino/auth/AuthenticatorType.java | Adds BASIC to supported authenticator types. |
| plugins/idp-basic/src/main/java/org/apache/gravitino/idp/auth/BasicAuthenticator.java | Implements HTTP Basic auth against IdP user metadata. |
| plugins/idp-basic/src/main/java/org/apache/gravitino/idp/auth/ServiceAdminInitializer.java | Initializes configured service-admin users on startup via env-provided passwords. |
| plugins/idp-basic/src/main/java/org/apache/gravitino/idp/IdpServerPluginBootstrap.java | Implements server plugin bootstrap to run IdP initialization once per JVM. |
| plugins/idp-basic/src/main/java/org/apache/gravitino/idp/storage/service/IdpUserMetaService.java | Adds existence check API used during initialization. |
| plugins/idp-basic/src/main/resources/META-INF/services/org.apache.gravitino.server.plugin.ServerPluginBootstrap | Registers IdP bootstrap provider for ServiceLoader. |
| clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClientBase.java | Adds withBasicAuth builder method. |
| clients/client-java/src/main/java/org/apache/gravitino/client/BasicTokenProvider.java | Adds Basic auth header token provider. |
| docs/security/how-to-authenticate.md | Documents Basic auth configuration and examples. |
| docs/index.md | Updates authentication documentation summary to include Basic. |
| server-common/build.gradle.kts | Adds idp-basic as test dependency for server-common tests. |
| plugins/idp-basic/build.gradle.kts | Adds dependencies/resources/copy tasks needed for plugin + standalone packaging. |
| server-common/src/test/java/org/apache/gravitino/server/authentication/TestBasicAuthentication.java | Adds server-side filter tests for Basic auth flow. |
| plugins/idp-basic/src/test/java/org/apache/gravitino/idp/auth/TestBasicAuthenticator.java | Adds unit tests for BasicAuthenticator parsing/authn behavior. |
| plugins/idp-basic/src/test/java/org/apache/gravitino/idp/auth/TestServiceAdminInitializer.java | Adds unit tests for service-admin initialization behavior and validation. |
| plugins/idp-basic/src/test/java/org/apache/gravitino/idp/TestIdpServerPluginBootstrap.java | Verifies ServiceLoader registration for the IdP bootstrap. |
| clients/client-java/src/test/java/org/apache/gravitino/client/TestBasicTokenProvider.java | Tests Basic token construction/encoding. |
…avoid IdpUserMetaService changes Rename ServerPluginBootstrap.initializeOnce to initialize, and check service admin existence via getIdpUserByUsername instead of adding idpUserExists to IdpUserMetaService. Co-authored-by: Cursor <cursoragent@cursor.com>
mchades
left a comment
There was a problem hiding this comment.
Missing update Python client?
…ginBootstrap SPI Fix RAT check failure on the META-INF/services registration file. Co-authored-by: Cursor <cursoragent@cursor.com>
Code Coverage Report
Files
|
| ./gradlew :plugins:idp-basic:test -PskipITs -PskipDockerTests=false -PskipWeb=true | ||
| for backend in h2 mysql postgresql; do | ||
| ./gradlew :plugins:idp-basic:test \ | ||
| -PtestMode=embedded \ |
There was a problem hiding this comment.
you also need to run the deploy testMode
There was a problem hiding this comment.
Got deploy mode has been added.
| -PjdbcBackend="${backend}" \ | ||
| -PskipDockerTests=false \ | ||
| -PskipWeb=true \ | ||
| --tests "org.apache.gravitino.idp.integration.test.IdpRESTApiIT" |
There was a problem hiding this comment.
use "org.apache.gravitino.idp.integration.test.**"
There was a problem hiding this comment.
Got code has been modified.
| metalake_name="metalake", | ||
| auth_data_provider=BasicAuthProvider("admin", "YourSecureGravitinoPassword"), | ||
| ) | ||
| ``` |
There was a problem hiding this comment.
could you plz add the shell example?
There was a problem hiding this comment.
God shell example has been added.
| } | ||
|
|
||
| private String requireBasicAuthHeader(byte[] tokenData) { | ||
| if (tokenData == null) { |
There was a problem hiding this comment.
using StringUtils.isBlank here, and the check below for 'authData ' can be simplified?
There was a problem hiding this comment.
Got the logical has been simplified
| throw new UnauthorizedException("Invalid username or password", BASIC_CHALLENGE); | ||
| } | ||
| return new BasicCredentials(username, password); | ||
| } catch (IllegalArgumentException e) { |
There was a problem hiding this comment.
Got cahtch base64 method, code has been modified.
| AuthConstants.AUTHORIZATION_BASIC_HEADER | ||
| + base64.b64encode(user_information.encode("utf-8")).decode("utf-8") | ||
| ).encode("utf-8") |
There was a problem hiding this comment.
Could you please improve the code's readability?
There was a problem hiding this comment.
Got.Revised the method.
| SIMPLE_AUTH_TYPE = "simple" | ||
| BASIC_AUTH_TYPE = "basic" | ||
| BASIC_USERNAME = "basic_username" | ||
| BASIC_PASSWORD = "basic_password" |
There was a problem hiding this comment.
Should you also update the Python client config user doc for these config items?
| * @param groupNames The group names the user belongs to. | ||
| */ | ||
| public IdpUser(String name, List<String> groupNames) { | ||
| public IdpUser(String name, String passwordHash, List<String> groupNames) { |
There was a problem hiding this comment.
Should you check the parameters?
There was a problem hiding this comment.
I split the constructor of IdpUser into two parts, one including passwordHash and the other not. The constructor that includes the "passwordHash" parameter will check that "passwordHash" cannot be empty.
| return "SELECT g.group_name as name," | ||
| + " '['" | ||
| + " || COALESCE(GROUP_CONCAT(" | ||
| + " CASE" | ||
| + " WHEN u.user_name IS NOT NULL AND u.user_name <> ''" | ||
| + " THEN '\"' || u.user_name || '\"'" | ||
| + " ELSE NULL" | ||
| + " END), '')" | ||
| + " || ']' as usernames" |
There was a problem hiding this comment.
Should this logic be handled within the Mapper?
There was a problem hiding this comment.
User and group mapper such logical patterns have been deleted.
| return currentProvider().selectIdpGroup(groupName); | ||
| } | ||
|
|
||
| public static String selectIdpGroupWithUsers(@Param("groupName") String groupName) { |
There was a problem hiding this comment.
should return List<String>
There was a problem hiding this comment.
This factory class returns SQL, it should be String.
…ss-control Add compileDistribution, deploy tests for MySQL/PostgreSQL, and DRY the embedded/deploy matrix while matching access-control integration test patterns. Co-authored-by: Cursor <cursoragent@cursor.com>
| * @return The built-in IdP user. | ||
| */ | ||
| public static IdpUser fromIdpUserWithGroupsPO(IdpUserWithGroupsPO userPO) { | ||
| Objects.requireNonNull(userPO, "userPO must not be null"); |
There was a problem hiding this comment.
use Preconditions.checkArgument to avoid throwing NPE
There was a problem hiding this comment.
All the methods in the class have been checked and modified.
Document HTTP Basic usage with username and password, aligned with the existing Simple mode shell example. Co-authored-by: Cursor <cursoragent@cursor.com>
…thenticator Use StringUtils.isBlank on authData after normalizing null tokenData to an empty string, reducing duplicate empty-header checks. Co-authored-by: Cursor <cursoragent@cursor.com>
…ator Catch IllegalArgumentException only around Base64.getDecoder().decode so credential parsing errors are not conflated with invalid base64 input. Co-authored-by: Cursor <cursoragent@cursor.com>
… readability Extract _build_basic_auth_token to separate credential encoding from header assembly, matching the structure of SimpleAuthProvider. Co-authored-by: Cursor <cursoragent@cursor.com>
Add basic_username and basic_password to the GVFS config table and include a Python usage example for built-in IDP Basic auth. Co-authored-by: Cursor <cursoragent@cursor.com>
…word hash Add a two-argument constructor for users without a loaded hash, validate constructor parameters, and update converters and call sites accordingly. Co-authored-by: Cursor <cursoragent@cursor.com>
…uctors Remove redundant null or empty checks in H2 and PostgreSQL join queries, and initialize passwordHash explicitly in the two-argument IdpUser constructor. Co-authored-by: Cursor <cursoragent@cursor.com>
Replace Objects.requireNonNull with Guava Preconditions.checkNotNull for consistency with other idp-basic modules. Co-authored-by: Cursor <cursoragent@cursor.com>
Set GRAVITINO_INITIAL_ADMIN_PASSWORD in the workflow and append it to gravitino-env.sh during deploy-mode IdpRESTApiIT so the packaged server can initialize configured service admins. Upload package-all server logs on failure for easier debugging. Co-authored-by: Cursor <cursoragent@cursor.com>
Deploy mode starts Gravitino via gravitino.sh without main() args; inheriting GRAVITINO_TEST from the module test env caused ArrayIndexOutOfBoundsException in GravitinoServer.main. Embedded IT still sets GRAVITINO_TEST via root build. Co-authored-by: Cursor <cursoragent@cursor.com>
| if: needs.changes.outputs.source_changes == 'true' && needs.changes.outputs.module_exists == 'true' | ||
| runs-on: ubuntu-22.04 | ||
| timeout-minutes: 60 | ||
| timeout-minutes: 90 |
| dev/ci/util_free_space.sh | ||
|
|
||
| - name: Run idp-basic tests | ||
| - name: Run idp-basic unit tests |
There was a problem hiding this comment.
unnecessary to seperate tests out
Merge unit and REST IT into one step, use nested loops for embedded/deploy across h2/mysql/postgresql, and drop redundant -PskipWeb on test tasks. Co-authored-by: Cursor <cursoragent@cursor.com>
What changes were proposed in this pull request?
Add built-in IdP Basic authentication and wire it into the Gravitino server, Java client, and distribution:
plugins:idp-basic:BasicAuthenticatorvalidates HTTP Basic credentials against relational IdP user metadata with Argon2id password hashes;ServiceAdminInitializercreates configured service admins on first startup whenGRAVITINO_INITIAL_ADMIN_PASSWORDis set.ServerPluginBootstrapSPI (ServerPluginBootstrapperinserver-common,IdpServerPluginBootstrapinidp-basic) so the server module does not compile-depend onidp-basic.GravitinoClient.builder(...).withBasicAuth(username, password)andBasicTokenProvider.docs/security/how-to-authenticate.md.Why are the changes needed?
Gravitino needs a built-in authentication path for deployments that do not use an external OAuth provider. This PR implements the
basicauthenticator mode, service-admin bootstrap for initial login, client support, and packaging so the feature is available in official binaries.Fix: #10965
Does this PR introduce any user-facing change?
Yes.
basicingravitino.authenticators(requiresgravitino.entity.store=relational).GRAVITINO_INITIAL_ADMIN_PASSWORD— JSON array ofusername:passwordentries for first-time service admin creation.GravitinoClientBase.Builder.withBasicAuth(String username, String password).How was this patch tested?
Unit tests added/updated in
plugins:idp-basic,server-common, andclients:client-java. Locally ran:./gradlew :plugins:idp-basic:test -PskipITs -PskipDockerTests=true./gradlew :server-common:test --tests "*BasicAuthentication*" -PskipITs -PskipDockerTests=true./gradlew :clients:client-java:test --tests "*BasicTokenProvider*" -PskipITs