Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 'aliasId' and 'aliasZid' Fields to ScimUser #2765

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
5556a1e
Add DB migration scripts
adrianhoelzl-sap Mar 5, 2024
b8426ee
Add fields 'aliasId' and 'aliasZid' to ScimUser class
adrianhoelzl-sap Mar 5, 2024
d96a5c4
Add fields 'aliasId' and 'aliasZid' to JSON deserialization of ScimUs…
adrianhoelzl-sap Mar 5, 2024
55c2368
Add fields 'aliasId' and 'aliasZid' to queries in JdbcScimUserProvisi…
adrianhoelzl-sap Mar 5, 2024
46a1a5d
Add unit tests for handling of alias properties in JdbcScimUserProvis…
adrianhoelzl-sap Mar 5, 2024
2a9cf25
Ignore alias properties in SCIM user endpoints
adrianhoelzl-sap Mar 5, 2024
cb54857
Add unit test for handling of alias properties in ScimUser.patch
adrianhoelzl-sap Mar 5, 2024
1ee07df
Fix Sonar issues
adrianhoelzl-sap Mar 6, 2024
a99b689
Revert changes to JsonDeserializer
adrianhoelzl-sap Mar 6, 2024
c6145b3
Increase test coverage
adrianhoelzl-sap Mar 6, 2024
9c25691
Move ScimUser test to correct package
adrianhoelzl-sap Mar 6, 2024
a951797
Fix further Sonar findings
adrianhoelzl-sap Mar 6, 2024
02bd32b
Merge branch 'develop' into feature/add-alias-id-and-alias-zid-fields…
adrianhoelzl-sap Mar 11, 2024
5e89664
Rework: remove setType call from assertThatIllegalArgumentException l…
adrianhoelzl-sap Mar 15, 2024
1f2614c
Rework: replace AssertJ isEqualTo with assertEquals in ScimUserTests.…
adrianhoelzl-sap Mar 15, 2024
45f13d0
Rework: refactor ScimUser.patch
adrianhoelzl-sap Mar 15, 2024
60cb8ca
Rework: remove usage of ReflectionTestUtils in JdbcScimUserProvisioni…
adrianhoelzl-sap Mar 15, 2024
22f80aa
Rework: add comment why alias properties are ignored to ScimUserEndpo…
adrianhoelzl-sap Mar 15, 2024
3061277
Rework: add equals method to ScimUser
adrianhoelzl-sap Mar 15, 2024
7d51c4f
Revert "Rework: add equals method to ScimUser"
adrianhoelzl-sap Mar 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 96 additions & 63 deletions model/src/main/java/org/cloudfoundry/identity/uaa/scim/ScimUser.java
swalchemist marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@
import java.util.Objects;
import java.util.Set;

import org.cloudfoundry.identity.uaa.EntityWithAlias;
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.lang.NonNull;
import org.springframework.util.Assert;

import com.fasterxml.jackson.annotation.JsonIgnore;
Expand All @@ -34,6 +36,8 @@
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;

import lombok.Setter;

