mirror of
https://github.com/iflytek/skillhub.git
synced 2026-09-24 00:55:35 +00:00
fix(auth): preserve Feishu OAuth browser redirect
Add safe phase-level OAuth diagnostics and redact callback credentials from request logs. Made-with: Proma Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
parent
2100996963
commit
c633904302
10 changed files with 136 additions and 10 deletions
|
|
@ -14,6 +14,7 @@ import org.springframework.web.util.ContentCachingRequestWrapper;
|
|||
import org.springframework.web.util.ContentCachingResponseWrapper;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.Locale;
|
||||
import java.util.Set;
|
||||
|
||||
/**
|
||||
|
|
@ -54,7 +55,7 @@ public class RequestLoggingFilter extends OncePerRequestFilter {
|
|||
private void logRequest(ContentCachingRequestWrapper request, ContentCachingResponseWrapper response, long duration) {
|
||||
String requestUri = request.getRequestURI();
|
||||
String queryString = request.getQueryString();
|
||||
String fullUrl = queryString != null ? requestUri + "?" + queryString : requestUri;
|
||||
String fullUrl = queryString != null ? requestUri + "?" + sanitizeQueryString(queryString) : requestUri;
|
||||
|
||||
String contentType = request.getContentType();
|
||||
String userAgent = request.getHeader("User-Agent");
|
||||
|
|
@ -74,6 +75,26 @@ public class RequestLoggingFilter extends OncePerRequestFilter {
|
|||
log.info(sb.toString());
|
||||
}
|
||||
|
||||
private String sanitizeQueryString(String queryString) {
|
||||
return java.util.Arrays.stream(queryString.split("&", -1))
|
||||
.map(parameter -> {
|
||||
int separator = parameter.indexOf('=');
|
||||
if (separator < 0) {
|
||||
return parameter;
|
||||
}
|
||||
String name = parameter.substring(0, separator).toLowerCase(Locale.ROOT);
|
||||
return isSensitiveQueryParameter(name)
|
||||
? parameter.substring(0, separator) + "=[REDACTED]"
|
||||
: parameter;
|
||||
})
|
||||
.collect(java.util.stream.Collectors.joining("&"));
|
||||
}
|
||||
|
||||
private boolean isSensitiveQueryParameter(String name) {
|
||||
return Set.of("code", "state", "error", "error_description", "error_uri", "access_token",
|
||||
"refresh_token", "id_token", "client_secret").contains(name);
|
||||
}
|
||||
|
||||
private boolean shouldSkip(String uri) {
|
||||
for (String prefix : SKIP_PREFIXES) {
|
||||
if (uri.startsWith(prefix)) {
|
||||
|
|
|
|||
|
|
@ -78,7 +78,7 @@ spring:
|
|||
# (contact:user.base:readonly, contact:user.email:readonly).
|
||||
authorization-grant-type: authorization_code
|
||||
client-authentication-method: client_secret_post
|
||||
redirect-uri: "{baseUrl}/login/oauth2/code/{registrationId}"
|
||||
redirect-uri: "${OAUTH2_FEISHU_REDIRECT_URI:{baseUrl}/login/oauth2/code/{registrationId}}"
|
||||
client-name: ${OAUTH2_FEISHU_DISPLAY_NAME:飞书}
|
||||
provider:
|
||||
github:
|
||||
|
|
|
|||
|
|
@ -104,6 +104,28 @@ class RequestLoggingFilterTest {
|
|||
assertThat(loggedMessages()).noneMatch(message -> message.contains("Headers: {"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void doFilterInternal_redactsOAuthCallbackQueryParameters() throws Exception {
|
||||
RequestLoggingFilter filter = new RequestLoggingFilter();
|
||||
attachAppender();
|
||||
|
||||
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/login/oauth2/code/feishu");
|
||||
request.setQueryString("code=authorization-code&state=csrf-state&scope=contact:user.base:readonly");
|
||||
MockHttpServletResponse response = new MockHttpServletResponse();
|
||||
|
||||
filter.doFilter(request, response, (req, res) -> {});
|
||||
|
||||
String message = loggedMessages().stream()
|
||||
.filter(entry -> entry.contains("GET /login/oauth2/code/feishu"))
|
||||
.findFirst()
|
||||
.orElseThrow();
|
||||
assertThat(message).contains("code=[REDACTED]");
|
||||
assertThat(message).contains("state=[REDACTED]");
|
||||
assertThat(message).contains("scope=contact:user.base:readonly");
|
||||
assertThat(message).doesNotContain("authorization-code");
|
||||
assertThat(message).doesNotContain("csrf-state");
|
||||
}
|
||||
|
||||
@Test
|
||||
void doFilterInternal_shouldKeepCachingWrapperForRegularApiResponses() throws Exception {
|
||||
RequestLoggingFilter filter = new RequestLoggingFilter();
|
||||
|
|
|
|||
|
|
@ -8,6 +8,8 @@ import java.time.Duration;
|
|||
import java.util.LinkedHashMap;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.beans.factory.annotation.Value;
|
||||
import org.springframework.http.HttpHeaders;
|
||||
|
|
@ -33,6 +35,7 @@ import org.springframework.web.client.RestClient;
|
|||
public class FeishuOAuth2AccessTokenResponseClient
|
||||
implements OAuth2AccessTokenResponseClient<OAuth2AuthorizationCodeGrantRequest> {
|
||||
|
||||
private static final Logger log = LoggerFactory.getLogger(FeishuOAuth2AccessTokenResponseClient.class);
|
||||
private static final String FEISHU_PROVIDER = "feishu";
|
||||
private static final String V2 = "v2";
|
||||
private static final String V3 = "v3";
|
||||
|
|
@ -100,16 +103,23 @@ public class FeishuOAuth2AccessTokenResponseClient
|
|||
requestBody.put("code_verifier", verifier);
|
||||
}
|
||||
|
||||
String tokenEndpoint = tokenUri(authorizationCodeGrantRequest);
|
||||
log.info("Feishu token exchange started: protocolVersion={}, endpointHost={}, redirectUriPresent={}, pkcePresent={}",
|
||||
protocolVersion,
|
||||
endpointHost(tokenEndpoint),
|
||||
redirectUri != null && !redirectUri.isBlank(),
|
||||
codeVerifier instanceof String verifier && !verifier.isBlank());
|
||||
try {
|
||||
return restClient.post()
|
||||
.uri(tokenUri(authorizationCodeGrantRequest))
|
||||
.uri(tokenEndpoint)
|
||||
.contentType(MediaType.parseMediaType("application/json; charset=utf-8"))
|
||||
.header(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE)
|
||||
.body(requestBody)
|
||||
.exchange((request, response) -> {
|
||||
int status = response.getStatusCode().value();
|
||||
log.info("Feishu token exchange response: httpStatus={}", status);
|
||||
if (!response.getStatusCode().is2xxSuccessful()) {
|
||||
throw tokenError("Feishu token endpoint returned HTTP "
|
||||
+ response.getStatusCode().value());
|
||||
throw tokenError("Feishu token endpoint returned HTTP " + status);
|
||||
}
|
||||
return parseResponse(readBounded(response.getBody()));
|
||||
});
|
||||
|
|
@ -154,6 +164,11 @@ public class FeishuOAuth2AccessTokenResponseClient
|
|||
if (scope != null) {
|
||||
tokenResponse.scopes(Set.of(scope.trim().split("\\s+")));
|
||||
}
|
||||
log.info("Feishu token exchange parsed: businessCode=0, accessTokenPresent={}, refreshTokenPresent={}, expiresInSeconds={}, scopePresent={}",
|
||||
accessToken != null,
|
||||
refreshToken != null,
|
||||
expiresIn,
|
||||
scope != null);
|
||||
return tokenResponse.build();
|
||||
} catch (OAuth2AuthorizationException exception) {
|
||||
throw exception;
|
||||
|
|
@ -182,6 +197,14 @@ public class FeishuOAuth2AccessTokenResponseClient
|
|||
return normalized;
|
||||
}
|
||||
|
||||
private static String endpointHost(String endpoint) {
|
||||
try {
|
||||
return java.net.URI.create(endpoint).getHost();
|
||||
} catch (IllegalArgumentException exception) {
|
||||
return "invalid";
|
||||
}
|
||||
}
|
||||
|
||||
private static String text(JsonNode node, String field) {
|
||||
JsonNode value = node.get(field);
|
||||
return value != null && value.isTextual() && !value.textValue().isBlank()
|
||||
|
|
|
|||
|
|
@ -95,12 +95,19 @@ public class FeishuOAuth2UserService implements ProviderOAuth2UserService {
|
|||
String userInfoUri = userRequest.getClientRegistration().getProviderDetails()
|
||||
.getUserInfoEndpoint().getUri();
|
||||
|
||||
log.info("Feishu userinfo started: endpointHost={}, accessTokenPresent={}",
|
||||
endpointHost(userInfoUri),
|
||||
userRequest.getAccessToken().getTokenValue() != null
|
||||
&& !userRequest.getAccessToken().getTokenValue().isBlank());
|
||||
FeishuUserResponse response;
|
||||
try {
|
||||
response = restClient.get()
|
||||
.uri(userInfoUri)
|
||||
.header(HttpHeaders.AUTHORIZATION, "Bearer " + userRequest.getAccessToken().getTokenValue())
|
||||
.exchange((request, clientResponse) -> readBounded(clientResponse.getBody()));
|
||||
.exchange((request, clientResponse) -> {
|
||||
log.info("Feishu userinfo response: httpStatus={}", clientResponse.getStatusCode().value());
|
||||
return readBounded(clientResponse.getBody());
|
||||
});
|
||||
} catch (Exception e) {
|
||||
// Exception class only: the message can quote the request URI, which holds the token.
|
||||
// Nothing downstream logs this failure, so without this line it would be silent.
|
||||
|
|
@ -128,6 +135,14 @@ public class FeishuOAuth2UserService implements ProviderOAuth2UserService {
|
|||
);
|
||||
}
|
||||
|
||||
log.info("Feishu userinfo parsed: businessCode=0, openIdPresent={}, unionIdPresent={}, emailPresent={}, displayNamePresent={}",
|
||||
response.data().openId() != null && !response.data().openId().isBlank(),
|
||||
response.data().unionId() != null && !response.data().unionId().isBlank(),
|
||||
(response.data().enterpriseEmail() != null && !response.data().enterpriseEmail().isBlank())
|
||||
|| (response.data().email() != null && !response.data().email().isBlank()),
|
||||
(response.data().name() != null && !response.data().name().isBlank())
|
||||
|| (response.data().enName() != null && !response.data().enName().isBlank()));
|
||||
|
||||
String userNameAttributeName = userRequest.getClientRegistration().getProviderDetails()
|
||||
.getUserInfoEndpoint().getUserNameAttributeName();
|
||||
|
||||
|
|
@ -156,6 +171,14 @@ public class FeishuOAuth2UserService implements ProviderOAuth2UserService {
|
|||
return attributes;
|
||||
}
|
||||
|
||||
private static String endpointHost(String endpoint) {
|
||||
try {
|
||||
return java.net.URI.create(endpoint).getHost();
|
||||
} catch (IllegalArgumentException exception) {
|
||||
return "invalid";
|
||||
}
|
||||
}
|
||||
|
||||
private void putIfPresent(Map<String, Object> attributes, String key, String value) {
|
||||
if (value != null && !value.isBlank()) {
|
||||
attributes.put(key, value);
|
||||
|
|
|
|||
|
|
@ -1,6 +1,8 @@
|
|||
package com.iflytek.skillhub.auth.oauth;
|
||||
|
||||
import jakarta.servlet.ServletException;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
import jakarta.servlet.http.HttpServletRequest;
|
||||
import jakarta.servlet.http.HttpServletResponse;
|
||||
import org.springframework.security.core.AuthenticationException;
|
||||
|
|
@ -16,6 +18,8 @@ import java.io.IOException;
|
|||
@Component
|
||||
public class OAuth2LoginFailureHandler extends SimpleUrlAuthenticationFailureHandler {
|
||||
|
||||
private static final Logger log = LoggerFactory.getLogger(OAuth2LoginFailureHandler.class);
|
||||
|
||||
private final OAuthLoginFlowService oauthLoginFlowService;
|
||||
|
||||
public OAuth2LoginFailureHandler(OAuthLoginFlowService oauthLoginFlowService) {
|
||||
|
|
@ -28,6 +32,8 @@ public class OAuth2LoginFailureHandler extends SimpleUrlAuthenticationFailureHan
|
|||
throws IOException, ServletException {
|
||||
String returnTo = oauthLoginFlowService.consumeReturnTo(request.getSession(false));
|
||||
String redirectTarget = oauthLoginFlowService.resolveFailureRedirect(exception, returnTo);
|
||||
log.warn("OAuth login failed: exceptionType={}, returnToPresent={}, redirectPath={}",
|
||||
exception.getClass().getSimpleName(), returnTo != null, redirectTarget);
|
||||
if (redirectTarget != null) {
|
||||
getRedirectStrategy().sendRedirect(request, response, redirectTarget);
|
||||
return;
|
||||
|
|
|
|||
|
|
@ -6,6 +6,8 @@ import jakarta.servlet.ServletException;
|
|||
import jakarta.servlet.http.HttpServletRequest;
|
||||
import jakarta.servlet.http.HttpServletResponse;
|
||||
import java.io.IOException;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
import org.springframework.security.core.Authentication;
|
||||
import org.springframework.security.oauth2.core.user.OAuth2User;
|
||||
import org.springframework.security.web.authentication.SimpleUrlAuthenticationSuccessHandler;
|
||||
|
|
@ -22,6 +24,8 @@ import org.springframework.stereotype.Component;
|
|||
@Component
|
||||
public class OAuth2LoginSuccessHandler extends SimpleUrlAuthenticationSuccessHandler {
|
||||
|
||||
private static final Logger log = LoggerFactory.getLogger(OAuth2LoginSuccessHandler.class);
|
||||
|
||||
private final PlatformSessionService platformSessionService;
|
||||
private final OAuthLoginFlowService oauthLoginFlowService;
|
||||
|
||||
|
|
@ -43,6 +47,8 @@ public class OAuth2LoginSuccessHandler extends SimpleUrlAuthenticationSuccessHan
|
|||
}
|
||||
String returnTo = oauthLoginFlowService.consumeReturnTo(request.getSession(false));
|
||||
if (returnTo != null) {
|
||||
log.info("OAuth login succeeded: redirectPath={}, returnToPresent=true, sessionAttached=true",
|
||||
returnTo);
|
||||
// returnTo is a root-relative path (web client strips the base path). The redirect
|
||||
// strategy (DefaultRedirectStrategy) already prepends the request context path, which
|
||||
// reflects X-Forwarded-Prefix under forward-headers-strategy=framework — so the browser
|
||||
|
|
@ -52,6 +58,7 @@ public class OAuth2LoginSuccessHandler extends SimpleUrlAuthenticationSuccessHan
|
|||
clearAuthenticationAttributes(request);
|
||||
return;
|
||||
}
|
||||
log.info("OAuth login succeeded: redirectPath={}, returnToPresent=false, sessionAttached=true", "/");
|
||||
super.onAuthenticationSuccess(request, response, authentication);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -17,6 +17,8 @@ import java.util.Map;
|
|||
import java.util.Objects;
|
||||
import java.util.function.Function;
|
||||
import java.util.stream.Collectors;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.security.core.AuthenticationException;
|
||||
import org.springframework.security.oauth2.client.userinfo.DefaultOAuth2UserService;
|
||||
|
|
@ -35,6 +37,8 @@ import org.springframework.stereotype.Service;
|
|||
@Service
|
||||
public class OAuthLoginFlowService {
|
||||
|
||||
private static final Logger log = LoggerFactory.getLogger(OAuthLoginFlowService.class);
|
||||
|
||||
private final Map<String, OAuthClaimsExtractor> extractors;
|
||||
private final Map<String, ProviderOAuth2UserService> userServiceOverrides;
|
||||
private final AccessPolicy accessPolicy;
|
||||
|
|
@ -111,19 +115,25 @@ public class OAuthLoginFlowService {
|
|||
new OAuth2Error("unsupported_provider", "Unsupported: " + registrationId, null)
|
||||
);
|
||||
}
|
||||
return new LoadedProviderIdentity(
|
||||
upstreamUser,
|
||||
extractor.extract(request, upstreamUser)
|
||||
);
|
||||
OAuthClaims claims = extractor.extract(request, upstreamUser);
|
||||
log.info("OAuth provider identity loaded: provider={}, subjectPresent={}, emailPresent={}, displayNamePresent={}",
|
||||
registrationId,
|
||||
claims.subject() != null && !claims.subject().isBlank(),
|
||||
claims.email() != null && !claims.email().isBlank(),
|
||||
claims.providerLogin() != null && !claims.providerLogin().isBlank());
|
||||
return new LoadedProviderIdentity(upstreamUser, claims);
|
||||
});
|
||||
|
||||
PlatformPrincipal principal = authenticate(loadedIdentity.claims());
|
||||
log.info("OAuth identity authenticated: provider={}, principalCreated=true, rolesCount={}",
|
||||
loadedIdentity.claims().provider(), principal.platformRoles().size());
|
||||
return new AuthenticatedLoginContext(loadedIdentity.upstreamUser(), principal);
|
||||
}
|
||||
|
||||
public PlatformPrincipal authenticate(OAuthClaims claims) {
|
||||
AccessDecision decision = accessPolicy.evaluate(claims);
|
||||
|
||||
log.info("OAuth access policy evaluated: provider={}, decision={}", claims.provider(), decision);
|
||||
if (decision == AccessDecision.PENDING_APPROVAL) {
|
||||
LegacyPlatformIdentityDecision identityDecision = identityCore.evaluate(claims);
|
||||
ensureActiveCoreAllowsPlatformLogin(identityDecision);
|
||||
|
|
|
|||
|
|
@ -1,6 +1,8 @@
|
|||
package com.iflytek.skillhub.auth.oauth;
|
||||
|
||||
import jakarta.servlet.http.HttpServletRequest;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository;
|
||||
import org.springframework.security.oauth2.client.web.DefaultOAuth2AuthorizationRequestResolver;
|
||||
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
|
||||
|
|
@ -14,6 +16,8 @@ import org.springframework.stereotype.Component;
|
|||
public class SkillHubOAuth2AuthorizationRequestResolver
|
||||
implements org.springframework.security.oauth2.client.web.OAuth2AuthorizationRequestResolver {
|
||||
|
||||
private static final Logger log = LoggerFactory.getLogger(SkillHubOAuth2AuthorizationRequestResolver.class);
|
||||
|
||||
private final DefaultOAuth2AuthorizationRequestResolver delegate;
|
||||
private final OAuthLoginFlowService oauthLoginFlowService;
|
||||
|
||||
|
|
@ -47,6 +51,10 @@ public class SkillHubOAuth2AuthorizationRequestResolver
|
|||
HttpServletRequest request, OAuth2AuthorizationRequest authorizationRequest) {
|
||||
if (authorizationRequest != null) {
|
||||
oauthLoginFlowService.rememberReturnTo(request);
|
||||
log.info("OAuth authorization started: provider={}, redirectUri={}, returnToPresent={}",
|
||||
authorizationRequest.getAttribute("registration_id"),
|
||||
authorizationRequest.getRedirectUri(),
|
||||
request.getParameter("returnTo") != null);
|
||||
}
|
||||
return authorizationRequest;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -95,6 +95,12 @@ export default defineConfig({
|
|||
target: 'http://localhost:8080',
|
||||
changeOrigin: true,
|
||||
},
|
||||
'/login/oauth2': {
|
||||
target: 'http://localhost:8080',
|
||||
// Preserve the browser-facing localhost:3000 host so Spring's
|
||||
// post-login redirect does not send the SPA to localhost:8080.
|
||||
changeOrigin: false,
|
||||
},
|
||||
},
|
||||
},
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue