From 3de0b94a9a398ad842607c139a12d8066ff9e166 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Tue, 22 Sep 2026 15:24:31 +0800 Subject: [PATCH] fix(auth): harden oauth token and claim logging (#895) Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- docs/03-authentication-design.md | 7 ++ .../enterprise-identity-platform/design.md | 8 +- .../enterprise-identity-platform/tasks.md | 2 +- server/pom.xml | 1 + .../auth/oauth/CustomOidcUserService.java | 13 ++- .../oauth/DispatchingTokenResponseClient.java | 3 +- ...FeishuOAuth2AccessTokenResponseClient.java | 5 +- .../auth/oauth/GitLabClaimsExtractor.java | 12 ++- .../oauth/OAuth2TokenResponseClients.java | 96 ++++++++++++++++++ .../auth/oauth/CustomOidcUserServiceTest.java | 49 ++++++++++ .../auth/oauth/GitLabClaimsExtractorTest.java | 45 +++++++++ .../oauth/OAuth2TokenResponseClientsTest.java | 98 +++++++++++++++++++ 12 files changed, 321 insertions(+), 18 deletions(-) create mode 100644 server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuth2TokenResponseClients.java create mode 100644 server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2TokenResponseClientsTest.java diff --git a/docs/03-authentication-design.md b/docs/03-authentication-design.md index 19f6140f..9fa2a179 100644 --- a/docs/03-authentication-design.md +++ b/docs/03-authentication-design.md @@ -334,6 +334,13 @@ fallback,否则同一个人会被拆成两个平台账号)、只有在 Provi 所有权时才置 `emailVerified=true`、远程调用要有超时与响应大小上限、 claims 提取过程不记录 subject/email/token。 +钉钉公共 Provider 的主 subject 固定为 `unionId`。`openId` 是应用作用域,`userId` +是组织作用域,都不能作为登录时的自动 fallback;否则一次字段缺失或连接变化就可能让 +同一用户生成新的平台账号。当前版本不执行别名迁移。后续如果要兼容历史 `openId` / +`userId` 绑定、切换 subject,或把协议代码复用到企业钉钉连接,必须走显式迁移: +先离线生成候选 alias 与冲突报告,再经管理员确认写入 alias/binding 记录;运行时 +不得静默改键,也不得仅凭邮箱或昵称合并账号。 + #### 飞书 token 协议版本 飞书 token client 支持显式选择 `v2` 或 `v3`,默认值为 `v3`: diff --git a/openspec/changes/enterprise-identity-platform/design.md b/openspec/changes/enterprise-identity-platform/design.md index 9cf0fa9f..0b66d0ef 100644 --- a/openspec/changes/enterprise-identity-platform/design.md +++ b/openspec/changes/enterprise-identity-platform/design.md @@ -77,11 +77,11 @@ Provider Adapter 首批统一身份核心应先证明所有外部公共登录都会经过同一个账号状态、绑定、资料权威和 session 决策门禁;公共 OAuth 的持久化仍可保留现有 legacy binding,避免第一批迁移存量 GitHub/GitLab 身份。企业 Organization 登录复用同一核心并增加企业上下文。不能再新增 `FeishuLoginService`、`DingTalkLoginService` 这类各自建号、绑定和建 session 的旁路。 -### 现有飞书、钉钉 PR 与统一身份架构的接入边界(设计建议,尚未实现) +### 飞书、钉钉 Provider 与统一身份架构的接入边界 -2026-09-17 只读核对的开放 PR:飞书 [#696](https://github.com/iflytek/skillhub/pull/696),head `d3f1d5e65af8139a6021eca52a5d7a114156f3c3`;钉钉 [#467](https://github.com/iflytek/skillhub/pull/467),head `0b21fe2f34c6901abf5bc017e37f1d432e66fe33`。这里只说明接入方向,不构成代码 Review 通过或合并准备结论。 +2026-09-22 状态:R1-A 统一身份核心已合并;飞书公共 Provider Adapter 已合并但仍需真实厂商登录验收;钉钉公共 Provider Adapter 已合并并完成真实钉钉往返验证修正。二者仍是公共平台登录能力,不等同于每个 Organization 单独配置凭证、策略与成员上下文的企业登录。 -两者当前主要提供部署级公共 OAuth 登录:配置一个 Provider registration,匿名 catalog 暴露按钮,通过旧 callback/claims/binding 路径登录。它们不等同于每个 Organization 单独配置凭证、策略与成员上下文的企业登录。当前本地公共 OAuth 已经过 LegacyPlatformIdentityCoreBridge 的统一决策评估,但旧 binding persistence 仍是权威;企业 redirect 则经 Adapter、IdentityAssertion、EnterpriseIdentityAssociationService 和企业 session。不能描述为所有写入路径已经统一迁移完成。 +公共 Provider 当前主要提供部署级 OAuth 登录:配置一个 Provider registration,匿名 catalog 暴露按钮,通过公共 OAuth callback 进入统一身份核心门禁,同时 legacy `identity_binding` persistence 仍是公共 OAuth 的写入权威。企业 redirect 后续经 Adapter、IdentityAssertion、EnterpriseIdentityAssociationService 和企业 session。不能描述为所有写入路径已经统一迁移到 Binding V2。 接入分三批,而不是为每家厂商复制身份核心: @@ -91,7 +91,7 @@ Provider Adapter 公共按钮与企业连接可以并存:公共按钮使用平台配置,企业入口先确定 Organization/connection 再跳转其身份源。共享协议客户端与验证逻辑,不共享租户凭证或放宽租户边界。前端公共图标 resolver 当前只识别 GitHub/GitLab/OIDC,接入厂商 PR 时需同步支持其已提供的真实图标;不能只合后端然后声称完整入口已接通。 -身份坐标必须包含可信的组织/连接上下文、issuer 与 typed subject;裸 open_id/unionId/userId、昵称或邮箱不能代表全球唯一企业身份。钉钉 PR 的 unionId→openId→userId fallback 在字段可用性变化时可能改变主 subject,接入前需定义稳定主 subject 和有证明的 alias/迁移策略,不能静默改键。飞书的 open_id 需保留应用作用域;union_id 不自动证明跨应用或跨组织可合号。邮箱必须有可靠的验证依据才进入 VerifiedEmail,禁止仅因返回邮箱或名称相同而绑定。 +身份坐标必须包含可信的组织/连接上下文、issuer 与 typed subject;裸 open_id/unionId/userId、昵称或邮箱不能代表全球唯一企业身份。钉钉公共 Provider 的稳定主 subject 是 `unionId`,不以 `openId` 或 `userId` 做运行时 fallback:`openId` 按应用隔离,`userId` 按组织隔离,任一字段可用性变化都可能把同一人拆成不同平台账号。当前 R1-A2 不做别名迁移;如果将来需要接受历史 `openId`/`userId`、切换 subject 或支持企业钉钉连接,必须以独立迁移完成:先生成候选 alias 冲突报告,再由管理员/运维确认,最后写入显式 alias/binding 记录。运行时登录不得因为“找不到 unionId”而静默改用其他字段,也不得仅凭邮箱或昵称合号。飞书的 open_id 需保留应用作用域;union_id 不自动证明跨应用或跨组织可合号。邮箱必须有可靠的验证依据才进入 VerifiedEmail,禁止仅因返回邮箱或名称相同而绑定。 组织、部门与人员目录同步属于后续 provisioning connection,不能复用登录 token 暗中拉取通讯录。实现前分别验证公共登录回归、企业连接隔离、旧 binding 兼容、验证 email、账号禁用与回调防重放;当前没有执行真实飞书/钉钉登录验收。 diff --git a/openspec/changes/enterprise-identity-platform/tasks.md b/openspec/changes/enterprise-identity-platform/tasks.md index daf859cd..a5f4c13c 100644 --- a/openspec/changes/enterprise-identity-platform/tasks.md +++ b/openspec/changes/enterprise-identity-platform/tasks.md @@ -89,6 +89,6 @@ - [x] 9.2.1 Make ACTIVE public OAuth fail closed on unified-core Denied/Conflict before creating active or pending legacy bindings; LEGACY and SHADOW keep existing behavior. - [ ] 9.2.2 Optional follow-up: evaluate whether platform-scoped public OAuth should switch runtime write authority to V2 after R1-A is stable; do not block unified authentication rollout on this cutover. - [ ] 9.3 Rework the Feishu PR as a public Provider Adapter first, with stable subject, verified email semantics, catalog rendering, real login validation, and no enterprise side effects. -- [ ] 9.4 Rework the DingTalk PR as a public Provider Adapter first; define a stable primary subject and explicit alias/migration policy before any merge. +- [x] 9.4 Rework the DingTalk PR as a public Provider Adapter first; define a stable primary subject and explicit alias/migration policy before any merge. - [ ] 9.5 Add a repeatable public-provider acceptance path using real vendor test credentials or an explicit reference service for each provider; mocked catalog buttons only prove presentation and navigation. - [ ] 9.6 Add enterprise Feishu/DingTalk Login Connection adapters only after public-provider behavior is stable, reusing protocol code but adding Organization/LoginConnection context and enterprise session guards. diff --git a/server/pom.xml b/server/pom.xml index 8322b723..6c59c464 100644 --- a/server/pom.xml +++ b/server/pom.xml @@ -21,6 +21,7 @@ 21 21 21 + 1.17.5 UTF-8 diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/CustomOidcUserService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/CustomOidcUserService.java index 9dc967b5..d719899d 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/CustomOidcUserService.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/CustomOidcUserService.java @@ -49,15 +49,20 @@ public class CustomOidcUserService implements OAuth2UserService providerClients) { - this(providerClients, new DefaultAuthorizationCodeTokenResponseClient()); + this(providerClients, OAuth2TokenResponseClients.standard()); } DispatchingTokenResponseClient( diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/FeishuOAuth2AccessTokenResponseClient.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/FeishuOAuth2AccessTokenResponseClient.java index 5a03639a..d33e023e 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/FeishuOAuth2AccessTokenResponseClient.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/FeishuOAuth2AccessTokenResponseClient.java @@ -16,7 +16,6 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.http.client.ClientHttpRequestFactory; import org.springframework.http.client.SimpleClientHttpRequestFactory; -import org.springframework.security.oauth2.client.endpoint.DefaultAuthorizationCodeTokenResponseClient; import org.springframework.security.oauth2.client.endpoint.OAuth2AccessTokenResponseClient; import org.springframework.security.oauth2.client.endpoint.OAuth2AuthorizationCodeGrantRequest; import org.springframework.security.oauth2.core.OAuth2AccessToken; @@ -55,12 +54,12 @@ public class FeishuOAuth2AccessTokenResponseClient public FeishuOAuth2AccessTokenResponseClient( @Value("${OAUTH2_FEISHU_PROTOCOL_VERSION:v3}") String protocolVersion) { this(RestClient.builder().requestFactory(defaultRequestFactory()), - new DefaultAuthorizationCodeTokenResponseClient(), protocolVersion); + OAuth2TokenResponseClients.standard(), protocolVersion); } FeishuOAuth2AccessTokenResponseClient( RestClient.Builder restClientBuilder) { - this(restClientBuilder, new DefaultAuthorizationCodeTokenResponseClient(), V3); + this(restClientBuilder, OAuth2TokenResponseClients.standard(), V3); } FeishuOAuth2AccessTokenResponseClient( diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/GitLabClaimsExtractor.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/GitLabClaimsExtractor.java index 6f0a87b4..dcdfba39 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/GitLabClaimsExtractor.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/GitLabClaimsExtractor.java @@ -58,7 +58,8 @@ public class GitLabClaimsExtractor implements OAuthClaimsExtractor { boolean emailVerified = isConfirmed(attrs.get("confirmed_at")); - log.debug("Initial email from GitLab: {}, verified: {}", email, emailVerified); + log.debug("Initial GitLab email state: emailPresent={}, emailVerified={}", + email != null && !email.isBlank(), emailVerified); // If email is not verified or not present, try to fetch from emails API if (email == null || !emailVerified) { @@ -67,7 +68,7 @@ public class GitLabClaimsExtractor implements OAuthClaimsExtractor { if (primaryEmail != null) { email = primaryEmail.email(); emailVerified = true; - log.debug("Found verified email from GitLab API: {}", email); + log.debug("Found verified email from GitLab API: emailPresent=true"); } else { log.debug("No verified email found from GitLab emails API"); } @@ -80,8 +81,11 @@ public class GitLabClaimsExtractor implements OAuthClaimsExtractor { } String subject = String.valueOf(attrs.get("id")); - log.info("GitLab OAuth claims extracted - subject: {}, username: {}, email: {}, emailVerified: {}", - subject, username, email, emailVerified); + log.info("GitLab OAuth claims extracted: subjectPresent={}, usernamePresent={}, emailPresent={}, emailVerified={}", + subject != null && !subject.isBlank(), + username != null && !username.isBlank(), + email != null && !email.isBlank(), + emailVerified); return new OAuthClaims( "gitlab", diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuth2TokenResponseClients.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuth2TokenResponseClients.java new file mode 100644 index 00000000..8d252a2e --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuth2TokenResponseClients.java @@ -0,0 +1,96 @@ +package com.iflytek.skillhub.auth.oauth; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.time.Duration; +import java.util.List; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatusCode; +import org.springframework.http.client.ClientHttpResponse; +import org.springframework.http.client.SimpleClientHttpRequestFactory; +import org.springframework.http.converter.FormHttpMessageConverter; +import org.springframework.security.oauth2.client.endpoint.DefaultAuthorizationCodeTokenResponseClient; +import org.springframework.security.oauth2.client.http.OAuth2ErrorResponseErrorHandler; +import org.springframework.security.oauth2.core.http.converter.OAuth2AccessTokenResponseHttpMessageConverter; +import org.springframework.web.client.RestTemplate; + +/** + * Factory for the standard OAuth2 authorization-code token client used by public providers. + * + *

Spring's default client does not cap response size. Public provider endpoints should still be + * treated as untrusted remote IO, so the shared client keeps the standard parser but adds timeouts + * and a bounded response body before any converter sees the payload. + */ +final class OAuth2TokenResponseClients { + + static final int MAX_RESPONSE_BYTES = 64 * 1024; + + private static final Duration CONNECT_TIMEOUT = Duration.ofSeconds(5); + private static final Duration READ_TIMEOUT = Duration.ofSeconds(10); + + private OAuth2TokenResponseClients() { + } + + static DefaultAuthorizationCodeTokenResponseClient standard() { + DefaultAuthorizationCodeTokenResponseClient client = + new DefaultAuthorizationCodeTokenResponseClient(); + client.setRestOperations(standardRestTemplate()); + return client; + } + + /** Package-visible so tests can exercise the production HTTP policy directly. */ + static RestTemplate standardRestTemplate() { + SimpleClientHttpRequestFactory factory = new SimpleClientHttpRequestFactory(); + factory.setConnectTimeout(CONNECT_TIMEOUT); + factory.setReadTimeout(READ_TIMEOUT); + + RestTemplate template = new RestTemplate(List.of( + new FormHttpMessageConverter(), + new OAuth2AccessTokenResponseHttpMessageConverter() + )); + template.setRequestFactory(factory); + template.setErrorHandler(new OAuth2ErrorResponseErrorHandler()); + template.getInterceptors().add((request, body, execution) -> { + ClientHttpResponse response = execution.execute(request, body); + byte[] bytes = response.getBody().readNBytes(MAX_RESPONSE_BYTES + 1); + if (bytes.length > MAX_RESPONSE_BYTES) { + response.close(); + throw new IOException("OAuth2 token response exceeds " + + MAX_RESPONSE_BYTES + " bytes"); + } + return new BoundedClientHttpResponse(response, bytes); + }); + return template; + } + + /** Replays the already-read, size-checked body so Spring's converters can parse it normally. */ + private record BoundedClientHttpResponse(ClientHttpResponse delegate, byte[] body) + implements ClientHttpResponse { + + @Override + public HttpStatusCode getStatusCode() throws IOException { + return delegate.getStatusCode(); + } + + @Override + public String getStatusText() throws IOException { + return delegate.getStatusText(); + } + + @Override + public void close() { + delegate.close(); + } + + @Override + public InputStream getBody() { + return new ByteArrayInputStream(body); + } + + @Override + public HttpHeaders getHeaders() { + return delegate.getHeaders(); + } + } +} diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/CustomOidcUserServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/CustomOidcUserServiceTest.java index d32fc6ae..eb4a4d89 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/CustomOidcUserServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/CustomOidcUserServiceTest.java @@ -1,5 +1,9 @@ package com.iflytek.skillhub.auth.oauth; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import java.time.Instant; import java.util.List; @@ -7,6 +11,7 @@ import java.util.Map; import java.util.Set; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; +import org.slf4j.LoggerFactory; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.oauth2.client.oidc.userinfo.OidcUserRequest; @@ -15,6 +20,7 @@ import org.springframework.security.oauth2.client.userinfo.OAuth2UserService; import org.springframework.security.oauth2.core.AuthorizationGrantType; import org.springframework.security.oauth2.core.OAuth2AccessToken; import org.springframework.security.oauth2.core.OAuth2AuthenticationException; +import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.oidc.IdTokenClaimNames; import org.springframework.security.oauth2.core.oidc.OidcIdToken; import org.springframework.security.oauth2.core.oidc.OidcUserInfo; @@ -188,6 +194,49 @@ class CustomOidcUserServiceTest { assertThat(captured.subject()).isEqualTo("oidc-sub-unverified"); } + @Test + void loadUser_logsPresenceFlagsWithoutSubjectEmailOrFailureDescription() { + OAuthLoginFlowService loginFlowService = mock(OAuthLoginFlowService.class); + OAuth2UserService delegate = mock(); + CustomOidcUserService service = new CustomOidcUserService(loginFlowService, delegate); + OidcUserRequest request = oidcRequest(); + OidcUser upstreamUser = oidcUser(Map.of( + IdTokenClaimNames.SUB, "sensitive-oidc-subject", + "email", "sensitive@example.com", + "email_verified", true, + "preferred_username", "sensitive-user" + )); + when(delegate.loadUser(request)).thenReturn(upstreamUser); + when(loginFlowService.authenticate(any())) + .thenThrow(new OAuth2AuthenticationException( + new OAuth2Error("access_denied", + "subject sensitive-oidc-subject email sensitive@example.com", null))); + + Logger logger = (Logger) LoggerFactory.getLogger(CustomOidcUserService.class); + Level previousLevel = logger.getLevel(); + ListAppender appender = new ListAppender<>(); + appender.start(); + logger.setLevel(Level.DEBUG); + logger.addAppender(appender); + try { + assertThatThrownBy(() -> service.loadUser(request)) + .isInstanceOf(OAuth2AuthenticationException.class); + } finally { + logger.detachAppender(appender); + logger.setLevel(previousLevel); + appender.stop(); + } + + String logged = appender.list.stream() + .map(ILoggingEvent::getFormattedMessage) + .collect(java.util.stream.Collectors.joining("\n")); + assertThat(logged).contains("subjectPresent=true", "emailPresent=true", "errorCode=access_denied"); + assertThat(logged) + .doesNotContain("sensitive-oidc-subject") + .doesNotContain("sensitive@example.com") + .doesNotContain("sensitive-user"); + } + private static OidcUserRequest oidcRequest() { Instant issuedAt = Instant.parse("2026-04-24T00:00:00Z"); OidcIdToken idToken = new OidcIdToken( diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/GitLabClaimsExtractorTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/GitLabClaimsExtractorTest.java index 643be583..c94c9b09 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/GitLabClaimsExtractorTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/GitLabClaimsExtractorTest.java @@ -1,5 +1,9 @@ package com.iflytek.skillhub.auth.oauth; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.test.web.client.match.MockRestRequestMatchers.header; import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo; @@ -8,6 +12,7 @@ import static org.springframework.test.web.client.response.MockRestResponseCreat import java.time.Instant; import java.util.Map; import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.security.oauth2.client.registration.ClientRegistration; @@ -79,6 +84,46 @@ class GitLabClaimsExtractorTest { server.verify(); } + @Test + void extract_logsPresenceFlagsWithoutSubjectUsernameOrEmailValues() { + RestClient.Builder restClientBuilder = RestClient.builder(); + GitLabClaimsExtractor extractor = new GitLabClaimsExtractor(restClientBuilder); + Logger logger = (Logger) LoggerFactory.getLogger(GitLabClaimsExtractor.class); + Level previousLevel = logger.getLevel(); + ListAppender appender = new ListAppender<>(); + appender.start(); + logger.setLevel(Level.DEBUG); + logger.addAppender(appender); + try { + extractor.extract( + userRequest(), + new DefaultOAuth2User( + java.util.List.of(), + Map.of( + "id", "gitlab-subject-42", + "username", "alice-sensitive", + "email", "alice@gitlab.example", + "confirmed_at", "2026-04-16T08:00:00Z" + ), + "username" + ) + ); + } finally { + logger.detachAppender(appender); + logger.setLevel(previousLevel); + appender.stop(); + } + + String logged = appender.list.stream() + .map(ILoggingEvent::getFormattedMessage) + .collect(java.util.stream.Collectors.joining("\n")); + assertThat(logged).contains("subjectPresent=true", "usernamePresent=true", "emailPresent=true"); + assertThat(logged) + .doesNotContain("gitlab-subject-42") + .doesNotContain("alice-sensitive") + .doesNotContain("alice@gitlab.example"); + } + private OAuth2UserRequest userRequest() { ClientRegistration registration = ClientRegistration.withRegistrationId("gitlab") .clientId("client-id") diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2TokenResponseClientsTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2TokenResponseClientsTest.java new file mode 100644 index 00000000..cfd38da6 --- /dev/null +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2TokenResponseClientsTest.java @@ -0,0 +1,98 @@ +package com.iflytek.skillhub.auth.oauth; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo; +import static org.springframework.test.web.client.response.MockRestResponseCreators.withSuccess; + +import org.junit.jupiter.api.Test; +import org.springframework.http.MediaType; +import org.springframework.security.oauth2.client.endpoint.DefaultAuthorizationCodeTokenResponseClient; +import org.springframework.security.oauth2.client.endpoint.OAuth2AuthorizationCodeGrantRequest; +import org.springframework.security.oauth2.client.registration.ClientRegistration; +import org.springframework.security.oauth2.core.AuthorizationGrantType; +import org.springframework.security.oauth2.core.ClientAuthenticationMethod; +import org.springframework.security.oauth2.core.OAuth2AccessToken; +import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationExchange; +import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; +import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationResponse; +import org.springframework.test.web.client.MockRestServiceServer; +import org.springframework.web.client.RestTemplate; + +class OAuth2TokenResponseClientsTest { + + @Test + void standardClientParsesNormalJsonTokenResponse() { + RestTemplate restTemplate = OAuth2TokenResponseClients.standardRestTemplate(); + MockRestServiceServer server = MockRestServiceServer.createServer(restTemplate); + server.expect(requestTo("https://provider.example/token")) + .andRespond(withSuccess( + """ + { + "access_token": "access-token", + "token_type": "Bearer", + "expires_in": 3600 + } + """, + MediaType.APPLICATION_JSON + )); + DefaultAuthorizationCodeTokenResponseClient client = + new DefaultAuthorizationCodeTokenResponseClient(); + client.setRestOperations(restTemplate); + + var response = client.getTokenResponse(grantRequest("github")); + + assertThat(response.getAccessToken().getTokenValue()).isEqualTo("access-token"); + assertThat(response.getAccessToken().getTokenType()).isEqualTo(OAuth2AccessToken.TokenType.BEARER); + server.verify(); + } + + @Test + void standardClientRejectsOversizedTokenResponseWithoutEchoingBody() { + RestTemplate restTemplate = OAuth2TokenResponseClients.standardRestTemplate(); + MockRestServiceServer server = MockRestServiceServer.createServer(restTemplate); + String sensitivePadding = "secret-response-body-".repeat(4 * 1024); + server.expect(requestTo("https://provider.example/token")) + .andRespond(withSuccess( + "{\"access_token\":\"" + sensitivePadding + "\",\"token_type\":\"Bearer\",\"expires_in\":3600}", + MediaType.APPLICATION_JSON + )); + DefaultAuthorizationCodeTokenResponseClient client = + new DefaultAuthorizationCodeTokenResponseClient(); + client.setRestOperations(restTemplate); + + assertThatThrownBy(() -> client.getTokenResponse(grantRequest("gitlab"))) + .satisfies(error -> assertThat(error.getMessage()) + .contains("OAuth2 token response exceeds") + .doesNotContain("secret-response-body")); + server.verify(); + } + + private static OAuth2AuthorizationCodeGrantRequest grantRequest(String registrationId) { + ClientRegistration registration = ClientRegistration.withRegistrationId(registrationId) + .clientId("client-id") + .clientSecret("client-secret") + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC) + .redirectUri("https://skillhub.example/login/oauth2/code/" + registrationId) + .authorizationUri("https://provider.example/authorize") + .tokenUri("https://provider.example/token") + .userInfoUri("https://provider.example/userinfo") + .userNameAttributeName("id") + .build(); + OAuth2AuthorizationRequest authorizationRequest = OAuth2AuthorizationRequest.authorizationCode() + .authorizationUri("https://provider.example/authorize") + .clientId("client-id") + .redirectUri("https://skillhub.example/login/oauth2/code/" + registrationId) + .state("state-1") + .build(); + OAuth2AuthorizationResponse authorizationResponse = OAuth2AuthorizationResponse.success("code-1") + .redirectUri("https://skillhub.example/login/oauth2/code/" + registrationId) + .state("state-1") + .build(); + return new OAuth2AuthorizationCodeGrantRequest( + registration, + new OAuth2AuthorizationExchange(authorizationRequest, authorizationResponse) + ); + } +}