diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/cas/CasIdentityClaims.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/cas/CasIdentityClaims.java index 83f30a42..f4c9c805 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/cas/CasIdentityClaims.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/cas/CasIdentityClaims.java @@ -23,6 +23,9 @@ public record CasIdentityClaims( @Override public boolean emailVerified() { - return email != null && !email.isBlank(); + // CAS protocol does not verify email addresses — the attribute is passed through from the + // upstream directory (LDAP, AD, etc.) without cryptographic proof. Return false to prevent + // AccessPolicy implementations from trusting unverified claims. + return false; } } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/cas/CasLoginController.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/cas/CasLoginController.java index 1a8cc75b..13510b8d 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/cas/CasLoginController.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/cas/CasLoginController.java @@ -1,9 +1,9 @@ package com.iflytek.skillhub.auth.cas; import com.iflytek.skillhub.auth.identity.AccessDeniedByPolicyException; +import com.iflytek.skillhub.auth.identity.AccountDisabledException; +import com.iflytek.skillhub.auth.identity.AccountPendingException; import com.iflytek.skillhub.auth.identity.IdentityAuthenticator; -import com.iflytek.skillhub.auth.oauth.AccountDisabledException; -import com.iflytek.skillhub.auth.oauth.AccountPendingException; import com.iflytek.skillhub.auth.oauth.OAuthLoginRedirectSupport; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.auth.session.PlatformSessionService; @@ -17,10 +17,18 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.util.UriComponentsBuilder; +import java.security.SecureRandom; +import java.util.Base64; + /** * Handles CAS SSO login flow: redirect to CAS server and callback with ticket validation. * Delegates access-policy evaluation and principal provisioning to {@link IdentityAuthenticator} * so the same allow/deny/pending decisions apply to OAuth and CAS uniformly. + * + * CSRF protection: login() stores a random nonce in the session and passes it as the + * {@code state} parameter to the CAS login URL. callback() validates the nonce before + * processing the ticket, preventing attackers from forcing a victim into an authenticated + * session by crafting a callback URL with an attacker-controlled ticket. */ @Controller @RequestMapping("/api/v1/auth/cas") @@ -28,6 +36,11 @@ public class CasLoginController { private static final Logger log = LoggerFactory.getLogger(CasLoginController.class); + static final String SESSION_CAS_RETURN_TO_ATTRIBUTE = "skillhub.cas.returnTo"; + static final String SESSION_CAS_STATE_ATTRIBUTE = "skillhub.cas.state"; + + private static final SecureRandom SECURE_RANDOM = new SecureRandom(); + private final CasProperties casProperties; private final CasTicketValidator ticketValidator; private final IdentityAuthenticator identityAuthenticator; @@ -55,24 +68,33 @@ public class CasLoginController { return "redirect:/login?error=cas_disabled"; } + HttpSession session = request.getSession(true); + String sanitized = OAuthLoginRedirectSupport.sanitizeReturnTo(returnTo); if (sanitized != null) { - HttpSession session = request.getSession(true); - session.setAttribute(OAuthLoginRedirectSupport.SESSION_RETURN_TO_ATTRIBUTE, sanitized); + session.setAttribute(SESSION_CAS_RETURN_TO_ATTRIBUTE, sanitized); + } else { + session.removeAttribute(SESSION_CAS_RETURN_TO_ATTRIBUTE); } + String state = generateState(); + session.setAttribute(SESSION_CAS_STATE_ATTRIBUTE, state); + + String serviceUrl = casProperties.getServiceUrl() + "?state=" + state; + String casLoginUrl = UriComponentsBuilder .fromHttpUrl(casProperties.getServerUrl() + "/login") - .queryParam("service", casProperties.getServiceUrl()) + .queryParam("service", serviceUrl) .toUriString(); - log.debug("Redirecting to CAS login: {}", casLoginUrl); + log.debug("Redirecting to CAS login: endpoint={}", casProperties.getServerUrl()); return "redirect:" + casLoginUrl; } @GetMapping("/callback") public String callback( @RequestParam(required = false) String ticket, + @RequestParam(required = false) String state, HttpServletRequest request ) { if (!casProperties.isEnabled()) { @@ -85,6 +107,15 @@ public class CasLoginController { return "redirect:/login?error=missing_ticket"; } + // CSRF: validate state nonce before touching the ticket + HttpSession session = request.getSession(false); + if (!isValidState(session, state)) { + log.warn("CAS callback state mismatch — possible CSRF attempt"); + return "redirect:/login?error=invalid_state"; + } + assert session != null; // guaranteed by isValidState + session.removeAttribute(SESSION_CAS_STATE_ATTRIBUTE); + CasIdentityClaims claims; try { claims = ticketValidator.validate(ticket); @@ -99,11 +130,13 @@ public class CasLoginController { PlatformPrincipal principal = identityAuthenticator.authenticate(claims); sessionService.establishSession(principal, request); - HttpSession session = request.getSession(false); + // Read returnTo from session AFTER establishSession (which may rotate the session id + // but preserves attributes on the same session object) + HttpSession postAuthSession = request.getSession(false); String returnTo = null; - if (session != null) { - returnTo = (String) session.getAttribute(OAuthLoginRedirectSupport.SESSION_RETURN_TO_ATTRIBUTE); - session.removeAttribute(OAuthLoginRedirectSupport.SESSION_RETURN_TO_ATTRIBUTE); + if (postAuthSession != null) { + returnTo = (String) postAuthSession.getAttribute(SESSION_CAS_RETURN_TO_ATTRIBUTE); + postAuthSession.removeAttribute(SESSION_CAS_RETURN_TO_ATTRIBUTE); } String targetUrl = OAuthLoginRedirectSupport.sanitizeReturnTo(returnTo); @@ -127,4 +160,18 @@ public class CasLoginController { return "redirect:/login?error=internal_error"; } } + + private static String generateState() { + byte[] bytes = new byte[24]; + SECURE_RANDOM.nextBytes(bytes); + return Base64.getUrlEncoder().withoutPadding().encodeToString(bytes); + } + + private static boolean isValidState(HttpSession session, String incomingState) { + if (session == null || incomingState == null || incomingState.isBlank()) { + return false; + } + Object stored = session.getAttribute(SESSION_CAS_STATE_ATTRIBUTE); + return stored instanceof String storedState && storedState.equals(incomingState); + } } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/cas/CasTicketValidator.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/cas/CasTicketValidator.java index 182a54a6..a61e3db1 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/cas/CasTicketValidator.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/cas/CasTicketValidator.java @@ -70,7 +70,7 @@ public class CasTicketValidator { } String validationUrl = buildValidationUrl(ticket); - log.debug("Validating CAS ticket at: {}", validationUrl); + log.debug("Validating CAS ticket at endpoint: {}", casProperties.resolvedProtocolVersion().validatePath()); try { String response = restClient.get() @@ -90,8 +90,12 @@ public class CasTicketValidator { } catch (CasValidationException e) { throw e; } catch (Exception e) { - log.error("CAS ticket validation failed", e); - throw new CasValidationException("Failed to validate CAS ticket: " + e.getMessage(), e); + // Log full exception (including URL) at server level for operators; surface only the + // exception class to callers so the ticket query parameter never reaches downstream + // log streams or error redirects. + log.error("CAS ticket validation request failed", e); + throw new CasValidationException( + "Failed to validate CAS ticket: " + e.getClass().getSimpleName(), e); } } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/AccountDisabledException.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/AccountDisabledException.java similarity index 67% rename from server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/AccountDisabledException.java rename to server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/AccountDisabledException.java index 962165ec..913abe46 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/AccountDisabledException.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/AccountDisabledException.java @@ -1,10 +1,11 @@ -package com.iflytek.skillhub.auth.oauth; +package com.iflytek.skillhub.auth.identity; import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.core.OAuth2Error; /** - * OAuth authentication exception raised when the mapped platform account is disabled. + * Thrown when an authenticated external identity maps to a platform account that is disabled. + * Used by both OAuth and CAS flows. */ public class AccountDisabledException extends OAuth2AuthenticationException { diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/AccountPendingException.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/AccountPendingException.java similarity index 66% rename from server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/AccountPendingException.java rename to server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/AccountPendingException.java index 9a7ccef9..98ff9aef 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/AccountPendingException.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/AccountPendingException.java @@ -1,10 +1,11 @@ -package com.iflytek.skillhub.auth.oauth; +package com.iflytek.skillhub.auth.identity; import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.core.OAuth2Error; /** - * OAuth authentication exception raised when the mapped platform account is pending approval. + * Thrown when an authenticated external identity maps to a platform account that is pending approval. + * Used by both OAuth and CAS flows. */ public class AccountPendingException extends OAuth2AuthenticationException { diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityAuthenticator.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityAuthenticator.java index 254717f0..dccadb7a 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityAuthenticator.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityAuthenticator.java @@ -1,6 +1,6 @@ package com.iflytek.skillhub.auth.identity; -import com.iflytek.skillhub.auth.oauth.AccountPendingException; +import com.iflytek.skillhub.auth.identity.AccountPendingException; import com.iflytek.skillhub.auth.policy.AccessDecision; import com.iflytek.skillhub.auth.policy.AccessPolicy; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; @@ -28,7 +28,7 @@ public class IdentityAuthenticator { * * @throws AccountPendingException if the policy yields PENDING_APPROVAL * @throws AccessDeniedByPolicyException if the policy yields DENY - * @throws com.iflytek.skillhub.auth.oauth.AccountDisabledException if the user is disabled + * @throws com.iflytek.skillhub.auth.identity.AccountDisabledException if the user is disabled */ public PlatformPrincipal authenticate(IdentityClaims claims) { AccessDecision decision = accessPolicy.evaluate(claims); diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityBindingService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityBindingService.java index d6696712..a3c7018a 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityBindingService.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityBindingService.java @@ -71,10 +71,10 @@ public class IdentityBindingService { } if (user.getStatus() == UserStatus.PENDING) { - throw new com.iflytek.skillhub.auth.oauth.AccountPendingException(); + throw new com.iflytek.skillhub.auth.identity.AccountPendingException(); } if (user.getStatus() == UserStatus.DISABLED) { - throw new com.iflytek.skillhub.auth.oauth.AccountDisabledException(); + throw new com.iflytek.skillhub.auth.identity.AccountDisabledException(); } Set roles = roleBindingRepo.findByUserId(user.getId()).stream() @@ -97,9 +97,9 @@ public class IdentityBindingService { UserAccount existingUser = userRepo.findById(existingBinding.getUserId()) .orElseThrow(() -> new IllegalStateException("User not found for binding")); if (existingUser.getStatus() == UserStatus.DISABLED) { - throw new com.iflytek.skillhub.auth.oauth.AccountDisabledException(); + throw new com.iflytek.skillhub.auth.identity.AccountDisabledException(); } - throw new com.iflytek.skillhub.auth.oauth.AccountPendingException(); + throw new com.iflytek.skillhub.auth.identity.AccountPendingException(); } UserAccount user = new UserAccount( diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowService.java index 7bce9d17..53d34b6e 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowService.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowService.java @@ -1,6 +1,8 @@ package com.iflytek.skillhub.auth.oauth; import com.iflytek.skillhub.auth.identity.AccessDeniedByPolicyException; +import com.iflytek.skillhub.auth.identity.AccountDisabledException; +import com.iflytek.skillhub.auth.identity.AccountPendingException; import com.iflytek.skillhub.auth.identity.IdentityAuthenticator; import com.iflytek.skillhub.auth.identity.IdentityClaims; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/cas/CasLoginControllerTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/cas/CasLoginControllerTest.java index 62f4a589..74604acc 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/cas/CasLoginControllerTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/cas/CasLoginControllerTest.java @@ -8,9 +8,9 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.iflytek.skillhub.auth.identity.AccessDeniedByPolicyException; +import com.iflytek.skillhub.auth.identity.AccountDisabledException; +import com.iflytek.skillhub.auth.identity.AccountPendingException; import com.iflytek.skillhub.auth.identity.IdentityAuthenticator; -import com.iflytek.skillhub.auth.oauth.AccountDisabledException; -import com.iflytek.skillhub.auth.oauth.AccountPendingException; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.auth.session.PlatformSessionService; import java.util.Map; @@ -50,6 +50,8 @@ class CasLoginControllerTest { controller = new CasLoginController(casProperties, ticketValidator, identityAuthenticator, sessionService); } + // ─── login() ─────────────────────────────────────────────────────────────── + @Test void login_redirectsToCasServer() { MockHttpServletRequest request = new MockHttpServletRequest(); @@ -61,12 +63,49 @@ class CasLoginControllerTest { } @Test - void login_storesReturnToInSession() { + void login_storesCasReturnToInSession() { MockHttpServletRequest request = new MockHttpServletRequest(); controller.login("/skills", request); - assertThat(request.getSession().getAttribute("skillhub.oauth.returnTo")).isEqualTo("/skills"); + assertThat(request.getSession(false)) + .isNotNull() + .extracting(s -> s.getAttribute(CasLoginController.SESSION_CAS_RETURN_TO_ATTRIBUTE)) + .isEqualTo("/skills"); + // Must NOT bleed into the shared OAuth key + assertThat(request.getSession(false) + .getAttribute("skillhub.oauth.returnTo")).isNull(); + } + + @Test + void login_storesStateNonceInSession() { + MockHttpServletRequest request = new MockHttpServletRequest(); + + controller.login(null, request); + + Object nonce = request.getSession(false) + .getAttribute(CasLoginController.SESSION_CAS_STATE_ATTRIBUTE); + assertThat(nonce).isInstanceOf(String.class); + assertThat((String) nonce).hasSizeGreaterThanOrEqualTo(20); + } + + @Test + void login_embedsStateInServiceUrl() { + MockHttpServletRequest request = new MockHttpServletRequest(); + + String result = controller.login(null, request); + + String nonce = (String) request.getSession(false) + .getAttribute(CasLoginController.SESSION_CAS_STATE_ATTRIBUTE); + // The service URL is percent-encoded when embedded as a query param in the CAS login URL. + // The state param itself may appear percent-encoded (= → %3D) or raw depending on how + // UriComponentsBuilder encodes the outer service param. Assert the nonce value appears + // in the redirect URL in either form. + assertThat(result).satisfiesAnyOf( + r -> assertThat(r).contains("state=" + nonce), + r -> assertThat(r).contains("state%3D" + nonce), + r -> assertThat(r).contains("state%3d" + nonce) + ); } @Test @@ -75,7 +114,8 @@ class CasLoginControllerTest { controller.login("https://evil.com", request); - assertThat(request.getSession(false)).isNull(); + assertThat(request.getSession(false) + .getAttribute(CasLoginController.SESSION_CAS_RETURN_TO_ATTRIBUTE)).isNull(); } @Test @@ -88,12 +128,22 @@ class CasLoginControllerTest { assertThat(result).isEqualTo("redirect:/login?error=cas_disabled"); } - @Test - void callback_successfulTicketValidation() { + // ─── callback() ──────────────────────────────────────────────────────────── + + private MockHttpServletRequest requestWithValidState(String returnTo) { MockHttpServletRequest request = new MockHttpServletRequest(); MockHttpSession session = new MockHttpSession(); - session.setAttribute("skillhub.oauth.returnTo", "/dashboard"); + session.setAttribute(CasLoginController.SESSION_CAS_STATE_ATTRIBUTE, "valid-nonce"); + if (returnTo != null) { + session.setAttribute(CasLoginController.SESSION_CAS_RETURN_TO_ATTRIBUTE, returnTo); + } request.setSession(session); + return request; + } + + @Test + void callback_successfulTicketValidation() { + MockHttpServletRequest request = requestWithValidState("/dashboard"); CasIdentityClaims claims = new CasIdentityClaims("zhangsan", "zhangsan@example.com", "Zhang San", Map.of()); PlatformPrincipal principal = new PlatformPrincipal("usr_123", "Zhang San", "zhangsan@example.com", null, "cas", Set.of("USER")); @@ -101,7 +151,7 @@ class CasLoginControllerTest { when(ticketValidator.validate("ST-12345")).thenReturn(claims); when(identityAuthenticator.authenticate(claims)).thenReturn(principal); - String result = controller.callback("ST-12345", request); + String result = controller.callback("ST-12345", "valid-nonce", request); assertThat(result).isEqualTo("redirect:/dashboard"); verify(sessionService).establishSession(eq(principal), eq(request)); @@ -109,7 +159,7 @@ class CasLoginControllerTest { @Test void callback_usesDefaultTargetWhenNoReturnTo() { - MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletRequest request = requestWithValidState(null); CasIdentityClaims claims = new CasIdentityClaims("user1", null, "User One", Map.of()); PlatformPrincipal principal = new PlatformPrincipal("usr_456", "User One", null, null, "cas", Set.of("USER")); @@ -117,17 +167,14 @@ class CasLoginControllerTest { when(ticketValidator.validate("ST-99999")).thenReturn(claims); when(identityAuthenticator.authenticate(claims)).thenReturn(principal); - String result = controller.callback("ST-99999", request); + String result = controller.callback("ST-99999", "valid-nonce", request); assertThat(result).isEqualTo("redirect:/dashboard"); } @Test void callback_sanitizesUnsafeReturnTo() { - MockHttpServletRequest request = new MockHttpServletRequest(); - MockHttpSession session = new MockHttpSession(); - session.setAttribute("skillhub.oauth.returnTo", "https://evil.example/steal"); - request.setSession(session); + MockHttpServletRequest request = requestWithValidState("https://evil.example/steal"); CasIdentityClaims claims = new CasIdentityClaims("u", null, "U", Map.of()); PlatformPrincipal principal = new PlatformPrincipal("usr_x", "U", null, null, "cas", Set.of("USER")); @@ -135,16 +182,16 @@ class CasLoginControllerTest { when(ticketValidator.validate("ST-evil")).thenReturn(claims); when(identityAuthenticator.authenticate(claims)).thenReturn(principal); - String result = controller.callback("ST-evil", request); + String result = controller.callback("ST-evil", "valid-nonce", request); assertThat(result).isEqualTo("redirect:/dashboard"); } @Test void callback_missingTicket_redirectsWithError() { - MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletRequest request = requestWithValidState(null); - String result = controller.callback(null, request); + String result = controller.callback(null, "valid-nonce", request); assertThat(result).isEqualTo("redirect:/login?error=missing_ticket"); verify(ticketValidator, never()).validate(any()); @@ -152,9 +199,9 @@ class CasLoginControllerTest { @Test void callback_blankTicket_redirectsWithError() { - MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletRequest request = requestWithValidState(null); - String result = controller.callback(" ", request); + String result = controller.callback(" ", "valid-nonce", request); assertThat(result).isEqualTo("redirect:/login?error=missing_ticket"); } @@ -162,60 +209,103 @@ class CasLoginControllerTest { @Test void callback_whenDisabled_redirectsWithError() { casProperties.setEnabled(false); - MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletRequest request = requestWithValidState(null); - String result = controller.callback("ST-12345", request); + String result = controller.callback("ST-12345", "valid-nonce", request); assertThat(result).isEqualTo("redirect:/login?error=cas_disabled"); } @Test - void callback_accountPending_redirectsToPendingApproval() { + void callback_missingState_rejectsCsrf() { + MockHttpServletRequest request = requestWithValidState(null); + + String result = controller.callback("ST-csrf", null, request); + + assertThat(result).isEqualTo("redirect:/login?error=invalid_state"); + verify(ticketValidator, never()).validate(any()); + } + + @Test + void callback_wrongState_rejectsCsrf() { + MockHttpServletRequest request = requestWithValidState(null); + + String result = controller.callback("ST-csrf", "wrong-nonce", request); + + assertThat(result).isEqualTo("redirect:/login?error=invalid_state"); + verify(ticketValidator, never()).validate(any()); + } + + @Test + void callback_noSession_rejectsCsrf() { MockHttpServletRequest request = new MockHttpServletRequest(); + String result = controller.callback("ST-csrf", "any-state", request); + + assertThat(result).isEqualTo("redirect:/login?error=invalid_state"); + verify(ticketValidator, never()).validate(any()); + } + + @Test + void callback_accountPending_redirectsToPendingApproval() { + MockHttpServletRequest request = requestWithValidState(null); + CasIdentityClaims claims = new CasIdentityClaims("pending-user", null, "Pending", Map.of()); when(ticketValidator.validate("ST-pending")).thenReturn(claims); when(identityAuthenticator.authenticate(claims)).thenThrow(new AccountPendingException()); - String result = controller.callback("ST-pending", request); + String result = controller.callback("ST-pending", "valid-nonce", request); assertThat(result).isEqualTo("redirect:/pending-approval"); } @Test void callback_accountDisabled_redirectsToAccessDenied() { - MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletRequest request = requestWithValidState(null); CasIdentityClaims claims = new CasIdentityClaims("disabled-user", null, "Disabled", Map.of()); when(ticketValidator.validate("ST-disabled")).thenReturn(claims); when(identityAuthenticator.authenticate(claims)).thenThrow(new AccountDisabledException()); - String result = controller.callback("ST-disabled", request); + String result = controller.callback("ST-disabled", "valid-nonce", request); assertThat(result).isEqualTo("redirect:/access-denied"); } @Test void callback_accessPolicyDeny_redirectsToAccessDenied() { - MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletRequest request = requestWithValidState(null); CasIdentityClaims claims = new CasIdentityClaims("denied-user", "denied@bad.example", "Denied", Map.of()); when(ticketValidator.validate("ST-denied")).thenReturn(claims); when(identityAuthenticator.authenticate(claims)).thenThrow(new AccessDeniedByPolicyException()); - String result = controller.callback("ST-denied", request); + String result = controller.callback("ST-denied", "valid-nonce", request); assertThat(result).isEqualTo("redirect:/access-denied"); } @Test void callback_validationFailed_redirectsWithError() { - MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletRequest request = requestWithValidState(null); when(ticketValidator.validate("ST-invalid")).thenThrow(new CasValidationException("Invalid ticket")); - String result = controller.callback("ST-invalid", request); + String result = controller.callback("ST-invalid", "valid-nonce", request); assertThat(result).isEqualTo("redirect:/login?error=cas_validation_failed"); } + + @Test + void callback_unexpectedError_redirectsWithInternalError() { + MockHttpServletRequest request = requestWithValidState(null); + + CasIdentityClaims claims = new CasIdentityClaims("user", null, "User", Map.of()); + when(ticketValidator.validate("ST-broken")).thenReturn(claims); + when(identityAuthenticator.authenticate(claims)).thenThrow(new RuntimeException("DB down")); + + String result = controller.callback("ST-broken", "valid-nonce", request); + + assertThat(result).isEqualTo("redirect:/login?error=internal_error"); + } } diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityAuthenticatorTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityAuthenticatorTest.java index dbb4a9f9..50dcf171 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityAuthenticatorTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityAuthenticatorTest.java @@ -7,7 +7,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import com.iflytek.skillhub.auth.oauth.AccountPendingException; +import com.iflytek.skillhub.auth.identity.AccountPendingException; import com.iflytek.skillhub.auth.oauth.OAuthClaims; import com.iflytek.skillhub.auth.policy.AccessDecision; import com.iflytek.skillhub.auth.policy.AccessPolicy; diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingServiceTest.java index 79e5df74..edc93f6c 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingServiceTest.java @@ -10,9 +10,9 @@ import static org.mockito.Mockito.when; import com.iflytek.skillhub.auth.entity.IdentityBinding; import com.iflytek.skillhub.auth.entity.Role; import com.iflytek.skillhub.auth.entity.UserRoleBinding; -import com.iflytek.skillhub.auth.oauth.AccountDisabledException; +import com.iflytek.skillhub.auth.identity.AccountDisabledException; import com.iflytek.skillhub.auth.oauth.OAuthClaims; -import com.iflytek.skillhub.auth.oauth.AccountPendingException; +import com.iflytek.skillhub.auth.identity.AccountPendingException; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.auth.repository.IdentityBindingRepository; import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository;