Skip to content

feat: add dedicated Logto OIDC config with custom JWS algorithm support#99

Open
SXFreell wants to merge 1 commit into
halo-sigs:mainfrom
SXFreell:main
Open

feat: add dedicated Logto OIDC config with custom JWS algorithm support#99
SXFreell wants to merge 1 commit into
halo-sigs:mainfrom
SXFreell:main

Conversation

@SXFreell

@SXFreell SXFreell commented Mar 30, 2026

Copy link
Copy Markdown

概述

新增 Logto 专用 OIDC 配置,并支持自定义 ID Token 签名算法。

变更内容

  • 为 OIDC ID Token 校验增加显式 jwsAlgorithm 配置
  • 新增 Logto 专用认证提供方和客户端注册配置
  • 新增 Logto 专用设置表单,使用单一 endpoint 字段完成配置
  • 根据配置的 endpoint 自动推导 Logto 的 issuerauthorizationtokenuserinfojwks 地址
  • 保持现有 SSO 自定义 OIDC 配置行为不变
  • 补充 Logto 算法解析、endpoint 规范化以及 SSO 兼容性的回归测试

变更原因

Logto 返回的 ID Token 可能使用 ES384 等非 RS256 签名算法,而插件此前默认按 RS256 处理,导致登录绑定失败。

同时,Logto 的 issuer 配置如果填写不准确,容易触发 ID Token 的 iss 校验失败。本次改动通过专用配置和 endpoint 自动推导,减少了这类配置错误。

验证情况

  • 执行 ./gradlew test
  • 确认现有 SSO 自定义 OIDC 配置链路未受影响
  • 确认 Logto 的 issuer 与 OIDC 相关端点能够正确推导

Fixes #97

新增 Logto 专用 OIDC 配置,并支持自定义 ID Token 签名算法。

Copilot AI review requested due to automatic review settings March 30, 2026 07:42
@f2c-ci-robot f2c-ci-robot Bot added the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Mar 30, 2026
@f2c-ci-robot

f2c-ci-robot Bot commented Mar 30, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign ruibaby for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a dedicated Logto OIDC provider/configuration path and introduces an explicit jwsAlgorithm setting to support non-RS256 ID Token signatures (e.g., ES384), while keeping the existing custom SSO OIDC behavior intact.

Changes:

  • Add Logto-specific AuthProvider + client registration + setting form (single endpoint), and derive issuer/OIDC endpoints from it.
  • Add jwsAlgorithm to Oauth2ClientRegistrationSpec, propagate it into ClientRegistration metadata, and update ID Token decoder algorithm resolution to honor it.
  • Add regression tests for Logto endpoint normalization, JWS algorithm resolution precedence/fallback, and extension resource contents.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/main/java/run/halo/oauth/OauthClientRegistrationRepository.java Adds Logto config parsing + endpoint derivation and injects jwsAlgorithm into provider metadata.
src/main/java/run/halo/oauth/Oauth2ClientRegistration.java Replaces configurationMetadata with explicit jwsAlgorithm field in the spec model.
src/main/java/run/halo/oauth/HaloOAuth2AuthenticationWebFilter.java Centralizes JWS algorithm resolution with explicit override support.
src/main/resources/extensions/auth-provider.yaml Adds logto AuthProvider definition and fixes indentation for SSO configMapRef.
src/main/resources/extensions/client-registrations.yaml Adds logto client registration (including jwsAlgorithm: ES384).
src/main/resources/extensions/setting.yaml Adds dedicated Logto settings form with endpoint field.
src/main/resources/static/logto.svg Adds Logto logo asset for provider UI.
src/test/java/run/halo/oauth/OauthClientRegistrationRepositoryTest.java Adds regression tests for SSO behavior and Logto endpoint derivation + metadata.
src/test/java/run/halo/oauth/HaloOAuth2AuthenticationWebFilterTest.java Adds tests for JWS algorithm resolution precedence and fallback behavior.
src/test/java/run/halo/oauth/Oauth2ExtensionResourcesTest.java Validates shipped extension YAML contains Logto resources and no private endpoint examples.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

