From 8435ee1ab16501a70849104c83c8826489fd77bc Mon Sep 17 00:00:00 2001 From: Gal Eyal Date: Mon, 27 Jul 2026 21:47:59 +0300 Subject: [PATCH] fix(auth): read device-code state via ObjectMapper conversion, not cast The shared RedisTemplate uses GenericJackson2JsonRedisSerializer with the application ObjectMapper, which embeds no type information, so stored DeviceCodeData deserializes as a LinkedHashMap. The typed casts in pollToken and authorizeDeviceCode then throw ClassCastException on every call, making the whole device authorization flow unusable (every poll returns 500). Convert the raw value with ObjectMapper.convertValue instead of casting; this reads both the current untyped map format and any typed format, so no stored-data migration is needed. Adds bean setters to DeviceCodeData for map conversion and regression tests that feed the service exactly what Redis returns in production (untyped maps). Fixes #604 Co-Authored-By: Claude Fable 5 Signed-off-by: Gal Eyal --- .../auth/device/DeviceAuthService.java | 19 +++- .../skillhub/auth/device/DeviceCodeData.java | 2 + .../auth/device/DeviceAuthServiceTest.java | 106 ++++++++++++++++++ 3 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java index e838c9a8..5061f854 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java @@ -1,5 +1,6 @@ package com.iflytek.skillhub.auth.device; +import com.fasterxml.jackson.databind.ObjectMapper; import com.iflytek.skillhub.auth.token.ApiTokenService; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import org.springframework.beans.factory.annotation.Value; @@ -33,14 +34,17 @@ public class DeviceAuthService { private final RedisTemplate redisTemplate; private final ApiTokenService apiTokenService; + private final ObjectMapper objectMapper; private final String verificationUri; private final SecureRandom random = new SecureRandom(); public DeviceAuthService(RedisTemplate redisTemplate, ApiTokenService apiTokenService, + ObjectMapper objectMapper, @Value("${skillhub.device-auth.verification-uri:/cli/auth}") String verificationUri) { this.redisTemplate = redisTemplate; this.apiTokenService = apiTokenService; + this.objectMapper = objectMapper; this.verificationUri = verificationUri; } @@ -71,7 +75,7 @@ public class DeviceAuthService { throw new DomainBadRequestException("error.deviceAuth.userCode.invalid"); } - DeviceCodeData data = (DeviceCodeData) redisTemplate.opsForValue().get(DEVICE_CODE_PREFIX + deviceCode); + DeviceCodeData data = readDeviceCodeData(deviceCode); if (data == null) { throw new DomainBadRequestException("error.deviceAuth.deviceCode.expired"); } @@ -97,7 +101,7 @@ public class DeviceAuthService { * into an API token exactly once. */ public DeviceTokenResponse pollToken(String deviceCode) { - DeviceCodeData data = (DeviceCodeData) redisTemplate.opsForValue().get(DEVICE_CODE_PREFIX + deviceCode); + DeviceCodeData data = readDeviceCodeData(deviceCode); if (data == null) { throw new DomainBadRequestException("error.deviceAuth.deviceCode.invalid"); @@ -147,6 +151,17 @@ public class DeviceAuthService { } } + /** + * Reads device-code state from Redis. The shared template's JSON value + * serializer carries no type information, so values deserialize as plain + * maps; convert explicitly instead of casting (a direct cast throws + * {@code ClassCastException} on every read). + */ + private DeviceCodeData readDeviceCodeData(String deviceCode) { + Object raw = redisTemplate.opsForValue().get(DEVICE_CODE_PREFIX + deviceCode); + return raw == null ? null : objectMapper.convertValue(raw, DeviceCodeData.class); + } + private String generateRandomDeviceCode() { byte[] bytes = new byte[32]; random.nextBytes(bytes); diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceCodeData.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceCodeData.java index 015896b7..7c44a22d 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceCodeData.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceCodeData.java @@ -19,7 +19,9 @@ public class DeviceCodeData implements Serializable { } public String getDeviceCode() { return deviceCode; } + public void setDeviceCode(String deviceCode) { this.deviceCode = deviceCode; } public String getUserCode() { return userCode; } + public void setUserCode(String userCode) { this.userCode = userCode; } public DeviceCodeStatus getStatus() { return status; } public void setStatus(DeviceCodeStatus status) { this.status = status; } public String getUserId() { return userId; } diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java new file mode 100644 index 00000000..fca992b2 --- /dev/null +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java @@ -0,0 +1,106 @@ +package com.iflytek.skillhub.auth.device; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.auth.token.ApiTokenService; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.data.redis.core.RedisTemplate; +import org.springframework.data.redis.core.ValueOperations; + +import java.util.LinkedHashMap; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.startsWith; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class DeviceAuthServiceTest { + + private static final String DEVICE_CODE = "device-code-1"; + private static final String USER_CODE = "ABCD-2345"; + + @Mock + private RedisTemplate redisTemplate; + + @Mock + private ValueOperations valueOperations; + + @Mock + private ApiTokenService apiTokenService; + + private DeviceAuthService service; + + @BeforeEach + void setUp() { + lenient().when(redisTemplate.opsForValue()).thenReturn(valueOperations); + service = new DeviceAuthService(redisTemplate, apiTokenService, new ObjectMapper(), "/cli/auth"); + } + + /** + * The shared RedisTemplate's JSON serializer keeps no type information, so + * stored DeviceCodeData comes back as a plain map. A typed cast used to + * throw ClassCastException on every poll; the service must convert instead. + */ + private static Map storedDeviceCode(DeviceCodeStatus status, String userId) { + Map raw = new LinkedHashMap<>(); + raw.put("deviceCode", DEVICE_CODE); + raw.put("userCode", USER_CODE); + raw.put("status", status.name()); + raw.put("userId", userId); + return raw; + } + + @Test + void pollTokenReturnsPendingWhenRedisValueIsUntypedMap() { + when(valueOperations.get("device:code:" + DEVICE_CODE)) + .thenReturn(storedDeviceCode(DeviceCodeStatus.PENDING, null)); + + DeviceTokenResponse response = service.pollToken(DEVICE_CODE); + + assertThat(response.error()).isEqualTo("authorization_pending"); + } + + @Test + void pollTokenRedeemsAuthorizedCodeFromUntypedMap() { + when(valueOperations.get("device:code:" + DEVICE_CODE)) + .thenReturn(storedDeviceCode(DeviceCodeStatus.AUTHORIZED, "usr_1")); + when(valueOperations.setIfAbsent(eq("device:claim:" + DEVICE_CODE), any(), anyLong(), any())) + .thenReturn(Boolean.TRUE); + when(apiTokenService.rotateToken(eq("usr_1"), any(), any())) + .thenReturn(new ApiTokenService.TokenCreateResult("sk_test_token", null)); + + DeviceTokenResponse response = service.pollToken(DEVICE_CODE); + + assertThat(response.accessToken()).isEqualTo("sk_test_token"); + } + + @Test + void pollTokenRejectsUnknownDeviceCode() { + when(valueOperations.get("device:code:" + DEVICE_CODE)).thenReturn(null); + + assertThatThrownBy(() -> service.pollToken(DEVICE_CODE)) + .isInstanceOf(DomainBadRequestException.class); + } + + @Test + void authorizeDeviceCodeMarksPendingCodeFromUntypedMap() { + when(valueOperations.get("device:usercode:" + USER_CODE)).thenReturn(DEVICE_CODE); + when(valueOperations.get("device:code:" + DEVICE_CODE)) + .thenReturn(storedDeviceCode(DeviceCodeStatus.PENDING, null)); + + service.authorizeDeviceCode(USER_CODE, "usr_1"); + + verify(valueOperations).set(startsWith("device:code:"), any(DeviceCodeData.class), anyLong(), any()); + } +}