diff --git a/src/main/java/com/descope/exception/ErrorCode.java b/src/main/java/com/descope/exception/ErrorCode.java index 27744bf4..f8071c3b 100644 --- a/src/main/java/com/descope/exception/ErrorCode.java +++ b/src/main/java/com/descope/exception/ErrorCode.java @@ -22,6 +22,19 @@ public class ErrorCode { // rate limit public static final String RATE_LIMIT_EXCEEDED = "E130429"; + // conflicts - the new identifier already belongs to another user and the caller asked to + // fail instead of letting the server merge the two users + public static final String USER_UPDATE_CONFLICT = "E111127"; + public static final String AUTH_USER_UPDATE_CONFLICT = "E062125"; + + // other user errors + public static final String USER_ALREADY_EXISTS = "E062107"; + public static final String USER_NOT_FOUND = "E112102"; + + // server common + public static final String BAD_REQUEST = "E011001"; + public static final String VALIDATION_FAILURE = "E011003"; + // No keys public static final String INVALID_SIGNING_KEY = "J010001"; } diff --git a/src/main/java/com/descope/exception/UserConflictException.java b/src/main/java/com/descope/exception/UserConflictException.java new file mode 100644 index 00000000..25cb441b --- /dev/null +++ b/src/main/java/com/descope/exception/UserConflictException.java @@ -0,0 +1,16 @@ +package com.descope.exception; + +/** + * Thrown when an update cannot be applied because the new identifier already belongs to another + * user and the caller asked to fail instead of merging, by passing {@code failOnConflict}. + * + *

Without {@code failOnConflict} the server merges the two users and deletes the other one, + * so catching this exception is the only way to be told about the collision. + */ +public class UserConflictException extends DescopeException { + + public UserConflictException(String message, String code) { + super(message); + setCode(code); + } +} diff --git a/src/main/java/com/descope/proxy/impl/AbstractProxyImpl.java b/src/main/java/com/descope/proxy/impl/AbstractProxyImpl.java index 2c648d20..a8f13176 100644 --- a/src/main/java/com/descope/proxy/impl/AbstractProxyImpl.java +++ b/src/main/java/com/descope/proxy/impl/AbstractProxyImpl.java @@ -3,6 +3,7 @@ import com.descope.exception.ErrorCode; import com.descope.exception.RateLimitExceededException; import com.descope.exception.ServerCommonException; +import com.descope.exception.UserConflictException; import com.descope.model.client.Client; import com.descope.model.client.SdkInfo; import com.fasterxml.jackson.annotation.JsonInclude.Include; @@ -130,6 +131,12 @@ public R handleResponse(ClassicHttpResponse response) throws HttpException, IOEx errorDetails.getErrorCode(), getRetryHeader(res)); } + if (ErrorCode.USER_UPDATE_CONFLICT.equals(errorDetails.getErrorCode()) + || ErrorCode.AUTH_USER_UPDATE_CONFLICT.equals(errorDetails.getErrorCode())) { + throw new UserConflictException( + errorDetails.getActualMessage(), + errorDetails.getErrorCode()); + } throw ServerCommonException.genericServerError( errorDetails.getActualMessage(), StringUtils.isBlank(errorDetails.getErrorCode()) diff --git a/src/test/java/com/descope/proxy/impl/AbstractProxyImplTest.java b/src/test/java/com/descope/proxy/impl/AbstractProxyImplTest.java index 531116c1..4645ce5d 100644 --- a/src/test/java/com/descope/proxy/impl/AbstractProxyImplTest.java +++ b/src/test/java/com/descope/proxy/impl/AbstractProxyImplTest.java @@ -1,6 +1,7 @@ package com.descope.proxy.impl; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -9,6 +10,8 @@ import static org.mockito.Mockito.mockStatic; import com.descope.exception.DescopeException; +import com.descope.exception.ErrorCode; +import com.descope.exception.UserConflictException; import com.descope.model.client.SdkInfo; import java.io.ByteArrayInputStream; import java.io.IOException; @@ -158,6 +161,58 @@ void testSuccessOnThirdRetry() throws IOException { } } + @Test + @SuppressWarnings({"unchecked", "rawtypes"}) + void testConflictErrorCodesThrowUserConflictException() throws IOException { + Map conflictResponses = new java.util.LinkedHashMap<>(); + conflictResponses.put(ErrorCode.USER_UPDATE_CONFLICT, 409); + conflictResponses.put(ErrorCode.AUTH_USER_UPDATE_CONFLICT, 400); + for (Map.Entry entry : conflictResponses.entrySet()) { + CloseableHttpClient mockClient = mock(CloseableHttpClient.class); + doAnswer(inv -> { + HttpClientResponseHandler handler = (HttpClientResponseHandler) inv.getArgument(1); + return handler.handleResponse(errorResponse(entry.getValue(), + "{\"errorCode\":\"" + entry.getKey() + "\",\"errorDescription\":\"conflict\"}")); + }).when(mockClient).execute(any(ClassicHttpRequest.class), any(HttpClientResponseHandler.class)); + + try (MockedStatic mockedHttpClients = mockStatic(HttpClients.class)) { + mockedHttpClients.when(HttpClients::createDefault).thenReturn(mockClient); + ApiProxyImpl proxy = new ApiProxyImpl((SdkInfo) null); + UserConflictException thrown = assertThrows(UserConflictException.class, + () -> proxy.get(URI.create("http://localhost/test"), Map.class)); + assertEquals(entry.getKey(), thrown.getCode()); + assertEquals("conflict", thrown.getMessage()); + } + } + } + + @Test + @SuppressWarnings({"unchecked", "rawtypes"}) + void testGenericErrorCodesDoNotThrowUserConflictException() throws IOException { + // These are generic bad request / validation codes reused all over the API, mapping them to a + // conflict would misreport unrelated failures (descope/etc#18009) + List genericCodes = Arrays.asList( + ErrorCode.BAD_REQUEST, ErrorCode.VALIDATION_FAILURE, ErrorCode.USER_NOT_FOUND, + ErrorCode.USER_ALREADY_EXISTS); + for (String errorCode : genericCodes) { + CloseableHttpClient mockClient = mock(CloseableHttpClient.class); + doAnswer(inv -> { + HttpClientResponseHandler handler = (HttpClientResponseHandler) inv.getArgument(1); + return handler.handleResponse(errorResponse(400, + "{\"errorCode\":\"" + errorCode + "\",\"errorDescription\":\"error\"}")); + }).when(mockClient).execute(any(ClassicHttpRequest.class), any(HttpClientResponseHandler.class)); + + try (MockedStatic mockedHttpClients = mockStatic(HttpClients.class)) { + mockedHttpClients.when(HttpClients::createDefault).thenReturn(mockClient); + ApiProxyImpl proxy = new ApiProxyImpl((SdkInfo) null); + DescopeException thrown = assertThrows(DescopeException.class, + () -> proxy.get(URI.create("http://localhost/test"), Map.class)); + assertFalse(thrown instanceof UserConflictException, "Unexpected conflict for " + errorCode); + assertEquals(errorCode, thrown.getCode()); + } + } + } + // --- helpers --- private ClassicHttpResponse retryableResponse(int statusCode) { diff --git a/src/test/java/com/descope/sdk/mgmt/impl/UserServiceImplTest.java b/src/test/java/com/descope/sdk/mgmt/impl/UserServiceImplTest.java index bbb82b4b..d768ce19 100644 --- a/src/test/java/com/descope/sdk/mgmt/impl/UserServiceImplTest.java +++ b/src/test/java/com/descope/sdk/mgmt/impl/UserServiceImplTest.java @@ -19,8 +19,10 @@ import com.descope.enums.DeliveryMethod; import com.descope.exception.DescopeException; +import com.descope.exception.ErrorCode; import com.descope.exception.RateLimitExceededException; import com.descope.exception.ServerCommonException; +import com.descope.exception.UserConflictException; import com.descope.model.auth.AssociatedTenant; import com.descope.model.auth.AuthenticationInfo; import com.descope.model.auth.AuthenticationServices; @@ -965,6 +967,71 @@ void testFunctionalFullCycle() { userService.delete(newLoginId); } + // Conflict behavior on POST /v1/mgmt/user/update/email when the new email is already another + // user's login ID (descope/etc#17716, #18009, #18488). Requires the login ID to be the email + // itself, which is what makes the update hit the external-ID unique constraint. + @RetryingTest(value = 3, suspendForMs = 30000, onExceptions = RateLimitExceededException.class) + void testFunctionalUpdateEmailWithFailOnConflictKeepsBothUsers() { + String emailA = TestUtils.getRandomName("test-") + "@descope.com"; + String emailB = TestUtils.getRandomName("test-") + "@descope.com"; + String userIdA = userService.create(emailA, + UserRequest.builder().email(emailA).verifiedEmail(true).build()).getUser().getUserId(); + String userIdB = userService.create(emailB, + UserRequest.builder().email(emailB).verifiedEmail(true).build()).getUser().getUserId(); + try { + DescopeException thrown = assertThrows(DescopeException.class, + () -> userService.updateEmail(emailA, emailB, true, true)); + // The conflict answers E111127 / HTTP 409 once descope/backend#2657 is deployed, and the SDK + // surfaces it as UserConflictException. Before that deploy the endpoint still answers the + // generic E111112 / HTTP 500, so both are accepted here - drop the else branch once the + // backend change is live everywhere. + if (thrown instanceof UserConflictException) { + assertEquals(ErrorCode.USER_UPDATE_CONFLICT, thrown.getCode()); + } else { + assertNotNull(thrown.getCode()); + } + // What must hold in any case: opting out of the merge leaves both users untouched. + UserResponse userA = userService.loadByUserId(userIdA).getUser(); + assertEquals(emailA, userA.getEmail()); + Assertions.assertThat(userA.getLoginIds()).contains(emailA); + UserResponse userB = userService.loadByUserId(userIdB).getUser(); + assertEquals(emailB, userB.getEmail()); + Assertions.assertThat(userB.getLoginIds()).contains(emailB); + } finally { + deleteUserQuietly(emailA); + deleteUserQuietly(emailB); + } + } + + @RetryingTest(value = 3, suspendForMs = 30000, onExceptions = RateLimitExceededException.class) + void testFunctionalUpdateEmailWithoutFailOnConflictDeletesTheOtherUser() { + String emailA = TestUtils.getRandomName("test-") + "@descope.com"; + String emailB = TestUtils.getRandomName("test-") + "@descope.com"; + String userIdA = userService.create(emailA, + UserRequest.builder().email(emailA).verifiedEmail(true).build()).getUser().getUserId(); + String userIdB = userService.create(emailB, + UserRequest.builder().email(emailB).verifiedEmail(true).build()).getUser().getUserId(); + try { + // Default behavior: the call succeeds and the colliding user is silently merged away. + // This is the data loss failOnConflict exists to prevent, so it is asserted explicitly. + UserResponse updated = userService.updateEmail(emailA, emailB, true).getUser(); + assertEquals(userIdA, updated.getUserId()); + assertEquals(emailB, updated.getEmail()); + assertThrows(DescopeException.class, () -> userService.loadByUserId(userIdB)); + } finally { + deleteUserQuietly(emailB); + deleteUserQuietly(emailA); + } + } + + private void deleteUserQuietly(String loginId) { + try { + userService.delete(loginId); + } catch (DescopeException ignored) { + // The user may already be gone - a merge deletes the colliding user. + } + } + @RetryingTest(value = 3, suspendForMs = 30000, onExceptions = RateLimitExceededException.class) void testFunctionalTestUsers() { String loginId = TestUtils.getRandomName("u-");