var configuredJwsAlgorithm = configurationMetadata.get(JWS_ALGORITHM_METADATA_KEY);
if (configuredJwsAlgorithm instanceof String jwsAlgorithm
&& StringUtils.hasText(jwsAlgorithm)) {
return SignatureAlgorithm.from(jwsAlgorithm);

Copilot AI Mar 30, 2026

Copy link

Choose a reason for hiding this comment

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

resolveJwsAlgorithm calls SignatureAlgorithm.from(jwsAlgorithm) for the explicitly configured value without guarding against invalid/unknown algorithms. If a user misconfigures jwsAlgorithm, this will throw IllegalArgumentException and can break the authentication flow; consider catching it (similar to the metadata path) and either fallback to provider metadata/default or surface a clearer configuration error.

Suggested change
return SignatureAlgorithm.from(jwsAlgorithm);
try {
return SignatureAlgorithm.from(jwsAlgorithm);
} catch (IllegalArgumentException ex) {
log.warn(
"Invalid JWS algorithm '{}' configured for client '{}' in metadata key '{}'; "
+ "falling back to provider metadata/default.",
jwsAlgorithm,
clientRegistration.getRegistrationId(),
JWS_ALGORITHM_METADATA_KEY,
ex
);
}

Copilot uses AI. Check for mistakes.
@Component
public class HaloOAuth2AuthenticationWebFilter implements AuthenticationSecurityWebFilter {

static final String JWS_ALGORITHM_METADATA_KEY = "jwsAlgorithm";

Copilot AI Mar 30, 2026

Copy link

Choose a reason for hiding this comment

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

The metadata key "jwsAlgorithm" is defined separately here and in OauthClientRegistrationRepository. Duplicating the literal/constant in multiple places risks future drift (one side changes and the other doesn’t), which would silently disable the override; consider centralizing the key in a shared constant (or referencing one class’s constant) to keep writer/reader aligned.

Suggested change
static final String JWS_ALGORITHM_METADATA_KEY = "jwsAlgorithm";
static final String JWS_ALGORITHM_METADATA_KEY =
OauthClientRegistrationRepository.JWS_ALGORITHM_METADATA_KEY;

Copilot uses AI. Check for mistakes.
Comment on lines +80 to +87
if (StringUtils.isBlank(value) || "{}".equals(value.trim())) {
String legacyValue = data.getOrDefault(ADVANCED_OIDC_SETTING_GROUP, "{}");
if (!"{}".equals(legacyValue.trim())) {
SsoClientConf ssoClientConf =
JsonUtils.jsonToObject(legacyValue, SsoClientConf.class);
return SsoClientRegistration(ssoClientConf, authProvider);
}
}

Copilot AI Mar 30, 2026

Copy link

Choose a reason for hiding this comment

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

LOGTO_SETTING_GROUP handling attempts to fall back to ADVANCED_OIDC_SETTING_GROUP data from the same provider ConfigMap. However, the shipped extension resources use different ConfigMaps for sso (oauth2-sso-config) and logto (oauth2-logto-config), so this fallback won’t migrate existing SSO-based Logto settings unless users manually copy the old group key into the new ConfigMap. If migration is intended, consider fetching from the SSO ConfigMap explicitly; otherwise removing this branch would avoid confusing, hard-to-reason-about behavior.

Suggested change
if (StringUtils.isBlank(value) || "{}".equals(value.trim())) {
String legacyValue = data.getOrDefault(ADVANCED_OIDC_SETTING_GROUP, "{}");
if (!"{}".equals(legacyValue.trim())) {
SsoClientConf ssoClientConf =
JsonUtils.jsonToObject(legacyValue, SsoClientConf.class);
return SsoClientRegistration(ssoClientConf, authProvider);
}
}

Copilot uses AI. Check for mistakes.
Comment on lines +65 to +76
private static SignatureAlgorithm resolveJwsAlgorithm(ClientRegistration registration) {
try {
Method method = HaloOAuth2AuthenticationWebFilter.class.getDeclaredMethod(
"resolveJwsAlgorithm", ClientRegistration.class
);
method.setAccessible(true);
return (SignatureAlgorithm) method.invoke(null, registration);
} catch (NoSuchMethodException e) {
fail("Expected resolveJwsAlgorithm(ClientRegistration) helper to exist", e);
} catch (IllegalAccessException | InvocationTargetException e) {
fail("Failed to invoke resolveJwsAlgorithm(ClientRegistration)", e);
}

Copilot AI Mar 30, 2026

Copy link

Choose a reason for hiding this comment

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

This test uses reflection to invoke HaloOAuth2AuthenticationWebFilter.resolveJwsAlgorithm, but the method is package-visible and the test is in the same run.halo.oauth package, so it can be called directly. Dropping reflection would make the test simpler and less brittle (no setAccessible(true) / NoSuchMethodException handling needed).

Copilot uses AI. Check for mistakes.
Comment on lines +30 to +34
assertThat(content).contains("name: logto");
assertThat(content).contains("clientName: \"Logto\"");
assertThat(content).contains("redirectUri: \"{baseUrl}/login/oauth2/code/logto\"");
assertThat(content).contains("jwsAlgorithm: \"ES384\"");
assertThat(content).doesNotContain("configurationMetadata:");

Copilot AI Mar 30, 2026

Copy link

Choose a reason for hiding this comment

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

clientRegistrationsShouldContainLogtoRegistrationAndExplicitJwsAlgorithm asserts the entire client-registrations.yaml does not contain configurationMetadata:. That’s a broad, file-level invariant and could cause unrelated future changes (e.g., another provider legitimately adding metadata) to break this test; consider scoping the assertion to the logto registration block instead.

Suggested change
assertThat(content).contains("name: logto");
assertThat(content).contains("clientName: \"Logto\"");
assertThat(content).contains("redirectUri: \"{baseUrl}/login/oauth2/code/logto\"");
assertThat(content).contains("jwsAlgorithm: \"ES384\"");
assertThat(content).doesNotContain("configurationMetadata:");
var logtoNameMarker = "name: logto";
int logtoIndex = content.indexOf(logtoNameMarker);
assertThat(logtoIndex).as("logto client registration should exist").isGreaterThanOrEqualTo(0);
int nextRegistrationIndex = content.indexOf("\n- name:", logtoIndex + logtoNameMarker.length());
String logtoRegistrationBlock = nextRegistrationIndex >= 0
? content.substring(logtoIndex, nextRegistrationIndex)
: content.substring(logtoIndex);
assertThat(logtoRegistrationBlock).contains("name: logto");
assertThat(logtoRegistrationBlock).contains("clientName: \"Logto\"");
assertThat(logtoRegistrationBlock).contains("redirectUri: \"{baseUrl}/login/oauth2/code/logto\"");
assertThat(logtoRegistrationBlock).contains("jwsAlgorithm: \"ES384\"");
assertThat(logtoRegistrationBlock).doesNotContain("configurationMetadata:");

Copilot uses AI. Check for mistakes.
@f2c-ci-robot f2c-ci-robot Bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-note Denotes a PR that will be considered when it comes time to generate release notes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Logto支持问题

2 participants