Skip to content

Commit 5264342

Browse files
DmytroZaichenkoDevDmytro Zaichenko
DmytroZaichenkoDev
and
Dmytro Zaichenko
authored
fix: [Toolset] Toolset sign-in succeeds when AS /token returns 200 with error payload (#1361)
Co-authored-by: Dmytro Zaichenko <dmytro_zaichenko@epam.com>
1 parent b2e411b commit 5264342

File tree

3 files changed

+112
-10
lines changed

3 files changed

+112
-10
lines changed

‎credentials/src/main/java/com/epam/aidial/core/credentials/service/ResourceAuthorizationClient.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import com.epam.aidial.core.credentials.service.metadata.HttpHeadersHandler;
44
import com.epam.aidial.core.credentials.util.JsonMapperUtil;
55
import com.epam.aidial.core.storage.http.HttpException;
6+
import com.epam.aidial.core.storage.http.HttpStatus;
67
import com.google.common.annotations.VisibleForTesting;
78
import lombok.SneakyThrows;
89
import lombok.extern.slf4j.Slf4j;
@@ -91,6 +92,8 @@ private <R> R execute(HttpRequest request, Class<R> responseType) {
9192
}
9293
}
9394

95+
checkOauthError(body, request.uri());
96+
9497
return JsonMapperUtil.convertToObject(body, responseType);
9598
} catch (ConnectException e) {
9699
if (hasUnresolvedAddressException(e)) {
@@ -114,4 +117,23 @@ private static boolean hasUnresolvedAddressException(Throwable ex) {
114117
private java.time.Duration createRequestConfig() {
115118
return java.time.Duration.ofSeconds(30);
116119
}
120+
121+
/**
122+
* Some OAuth Authorization Servers return HTTP 200 with an error payload
123+
* instead of a proper error status code. Detect and handle this case.
124+
*/
125+
private static void checkOauthError(String body, URI uri) {
126+
if (body == null || body.isBlank()) {
127+
return;
128+
}
129+
var node = JsonMapperUtil.convertToObject(body, java.util.Map.class);
130+
if (node != null && node.containsKey("error")) {
131+
String error = String.valueOf(node.get("error"));
132+
String description = node.containsKey("error_description")
133+
? String.valueOf(node.get("error_description"))
134+
: "no description";
135+
log.debug("OAuth error in 200 response from {}: error={}, description={}", uri, error, description);
136+
throw new HttpException(HttpStatus.BAD_REQUEST, "Authorization server returned error: %s (%s)".formatted(error, description));
137+
}
138+
}
117139
}

‎credentials/src/main/java/com/epam/aidial/core/credentials/service/TokenService.java‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,7 @@ public TokenResponse getToken(String resourceId,
2828
.redirectUri(resourceAuthSettings.getRedirectUri())
2929
.build();
3030

31-
TokenResponse tokenResponse = resourceAuthorizationClient.executePost(
32-
resourceAuthSettings.getTokenEndpoint(),
33-
tokenRequest.buildFormData(),
34-
"application/x-www-form-urlencoded",
35-
TokenResponse.class);
31+
TokenResponse tokenResponse = doTokenCall(resourceAuthSettings.getTokenEndpoint(), tokenRequest.buildFormData());
3632
log.debug("Finished Resource {} token retrieval", resourceId);
3733
return tokenResponse;
3834
}
@@ -48,12 +44,15 @@ public TokenResponse getToken(String resourceId,
4844
.refreshToken(refreshToken)
4945
.build();
5046

51-
TokenResponse tokenResponse = resourceAuthorizationClient.executePost(
52-
resourceAuthSettings.getTokenEndpoint(),
53-
tokenRequest.buildFormData(),
54-
"application/x-www-form-urlencoded",
55-
TokenResponse.class);
47+
TokenResponse tokenResponse = doTokenCall(resourceAuthSettings.getTokenEndpoint(), tokenRequest.buildFormData());
5648
log.debug("Finished Resource {} refresh token retrieval", resourceId);
5749
return tokenResponse;
5850
}
51+
52+
private TokenResponse doTokenCall(String tokenEndpoint, String tokenRequest) {
53+
return resourceAuthorizationClient.executePost(
54+
tokenEndpoint, tokenRequest,
55+
"application/x-www-form-urlencoded",
56+
TokenResponse.class);
57+
}
5958
}

‎credentials/src/test/java/com/epam/aidial/core/credentials/service/ResourceAuthorizationClientTest.java‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,87 @@ void testExecutePost_ConnectException() throws Exception {
226226
assertEquals("Cannot connect to https://example.com/resource", exception.getMessage());
227227
}
228228

229+
@Test
230+
void testExecuteGet_OauthErrorInSuccessResponse() throws Exception {
231+
// Given
232+
String url = "https://example.com/token";
233+
String errorResponse = "{\"error\":\"invalid_grant\",\"error_description\":\"The authorization code has expired\"}";
234+
HttpResponse<String> httpResponseMock = mock(HttpResponse.class);
235+
when(httpClientMock.send(any(HttpRequest.class), any(HttpResponse.BodyHandler.class))).thenReturn(httpResponseMock);
236+
when(httpResponseMock.statusCode()).thenReturn(200);
237+
when(httpResponseMock.body()).thenReturn(errorResponse);
238+
239+
// When
240+
HttpException exception = assertThrows(HttpException.class,
241+
() -> resourceAuthorizationClient.executeGet(url, TestResponse.class));
242+
243+
// Then
244+
assertEquals(400, exception.getStatus().getCode());
245+
assertTrue(exception.getMessage().contains("invalid_grant"));
246+
assertTrue(exception.getMessage().contains("The authorization code has expired"));
247+
}
248+
249+
@Test
250+
void testExecutePost_OauthErrorInSuccessResponse() throws Exception {
251+
// Given
252+
String url = "https://example.com/token";
253+
TestRequest requestPayload = new TestRequest("testValue");
254+
String errorResponse = "{\"error\":\"invalid_client\",\"error_description\":\"Invalid redirect_uri\"}";
255+
HttpResponse<String> httpResponseMock = mock(HttpResponse.class);
256+
when(httpClientMock.send(any(HttpRequest.class), any(HttpResponse.BodyHandler.class))).thenReturn(httpResponseMock);
257+
when(httpResponseMock.statusCode()).thenReturn(200);
258+
when(httpResponseMock.body()).thenReturn(errorResponse);
259+
260+
// When
261+
HttpException exception = assertThrows(HttpException.class,
262+
() -> resourceAuthorizationClient.executePost(url, requestPayload,
263+
ContentType.APPLICATION_JSON.toString(), TestResponse.class));
264+
265+
// Then
266+
assertEquals(400, exception.getStatus().getCode());
267+
assertTrue(exception.getMessage().contains("invalid_client"));
268+
assertTrue(exception.getMessage().contains("Invalid redirect_uri"));
269+
}
270+
271+
@Test
272+
void testExecutePost_OauthErrorWithoutDescription() throws Exception {
273+
// Given
274+
String url = "https://example.com/token";
275+
TestRequest requestPayload = new TestRequest("testValue");
276+
String errorResponse = "{\"error\":\"server_error\"}";
277+
HttpResponse<String> httpResponseMock = mock(HttpResponse.class);
278+
when(httpClientMock.send(any(HttpRequest.class), any(HttpResponse.BodyHandler.class))).thenReturn(httpResponseMock);
279+
when(httpResponseMock.statusCode()).thenReturn(200);
280+
when(httpResponseMock.body()).thenReturn(errorResponse);
281+
282+
// When
283+
HttpException exception = assertThrows(HttpException.class,
284+
() -> resourceAuthorizationClient.executePost(url, requestPayload,
285+
ContentType.APPLICATION_JSON.toString(), TestResponse.class));
286+
287+
// Then
288+
assertEquals(400, exception.getStatus().getCode());
289+
assertTrue(exception.getMessage().contains("server_error"));
290+
assertTrue(exception.getMessage().contains("no description"));
291+
}
292+
293+
@Test
294+
void testExecuteGet_ValidResponseNotTreatedAsOauthError() throws Exception {
295+
String url = "https://example.com/resource";
296+
String jsonResponse = "{\"key\":\"value\"}";
297+
HttpResponse<String> httpResponseMock = mock(HttpResponse.class);
298+
when(httpClientMock.send(any(HttpRequest.class), any(HttpResponse.BodyHandler.class))).thenReturn(httpResponseMock);
299+
when(httpResponseMock.statusCode()).thenReturn(200);
300+
when(httpResponseMock.body()).thenReturn(jsonResponse);
301+
302+
// When
303+
TestResponse actualResponse = resourceAuthorizationClient.executeGet(url, TestResponse.class);
304+
305+
// Then
306+
assertNotNull(actualResponse);
307+
assertEquals("value", actualResponse.getKey());
308+
}
309+
229310
@Data
230311
@AllArgsConstructor
231312
@NoArgsConstructor

0 commit comments

Comments
 (0)