diff --git a/metrics-data/src/main/java/org/cloudfoundry/identity/uaa/util/JsonUtils.java b/metrics-data/src/main/java/org/cloudfoundry/identity/uaa/util/JsonUtils.java index c0040b15ba9..0baa2c0ecfc 100644 --- a/metrics-data/src/main/java/org/cloudfoundry/identity/uaa/util/JsonUtils.java +++ b/metrics-data/src/main/java/org/cloudfoundry/identity/uaa/util/JsonUtils.java @@ -16,6 +16,7 @@ package org.cloudfoundry.identity.uaa.util; import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; @@ -55,6 +56,15 @@ public static T readValue(String s, Class clazz) throws JsonUtilException } } + public static Map readValueAsMap(final String input) { + try { + final JsonNode rootNode = objectMapper.readTree(input); + return getNodeAsMap(rootNode); + } catch (final JsonProcessingException e) { + throw new JsonUtilException(e); + } + } + public static T readValue(byte[] data, Class clazz) throws JsonUtilException { try { if (data!=null && data.length>0) { diff --git a/metrics-data/src/test/java/org/cloudfoundry/identity/uaa/util/JsonUtilsTest.java b/metrics-data/src/test/java/org/cloudfoundry/identity/uaa/util/JsonUtilsTest.java index c4ba1e837b3..4d7917c21b7 100644 --- a/metrics-data/src/test/java/org/cloudfoundry/identity/uaa/util/JsonUtilsTest.java +++ b/metrics-data/src/test/java/org/cloudfoundry/identity/uaa/util/JsonUtilsTest.java @@ -1,9 +1,14 @@ package org.cloudfoundry.identity.uaa.util; import com.fasterxml.jackson.core.type.TypeReference; + +import org.assertj.core.api.Assertions; import org.cloudfoundry.identity.uaa.metrics.UrlGroup; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import java.util.List; import java.util.Map; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -42,6 +47,26 @@ void testReadValueByteClass() { assertNull(JsonUtils.readValue((byte[]) null, UrlGroup.class)); } + @Test + void testReadValueAsMap() { + final String jsonInput = "{\"prop1\":\"abc\",\"prop2\":{\"prop2a\":\"def\",\"prop2b\":\"ghi\"},\"prop3\":[\"jkl\",\"mno\"]}"; + final Map map = JsonUtils.readValueAsMap(jsonInput); + Assertions.assertThat(map).isNotNull(); + Assertions.assertThat(map.get("prop1")).isNotNull().isEqualTo("abc"); + Assertions.assertThat(map.get("prop2")).isNotNull().isInstanceOf(Map.class); + Assertions.assertThat(((Map) map.get("prop2")).get("prop2a")).isNotNull().isEqualTo("def"); + Assertions.assertThat(((Map) map.get("prop2")).get("prop2b")).isNotNull().isEqualTo("ghi"); + Assertions.assertThat(map.get("prop3")).isNotNull().isInstanceOf(List.class); + Assertions.assertThat((List) map.get("prop3")).containsExactly("jkl", "mno"); + } + + @ParameterizedTest + @ValueSource(strings = {"{", "}", "{\"prop1\":\"abc\","}) + void testReadValueAsMap_Invalid(final String input) { + Assertions.assertThatExceptionOfType(JsonUtils.JsonUtilException.class) + .isThrownBy(() -> JsonUtils.readValueAsMap(input)); + } + @Test void testReadValueBytes() { assertNotNull(JsonUtils.readValue(jsonTestObjectString.getBytes(), new TypeReference>() {})); diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/scim/ScimUser.java b/model/src/main/java/org/cloudfoundry/identity/uaa/scim/ScimUser.java index 7131d1697d5..0700de6c6c3 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/scim/ScimUser.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/scim/ScimUser.java @@ -12,20 +12,27 @@ *******************************************************************************/ package org.cloudfoundry.identity.uaa.scim; -import com.fasterxml.jackson.annotation.JsonIgnore; -import com.fasterxml.jackson.annotation.JsonInclude; -import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.databind.annotation.JsonDeserialize; -import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import static java.util.Optional.ofNullable; +import static org.springframework.util.StringUtils.hasText; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Date; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Objects; +import java.util.Set; + import org.cloudfoundry.identity.uaa.approval.Approval; import org.cloudfoundry.identity.uaa.impl.JsonDateSerializer; import org.cloudfoundry.identity.uaa.scim.impl.ScimUserJsonDeserializer; import org.springframework.util.Assert; -import java.util.*; - -import static java.util.Optional.ofNullable; -import static org.springframework.util.StringUtils.hasText; +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; /** * Object to hold SCIM data for Jackson to map to and from JSON @@ -772,6 +779,10 @@ public void patch(ScimUser patch) { } } + if (hasText(patch.getOrigin()) && !patch.getOrigin().equals(getOrigin())) { + throw new IllegalArgumentException("Cannot change origin in patch of user."); + } + //Merge simple Attributes, that are stored ofNullable(patch.getUserName()).ifPresent(this::setUserName); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimUserProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimUserProvisioning.java index ee2e52ed322..cc22c92f194 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimUserProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimUserProvisioning.java @@ -273,17 +273,23 @@ public String extractPhoneNumber(final ScimUser user) { } @Override - public ScimUser update(final String id, final ScimUser user, String zoneId) throws InvalidScimResourceException { + public ScimUser update(final String id, final ScimUser user, final String zoneId) throws InvalidScimResourceException { logger.debug("Updating user " + user.getUserName()); final String origin = hasText(user.getOrigin()) ? user.getOrigin() : OriginKeys.UAA; user.setOrigin(origin); - if (isCheckOriginEnabled(zoneId)) { - checkOrigin(origin, zoneId); + + // check if the origin was changed + final ScimUser existingUser = retrieve(id, zoneId); + if (!origin.equals(existingUser.getOrigin())) { + throw new InvalidScimResourceException("Cannot change user's origin in update operation."); } + ScimUtils.validate(user); int updated = jdbcTemplate.update(UPDATE_USER_SQL, ps -> { int pos = 1; Timestamp t = new Timestamp(new Date().getTime()); + + // placeholders in UPDATE ps.setInt(pos++, user.getVersion() + 1); ps.setTimestamp(pos++, t); ps.setString(pos++, user.getUserName()); @@ -296,9 +302,11 @@ public ScimUser update(final String id, final ScimUser user, String zoneId) thro ps.setString(pos++, origin); ps.setString(pos++, hasText(user.getExternalId())?user.getExternalId():null); ps.setString(pos++, user.getSalt()); + + // placeholders in WHERE ps.setString(pos++, id); ps.setInt(pos++, user.getVersion()); - ps.setString(pos++, zoneId); + ps.setString(pos, zoneId); }); ScimUser result = retrieve(id, zoneId); if (updated == 0) { @@ -574,10 +582,6 @@ private void validateUserLimit(String zoneId, UserConfig userConfig) { } } - private boolean isCheckOriginEnabled(String zoneId) { - return isCheckOriginEnabled(getUserConfig(zoneId)); - } - private boolean isCheckOriginEnabled(UserConfig userConfig) { return userConfig != null && userConfig.isCheckOriginEnabled(); } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/scim/ScimUserTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/scim/ScimUserTests.java index 2b29921c8eb..a31e1c9c449 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/scim/ScimUserTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/scim/ScimUserTests.java @@ -12,6 +12,7 @@ *******************************************************************************/ package org.cloudfoundry.identity.uaa.scim; +import org.assertj.core.api.Assertions; import org.cloudfoundry.identity.uaa.approval.Approval; import org.cloudfoundry.identity.uaa.scim.ScimUser.Group; import org.cloudfoundry.identity.uaa.util.JsonUtils; @@ -583,6 +584,13 @@ public void testPatchUserDropNameSubAttributes() { assertNull(user.getName().getFamilyName()); } + @Test + public void testPatchUserRejectChangingOrigin() { + patch.setOrigin("some-new-origin"); + Assertions.assertThatIllegalArgumentException().isThrownBy(() -> user.patch(patch)) + .withMessage("Cannot change origin in patch of user."); + } + @Test public void testPatchUserDropNonUsedAttributes() { int pos = 0; diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimUserProvisioningTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimUserProvisioningTests.java index 83a62e847b7..d1a07443069 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimUserProvisioningTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimUserProvisioningTests.java @@ -398,31 +398,29 @@ void countUsersAcrossAllZones() { } @Test - void validateOriginAndExternalIDDuringCreateAndUpdate() { - String origin = "test-"+randomString(); + void validateExternalIdDuringCreateAndUpdate() { + final String origin = "test-"+randomString(); addIdentityProvider(jdbcTemplate, IdentityZone.getUaaZoneId(), origin); - String externalId = "testId"; - ScimUser user = new ScimUser(null, "jo@foo.com", "Jo", "User"); + final String externalId = "testId"; + final ScimUser user = new ScimUser(null, "jo@foo.com", "Jo", "User"); user.setOrigin(origin); user.setExternalId(externalId); user.addEmail("jo@blah.com"); - ScimUser created = jdbcScimUserProvisioning.createUser(user, "j7hyqpassX", currentIdentityZoneId); + final ScimUser created = jdbcScimUserProvisioning.createUser(user, "j7hyqpassX", currentIdentityZoneId); assertEquals("jo@foo.com", created.getUserName()); assertNotNull(created.getId()); assertNotSame(user.getId(), created.getId()); - Map map = jdbcTemplate.queryForMap("select * from users where id=?", created.getId()); + final Map map = jdbcTemplate.queryForMap("select * from users where id=?", created.getId()); assertEquals(user.getUserName(), map.get("userName")); assertEquals(user.getUserType(), map.get(UaaAuthority.UAA_USER.getUserType())); assertNull(created.getGroups()); assertEquals(origin, created.getOrigin()); assertEquals(externalId, created.getExternalId()); - String origin2 = "test2-"+randomString(); - addIdentityProvider(jdbcTemplate, IdentityZone.getUaaZoneId(), origin2); - String externalId2 = "testId2"; - created.setOrigin(origin2); + + // update external ID + final String externalId2 = "testId2"; created.setExternalId(externalId2); - ScimUser updated = jdbcScimUserProvisioning.update(created.getId(), created, currentIdentityZoneId); - assertEquals(origin2, updated.getOrigin()); + final ScimUser updated = jdbcScimUserProvisioning.update(created.getId(), created, currentIdentityZoneId); assertEquals(externalId2, updated.getExternalId()); } @@ -539,6 +537,31 @@ void updateCannotModifyGroups() { assertNull(joe.getGroups()); } + @Test + void updateCannotModifyOrigin() { + final String userId = UUID.randomUUID().toString(); + + final ScimUser userToCreate = new ScimUser(userId, "john.doe", "John", "Doe"); + userToCreate.setPassword("some-password"); + userToCreate.setOrigin("origin1"); + userToCreate.setZoneId(currentIdentityZoneId); + userToCreate.setPhoneNumbers(Collections.singletonList(new PhoneNumber("12345"))); + userToCreate.setPrimaryEmail("john.doe@example.com"); + addUser(jdbcTemplate, userToCreate); + + final ScimUser scimUser = jdbcScimUserProvisioning.retrieve(userId, currentIdentityZoneId); + + // change origin + scimUser.setOrigin("origin2"); + + final InvalidScimResourceException exception = assertThrows(InvalidScimResourceException.class, () -> + jdbcScimUserProvisioning.update(userId, scimUser, currentIdentityZoneId) + ); + + assertEquals(HttpStatus.BAD_REQUEST, exception.getStatus()); + assertEquals("Cannot change user's origin in update operation.", exception.getMessage()); + } + @Test void updateWithWrongVersionIsError() { ScimUser jo = new ScimUser(null, "josephine", "Jo", "NewUser"); @@ -556,12 +579,19 @@ void updateWithBadUsernameIsError() { @Test void updateWithBadUsernameIsOk_For_Non_UAA() { - ScimUser jo = new ScimUser(null, "jo$ephine", "Jo", "NewUser"); - jo.setOrigin(OriginKeys.LDAP); - jo.addEmail("jo@blah.com"); - ScimUser joe = jdbcScimUserProvisioning.update(joeId, jo, currentIdentityZoneId); - assertEquals("jo$ephine", joe.getUserName()); - assertEquals(OriginKeys.LDAP, joe.getOrigin()); + final String id = UUID.randomUUID().toString(); + final ScimUser user = new ScimUser(id, "josephine", "Jo", "NewUser"); + user.setOrigin(OriginKeys.LDAP); + user.setZoneId(currentIdentityZoneId); + user.addEmail("jo@blah.com"); + user.setPhoneNumbers(Collections.singletonList(new PhoneNumber("12345"))); + addUser(jdbcTemplate, user); + + final ScimUser updatePayload = jdbcScimUserProvisioning.retrieve(id, currentIdentityZoneId); + updatePayload.setUserName("jo$ephine"); + final ScimUser userAfterUpdate = jdbcScimUserProvisioning.update(id, updatePayload, currentIdentityZoneId); + assertEquals("jo$ephine", userAfterUpdate.getUserName()); + assertEquals(OriginKeys.LDAP, userAfterUpdate.getOrigin()); } @Test @@ -1115,33 +1145,6 @@ void cannotCreateUserWithInvalidOrigin() { idzManager.getCurrentIdentityZone().getConfig().getUserConfig().setCheckOriginEnabled(false); } - @Test - void cannotUpdateUserWithInvalidOrigin() { - String validOrigin = "validOrigin-"+randomString(); - String invalidOrigin = "invalidOrigin-"+randomString(); - addIdentityProvider(jdbcTemplate, currentIdentityZoneId, validOrigin); - String userId = "user-"+randomString(); - ScimUser scimUser = new ScimUser(userId, "user@example.com", "User", "Example"); - ScimUser.Email email = new ScimUser.Email(); - email.setValue(userId+"@example.com"); - scimUser.setEmails(Collections.singletonList(email)); - scimUser.setPassword(randomString()); - scimUser.setOrigin(validOrigin); - try { - jdbcScimUserProvisioning.create(scimUser, currentIdentityZoneId); - } catch (Exception e) { - fail("Can't create test user"); - } - scimUser.setOrigin(invalidOrigin); - idzManager.getCurrentIdentityZone().getConfig().getUserConfig().setCheckOriginEnabled(true); - assertThrowsWithMessageThat( - InvalidScimResourceException.class, - () -> jdbcScimUserProvisioning.update(userId, scimUser, currentIdentityZoneId), - containsString("Invalid origin") - ); - idzManager.getCurrentIdentityZone().getConfig().getUserConfig().setCheckOriginEnabled(false); - } - private static String createUserForDelete(final JdbcTemplate jdbcTemplate, String zoneId) { String randomUserId = UUID.randomUUID().toString(); addUser(jdbcTemplate, randomUserId, randomUserId, "password", randomUserId + "@delete.com", "ToDelete", "User", "+1-234-5678910", zoneId); @@ -1170,6 +1173,21 @@ private static void addUser(final JdbcTemplate jdbcTemplate, jdbcTemplate.execute(addUserSql); } + private static void addUser(final JdbcTemplate jdbcTemplate, final ScimUser scimUser) { + String addUserSql = String.format( + "insert into users (id, username, password, email, givenName, familyName, phoneNumber, identity_zone_id, origin) values ('%s','%s','%s','%s','%s','%s','%s','%s', '%s')", + scimUser.getId(), + scimUser.getUserName(), + scimUser.getPassword(), + scimUser.getPrimaryEmail(), + scimUser.getName().getGivenName(), + scimUser.getName().getFamilyName(), + scimUser.getPhoneNumbers().get(0), + scimUser.getZoneId(), + scimUser.getOrigin()); + jdbcTemplate.execute(addUserSql); + } + private static void createRandomUserInZone( final JdbcTemplate jdbcTemplate, final RandomValueStringGenerator generator, diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointDocs.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointDocs.java index 7a8e7d9d5bd..3c531e00f35 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointDocs.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointDocs.java @@ -211,7 +211,7 @@ class ScimUserEndpointDocs extends EndpointDocs { fieldWithPath("approvals").ignored().type(ARRAY).description("Approvals are not created at this time"), fieldWithPath("active").optional(true).type(BOOLEAN).description(userActiveDescription), fieldWithPath("verified").optional(true).type(BOOLEAN).description(userVerifiedDescription), - fieldWithPath("origin").optional(OriginKeys.UAA).type(STRING).description(userOriginDescription), + fieldWithPath("origin").optional(OriginKeys.UAA).type(STRING).description(userOriginDescription + " The `origin` value cannot be changed in an update operation."), fieldWithPath("zoneId").ignored().type(STRING).description(userZoneIdDescription), fieldWithPath("passwordLastModified").ignored().type(STRING).description(passwordLastModifiedDescription), fieldWithPath("externalId").optional(null).type(STRING).description(externalIdDescription), @@ -271,7 +271,7 @@ class ScimUserEndpointDocs extends EndpointDocs { fieldWithPath("approvals").ignored().type(ARRAY).description("Approvals are not created at this time"), fieldWithPath("active").optional(true).type(BOOLEAN).description(userActiveDescription), fieldWithPath("verified").optional(true).type(BOOLEAN).description(userVerifiedDescription), - fieldWithPath("origin").optional(OriginKeys.UAA).type(STRING).description(userOriginDescription), + fieldWithPath("origin").optional(OriginKeys.UAA).type(STRING).description(userOriginDescription + " The `origin` value cannot be changed in a patch operation."), fieldWithPath("zoneId").ignored().type(STRING).description(userZoneIdDescription), fieldWithPath("passwordLastModified").ignored().type(STRING).description(passwordLastModifiedDescription), fieldWithPath("externalId").optional(null).type(STRING).description(externalIdDescription), 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 4b0e5907629..ef0cc573670 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 @@ -1,10 +1,51 @@ package org.cloudfoundry.identity.uaa.scim.endpoints; -import com.fasterxml.jackson.core.type.TypeReference; -import com.google.common.collect.Lists; +import static org.cloudfoundry.identity.uaa.codestore.ExpiringCodeType.REGISTRATION; +import static org.cloudfoundry.identity.uaa.invitations.InvitationsEndpoint.USER_ID; +import static org.cloudfoundry.identity.uaa.mock.util.MockMvcUtils.CookieCsrfPostProcessor.cookieCsrf; +import static org.cloudfoundry.identity.uaa.zone.IdentityZoneSwitchingFilter.HEADER; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.CoreMatchers.startsWith; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.core.Is.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.springframework.http.MediaType.APPLICATION_JSON; +import static org.springframework.http.MediaType.APPLICATION_JSON_UTF8; +import static org.springframework.security.oauth2.common.util.OAuth2Utils.CLIENT_ID; +import static org.springframework.security.oauth2.common.util.OAuth2Utils.REDIRECT_URI; +import static org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers.authenticated; +import static org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers.unauthenticated; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; +import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrl; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; +import static org.springframework.util.StringUtils.hasText; + +import java.nio.charset.Charset; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.UUID; + import org.apache.commons.lang3.RandomStringUtils; import org.apache.http.NameValuePair; import org.apache.http.client.utils.URLEncodedUtils; +import org.assertj.core.api.Assertions; import org.cloudfoundry.identity.uaa.DefaultTestContext; import org.cloudfoundry.identity.uaa.account.UserAccountStatus; import org.cloudfoundry.identity.uaa.approval.Approval; @@ -29,10 +70,10 @@ import org.cloudfoundry.identity.uaa.scim.ScimUserProvisioning; import org.cloudfoundry.identity.uaa.scim.exception.UserAlreadyVerifiedException; import org.cloudfoundry.identity.uaa.scim.test.JsonObjectMatcherUtils; -import org.cloudfoundry.identity.uaa.util.AlphanumericRandomValueStringGenerator; import org.cloudfoundry.identity.uaa.test.TestClient; import org.cloudfoundry.identity.uaa.test.ZoneSeeder; import org.cloudfoundry.identity.uaa.test.ZoneSeederExtension; +import org.cloudfoundry.identity.uaa.util.AlphanumericRandomValueStringGenerator; import org.cloudfoundry.identity.uaa.util.JsonUtils; import org.cloudfoundry.identity.uaa.util.SetServerNameRequestPostProcessor; import org.cloudfoundry.identity.uaa.zone.IdentityZone; @@ -51,6 +92,7 @@ import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.security.oauth2.common.util.RandomValueStringGenerator; import org.springframework.security.oauth2.provider.ClientDetails; import org.springframework.security.oauth2.provider.client.BaseClientDetails; @@ -61,46 +103,8 @@ import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; import org.springframework.web.context.WebApplicationContext; -import java.nio.charset.Charset; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; -import java.util.Map; - -import static org.cloudfoundry.identity.uaa.codestore.ExpiringCodeType.REGISTRATION; -import static org.cloudfoundry.identity.uaa.invitations.InvitationsEndpoint.USER_ID; -import static org.cloudfoundry.identity.uaa.mock.util.MockMvcUtils.CookieCsrfPostProcessor.cookieCsrf; -import static org.cloudfoundry.identity.uaa.zone.IdentityZoneSwitchingFilter.HEADER; -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.CoreMatchers.notNullValue; -import static org.hamcrest.CoreMatchers.startsWith; -import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.core.Is.is; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; -import static org.springframework.http.MediaType.APPLICATION_JSON; -import static org.springframework.http.MediaType.APPLICATION_JSON_UTF8; -import static org.springframework.security.oauth2.common.util.OAuth2Utils.CLIENT_ID; -import static org.springframework.security.oauth2.common.util.OAuth2Utils.REDIRECT_URI; -import static org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers.authenticated; -import static org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers.unauthenticated; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; -import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrl; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; -import static org.springframework.util.StringUtils.hasText; +import com.fasterxml.jackson.core.type.TypeReference; +import com.google.common.collect.Lists; @ExtendWith(ZoneSeederExtension.class) @DefaultTestContext @@ -440,12 +444,7 @@ void patch_user_to_inactive_then_login() throws Exception { user.setVerified(true); boolean active = true; user.setActive(active); - mockMvc.perform( - patch("/Users/" + user.getId()) - .header("Authorization", "Bearer " + scimReadWriteToken) - .header("If-Match", "\"" + user.getVersion() + "\"") - .contentType(APPLICATION_JSON) - .content(JsonUtils.writeValueAsString(user))) + patchUser(user, scimReadWriteToken, user.getVersion()) .andExpect(status().isOk()) .andExpect(jsonPath("$.active", equalTo(active))); @@ -453,17 +452,31 @@ void patch_user_to_inactive_then_login() throws Exception { active = false; user.setActive(active); - mockMvc.perform( - patch("/Users/" + user.getId()) - .header("Authorization", "Bearer " + scimReadWriteToken) - .header("If-Match", "\"" + (user.getVersion() + 1) + "\"") - .contentType(APPLICATION_JSON) - .content(JsonUtils.writeValueAsString(user))) + patchUser(user, scimReadWriteToken, user.getVersion() + 1) .andExpect(status().isOk()) .andExpect(jsonPath("$.active", equalTo(active))); performAuthentication(user, false); + } + private ResultActions patchUser(final ScimUser user, final String token, final int version) throws Exception { + return mockMvc.perform( + patch("/Users/" + user.getId()) + .header("Authorization", "Bearer " + token) + .header("If-Match", "\"" + version + "\"") + .contentType(APPLICATION_JSON) + .content(JsonUtils.writeValueAsString(user)) + ); + } + + @Test + void testPatchUserShouldRejectChangingOrigin() throws Exception { + final ScimUser scimUser = setUpScimUser(); + scimUser.setOrigin("some-new-origin"); + patchUser(scimUser, scimReadWriteToken, scimUser.getVersion()) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error_description", equalTo("Cannot change origin in patch of user."))) + .andExpect(jsonPath("$.error", equalTo("invalid_scim_resource"))); } @Test @@ -686,9 +699,9 @@ void testForcePasswordExpireAccountInvalid() throws Exception { @Test void testForcePasswordExpireAccountExternalUser() throws Exception { - ScimUser user = createUser(uaaAdminToken); - user.setOrigin("NOT_UAA"); - updateUser(uaaAdminToken, HttpStatus.OK.value(), user); + ScimUser userToCreate = getScimUser(); + userToCreate.setOrigin("NOT_UAA"); + ScimUser user = createUser(userToCreate, uaaAdminToken, null); UserAccountStatus alteredAccountStatus = new UserAccountStatus(); alteredAccountStatus.setPasswordChangeRequired(true); @@ -1068,6 +1081,22 @@ void testUpdateUser_No_Username_Returns_400() throws Exception { updateUser(scimReadWriteToken, HttpStatus.BAD_REQUEST.value()); } + @Test + void testUpdateUser_ChangingOriginReturns400() throws Exception { + final ScimUser scimUser = setUpScimUser(IdentityZone.getUaa()); + scimUser.setOrigin(UUID.randomUUID().toString()); + final MvcResult result = updateUserAndReturnResult(scimReadWriteToken, scimUser); + final MockHttpServletResponse response = result.getResponse(); + Assertions.assertThat(response).isNotNull(); + Assertions.assertThat(response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value()); + final Map responseBody = JsonUtils.readValueAsMap(response.getContentAsString()); + Assertions.assertThat(responseBody) + .isNotNull() + .containsEntry("error_description", "Cannot change user's origin in update operation.") + .containsEntry("error", "invalid_scim_resource") + .containsEntry("message", "Cannot change user's origin in update operation."); + } + @Test void testUpdateUserWithScimCreateToken() throws Exception { updateUser(scimCreateToken, HttpStatus.FORBIDDEN.value()); @@ -1467,6 +1496,16 @@ private ScimUser updateUser(String token, int status, ScimUser user) throws Exce } } + private MvcResult updateUserAndReturnResult(final String token, final ScimUser scimUser) throws Exception { + final MockHttpServletRequestBuilder request = put("/Users/" + scimUser.getId()) + .header("Authorization", "Bearer " + token) + .header("If-Match", "\"" + scimUser.getVersion() + "\"") + .accept(APPLICATION_JSON) + .contentType(APPLICATION_JSON) + .content(JsonUtils.writeValueAsBytes(scimUser)); + return mockMvc.perform(request).andReturn(); + } + private ResultActions updateAccountStatus(ScimUser user, UserAccountStatus alteredAccountStatus) throws Exception { String jsonStatus = JsonUtils.writeValueAsString(alteredAccountStatus); return mockMvc