From c2cd83994448b7303e5a32cad8ecd2e6266b5c8d Mon Sep 17 00:00:00 2001 From: Enrico Vianello Date: Thu, 13 Jun 2024 15:51:13 +0200 Subject: [PATCH] Add authorities list to SCIM user and rename isAdmin to isVoAdmin (#788) --- .../iam/api/scim/converter/UserConverter.java | 6 ++- .../mw/iam/api/scim/model/ScimIndigoUser.java | 45 ++++--------------- .../infn/mw/iam/api/scim/model/ScimUser.java | 12 ++--- .../mw/iam/config/scim/ScimProperties.java | 9 ++++ .../src/main/resources/application.yml | 1 + .../mw/iam/test/scim/ScimRestUtilsMvc.java | 2 - .../ScimAccountAuthoritiesConverterTests.java | 16 +++++-- .../user/ScimUserServiceTests.java | 2 +- .../me/ScimMeFullResponseEndpointTests.java | 12 +++-- .../test/scim/user/ScimUserCreationTests.java | 5 ++- 10 files changed, 55 insertions(+), 55 deletions(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/scim/converter/UserConverter.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/scim/converter/UserConverter.java index d43977bdf..c2a1aebfd 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/scim/converter/UserConverter.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/scim/converter/UserConverter.java @@ -243,8 +243,6 @@ public ScimUser dtoFromEntity(IamAccount entity) { .build())); } - builder.isAdmin(entity.getAuthorities().stream().anyMatch(a -> a.isAdminAuthority())); - if (properties.isIncludeManagedGroups()) { groupManagerService.getManagedGroupInfoForAccount(entity) .getManagedGroups() @@ -255,6 +253,10 @@ public ScimUser dtoFromEntity(IamAccount entity) { .build())); } + if (properties.isIncludeAuthorities()) { + entity.getAuthorities().forEach(a -> builder.addAuthority(a.getAuthority())); + } + return builder.build(); } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/scim/model/ScimIndigoUser.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/scim/model/ScimIndigoUser.java index 35e91ab35..daac57df2 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/scim/model/ScimIndigoUser.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/scim/model/ScimIndigoUser.java @@ -18,7 +18,6 @@ import java.util.Date; import java.util.LinkedList; import java.util.List; -import java.util.Objects; import javax.validation.Valid; @@ -42,7 +41,7 @@ public enum INDIGO_USER_SCHEMA { X509_CERTS(ScimConstants.INDIGO_USER_SCHEMA + ".x509Certificates"), AUP_SIGNATURE_TIME(ScimConstants.INDIGO_USER_SCHEMA + ".aupSignatureTime"), LABELS(ScimConstants.INDIGO_USER_SCHEMA + ".labels"), - IS_ADMIN(ScimConstants.INDIGO_USER_SCHEMA + ".isAdmin"), + AUTHORITIES(ScimConstants.INDIGO_USER_SCHEMA + ".authorities"), ATTRIBUTES(ScimConstants.INDIGO_USER_SCHEMA + ".attributes"), MANAGED_GROUPS(ScimConstants.INDIGO_USER_SCHEMA + ".managedGroups"); // @formatter:on @@ -76,8 +75,7 @@ public String toString() { @JsonSerialize(using = JsonDateSerializer.class) private final Date endTime; - @JsonProperty(value = "isAdmin") - private final Boolean isAdmin; + private final List authorities; @Valid private final List attributes; @@ -100,9 +98,9 @@ private ScimIndigoUser(@JsonProperty("oidcIds") List oidcIds, this.aupSignatureTime = aupSignatureTime; this.endTime = endTime; this.labels = null; + this.authorities = null; this.attributes = null; this.managedGroups = null; - this.isAdmin = null; } private ScimIndigoUser(Builder b) { @@ -115,7 +113,7 @@ private ScimIndigoUser(Builder b) { this.labels = b.labels; this.attributes = b.attributes; this.managedGroups = b.managedGroups; - this.isAdmin = b.isAdmin; + this.authorities = b.authorities; } public List getSshKeys() { @@ -145,8 +143,8 @@ public List getLabels() { return labels; } - public Boolean isAdmin() { - return isAdmin; + public List getAuthorities() { + return authorities; } public List getAttributes() { @@ -177,7 +175,7 @@ public static class Builder { private Date aupSignatureTime; private Date endTime; - private Boolean isAdmin; + private List authorities = Lists.newLinkedList(); private List attributes = Lists.newLinkedList(); private List managedGroups = Lists.newLinkedList(); @@ -224,8 +222,8 @@ public Builder addLabel(ScimLabel label) { return this; } - public Builder isAdmin(Boolean isAdmin) { - this.isAdmin = isAdmin; + public Builder addAuthority(String authority) { + authorities.add(authority); return this; } @@ -243,31 +241,6 @@ public ScimIndigoUser build() { return new ScimIndigoUser(this); } - @Override - public int hashCode() { - return Objects.hash(attributes, aupSignatureTime, certificates, endTime, isAdmin, labels, - managedGroups, oidcIds, samlIds, sshKeys); - } - - @Override - public boolean equals(Object obj) { - if (this == obj) - return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - Builder other = (Builder) obj; - return Objects.equals(attributes, other.attributes) - && Objects.equals(aupSignatureTime, other.aupSignatureTime) - && Objects.equals(certificates, other.certificates) - && Objects.equals(endTime, other.endTime) && Objects.equals(isAdmin, other.isAdmin) - && Objects.equals(labels, other.labels) - && Objects.equals(managedGroups, other.managedGroups) - && Objects.equals(oidcIds, other.oidcIds) && Objects.equals(samlIds, other.samlIds) - && Objects.equals(sshKeys, other.sshKeys); - } - } } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/scim/model/ScimUser.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/scim/model/ScimUser.java index 7cf662cab..dc5e60bb3 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/scim/model/ScimUser.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/scim/model/ScimUser.java @@ -457,19 +457,19 @@ public Builder aupSignatureTime(Date signatureTime) { return this; } - public Builder isAdmin(Boolean isAdmin) { + public Builder endTime(Date endTime) { - Preconditions.checkNotNull(isAdmin, "Null isAdmin"); + Preconditions.checkNotNull(endTime, "Null membership end-time"); - indigoUserBuilder.isAdmin(isAdmin); + indigoUserBuilder.endTime(endTime); return this; } - public Builder endTime(Date endTime) { + public Builder addAuthority(String authority) { - Preconditions.checkNotNull(endTime, "Null membership end-time"); + Preconditions.checkNotNull(authority, "Null authority"); - indigoUserBuilder.endTime(endTime); + indigoUserBuilder.addAuthority(authority); return this; } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/scim/ScimProperties.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/scim/ScimProperties.java index 7ce965be2..8f64ac280 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/scim/ScimProperties.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/scim/ScimProperties.java @@ -61,6 +61,7 @@ public void setName(String name) { List includeLabels = Lists.newArrayList(); List includeAttributes = Lists.newArrayList(); + boolean includeAuthorities = false; boolean includeManagedGroups = false; public List getIncludeLabels() { @@ -79,6 +80,14 @@ public void setIncludeAttributes(List includeAttributes) { this.includeAttributes = includeAttributes; } + public boolean isIncludeAuthorities() { + return includeAuthorities; + } + + public void setIncludeAuthorities(boolean includeAuthorities) { + this.includeAuthorities = includeAuthorities; + } + public boolean isIncludeManagedGroups() { return includeManagedGroups; } diff --git a/iam-login-service/src/main/resources/application.yml b/iam-login-service/src/main/resources/application.yml index a3de7e80e..24cd9ec8a 100644 --- a/iam-login-service/src/main/resources/application.yml +++ b/iam-login-service/src/main/resources/application.yml @@ -282,4 +282,5 @@ lifecycle: enabled: ${IAM_LIFECYCLE_EXPIRED_ACCOUNT_TASK_ENABLED:true} scim: + include_authorities: ${IAM_SCIM_INCLUDE_AUTHORITIES:false} include_managed_groups: ${IAM_SCIM_INCLUDE_MANAGED_GROUPS:false} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/ScimRestUtilsMvc.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/ScimRestUtilsMvc.java index c3df85dbd..ef2a24dfd 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/ScimRestUtilsMvc.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/ScimRestUtilsMvc.java @@ -56,8 +56,6 @@ public ResultActions postUser(ScimUser user, HttpStatus expectedStatus) throws E return doPost(getUsersLocation(), user, SCIM_CONTENT_TYPE, expectedStatus); } - - public ScimUser getUser(String uuid) throws Exception { return mapper.readValue(getUser(uuid, OK).andReturn().getResponse().getContentAsString(), diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/converter/ScimAccountAuthoritiesConverterTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/converter/ScimAccountAuthoritiesConverterTests.java index 6fa9c63fa..38ea4db4e 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/converter/ScimAccountAuthoritiesConverterTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/converter/ScimAccountAuthoritiesConverterTests.java @@ -26,6 +26,7 @@ import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.test.context.support.WithMockUser; +import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.junit4.SpringRunner; import it.infn.mw.iam.api.account.authority.AccountAuthorityService; @@ -41,8 +42,10 @@ @RunWith(SpringRunner.class) @IamMockMvcIntegrationTest +@TestPropertySource(properties = {"scim.include_authorities=true"}) public class ScimAccountAuthoritiesConverterTests { + private static final String USER_AUTHORITY = "ROLE_USER"; private static final String GM_AUTHORITY = "ROLE_GM" + UUID.randomUUID(); private static final String ADMIN_AUTHORITY = "ROLE_ADMIN"; @@ -87,18 +90,25 @@ public void testAuthoritiesReturnedIfAllowedByConfigurationSerializedByDefault() ScimUser user = scimUtils.getUser(testAccount.getUuid()); - assertThat(user.getIndigoUser().isAdmin(), equalTo(false)); + assertThat(user.getIndigoUser().getAuthorities().size(), equalTo(1)); + assertThat(user.getIndigoUser().getAuthorities().get(0), equalTo(USER_AUTHORITY)); authorityService.addAuthorityToAccount(testAccount, GM_AUTHORITY); user = scimUtils.getUser(testAccount.getUuid()); - assertThat(user.getIndigoUser().isAdmin(), equalTo(false)); + assertThat(user.getIndigoUser().getAuthorities().size(), equalTo(2)); + assertThat(user.getIndigoUser().getAuthorities().contains(USER_AUTHORITY), equalTo(true)); + assertThat(user.getIndigoUser().getAuthorities().contains(GM_AUTHORITY), equalTo(true)); authorityService.addAuthorityToAccount(testAccount, ADMIN_AUTHORITY); user = scimUtils.getUser(testAccount.getUuid()); - assertThat(user.getIndigoUser().isAdmin(), equalTo(true)); + assertThat(user.getIndigoUser().getAuthorities().size(), equalTo(3)); + assertThat(user.getIndigoUser().getAuthorities().contains(USER_AUTHORITY), equalTo(true)); + assertThat(user.getIndigoUser().getAuthorities().contains(GM_AUTHORITY), equalTo(true)); + assertThat(user.getIndigoUser().getAuthorities().contains(ADMIN_AUTHORITY), equalTo(true)); + } } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/core/provisioning/user/ScimUserServiceTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/core/provisioning/user/ScimUserServiceTests.java index 9954037d0..7255dcb89 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/core/provisioning/user/ScimUserServiceTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/core/provisioning/user/ScimUserServiceTests.java @@ -191,9 +191,9 @@ public void createAndReplaceUserCheckingNotWritableFieldsAreIgnoredTest() { .addEmail(TESTUSER_EMAIL) // mandatory .name(TESTUSER_NAME) // mandatory .addAttribute(TESTUSER_ATTRIBUTE) // not writable - .isAdmin(true) // not writable .addLabel(TESTUSER_LABEL) // not writable .addManagedGroup(TESTUSER_GROUP_REF) // not writable + .addAuthority("ROLE_ADMIN") // not writable .build(); userService.create(scimUser); diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/me/ScimMeFullResponseEndpointTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/me/ScimMeFullResponseEndpointTests.java index 8031bc3d5..81824cb5c 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/me/ScimMeFullResponseEndpointTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/me/ScimMeFullResponseEndpointTests.java @@ -17,8 +17,9 @@ import static it.infn.mw.iam.api.scim.model.ScimConstants.SCIM_CONTENT_TYPE; import static it.infn.mw.iam.api.scim.model.ScimIndigoUser.INDIGO_USER_SCHEMA.ATTRIBUTES; -import static it.infn.mw.iam.api.scim.model.ScimIndigoUser.INDIGO_USER_SCHEMA.IS_ADMIN; +import static it.infn.mw.iam.api.scim.model.ScimIndigoUser.INDIGO_USER_SCHEMA.AUTHORITIES; import static it.infn.mw.iam.api.scim.model.ScimIndigoUser.INDIGO_USER_SCHEMA.MANAGED_GROUPS; +import static org.hamcrest.CoreMatchers.hasItem; import static org.hamcrest.Matchers.hasSize; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; @@ -41,7 +42,7 @@ @RunWith(SpringJUnit4ClassRunner.class) @IamMockMvcIntegrationTest @TestPropertySource(properties = {"scim.include_attributes[0].name=affiliation", - "scim.include_managed_groups=true"}) + "scim.include_managed_groups=true", "scim.include_authorities=true"}) public class ScimMeFullResponseEndpointTests { private final static String ME_ENDPOINT = "/scim/Me"; @@ -74,8 +75,11 @@ public void meEndpointFullUserInfo() throws Exception { .andExpect(jsonPath("$." + ATTRIBUTES, hasSize(1))) .andExpect(jsonPath("$." + ATTRIBUTES + "[0].name").value("affiliation")) .andExpect(jsonPath("$." + ATTRIBUTES + "[0].value").value("INFN-CNAF")) - .andExpect(jsonPath("$." + IS_ADMIN).exists()) - .andExpect(jsonPath("$." + IS_ADMIN).value("false")) + .andExpect(jsonPath("$." + AUTHORITIES).exists()) + .andExpect(jsonPath("$." + AUTHORITIES).isArray()) + .andExpect(jsonPath("$." + AUTHORITIES, hasSize(2))) + .andExpect(jsonPath("$." + AUTHORITIES, hasItem("ROLE_USER"))) + .andExpect(jsonPath("$." + AUTHORITIES, hasItem("ROLE_GM:c617d586-54e6-411d-8e38-64967798fa8a"))) .andExpect(jsonPath("$." + MANAGED_GROUPS).exists()) .andExpect(jsonPath("$." + MANAGED_GROUPS).isArray()) .andExpect(jsonPath("$." + MANAGED_GROUPS, hasSize(1))) diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/user/ScimUserCreationTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/user/ScimUserCreationTests.java index 92f78cee0..dce25dabd 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/user/ScimUserCreationTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/user/ScimUserCreationTests.java @@ -46,6 +46,7 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; import org.springframework.security.crypto.password.PasswordEncoder; +import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.web.servlet.MockMvc; @@ -73,6 +74,7 @@ @SpringBootTest( classes = {IamLoginService.class, CoreControllerTestSupport.class, ScimRestUtilsMvc.class}, webEnvironment = WebEnvironment.MOCK) +@TestPropertySource(properties = {"scim.include_authorities=true"}) public class ScimUserCreationTests extends ScimUserTestSupport { @Autowired @@ -450,6 +452,7 @@ public void testUserCreationWithEndTimeAndBasicAuthorities() throws Exception { ScimUser user = buildUser("user_with_exp_time", "userwithexptime@email.test", "User", "Test") .endTime(expTime) + .addAuthority("ROLE_ADMIN") .build(); ScimUser createdUser = scimUtils.postUser(user); @@ -458,7 +461,7 @@ public void testUserCreationWithEndTimeAndBasicAuthorities() throws Exception { assertThat(user.getEmails().get(0).getValue(), equalTo(createdUser.getEmails().get(0).getValue())); assertNull(createdUser.getIndigoUser().getEndTime()); - assertThat(createdUser.getIndigoUser().isAdmin(), equalTo(false)); + assertThat(createdUser.getIndigoUser().getAuthorities().contains("ROLE_ADMIN"), equalTo(false)); } @Test