/**
* Object to hold SCIM data for Jackson to map to and from JSON
*
Expand All @@ -45,7 +49,7 @@
*/
@JsonInclude(JsonInclude.Include.NON_NULL)
@JsonDeserialize(using = ScimUserJsonDeserializer.class)
public class ScimUser extends ScimCore<ScimUser> {
public class ScimUser extends ScimCore<ScimUser> implements EntityWithAlias {

@JsonInclude(JsonInclude.Include.NON_NULL)
public static final class Group {
Expand Down Expand Up @@ -349,6 +353,14 @@ public int hashCode() {

private String zoneId = null;

@JsonIgnore
@Setter
private String aliasZid = null;

@JsonIgnore
@Setter
private String aliasId = null;

private String salt = null;

private Date passwordLastModified = null;
Expand Down Expand Up @@ -413,7 +425,7 @@ public Set<Group> getGroups() {
return groups;
}

public void setGroups(Collection<Group> groups) {
public void setGroups(@NonNull Collection<Group> groups) {
this.groups = new LinkedHashSet<>(groups);
}

Expand Down Expand Up @@ -532,6 +544,7 @@ public ScimUser setExternalId(String externalId) {
return this;
}

@Override
public String getZoneId() {
return zoneId;
}
Expand All @@ -540,6 +553,16 @@ public void setZoneId(String zoneId) {
this.zoneId = zoneId;
}

@Override
public String getAliasId() {
return aliasId;
}

@Override
public String getAliasZid() {
return aliasZid;
}

public String getSalt() {
return salt;
}
Expand Down Expand Up @@ -718,6 +741,67 @@ List<String> wordList() {
public void patch(ScimUser patch) {
//Delete Attributes specified in Meta.attributes
String[] attributes = ofNullable(patch.getMeta().getAttributes()).orElse(new String[0]);
removeAttributesForPatch(patch, attributes);

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);

setActive(patch.isActive());
setVerified(patch.isVerified());

//Merge complex attributes
ScimUser.Name patchName = patch.getName();
if (patchName != null) {
patchName(patchName);
}

ofNullable(patch.getDisplayName()).ifPresent(
this::setDisplayName
);
ofNullable(patch.getNickName()).ifPresent(this::setNickName);
ofNullable(patch.getTimezone()).ifPresent(this::setTimezone);
ofNullable(patch.getTitle()).ifPresent(this::setTitle);
ofNullable(patch.getProfileUrl()).ifPresent(this::setProfileUrl);
ofNullable(patch.getLocale()).ifPresent(this::setLocale);
ofNullable(patch.getPreferredLanguage()).ifPresent(this::setPreferredLanguage);

//Only one email stored, use Primary or first.
if (patch.getEmails() != null && patch.getEmails().size()>0) {
ScimUser.Email primary = null;
for (ScimUser.Email email : patch.getEmails()) {
if (email.isPrimary()) {
primary = email;
break;
}
}
List<Email> currentEmails = ofNullable(getEmails()).orElse(new ArrayList());
if (primary != null) {
for (Email e : currentEmails) {
e.setPrimary(false);
}
}
currentEmails.addAll(patch.getEmails());
setEmails(currentEmails);
}

//Only one PhoneNumber stored, use first, as primary does not exist
if (patch.getPhoneNumbers() != null && patch.getPhoneNumbers().size()>0) {
List<PhoneNumber> current = ofNullable(getPhoneNumbers()).orElse(new ArrayList<>());
for (int index=0; index<patch.getPhoneNumbers().size(); index++) {
current.add(index, patch.getPhoneNumbers().get(index));
}
setPhoneNumbers(current);
}

ofNullable(patch.getAliasId()).ifPresent(this::setAliasId);
ofNullable(patch.getAliasZid()).ifPresent(this::setAliasZid);
}

private void removeAttributesForPatch(final ScimUser patch, final String[] attributes) {
for (String attribute : attributes) {
switch (attribute.toUpperCase()) {
case "USERNAME":
Expand Down Expand Up @@ -778,67 +862,16 @@ public void patch(ScimUser patch) {
throw new IllegalArgumentException(String.format("Attribute %s cannot be removed using \"Meta.attributes\"", attribute));
}
}

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);

setActive(patch.isActive());
setVerified(patch.isVerified());

//Merge complex attributes
ScimUser.Name patchName = patch.getName();
if (patchName != null) {
ScimUser.Name currentName = ofNullable(getName()).orElse(new Name());
ofNullable(patchName.getFamilyName()).ifPresent(currentName::setFamilyName);
ofNullable(patchName.getGivenName()).ifPresent(currentName::setGivenName);
ofNullable(patchName.getMiddleName()).ifPresent(currentName::setMiddleName);
ofNullable(patchName.getFormatted()).ifPresent(currentName::setFormatted);
ofNullable(patchName.getHonorificPrefix()).ifPresent(currentName::setHonorificPrefix);
ofNullable(patchName.getHonorificSuffix()).ifPresent(currentName::setHonorificSuffix);
setName(currentName);
}

ofNullable(patch.getDisplayName()).ifPresent(
this::setDisplayName
);
ofNullable(patch.getNickName()).ifPresent(this::setNickName);
ofNullable(patch.getTimezone()).ifPresent(this::setTimezone);
ofNullable(patch.getTitle()).ifPresent(this::setTitle);
ofNullable(patch.getProfileUrl()).ifPresent(this::setProfileUrl);
ofNullable(patch.getLocale()).ifPresent(this::setLocale);
ofNullable(patch.getPreferredLanguage()).ifPresent(this::setPreferredLanguage);

//Only one email stored, use Primary or first.
if (patch.getEmails() != null && patch.getEmails().size()>0) {
ScimUser.Email primary = null;
for (ScimUser.Email email : patch.getEmails()) {
if (email.isPrimary()) {
primary = email;
break;
}
}
List<Email> currentEmails = ofNullable(getEmails()).orElse(new ArrayList());
if (primary != null) {
for (Email e : currentEmails) {
e.setPrimary(false);
}
}
currentEmails.addAll(patch.getEmails());
setEmails(currentEmails);
}

//Only one PhoneNumber stored, use first, as primary does not exist
if (patch.getPhoneNumbers() != null && patch.getPhoneNumbers().size()>0) {
List<PhoneNumber> current = ofNullable(getPhoneNumbers()).orElse(new ArrayList<>());
for (int index=0; index<patch.getPhoneNumbers().size(); index++) {
current.add(index, patch.getPhoneNumbers().get(index));
}
setPhoneNumbers(current);
}
}

private void patchName(@NonNull final Name patchName) {
Name currentName = ofNullable(getName()).orElse(new Name());
ofNullable(patchName.getFamilyName()).ifPresent(currentName::setFamilyName);
ofNullable(patchName.getGivenName()).ifPresent(currentName::setGivenName);
ofNullable(patchName.getMiddleName()).ifPresent(currentName::setMiddleName);
ofNullable(patchName.getFormatted()).ifPresent(currentName::setFormatted);
ofNullable(patchName.getHonorificPrefix()).ifPresent(currentName::setHonorificPrefix);
ofNullable(patchName.getHonorificSuffix()).ifPresent(currentName::setHonorificSuffix);
setName(currentName);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
*******************************************************************************/
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;
Expand All @@ -30,7 +29,9 @@
import java.util.LinkedList;
import java.util.List;
import java.util.Set;
import java.util.UUID;

import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.junit.Assert.*;

/**
Expand Down Expand Up @@ -363,8 +364,6 @@ public void testGroupSettersGetters() {
Group group2 = new Group("id", "display", Group.Type.DIRECT);
assertEquals(group1, group2);
assertEquals(group2, group1);
assertEquals(group1, group1);
assertEquals(group2, group2);
assertNotEquals(null, group1);
assertNotEquals(group1, new Object());
group1.setValue(null);
Expand Down Expand Up @@ -451,14 +450,12 @@ public void testPhoneNumber() {
assertEquals("type", p1.getType());
ScimUser user = new ScimUser();
user.setPhoneNumbers(Collections.singletonList(p1));
try {
p1.setType(null);
user.addPhoneNumber(p1.getValue());
fail();
}catch (IllegalArgumentException ignored) {

}

// should reject adding duplicate phone number if the existing has a type set to null
p1.setType(null);
assertThatIllegalArgumentException()
.isThrownBy(() -> user.addPhoneNumber(p1.getValue()))
.withMessageStartingWith("Already contains phoneNumber");
}

@Test
Expand Down Expand Up @@ -495,6 +492,33 @@ public void test_patch_previous_logon() {
assertNull(user.getPreviousLogonTime());
}

@Test
public void testPatchAliasId() {
final String aliasId = UUID.randomUUID().toString();
patch.setAliasId(aliasId);
user.patch(patch);
assertEquals(aliasId, user.getAliasId());
}

@Test
public void testPatchAliasZid() {
final String aliasZid = UUID.randomUUID().toString();
patch.setAliasZid(aliasZid);
user.patch(patch);
assertEquals(aliasZid, user.getAliasZid());
}

@Test
public void testAliasPropertiesGettersAndSetters() {
final String aliasId = UUID.randomUUID().toString();
final String aliasZid = UUID.randomUUID().toString();

final ScimUser scimUser = new ScimUser("id", "uname", "gname", "fname");
scimUser.setAliasId(aliasId);
scimUser.setAliasZid(aliasZid);
assertEquals(aliasId, scimUser.getAliasId());
assertEquals(aliasZid, scimUser.getAliasZid());
}

@Test
public void testPatchUserSetPrimaryEmail() {
Expand Down Expand Up @@ -587,7 +611,7 @@ public void testPatchUserDropNameSubAttributes() {
@Test
public void testPatchUserRejectChangingOrigin() {
patch.setOrigin("some-new-origin");
Assertions.assertThatIllegalArgumentException().isThrownBy(() -> user.patch(patch))
assertThatIllegalArgumentException().isThrownBy(() -> user.patch(patch))
.withMessage("Cannot change origin in patch of user.");
}

Expand Down Expand Up @@ -653,20 +677,6 @@ public void testPatchUserDropNonUsedAttributes() {
setAndPatchAndValidate("displayname", --pos);

assertEquals(0, pos);














}

public void setAndPatchAndValidate(String attribute, int nullable) {
Expand Down
Loading
Loading