Skip to content

[feat](plugin) Support multi-root auth plugin loading and normalize OIDC access token auth#62159

Merged
CalvinKirs merged 1 commit into
apache:masterfrom
CalvinKirs:oidc-regression-test
Apr 13, 2026
Merged

[feat](plugin) Support multi-root auth plugin loading and normalize OIDC access token auth#62159
CalvinKirs merged 1 commit into
apache:masterfrom
CalvinKirs:oidc-regression-test

Conversation

@CalvinKirs
Copy link
Copy Markdown
Member

@CalvinKirs CalvinKirs commented Apr 7, 2026

#60361

Problem Summary:
FE authentication plugin loading currently assumes a single plugin root in several runtime paths, which makes
external plugin deployment less flexible and inconsistent across callers.

In addition, OIDC token-based authentication uses inconsistent credential typing across MySQL authentication and
authentication_chain fallback flows, and the client-visible error message handling is not aligned with access-token-
specific failures.

This change makes plugin root parsing reusable across FE runtime paths, allows configuring multiple authentication
or authorization plugin roots with a comma-separated list, normalizes OIDC access token requests to use
OAUTH_TOKEN, broadens OIDC request detection in fallback authentication flow, and aligns access-token-related
failure messages. It also marks authentication_chain as mutable so the fallback chain can be adjusted dynamically.

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

…IDC access token auth

  Problem Summary:
  FE authentication plugin loading currently assumes a single plugin root in several runtime paths, which makes
  external plugin deployment less flexible and inconsistent across callers.

  In addition, OIDC token-based authentication uses inconsistent credential typing across MySQL authentication and
  authentication_chain fallback flows, and the client-visible error message handling is not aligned with access-token-
  specific failures.

  This change makes plugin root parsing reusable across FE runtime paths, allows configuring multiple authentication
  or authorization plugin roots with a comma-separated list, normalizes OIDC access token requests to use
  `OAUTH_TOKEN`, broadens OIDC request detection in fallback authentication flow, and aligns access-token-related
  failure messages. It also marks `authentication_chain` as mutable so the fallback chain can be adjusted dynamically.
@hello-stephen
Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@CalvinKirs
Copy link
Copy Markdown
Member Author

run buildall

@hello-stephen
Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 43.10% (25/58) 🎉
Increment coverage report
Complete coverage report

@CalvinKirs
Copy link
Copy Markdown
Member Author

/review

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings:

  1. fe/fe-core/src/main/java/org/apache/doris/common/util/ClassLoaderUtils.java: loadServicesFromDirectory() still interprets each configured entry as a flat jar directory, even though this PR changes authentication_plugins_dir / authorization_plugins_dir to documented plugin roots. AuthenticatorManager.loadCustomerFactories() and AccessControllerManager.loadAccessControllerPlugins() still call this helper, so a deployment laid out as <root>/<plugin>/<plugin>.jar will work for the new AuthenticationPluginManager paths but silently fail for legacy authentication_type=<plugin_name> and authorization factory loading.
  2. fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/integration/AuthenticationIntegrationAuthenticator.java and .../plugin/AuthenticationPluginAuthenticator.java: the sanitization branch for the old "OIDC token signature validation failed" prefix was removed. Older OIDC plugins or mixed-version deployments that still emit that message now fall through to startsWith("OIDC token ") and the full raw detail is returned to the client, which is a behavior regression and weakens the previous redaction.

Critical checkpoints:

  • Goal of the task: Partially achieved. Multi-root loading was updated for the new authentication plugin manager paths, but not for the remaining legacy loader paths, and the OIDC message normalization is not backward compatible with older emitters. Existing unit tests only cover the updated paths, not these regressions.
  • Small / clear / focused change: Mostly yes, but the config semantic change was not applied consistently to all parallel loading paths.
  • Concurrency: No new concurrency issue found in the touched code paths.
  • Lifecycle / static initialization: Acceptable. Switching pluginDirMapping to Supplier<String> correctly avoids stale config snapshots.
  • Configuration changes: Yes. authentication_chain becoming mutable looks fine, but the new plugin root semantics are inconsistent across callers.
  • Compatibility / incompatible changes: Problematic. Client-visible OIDC failure handling is not backward compatible with older plugins/message formats.
  • Parallel code paths: Problematic. Multi-root handling was updated in AuthenticationIntegrationRuntime and AuthenticationPluginAuthenticator, but not in the ClassLoaderUtils.loadServicesFromDirectory() callers used by legacy authentication and authorization loading.
  • Special conditional checks: The new OIDC message-prefix checks are incomplete for the old signature-validation prefix.
  • Test coverage: Insufficient for the changed surface. Added tests cover only the new manager paths and OAUTH_TOKEN plumbing, not legacy ClassLoaderUtils callers or old-message compatibility.
  • Observability: No additional observability requirement identified here.
  • Transaction / persistence: Not applicable.
  • Data writes / modifications: Not applicable.
  • FE/BE variable passing: Not applicable.
  • Performance: No material performance regression found in the touched code.
  • Other issues: None beyond the two findings above.

