diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpoints.java index 5755c4f4a68..e277937385c 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpoints.java @@ -555,6 +555,7 @@ public SearchResults findUsers( } @PatchMapping("/Users/{userId}/status") + @ResponseBody public UserAccountStatus updateAccountStatus(@RequestBody UserAccountStatus status, @PathVariable String userId) { ScimUser user = scimUserProvisioning.retrieve(userId, identityZoneManager.getCurrentIdentityZoneId()); diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsMockMvcTests.java index 07473c027fd..436e95fd7b0 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsMockMvcTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsMockMvcTests.java @@ -87,7 +87,6 @@ @ExtendWith(ZoneSeederExtension.class) @DefaultTestContext class ScimUserEndpointsMockMvcTests { - private static final MediaType APPLICATION_JSON_UTF8 = new MediaType("application", "json", java.nio.charset.StandardCharsets.UTF_8); private static final String HTTP_REDIRECT_EXAMPLE_COM = "http://redirect.example.com"; private static final String USER_PASSWORD = "pas5Word"; private String scimReadWriteToken; @@ -630,7 +629,27 @@ void unlockAccount() throws Exception { alteredAccountStatus.setLocked(false); updateAccountStatus(userToLockout, alteredAccountStatus) .andExpect(status().isOk()) - .andExpect(content().contentType(APPLICATION_JSON_UTF8)) + .andExpect(content().contentType(APPLICATION_JSON)) + .andExpect(content().string(JsonUtils.writeValueAsString(alteredAccountStatus))); + + attemptLogin(userToLockout) + .andExpect(redirectedUrl("/")); + } + + @Test + void unlockAccountWithoutAcceptHeader() throws Exception { + // Regression test: without @ResponseBody on updateAccountStatus, a caller + // that omits Accept: application/json (e.g. uaa-cli's unlock-user, which + // issues a raw PATCH via its curl helper) gets routed into Thymeleaf view + // resolution instead of the normal HttpMessageConverter path, and 500s. + ScimUser userToLockout = createUser(uaaAdminToken); + attemptUnsuccessfulLogin(5, userToLockout.getUserName(), ""); + + UserAccountStatus alteredAccountStatus = new UserAccountStatus(); + alteredAccountStatus.setLocked(false); + updateAccountStatusWithoutAcceptHeader(userToLockout, alteredAccountStatus) + .andExpect(status().isOk()) + .andExpect(content().contentType(APPLICATION_JSON)) .andExpect(content().string(JsonUtils.writeValueAsString(alteredAccountStatus))); attemptLogin(userToLockout) @@ -644,7 +663,7 @@ void accountStatusEmptyPatchDoesNotUnlock() throws Exception { updateAccountStatus(userToLockout, new UserAccountStatus()) .andExpect(status().isOk()) - .andExpect(content().contentType(APPLICATION_JSON_UTF8)) + .andExpect(content().contentType(APPLICATION_JSON)) .andExpect(content().string("{}")); attemptLogin(userToLockout) @@ -672,7 +691,7 @@ void unlockAccountWhenNotLocked() throws Exception { alteredAccountStatus.setLocked(false); updateAccountStatus(userToLockout, alteredAccountStatus) .andExpect(status().isOk()) - .andExpect(content().contentType(APPLICATION_JSON_UTF8)) + .andExpect(content().contentType(APPLICATION_JSON)) .andExpect(content().string(JsonUtils.writeValueAsString(alteredAccountStatus))); attemptLogin(userToLockout) @@ -716,7 +735,7 @@ void forcePasswordChange() throws Exception { updateAccountStatus(user, alteredAccountStatus) .andExpect(status().isOk()) - .andExpect(content().contentType(APPLICATION_JSON_UTF8)) + .andExpect(content().contentType(APPLICATION_JSON)) .andExpect(content().string(JsonUtils.writeValueAsString(alteredAccountStatus))); assertThat(usersRepository.checkPasswordChangeIndividuallyRequired(user.getId(), IdentityZoneHolder.get().getId())).isTrue(); @@ -1389,6 +1408,17 @@ private ResultActions updateAccountStatus(ScimUser user, UserAccountStatus alter ); } + private ResultActions updateAccountStatusWithoutAcceptHeader(ScimUser user, UserAccountStatus alteredAccountStatus) throws Exception { + String jsonStatus = JsonUtils.writeValueAsString(alteredAccountStatus); + return mockMvc + .perform( + patch("/Users/" + user.getId() + "/status") + .header("Authorization", "Bearer " + uaaAdminToken) + .contentType(APPLICATION_JSON) + .content(jsonStatus) + ); + } + private ResultActions attemptLogin(ScimUser user) throws Exception { return mockMvc .perform(post("/login.do") diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsMockMvcZonePathTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsMockMvcZonePathTests.java index e59cb1c746e..e558e2406b6 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsMockMvcZonePathTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsMockMvcZonePathTests.java @@ -96,7 +96,6 @@ @DefaultTestContext @EnabledIfZonePathsEnabled class ScimUserEndpointsMockMvcZonePathTests { - private static final MediaType APPLICATION_JSON_UTF8 = new MediaType("application", "json", java.nio.charset.StandardCharsets.UTF_8); private static final String HTTP_REDIRECT_EXAMPLE_COM = "http://redirect.example.com"; private static final String USER_PASSWORD = "pas5Word"; private String scimReadWriteToken; @@ -660,7 +659,7 @@ void unlockAccount(ZoneResolutionMode mode) throws Exception { alteredAccountStatus.setLocked(false); updateAccountStatus(userToLockout, alteredAccountStatus) .andExpect(status().isOk()) - .andExpect(content().contentType(APPLICATION_JSON_UTF8)) + .andExpect(content().contentType(APPLICATION_JSON)) .andExpect(content().string(JsonUtils.writeValueAsString(alteredAccountStatus))); attemptLogin(userToLockout) @@ -675,7 +674,7 @@ void accountStatusEmptyPatchDoesNotUnlock(ZoneResolutionMode mode) throws Except updateAccountStatus(userToLockout, new UserAccountStatus()) .andExpect(status().isOk()) - .andExpect(content().contentType(APPLICATION_JSON_UTF8)) + .andExpect(content().contentType(APPLICATION_JSON)) .andExpect(content().string("{}")); attemptLogin(userToLockout) @@ -703,7 +702,7 @@ void unlockAccountWhenNotLocked() throws Exception { alteredAccountStatus.setLocked(false); updateAccountStatus(userToLockout, alteredAccountStatus) .andExpect(status().isOk()) - .andExpect(content().contentType(APPLICATION_JSON_UTF8)) + .andExpect(content().contentType(APPLICATION_JSON)) .andExpect(content().string(JsonUtils.writeValueAsString(alteredAccountStatus))); attemptLogin(userToLockout) @@ -747,7 +746,7 @@ void forcePasswordChange() throws Exception { updateAccountStatus(user, alteredAccountStatus) .andExpect(status().isOk()) - .andExpect(content().contentType(APPLICATION_JSON_UTF8)) + .andExpect(content().contentType(APPLICATION_JSON)) .andExpect(content().string(JsonUtils.writeValueAsString(alteredAccountStatus))); assertThat(usersRepository.checkPasswordChangeIndividuallyRequired(user.getId(), IdentityZoneHolder.get().getId())).isTrue();