From e652dc8d8663df2238d10805c40a3d4e603ccbc6 Mon Sep 17 00:00:00 2001 From: Benjamin Faal Date: Tue, 12 Jan 2021 15:55:28 +0100 Subject: [PATCH 1/2] Make user info response status check accept all 2xx statuses This would otherwise fail on anything in the 2xx range thats not 200 OK. For example when i use the user-info-uri: https://vl.api.np.km.playstation.net/vl/api/v1/mobile/users/me/info it returns 201 Created (dont ask me why Sony chose that). Anyway i think this status check will still cover most cases by checking if its a 2xx status. Also this just works in the non reactive version: org.springframework.security.oauth2.client.userinfo.DefaultOAuth2UserService because it try catches the request. --- .../client/userinfo/DefaultReactiveOAuth2UserService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultReactiveOAuth2UserService.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultReactiveOAuth2UserService.java index 8d9cf1d92c6..59700889480 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultReactiveOAuth2UserService.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultReactiveOAuth2UserService.java @@ -108,7 +108,7 @@ public Mono loadUser(OAuth2UserRequest userRequest) throws OAuth2Aut authenticationMethod); // @formatter:off Mono> userAttributes = requestHeadersSpec.retrieve() - .onStatus((s) -> s != HttpStatus.OK, (response) -> + .onStatus((s) -> !s.is2xxSuccessful(), (response) -> parse(response) .map((userInfoErrorResponse) -> { String description = userInfoErrorResponse.getErrorObject().getDescription(); From c4f6f3f3d8b6437f9de331c07b70761a08db4196 Mon Sep 17 00:00:00 2001 From: Benjamin Faal Date: Wed, 20 Jan 2021 18:29:39 +0100 Subject: [PATCH 2/2] Make user info response status check http error statuses --- .../DefaultReactiveOAuth2UserService.java | 2 +- ...DefaultReactiveOAuth2UserServiceTests.java | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultReactiveOAuth2UserService.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultReactiveOAuth2UserService.java index 59700889480..513d72b16bd 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultReactiveOAuth2UserService.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultReactiveOAuth2UserService.java @@ -108,7 +108,7 @@ public Mono loadUser(OAuth2UserRequest userRequest) throws OAuth2Aut authenticationMethod); // @formatter:off Mono> userAttributes = requestHeadersSpec.retrieve() - .onStatus((s) -> !s.is2xxSuccessful(), (response) -> + .onStatus(HttpStatus::isError, (response) -> parse(response) .map((userInfoErrorResponse) -> { String description = userInfoErrorResponse.getErrorObject().getDescription(); diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/userinfo/DefaultReactiveOAuth2UserServiceTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/userinfo/DefaultReactiveOAuth2UserServiceTests.java index e905c1a9e90..ee3dde2cccc 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/userinfo/DefaultReactiveOAuth2UserServiceTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/userinfo/DefaultReactiveOAuth2UserServiceTests.java @@ -51,6 +51,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -144,6 +145,24 @@ public void loadUserWhenUserInfoSuccessResponseThenReturnUser() { assertThat(userAuthority.getAttributes()).isEqualTo(user.getAttributes()); } + @Test + public void loadUserWhenUserInfo201CreatedResponseThenReturnUser() { + // @formatter:off + String userInfoResponse = "{\n" + + " \"id\": \"user1\",\n" + + " \"first-name\": \"first\",\n" + + " \"last-name\": \"last\",\n" + + " \"middle-name\": \"middle\",\n" + + " \"address\": \"address\",\n" + + " \"email\": \"user1@example.com\"\n" + + "}\n"; + // @formatter:on + this.server.enqueue( + new MockResponse().setResponseCode(201).setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE).setBody(userInfoResponse)); + assertThatNoException() + .isThrownBy(() -> this.userService.loadUser(oauth2UserRequest()).block()); + } + // gh-5500 @Test public void loadUserWhenAuthenticationMethodHeaderSuccessResponseThenHttpMethodGet() throws Exception {