Skip to content

Conversation

@izumo27
Copy link
Contributor

@izumo27 izumo27 commented Dec 9, 2025

Motivation

The client’s OAuth2 authentication plugin can be used for OIDC.
So, the authorization server metadata path is fixed to /.well-known/openid-configuration.

However, RFC 8414 defines /.well-known/oauth-authorization-server as the default, and some users may want to configure a different path.
https://datatracker.ietf.org/doc/html/rfc8414#section-3
This PR makes the authorization server metadata path configurable.

Modifications

  • Added the wellKnownMetadataPath parameter to make the authorization server metadata path configurable.
  • Added the AuthenticationOAuth2StandardAuthzServer class and the clientCredentialsWithStandardAuthzServerBuilder builder, which preconfigure the standard path /.well-known/openid-configuration as defined in RFC 8414.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • Added tests for configurable authorization server metadata path

Does this pull request potentially affect one of the following parts:

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: izumo27#6

@github-actions github-actions bot added the doc-required Your PR changes impact docs and you will update later. label Dec 9, 2025
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 72.22222% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.38%. Comparing base (3937788) to head (3f4049c).
⚠️ Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
...auth2/AuthenticationOAuth2StandardAuthzServer.java 52.94% 6 Missing and 2 partials ⚠️
.../auth/oauth2/protocol/DefaultMetadataResolver.java 75.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #25052       +/-   ##
=============================================
+ Coverage     38.50%   74.38%   +35.88%     
- Complexity    13234    34133    +20899     
=============================================
  Files          1863     1922       +59     
  Lines        146150   150408     +4258     
  Branches      16973    17471      +498     
=============================================
+ Hits          56272   111878    +55606     
+ Misses        82185    29604    -52581     
- Partials       7693     8926     +1233     
Flag Coverage Δ
inttests 26.38% <0.00%> (-0.01%) ⬇️
systests 22.94% <0.00%> (+0.11%) ⬆️
unittests 73.91% <72.22%> (+39.27%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
.../impl/auth/oauth2/AuthenticationFactoryOAuth2.java 94.11% <100.00%> (+94.11%) ⬆️
...client/impl/auth/oauth2/ClientCredentialsFlow.java 80.00% <100.00%> (+80.00%) ⬆️
...pache/pulsar/client/impl/auth/oauth2/FlowBase.java 67.85% <100.00%> (+67.85%) ⬆️
.../auth/oauth2/protocol/DefaultMetadataResolver.java 71.42% <75.00%> (+71.42%) ⬆️
...auth2/AuthenticationOAuth2StandardAuthzServer.java 52.94% <52.94%> (ø)

... and 1417 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Technoboy- Technoboy- added this to the 4.2.0 milestone Dec 12, 2025
Copy link
Contributor

@Technoboy- Technoboy- left a comment

Choose a reason for hiding this comment

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

@poorbarcode @lhotari could you help review this ?

Comment on lines +33 to +70
public class AuthenticationOAuth2StandardAuthzServer extends AuthenticationOAuth2 {

private static final long serialVersionUID = 1L;

public AuthenticationOAuth2StandardAuthzServer() {
super();
}

AuthenticationOAuth2StandardAuthzServer(Flow flow, Clock clock) {
super(flow, clock);
}

@Override
public void configure(String encodedAuthParamString) {
if (StringUtils.isBlank(encodedAuthParamString)) {
throw new IllegalArgumentException("No authentication parameters were provided");
}
Map<String, String> params;
try {
params = AuthenticationUtil.configureFromJsonString(encodedAuthParamString);
} catch (IOException e) {
throw new IllegalArgumentException("Malformed authentication parameters", e);
}

// Always set the OAuth 2.0 standard metadata path
params.put(FlowBase.CONFIG_PARAM_WELL_KNOWN_METADATA_PATH,
DefaultMetadataResolver.OAUTH_WELL_KNOWN_METADATA_PATH);

String type = params.getOrDefault(CONFIG_PARAM_TYPE, TYPE_CLIENT_CREDENTIALS);
switch(type) {
case TYPE_CLIENT_CREDENTIALS:
this.flow = ClientCredentialsFlow.fromParameters(params);
break;
default:
throw new IllegalArgumentException("Unsupported authentication type: " + type);
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

What's the purpose of this class? Could we eliminate this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This class allows users to switch authentication plugins and use the standard metadata path defined in RFC 8414 without additional configuration.
It’s especially useful for CLI tool, where ClientCredentialsBuilder cannot be used.

@lhotari
Copy link
Member

lhotari commented Dec 15, 2025

However, RFC 8414 defines /.well-known/oauth-authorization-server as the default, and some users may want to configure a different path.
https://datatracker.ietf.org/doc/html/rfc8414#section-3
This PR makes the authorization server metadata path configurable

@izumo27 It seems that according to RFC 8414, the logic is also different than what there is in the current implementation.

https://www.rfc-editor.org/rfc/rfc8414.html#section-3.1
More specifically this part:

If the issuer identifier value contains a path component, any
terminating "/" MUST be removed before inserting "/.well-known/" and
the well-known URI suffix between the host component and the path
component. The client would make the following request when the
issuer identifier is "https://example.com/issuer1" and the well-known
URI suffix is "oauth-authorization-server" to obtain the metadata,
since the issuer identifier contains a path component:

 GET /.well-known/oauth-authorization-server/issuer1 HTTP/1.1
 Host: example.com

Instead of appending .well-known/oauth-authorization-server to the path, the logic should be to prefix the path with /.well-known/oauth-authorization-server.

@izumo27
Copy link
Contributor Author

izumo27 commented Dec 18, 2025

@lhotari Thank you for the comment. I’ve fixed it.

  • Changed the logic to insert well-known path before the issuer path when wellKnownMetadataPath is configured.
  • For OIDC, keep the existing implementation that appends well-known path after the issuer path when wellKnownMetadataPath is empty.

The RP would make the following request to the Issuer https://example.com/issuer1 to obtain its configuration information, since the Issuer contains a path component:

  GET /issuer1/.well-known/openid-configuration HTTP/1.1
  Host: example.com

https://openid.net/specs/openid-connect-discovery-1_0.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-required Your PR changes impact docs and you will update later. ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants