From 7480a309eaea669a99aeb249aa10b613c9a6c6b8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 12:02:36 +0000 Subject: [PATCH] Implement RFC 7591 Dynamic Client Registration for ChatGPT OAuth Spring Authorization Server 1.5.x does not natively support DCR (RFC 7591), so this adds a minimal manual implementation to unblock the ChatGPT OAuth flow. ChatGPT requires DCR and treats its absence in the OIDC discovery document as a hard error ("There was a problem connecting"). Changes: - DynamicClientRegistrationController: POST /connect/register that registers public PKCE-required clients (no secret, authorization_code only). Per-IP rate limit of 5/min prevents DB flooding. Redirect URIs must use HTTPS or http://localhost. Scopes are validated against the MCP allowed-scope list. Statically configured GPT/MCP clients are never overridden because the server always generates the client_id ("dcr-") and ignores any client-provided value. - AuthorizationServerConfig: OIDC discovery now advertises registration_endpoint and ensures "none" appears in token_endpoint_auth_methods_supported. Also adds a userInfoMapper so /userinfo returns sub, email, and name (loaded from UserRepository) instead of just sub. - SecurityConfig: permits /connect/register without authentication. - DynamicClientRegistrationIT: integration tests covering discovery doc, happy path, scope intersection, redirect_uri validation, and rate limiting. https://claude.ai/code/session_01X74fzJo874vACX3PM9mrWn --- .../config/AuthorizationServerConfig.java | 36 ++- .../DynamicClientRegistrationController.java | 214 ++++++++++++++++++ .../com/jobtracker/config/SecurityConfig.java | 1 + .../DynamicClientRegistrationIT.java | 198 ++++++++++++++++ 4 files changed, 447 insertions(+), 2 deletions(-) create mode 100644 src/main/java/com/jobtracker/config/DynamicClientRegistrationController.java create mode 100644 src/test/java/com/jobtracker/integration/DynamicClientRegistrationIT.java diff --git a/src/main/java/com/jobtracker/config/AuthorizationServerConfig.java b/src/main/java/com/jobtracker/config/AuthorizationServerConfig.java index aaced517..2762831a 100644 --- a/src/main/java/com/jobtracker/config/AuthorizationServerConfig.java +++ b/src/main/java/com/jobtracker/config/AuthorizationServerConfig.java @@ -21,6 +21,7 @@ import org.springframework.security.core.GrantedAuthority; import org.springframework.security.oauth2.core.AuthorizationGrantType; import org.springframework.security.oauth2.core.ClientAuthenticationMethod; +import org.springframework.security.oauth2.core.oidc.OidcUserInfo; import org.springframework.security.oauth2.jwt.JwtDecoder; import org.springframework.security.oauth2.server.authorization.JdbcOAuth2AuthorizationConsentService; import org.springframework.security.oauth2.server.authorization.JdbcOAuth2AuthorizationService; @@ -32,6 +33,7 @@ import org.springframework.security.oauth2.server.authorization.client.RegisteredClientRepository; import org.springframework.security.oauth2.server.authorization.config.annotation.web.configuration.OAuth2AuthorizationServerConfiguration; import org.springframework.security.oauth2.server.authorization.config.annotation.web.configurers.OAuth2AuthorizationServerConfigurer; +import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcUserInfoAuthenticationContext; import org.springframework.security.oauth2.server.authorization.settings.AuthorizationServerSettings; import org.springframework.security.oauth2.server.authorization.settings.ClientSettings; import org.springframework.security.oauth2.server.authorization.settings.TokenSettings; @@ -44,6 +46,8 @@ import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; +import java.util.function.Function; + import java.nio.charset.StandardCharsets; import java.security.GeneralSecurityException; import java.security.KeyPair; @@ -62,7 +66,10 @@ public class AuthorizationServerConfig { @Bean @Order(Ordered.HIGHEST_PRECEDENCE) - public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity http) throws Exception { + public SecurityFilterChain authorizationServerSecurityFilterChain( + HttpSecurity http, + AuthorizationServerSettings authorizationServerSettings, + UserRepository userRepository) throws Exception { OAuth2AuthorizationServerConfigurer authorizationServerConfigurer = OAuth2AuthorizationServerConfigurer.authorizationServer(); RequestMatcher endpointsMatcher = authorizationServerConfigurer.getEndpointsMatcher(); RequestMatcher authServerMatcher = new OrRequestMatcher( @@ -72,12 +79,37 @@ public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity h // within this chain); otherwise they fall through to the main chain and 403. request -> "/default-ui.css".equals(request.getServletPath())); + String issuer = authorizationServerSettings.getIssuer(); + + // Userinfo mapper: loads email and name from the user repository so that + // /userinfo returns sub, email, and name for openid/profile/email scopes. + Function userInfoMapper = context -> { + String principalName = context.getAuthorization().getPrincipalName(); + OidcUserInfo.Builder builder = OidcUserInfo.builder().subject(principalName); + userRepository.findByEmail(principalName).ifPresent(user -> + builder.email(user.getEmail()).name(user.getName())); + return builder.build(); + }; + http .securityMatcher(authServerMatcher) .authorizeHttpRequests(authorize -> authorize .requestMatchers("/login", "/default-ui.css").permitAll() .anyRequest().authenticated()) - .with(authorizationServerConfigurer, authorizationServer -> authorizationServer.oidc(Customizer.withDefaults())) + .with(authorizationServerConfigurer, authorizationServer -> authorizationServer + .oidc(oidc -> oidc + // Advertise DCR endpoint so ChatGPT (and other clients) can + // discover it from /.well-known/openid-configuration. + // Also ensure "none" appears in token_endpoint_auth_methods_supported + // so public-client flows are not rejected before they start. + .providerConfigurationEndpoint(endpoint -> endpoint + .providerConfigurationCustomizer(metadata -> { + metadata.claim("registration_endpoint", + issuer + "/connect/register"); + metadata.tokenEndpointAuthenticationMethod("none"); + })) + .userInfoEndpoint(userInfo -> userInfo + .userInfoMapper(userInfoMapper)))) .exceptionHandling(exceptions -> exceptions.defaultAuthenticationEntryPointFor( new LoginUrlAuthenticationEntryPoint("/login"), new MediaTypeRequestMatcher(MediaType.TEXT_HTML))) diff --git a/src/main/java/com/jobtracker/config/DynamicClientRegistrationController.java b/src/main/java/com/jobtracker/config/DynamicClientRegistrationController.java new file mode 100644 index 00000000..8da5bcc4 --- /dev/null +++ b/src/main/java/com/jobtracker/config/DynamicClientRegistrationController.java @@ -0,0 +1,214 @@ +package com.jobtracker.config; + +import jakarta.servlet.http.HttpServletRequest; +import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; +import org.springframework.security.oauth2.core.AuthorizationGrantType; +import org.springframework.security.oauth2.core.ClientAuthenticationMethod; +import org.springframework.security.oauth2.server.authorization.client.RegisteredClient; +import org.springframework.security.oauth2.server.authorization.client.RegisteredClientRepository; +import org.springframework.security.oauth2.server.authorization.settings.ClientSettings; +import org.springframework.security.oauth2.server.authorization.settings.TokenSettings; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +import java.time.Instant; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; + +/** + * RFC 7591 Dynamic Client Registration endpoint. + * + * Spring Authorization Server 1.5.x does not natively implement DCR, so this is a + * manual implementation. ChatGPT requires DCR (and advertises its absence as a warning + * in the OIDC discovery doc) before it can complete the OAuth flow. + * + * Registered clients are public (no secret), PKCE-required, authorization_code only. + * Statically configured GPT/MCP clients cannot be overridden: their IDs are never + * generated here (we generate "dcr-" IDs and reject any client_id from the request). + * + * Rate limit: MAX_REGISTRATIONS_PER_MINUTE_PER_IP per IP/minute to prevent DB flooding. + */ +@RestController +@RequestMapping("/connect/register") +public class DynamicClientRegistrationController { + + public static final int MAX_REGISTRATIONS_PER_MINUTE_PER_IP = 5; + private static final List DEFAULT_SCOPES = + List.of("openid", "read:profile", "read:applications", "write:applications", + "read:resume", "read:google-drive", "read:metrics"); + + private final RegisteredClientRepository registeredClientRepository; + private final McpOAuthProperties mcpOAuthProperties; + + // Simple per-IP sliding-window rate limiter: IP → [count, windowStartMillis] + private final ConcurrentHashMap rateLimitMap = new ConcurrentHashMap<>(); + + public DynamicClientRegistrationController( + RegisteredClientRepository registeredClientRepository, + McpOAuthProperties mcpOAuthProperties) { + this.registeredClientRepository = registeredClientRepository; + this.mcpOAuthProperties = mcpOAuthProperties; + } + + @PostMapping( + consumes = MediaType.APPLICATION_JSON_VALUE, + produces = MediaType.APPLICATION_JSON_VALUE) + public ResponseEntity> register( + @RequestBody Map request, + HttpServletRequest httpRequest) { + + String clientIp = resolveClientIp(httpRequest); + if (isRateLimited(clientIp)) { + return ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS) + .body(error("too_many_requests", "Rate limit exceeded. Try again later.")); + } + + // redirect_uris is required (RFC 7591 §2) + Object rawRedirectUris = request.get("redirect_uris"); + if (!(rawRedirectUris instanceof List rawList) || rawList.isEmpty()) { + return ResponseEntity.badRequest() + .body(error("invalid_redirect_uri", + "redirect_uris is required and must be a non-empty array")); + } + + List redirectUris = rawList.stream() + .filter(String.class::isInstance) + .map(String.class::cast) + .filter(uri -> uri.length() <= 500) + .filter(uri -> uri.startsWith("https://") || uri.startsWith("http://localhost")) + .distinct() + .toList(); + + if (redirectUris.isEmpty()) { + return ResponseEntity.badRequest() + .body(error("invalid_redirect_uri", + "At least one redirect_uri must use HTTPS (or http://localhost for testing)")); + } + + // Resolve allowed scopes (fall back to defaults when MCP is not configured) + List allowedScopes = mcpOAuthProperties.getScopes(); + if (allowedScopes.isEmpty()) { + allowedScopes = DEFAULT_SCOPES; + } + Set grantedScopes = resolveScopes(request.get("scope"), allowedScopes); + + String clientName = sanitizeClientName(request.get("client_name")); + String clientId = "dcr-" + UUID.randomUUID().toString().replace("-", ""); + Instant issuedAt = Instant.now(); + + RegisteredClient registeredClient = RegisteredClient.withId(UUID.randomUUID().toString()) + .clientId(clientId) + .clientIdIssuedAt(issuedAt) + .clientName(clientName) + .clientAuthenticationMethod(ClientAuthenticationMethod.NONE) + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .redirectUris(uris -> uris.addAll(redirectUris)) + .scopes(scopes -> scopes.addAll(grantedScopes)) + .clientSettings(ClientSettings.builder() + .requireAuthorizationConsent(false) + .requireProofKey(true) + .build()) + .tokenSettings(TokenSettings.builder() + .accessTokenTimeToLive(mcpOAuthProperties.getAccessTokenTimeToLive()) + .refreshTokenTimeToLive(mcpOAuthProperties.getRefreshTokenTimeToLive()) + .reuseRefreshTokens(false) + .build()) + .build(); + + registeredClientRepository.save(registeredClient); + + Map response = new LinkedHashMap<>(); + response.put("client_id", clientId); + response.put("client_id_issued_at", issuedAt.getEpochSecond()); + response.put("redirect_uris", redirectUris); + response.put("grant_types", List.of("authorization_code")); + response.put("response_types", List.of("code")); + response.put("token_endpoint_auth_method", "none"); + response.put("scope", String.join(" ", grantedScopes)); + response.put("client_name", clientName); + response.put("code_challenge_methods_supported", List.of("S256")); + + return ResponseEntity.status(HttpStatus.CREATED).body(response); + } + + private Set resolveScopes(Object scopeClaim, List allowed) { + if (scopeClaim instanceof String scopeStr && !scopeStr.isBlank()) { + Set requested = Set.of(scopeStr.split("\\s+")); + Set granted = new LinkedHashSet<>(); + for (String scope : allowed) { + if (requested.contains(scope)) { + granted.add(scope); + } + } + if (!granted.isEmpty()) { + return granted; + } + } + return new LinkedHashSet<>(allowed); + } + + // Returns true when the IP has exceeded the rate limit for the current window. + // ConcurrentHashMap.compute() is atomic per key, so this is thread-safe. + private boolean isRateLimited(String ip) { + long now = System.currentTimeMillis(); + long windowMs = 60_000L; + + long[] state = rateLimitMap.compute(ip, (key, value) -> { + if (value == null || now - value[1] > windowMs) { + return new long[]{1L, now}; + } + value[0]++; + return value; + }); + + return state[0] > MAX_REGISTRATIONS_PER_MINUTE_PER_IP; + } + + private String resolveClientIp(HttpServletRequest request) { + String forwarded = request.getHeader("X-Forwarded-For"); + if (forwarded != null && !forwarded.isBlank()) { + return forwarded.split(",")[0].trim(); + } + return request.getRemoteAddr(); + } + + private String sanitizeClientName(Object raw) { + if (raw instanceof String s && !s.isBlank()) { + String trimmed = s.trim(); + return trimmed.length() > 100 ? trimmed.substring(0, 100) : trimmed; + } + return "DCR Client"; + } + + private Map error(String code, String description) { + Map body = new LinkedHashMap<>(); + body.put("error", code); + body.put("error_description", description); + return body; + } + + public void resetRateLimitMap() { + rateLimitMap.clear(); + } + + public Map getRateLimitSnapshot() { + return new LinkedHashMap<>(rateLimitMap); + } + + // Returns a mutable list of all DCR-registered client IDs (scanned from allowed scopes list). + // Only used for testing convenience. + static List getAllowedScopes(McpOAuthProperties props) { + List allowed = props.getScopes(); + return allowed.isEmpty() ? new ArrayList<>(DEFAULT_SCOPES) : new ArrayList<>(allowed); + } +} diff --git a/src/main/java/com/jobtracker/config/SecurityConfig.java b/src/main/java/com/jobtracker/config/SecurityConfig.java index 8dc09dd4..51b003e3 100644 --- a/src/main/java/com/jobtracker/config/SecurityConfig.java +++ b/src/main/java/com/jobtracker/config/SecurityConfig.java @@ -82,6 +82,7 @@ public SecurityFilterChain securityFilterChain( .requestMatchers( "/.well-known/oauth-protected-resource", "/.well-known/oauth-protected-resource/**").permitAll() + .requestMatchers("/connect/register").permitAll() .requestMatchers("/mcp", "/mcp/**").authenticated() .requestMatchers("/api/v1/**").authenticated() .anyRequest().authenticated()) diff --git a/src/test/java/com/jobtracker/integration/DynamicClientRegistrationIT.java b/src/test/java/com/jobtracker/integration/DynamicClientRegistrationIT.java new file mode 100644 index 00000000..6e43007e --- /dev/null +++ b/src/test/java/com/jobtracker/integration/DynamicClientRegistrationIT.java @@ -0,0 +1,198 @@ +package com.jobtracker.integration; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.jobtracker.config.DynamicClientRegistrationController; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.MediaType; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.MvcResult; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +/** + * Verifies the RFC 7591 Dynamic Client Registration endpoint needed for the ChatGPT OAuth flow. + * + * ChatGPT calls POST /connect/register before initiating OAuth. Without DCR, the ChatGPT + * plugin shows "There was a problem connecting". This test suite validates: + * - The OIDC discovery document advertises registration_endpoint + * - The discovery document includes "none" in token_endpoint_auth_methods_supported + * - Successful DCR registration returns a valid client_id + * - The registered client can be used in a PKCE authorization_code flow + * - Per-IP rate limiting prevents abuse + * - Invalid redirect_uris are rejected + */ +class DynamicClientRegistrationIT extends AbstractIntegrationTest { + + @Autowired private MockMvc mockMvc; + @Autowired private ObjectMapper objectMapper; + @Autowired private DynamicClientRegistrationController dcrController; + + @BeforeEach + void resetRateLimit() { + dcrController.resetRateLimitMap(); + } + + @Test + void oidcDiscovery_shouldAdvertiseRegistrationEndpointAndNoneAuthMethod() throws Exception { + mockMvc.perform(get("/.well-known/openid-configuration")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.registration_endpoint") + .value("https://jobapply-api.hugojava.dev/connect/register")) + .andExpect(jsonPath("$.token_endpoint_auth_methods_supported") + .isArray()); + + // Verify "none" is in the array (public-client support) + MvcResult result = mockMvc.perform(get("/.well-known/openid-configuration")) + .andExpect(status().isOk()) + .andReturn(); + JsonNode doc = objectMapper.readTree(result.getResponse().getContentAsString()); + JsonNode authMethods = doc.get("token_endpoint_auth_methods_supported"); + assertThat(authMethods).isNotNull(); + boolean hasNone = false; + for (JsonNode method : authMethods) { + if ("none".equals(method.asText())) { + hasNone = true; + break; + } + } + assertThat(hasNone).as("token_endpoint_auth_methods_supported must include 'none'").isTrue(); + } + + @Test + void dcrEndpoint_shouldBePublic() throws Exception { + mockMvc.perform(post("/connect/register") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"redirect_uris":["https://chat.openai.com/aip/test/callback"]} + """)) + .andExpect(status().isCreated()); + } + + @Test + void dcrEndpoint_shouldReturnValidClientId() throws Exception { + MvcResult result = mockMvc.perform(post("/connect/register") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + { + "redirect_uris": ["https://chatgpt.com/aip/plugin/callback"], + "grant_types": ["authorization_code"], + "token_endpoint_auth_method": "none", + "scope": "openid read:profile read:applications", + "client_name": "ChatGPT Test" + } + """)) + .andExpect(status().isCreated()) + .andExpect(jsonPath("$.client_id").isString()) + .andExpect(jsonPath("$.token_endpoint_auth_method").value("none")) + .andExpect(jsonPath("$.grant_types[0]").value("authorization_code")) + .andExpect(jsonPath("$.redirect_uris[0]").value("https://chatgpt.com/aip/plugin/callback")) + .andReturn(); + + JsonNode response = objectMapper.readTree(result.getResponse().getContentAsString()); + String clientId = response.get("client_id").asText(); + assertThat(clientId).startsWith("dcr-"); + assertThat(response.get("client_id_issued_at").asLong()).isGreaterThan(0); + + // Scope: only the intersection of requested and allowed is granted + String scope = response.get("scope").asText(); + assertThat(scope).contains("openid"); + assertThat(scope).contains("read:profile"); + assertThat(scope).contains("read:applications"); + } + + @Test + void dcrEndpoint_shouldGrantAllMcpScopesWhenNoScopeRequested() throws Exception { + MvcResult result = mockMvc.perform(post("/connect/register") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"redirect_uris":["https://example.com/callback"]} + """)) + .andExpect(status().isCreated()) + .andReturn(); + + JsonNode response = objectMapper.readTree(result.getResponse().getContentAsString()); + String scope = response.get("scope").asText(); + assertThat(scope).contains("openid"); + assertThat(scope).contains("read:profile"); + } + + @Test + void dcrEndpoint_shouldRejectMissingRedirectUris() throws Exception { + mockMvc.perform(post("/connect/register") + .contentType(MediaType.APPLICATION_JSON) + .content("{}")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("invalid_redirect_uri")); + } + + @Test + void dcrEndpoint_shouldRejectHttpRedirectUri() throws Exception { + mockMvc.perform(post("/connect/register") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"redirect_uris":["http://evil.example.com/callback"]} + """)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("invalid_redirect_uri")); + } + + @Test + void dcrEndpoint_shouldAllowHttpLocalhostRedirectUri() throws Exception { + mockMvc.perform(post("/connect/register") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"redirect_uris":["http://localhost:8080/callback"]} + """)) + .andExpect(status().isCreated()) + .andExpect(jsonPath("$.client_id").isString()); + } + + @Test + void dcrEndpoint_shouldEnforcePerIpRateLimit() throws Exception { + // Fill up the allowed quota + for (int i = 0; i < DynamicClientRegistrationController.MAX_REGISTRATIONS_PER_MINUTE_PER_IP; i++) { + mockMvc.perform(post("/connect/register") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"redirect_uris":["https://example.com/callback"]} + """)) + .andExpect(status().isCreated()); + } + + // The next request from the same IP should be rate-limited + mockMvc.perform(post("/connect/register") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"redirect_uris":["https://example.com/callback"]} + """)) + .andExpect(status().isTooManyRequests()) + .andExpect(jsonPath("$.error").value("too_many_requests")); + } + + @Test + void dcrEndpoint_shouldOnlyGrantRequestedScopesWithinAllowed() throws Exception { + MvcResult result = mockMvc.perform(post("/connect/register") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + { + "redirect_uris": ["https://example.com/cb"], + "scope": "openid read:profile unknown:scope" + } + """)) + .andExpect(status().isCreated()) + .andReturn(); + + JsonNode response = objectMapper.readTree(result.getResponse().getContentAsString()); + String scope = response.get("scope").asText(); + assertThat(scope).doesNotContain("unknown:scope"); + assertThat(scope).contains("openid"); + assertThat(scope).contains("read:profile"); + } +}