Overall opinion: the PR is close, but these two regressions should be addressed before merge.

throw new IOException("The specified path is not a directory: " + pluginRoot);
}

File[] jarFiles = jarDir.listFiles((dir, name) -> name.endsWith(".jar"));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helper is still used by AuthenticatorManager.loadCustomerFactories() and AccessControllerManager.loadAccessControllerPlugins(), but it still treats each configured entry as a flat jar directory. After this PR, the same configs are documented as plugin roots, and the newer AuthenticationPluginManager path now scans <root>/<plugin>/.... That means a deployment like authentication_plugins_dir=/opt/doris/plugins/authentication with jars under /opt/doris/plugins/authentication/my-auth/my-auth.jar will work for integration-based loading but fail for legacy authentication_type=<plugin_name> and authorization factory loading. The config semantic change needs to be applied consistently to these legacy callers too, or the docs/config text here stays misleading and introduces a real loading regression.

String detailMessage = Strings.nullToEmpty(exception.getMessage());
if (detailMessage.startsWith("OIDC token signature validation failed")) {
return "OIDC token signature validation failed";
if (detailMessage.startsWith("OIDC access token signature validation failed")) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This drops the old "OIDC token signature validation failed" sanitization path. In mixed-version or external-plugin setups that still emit the old prefix, the message now falls through the next branch (startsWith("OIDC token ")) and the full raw detail is sent back to the client instead of the generic redacted text they got before. That is a client-visible compatibility regression and should be preserved here (and in AuthenticationPluginAuthenticator) while adding the new OIDC access token ... variant.

Copy link
Copy Markdown
Contributor

@kaka11chen kaka11chen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Apr 13, 2026
@github-actions
Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@github-actions
Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@CalvinKirs
Copy link
Copy Markdown
Member Author

run cloud_p0

@CalvinKirs CalvinKirs merged commit 3de6d44 into apache:master Apr 13, 2026
33 of 34 checks passed
CalvinKirs added a commit to CalvinKirs/incubator-doris that referenced this pull request Apr 13, 2026
### What problem does this PR solve?

Issue Number: None

Related PR: apache#62159

Problem Summary: Preserve the original OIDC auth packet when native auth switch runs on branch-4.1, normalize OIDC access-token requests to OAUTH_TOKEN across auth packet extraction and authentication_chain fallback, and create AuthenticationPluginManager under AuthenticationPluginAuthenticator's classloader context so SPI lookup does not depend on the caller thread context classloader.

### Release note

None

### Check List (For Author)

- Test: FE unit test
    - mvn -f pom.xml -pl fe-core -am -Dskip.clean=true -Dtest=org.apache.doris.mysql.authenticate.AuthenticatorManagerTest,org.apache.doris.mysql.authenticate.MysqlAuthPacketCredentialExtractorTest,org.apache.doris.mysql.authenticate.plugin.AuthenticationPluginAuthenticatorTest test -DfailIfNoTests=false
- Behavior changed: Yes (branch-4.1 now preserves the original OIDC auth packet across native auth-switch fallback, uses OAUTH_TOKEN for OIDC access-token requests in fe-core auth flows, and resolves auth plugin SPI factories without relying on the caller thread context classloader)
- Does this need documentation: No
CalvinKirs added a commit to CalvinKirs/incubator-doris that referenced this pull request May 15, 2026
…IDC access token auth (apache#62159)

apache#60361

  Problem Summary:
FE authentication plugin loading currently assumes a single plugin root
in several runtime paths, which makes
external plugin deployment less flexible and inconsistent across
callers.

In addition, OIDC token-based authentication uses inconsistent
credential typing across MySQL authentication and
authentication_chain fallback flows, and the client-visible error
message handling is not aligned with access-token-
  specific failures.

This change makes plugin root parsing reusable across FE runtime paths,
allows configuring multiple authentication
or authorization plugin roots with a comma-separated list, normalizes
OIDC access token requests to use
`OAUTH_TOKEN`, broadens OIDC request detection in fallback
authentication flow, and aligns access-token-related
failure messages. It also marks `authentication_chain` as mutable so the
fallback chain can be adjusted dynamically.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. dev/4.1.x dev/4.1.x-conflict reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants