From b0f5a899bd02701d3663978b505e2fe3a482330f Mon Sep 17 00:00:00 2001 From: chasegawa Date: Fri, 13 Aug 2021 08:52:16 -0700 Subject: [PATCH 1/2] SHIBUI-1742 merged from 1740 code (master) --- .../JPAMetadataResolverServiceImpl.groovy | 4 +- .../admin/ui/domain/ActivatableType.java | 5 +++ .../admin/ui/domain/EntityDescriptor.java | 23 +++++++--- .../admin/ui/domain/IActivatable.java | 7 +++ .../ui/domain/filters/MetadataFilter.java | 24 +++++++--- .../ui/domain/resolvers/MetadataResolver.java | 44 +++++++++++-------- .../ui/security/service/UserService.java | 37 +++++++++++----- .../JPAEntityDescriptorServiceImpl.java | 4 +- .../ui/service/JPAFilterServiceImpl.java | 13 +++--- 9 files changed, 110 insertions(+), 51 deletions(-) create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/ActivatableType.java create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/IActivatable.java diff --git a/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy b/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy index 365125395..abd4ec060 100644 --- a/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy +++ b/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy @@ -559,8 +559,8 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService { } public edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver updateMetadataResolverEnabledStatus(edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver updatedResolver) throws ForbiddenException, MetadataFileNotFoundException, InitializationException { - // @TODO: when merged with groups, this should maybe be merged with group check as they have to have the role in the right group - if (!userService.currentUserHasExpectedRole(["ROLE_ADMIN", "ROLE_ENABLE"])) { + if (!userService.currentUserCanEnable(updatedResolver)) { +// if (!userService.currentUserHasExpectedRole(["ROLE_ADMIN", "ROLE_ENABLE"])) { throw new ForbiddenException("You do not have the permissions necessary to change the enable status of this filter.") } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/ActivatableType.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/ActivatableType.java new file mode 100644 index 000000000..8042d56d4 --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/ActivatableType.java @@ -0,0 +1,5 @@ +package edu.internet2.tier.shibboleth.admin.ui.domain; + +public enum ActivatableType { + ENTITY_DESCRIPTOR, METADATA_RESOLVER, FILTER +} \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/EntityDescriptor.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/EntityDescriptor.java index a3acff06b..185b43918 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/EntityDescriptor.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/EntityDescriptor.java @@ -33,11 +33,12 @@ import java.util.UUID; import java.util.stream.Collectors; +import static edu.internet2.tier.shibboleth.admin.ui.domain.ActivatableType.ENTITY_DESCRIPTOR; @Entity @EqualsAndHashCode(callSuper = true) @Audited -public class EntityDescriptor extends AbstractDescriptor implements org.opensaml.saml.saml2.metadata.EntityDescriptor, Ownable { +public class EntityDescriptor extends AbstractDescriptor implements org.opensaml.saml.saml2.metadata.EntityDescriptor, Ownable, IActivatable { @OneToMany(cascade = CascadeType.ALL) @JoinColumn(name = "entitydesc_addlmetdatlocations_id") @OrderColumn @@ -47,7 +48,7 @@ public class EntityDescriptor extends AbstractDescriptor implements org.opensaml @OneToOne(cascade = CascadeType.ALL) @NotAudited private AffiliationDescriptor affiliationDescriptor; - + @OneToOne(cascade = CascadeType.ALL) @NotAudited private AttributeAuthorityDescriptor attributeAuthorityDescriptor; @@ -61,7 +62,7 @@ public class EntityDescriptor extends AbstractDescriptor implements org.opensaml private List contactPersons = new ArrayList<>(); private String entityID; - + private String localId; @OneToOne(cascade = CascadeType.ALL) @@ -70,7 +71,7 @@ public class EntityDescriptor extends AbstractDescriptor implements org.opensaml @Getter @Setter private String idOfOwner; - + @OneToOne(cascade = CascadeType.ALL) @NotAudited private PDPDescriptor pdpDescriptor; @@ -254,6 +255,10 @@ public void setEntityID(String entityID) { this.entityID = entityID; } + public void setEnabled(Boolean serviceEnabled) { + this.serviceEnabled = (serviceEnabled == null) ? false : serviceEnabled; + } + @Override public void setID(String id) { this.localId = id; @@ -296,12 +301,16 @@ public String toString() { .add("id", id) .toString(); } - + public String getObjectId() { return entityID; } - + public OwnableType getOwnableType() { return OwnableType.ENTITY_DESCRIPTOR; } -} + + @Override public ActivatableType getActivatableType() { + return ENTITY_DESCRIPTOR; + } +} \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/IActivatable.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/IActivatable.java new file mode 100644 index 000000000..84a31078a --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/IActivatable.java @@ -0,0 +1,7 @@ +package edu.internet2.tier.shibboleth.admin.ui.domain; + +public interface IActivatable { + ActivatableType getActivatableType(); + + void setEnabled(Boolean enabled); +} \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java index 9363f5711..33b763231 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java @@ -5,6 +5,8 @@ import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; import edu.internet2.tier.shibboleth.admin.ui.domain.AbstractAuditable; +import edu.internet2.tier.shibboleth.admin.ui.domain.ActivatableType; +import edu.internet2.tier.shibboleth.admin.ui.domain.IActivatable; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.NoArgsConstructor; @@ -20,6 +22,8 @@ import javax.persistence.Transient; import java.util.UUID; +import static edu.internet2.tier.shibboleth.admin.ui.domain.ActivatableType.*; + /** * Domain class to store information about {@link org.opensaml.saml.metadata.resolver.filter.MetadataFilter} */ @@ -38,22 +42,26 @@ @JsonSubTypes.Type(value=NameIdFormatFilter.class, name="NameIDFormat")}) @Audited @AuditOverride(forClass = AbstractAuditable.class) -public abstract class MetadataFilter extends AbstractAuditable implements IConcreteMetadataFilterType { +public abstract class MetadataFilter extends AbstractAuditable implements IConcreteMetadataFilterType, IActivatable { - @JsonProperty("@type") - @Transient - String type; + private boolean filterEnabled; private String name; @Column(unique=true) private String resourceId = UUID.randomUUID().toString(); - private boolean filterEnabled; + @JsonProperty("@type") + @Transient + String type; @Transient private transient Integer version; + public ActivatableType getActivatableType() { + return FILTER; + } + @JsonGetter("version") public int getVersion() { if (version != null && version != 0) { @@ -61,4 +69,8 @@ public int getVersion() { } return this.hashCode(); } -} + + public void setEnabled(Boolean serviceEnabled) { + this.filterEnabled = (serviceEnabled == null) ? false : serviceEnabled; + } +} \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java index 13d573b56..a777aa3ca 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java @@ -6,6 +6,8 @@ import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; import edu.internet2.tier.shibboleth.admin.ui.domain.AbstractAuditable; +import edu.internet2.tier.shibboleth.admin.ui.domain.ActivatableType; +import edu.internet2.tier.shibboleth.admin.ui.domain.IActivatable; import edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityAttributesFilter; import edu.internet2.tier.shibboleth.admin.ui.domain.filters.MetadataFilter; import lombok.EqualsAndHashCode; @@ -28,6 +30,8 @@ import java.util.List; import java.util.UUID; +import static edu.internet2.tier.shibboleth.admin.ui.domain.ActivatableType.*; + @Entity @Inheritance(strategy = InheritanceType.TABLE_PER_CLASS) @EqualsAndHashCode(callSuper = true, exclude = {"version", "versionModifiedTimestamp"}) @@ -43,7 +47,7 @@ @JsonSubTypes.Type(value = ResourceBackedMetadataResolver.class, name = "ResourceBackedMetadataResolver")}) @Audited @AuditOverride(forClass = AbstractAuditable.class) -public class MetadataResolver extends AbstractAuditable { +public class MetadataResolver extends AbstractAuditable implements IActivatable { @JsonProperty("@type") @Transient @@ -84,31 +88,35 @@ public class MetadataResolver extends AbstractAuditable { @Transient private Integer version; - @JsonGetter("version") - public int getVersion() { - if (this.version != null && this.version != 0 ) { - return this.version; - } - return this.hashCode(); - } - public void addFilter(MetadataFilter metadataFilter) { //To make sure that Spring Data auditing infrastructure recognizes update and "touched" modifiedDate markAsModified(); this.metadataFilters.add(metadataFilter); } - public void markAsModified() { - this.versionModifiedTimestamp = System.currentTimeMillis(); - } - public void entityAttributesFilterIntoTransientRepresentation() { //expose explicit API to call to convert into transient representation //used in unit/integration tests where JPA's @PostLoad callback execution engine is not available this.metadataFilters - .stream() - .filter(EntityAttributesFilter.class::isInstance) - .map(EntityAttributesFilter.class::cast) - .forEach(EntityAttributesFilter::intoTransientRepresentation); + .stream() + .filter(EntityAttributesFilter.class::isInstance) + .map(EntityAttributesFilter.class::cast) + .forEach(EntityAttributesFilter::intoTransientRepresentation); + } + + @Override public ActivatableType getActivatableType() { + return METADATA_RESOLVER; + } + + @JsonGetter("version") + public int getVersion() { + if (this.version != null && this.version != 0 ) { + return this.version; + } + return this.hashCode(); + } + + public void markAsModified() { + this.versionModifiedTimestamp = System.currentTimeMillis(); } -} +} \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java index 2081f02f5..4c1bdc4c2 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java @@ -1,10 +1,10 @@ package edu.internet2.tier.shibboleth.admin.ui.security.service; -import java.util.HashSet; -import java.util.List; -import java.util.Optional; -import java.util.Set; +import java.util.*; +import edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor; +import edu.internet2.tier.shibboleth.admin.ui.domain.IActivatable; +import edu.internet2.tier.shibboleth.admin.ui.domain.filters.MetadataFilter; import org.apache.commons.lang.StringUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.core.context.SecurityContextHolder; @@ -25,6 +25,8 @@ import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository; import lombok.NoArgsConstructor; +import static edu.internet2.tier.shibboleth.admin.ui.security.service.UserAccess.*; + @Service @NoArgsConstructor public class UserService { @@ -47,10 +49,25 @@ public UserService(IGroupService groupService, OwnershipRepository ownershipRepo this.userRepository = userRepository; } + public boolean currentUserCanEnable(IActivatable activatableObject) { + switch (activatableObject.getActivatableType()) { + case ENTITY_DESCRIPTOR: { + if (getCurrentUserAccess() == ADMIN) { return true; } + if (currentUserHasExpectedRole(Arrays.asList("ROLE_ENABLE" )) && getCurrentUserGroup().getOwnerId().equals(((EntityDescriptor) activatableObject).getIdOfOwner())) { + return true; + } + } + case FILTER: + case METADATA_RESOLVER: + return currentUserHasExpectedRole(Arrays.asList("ROLE_ADMIN", "ROLE_ENABLE" )); + } + return false; + } + /** - * Current logic is pretty dumb, this will need to change/expand once a user can have more than one role. + * This basic logic assumes users only have a single role (despite users having a list of roles, we assume only 1 currently) */ - public boolean currentUserHasExpectedRole(List acceptedRoles) { + private boolean currentUserHasExpectedRole(List acceptedRoles) { User user = getCurrentUser(); return acceptedRoles.contains(user.getRole()); } @@ -91,15 +108,15 @@ public User getCurrentUser() { public UserAccess getCurrentUserAccess() { User user = getCurrentUser(); if (user == null) { - return UserAccess.NONE; + return NONE; } if (user.getRole().equals("ROLE_ADMIN")) { - return UserAccess.ADMIN; + return ADMIN; } if (user.getRole().equals("ROLE_USER")) { - return UserAccess.GROUP; + return GROUP; } - return UserAccess.NONE; + return NONE; } public Group getCurrentUserGroup() { diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java index 7752eafa2..668f2636b 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java @@ -391,8 +391,8 @@ public EntityDescriptorRepresentation updateEntityDescriptorEnabledStatus(String if (ed == null) { throw new EntityNotFoundException("Entity with resourceid[" + resourceId + "] was not found for update"); } - // @TODO: when merged with groups, this should maybe be merged with group check as they have to have the role in the right group - if (!userService.currentUserHasExpectedRole(Arrays.asList(new String[] { "ROLE_ADMIN", "ROLE_ENABLE" }))) { + if (!userService.currentUserCanEnable(ed)) { +// if (!userService.currentUserHasExpectedRole(Arrays.asList(new String[] { "ROLE_ADMIN", "ROLE_ENABLE" }))) { throw new ForbiddenException("You do not have the permissions necessary to change the enable status of this entity descriptor."); } ed.setServiceEnabled(status); diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAFilterServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAFilterServiceImpl.java index 23a70268a..c42bd7cad 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAFilterServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAFilterServiceImpl.java @@ -1,5 +1,6 @@ package edu.internet2.tier.shibboleth.admin.ui.service; +import edu.internet2.tier.shibboleth.admin.ui.domain.IActivatable; import edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityAttributesFilter; import edu.internet2.tier.shibboleth.admin.ui.domain.filters.MetadataFilter; import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.FilterRepresentation; @@ -117,13 +118,13 @@ public MetadataFilter updateFilterEnabledStatus(String metadataResolverId, Strin if (filterTobeUpdatedOptional.isEmpty()) { throw new EntityNotFoundException("Filter with resource id[" + resourceId + "] not found"); } - - // @TODO: when merged with groups, this should maybe be merged with group check as they have to have the role in the right group - if (!userService.currentUserHasExpectedRole(Arrays.asList(new String[] { "ROLE_ADMIN", "ROLE_ENABLE" }))) { + + MetadataFilter filterTobeUpdated = filterTobeUpdatedOptional.get(); + + if (!userService.currentUserCanEnable(filterTobeUpdated)) { throw new ForbiddenException("You do not have the permissions necessary to change the enable status of this filter."); } - - MetadataFilter filterTobeUpdated = filterTobeUpdatedOptional.get(); + filterTobeUpdated.setFilterEnabled(status); MetadataFilter persistedFilter = filterRepository.save(filterTobeUpdated); @@ -136,4 +137,4 @@ public MetadataFilter updateFilterEnabledStatus(String metadataResolverId, Strin return persistedFilter; } -} +} \ No newline at end of file From 0b12ec9ed27e8a0b4ae194796ba2101ce09915ea Mon Sep 17 00:00:00 2001 From: chasegawa Date: Mon, 16 Aug 2021 10:50:05 -0700 Subject: [PATCH 2/2] SHIBUI-2001 Wrapping up endpoint for enabling --- .../JPAMetadataResolverServiceImpl.groovy | 1 - .../ui/domain/filters/MetadataFilter.java | 6 +- .../ui/domain/resolvers/MetadataResolver.java | 4 +- .../ui/security/service/UserService.java | 13 +- .../JPAEntityDescriptorServiceImpl.java | 3 +- .../src/main/resources/application.properties | 4 +- ...ResolversControllerIntegrationTests.groovy | 9 +- .../security/service/UserServiceTests.groovy | 345 ++++++++++++++---- 8 files changed, 287 insertions(+), 98 deletions(-) diff --git a/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy b/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy index abd4ec060..03e041322 100644 --- a/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy +++ b/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy @@ -560,7 +560,6 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService { public edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver updateMetadataResolverEnabledStatus(edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver updatedResolver) throws ForbiddenException, MetadataFileNotFoundException, InitializationException { if (!userService.currentUserCanEnable(updatedResolver)) { -// if (!userService.currentUserHasExpectedRole(["ROLE_ADMIN", "ROLE_ENABLE"])) { throw new ForbiddenException("You do not have the permissions necessary to change the enable status of this filter.") } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java index 33b763231..548c97356 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java @@ -1,9 +1,6 @@ package edu.internet2.tier.shibboleth.admin.ui.domain.filters; -import com.fasterxml.jackson.annotation.JsonGetter; -import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.annotation.JsonSubTypes; -import com.fasterxml.jackson.annotation.JsonTypeInfo; +import com.fasterxml.jackson.annotation.*; import edu.internet2.tier.shibboleth.admin.ui.domain.AbstractAuditable; import edu.internet2.tier.shibboleth.admin.ui.domain.ActivatableType; import edu.internet2.tier.shibboleth.admin.ui.domain.IActivatable; @@ -58,6 +55,7 @@ public abstract class MetadataFilter extends AbstractAuditable implements IConcr @Transient private transient Integer version; + @JsonIgnore public ActivatableType getActivatableType() { return FILTER; } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java index a777aa3ca..995fab868 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java @@ -104,7 +104,9 @@ public void entityAttributesFilterIntoTransientRepresentation() { .forEach(EntityAttributesFilter::intoTransientRepresentation); } - @Override public ActivatableType getActivatableType() { + @Override + @JsonIgnore + public ActivatableType getActivatableType() { return METADATA_RESOLVER; } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java index 4c1bdc4c2..4118b8881 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java @@ -50,18 +50,18 @@ public UserService(IGroupService groupService, OwnershipRepository ownershipRepo } public boolean currentUserCanEnable(IActivatable activatableObject) { + if (currentUserIsAdmin()) { return true; } switch (activatableObject.getActivatableType()) { case ENTITY_DESCRIPTOR: { - if (getCurrentUserAccess() == ADMIN) { return true; } - if (currentUserHasExpectedRole(Arrays.asList("ROLE_ENABLE" )) && getCurrentUserGroup().getOwnerId().equals(((EntityDescriptor) activatableObject).getIdOfOwner())) { - return true; - } + return currentUserHasExpectedRole(Arrays.asList("ROLE_ENABLE" )) && getCurrentUserGroup().getOwnerId().equals(((EntityDescriptor) activatableObject).getIdOfOwner()); } + // Currently filters and providers dont have ownership, so we just look for the right role case FILTER: case METADATA_RESOLVER: - return currentUserHasExpectedRole(Arrays.asList("ROLE_ADMIN", "ROLE_ENABLE" )); + return currentUserHasExpectedRole(Arrays.asList("ROLE_ENABLE" )); + default: + return false; } - return false; } /** @@ -137,6 +137,7 @@ public Set getUserRoles(String username) { return result; } + // @TODO - probably delegate this out to something plugable at some point public boolean isAuthorizedFor(Ownable ownableObject) { switch (getCurrentUserAccess()) { case ADMIN: // Pure admin is authorized to do anything diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java index 668f2636b..764d75f6b 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java @@ -363,7 +363,7 @@ public EntityDescriptorRepresentation update(EntityDescriptorRepresentation edRe if (existingEd == null) { throw new EntityNotFoundException(String.format("The entity descriptor with entity id [%s] was not found for update.", edRep.getId())); } - if (edRep.isServiceEnabled() && !userService.currentUserIsAdmin()) { + if (edRep.isServiceEnabled() && !userService.currentUserCanEnable(existingEd)) { throw new ForbiddenException("You do not have the permissions necessary to enable this service."); } if (!userService.isAuthorizedFor(existingEd)) { @@ -392,7 +392,6 @@ public EntityDescriptorRepresentation updateEntityDescriptorEnabledStatus(String throw new EntityNotFoundException("Entity with resourceid[" + resourceId + "] was not found for update"); } if (!userService.currentUserCanEnable(ed)) { -// if (!userService.currentUserHasExpectedRole(Arrays.asList(new String[] { "ROLE_ADMIN", "ROLE_ENABLE" }))) { throw new ForbiddenException("You do not have the permissions necessary to change the enable status of this entity descriptor."); } ed.setServiceEnabled(status); diff --git a/backend/src/main/resources/application.properties b/backend/src/main/resources/application.properties index 83f2635e0..6f8b837e3 100644 --- a/backend/src/main/resources/application.properties +++ b/backend/src/main/resources/application.properties @@ -87,7 +87,7 @@ shibui.mail.text-email-template-path-prefix=/mail/text/ shibui.mail.html.email-template-path-prefix=/mail/html/ shibui.mail.system-email-address=doNotReply@shibui.org -shibui.roles=ROLE_ADMIN,ROLE_USER,ROLE_NONE +shibui.roles=ROLE_ADMIN,ROLE_ENABLE,ROLE_USER,ROLE_NONE #In order to enable authentication via configured pac4j library (with external SAMl Idp, for example) #This property must be set to true and pac4j properties configured. For sample pac4j properties, see application.yml @@ -97,4 +97,4 @@ shibui.roles=ROLE_ADMIN,ROLE_USER,ROLE_NONE #This property must be set to true in order to enable posting stats to beacon endpoint. Furthermore, appropriate #environment variables must be set for beacon publisher to be used (the ones that are set when running shib-ui in #docker container -shibui.beacon-enabled=true +shibui.beacon-enabled=true \ No newline at end of file diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy index b8487c9c8..05f6b62c8 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy @@ -4,6 +4,7 @@ import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.databind.SerializationFeature import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule import edu.internet2.tier.shibboleth.admin.ui.configuration.CustomPropertiesConfiguration +import edu.internet2.tier.shibboleth.admin.ui.configuration.StringTrimModule import edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityAttributesFilter import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.DynamicHttpMetadataResolver import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.FileBackedHttpMetadataResolver @@ -21,6 +22,7 @@ import org.springframework.boot.test.context.SpringBootTest import org.springframework.boot.test.context.TestConfiguration import org.springframework.boot.test.web.client.TestRestTemplate import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.Profile import org.springframework.http.HttpEntity import org.springframework.http.HttpHeaders import org.springframework.test.annotation.DirtiesContext @@ -63,6 +65,7 @@ class MetadataResolversControllerIntegrationTests extends Specification { mapper.enable(SerializationFeature.INDENT_OUTPUT) mapper.setSerializationInclusion(NON_NULL) mapper.registerModule(new JavaTimeModule()) + mapper.registerModule(new StringTrimModule()) metadataResolverRepository.deleteAll() } @@ -206,7 +209,7 @@ class MetadataResolversControllerIntegrationTests extends Specification { 'ResourceBacked' | _ 'Filesystem' | _ } - + @DirtiesContext def "SHIBUI-1992 - error creating FileBackedHTTPMetadata"() { def resolver = new FileBackedHttpMetadataResolver().with { @@ -360,10 +363,10 @@ class MetadataResolversControllerIntegrationTests extends Specification { } @TestConfiguration - static class Config { + static class LocalConfig { @Bean MetadataResolver metadataResolver() { new OpenSamlChainingMetadataResolver() } } -} +} \ No newline at end of file diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/UserServiceTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/UserServiceTests.groovy index b6431f79f..80ce68ab9 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/UserServiceTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/UserServiceTests.groovy @@ -1,19 +1,28 @@ package edu.internet2.tier.shibboleth.admin.ui.security.service -import java.time.LocalDateTime -import java.time.format.DateTimeFormatter - -import javax.persistence.EntityManager - +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.databind.SerializationFeature +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule +import com.fasterxml.jackson.datatype.jsr310.deser.LocalDateTimeDeserializer +import edu.internet2.tier.shibboleth.admin.ui.configuration.CoreShibUiConfiguration +import edu.internet2.tier.shibboleth.admin.ui.configuration.CustomPropertiesConfiguration +import edu.internet2.tier.shibboleth.admin.ui.configuration.ShibUIConfiguration +import edu.internet2.tier.shibboleth.admin.ui.domain.ActivatableType +import edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor +import edu.internet2.tier.shibboleth.admin.ui.domain.IActivatable +import edu.internet2.tier.shibboleth.admin.ui.security.model.* +import edu.internet2.tier.shibboleth.admin.ui.security.repository.GroupsRepository +import edu.internet2.tier.shibboleth.admin.ui.security.repository.OwnershipRepository +import edu.internet2.tier.shibboleth.admin.ui.security.repository.RoleRepository +import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository +import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.autoconfigure.domain.EntityScan import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest -import org.springframework.boot.test.context.SpringBootTest import org.springframework.boot.test.context.TestConfiguration import org.springframework.context.annotation.Bean import org.springframework.context.annotation.ComponentScan import org.springframework.context.annotation.Profile -import org.springframework.context.annotation.PropertySource import org.springframework.data.jpa.repository.config.EnableJpaRepositories import org.springframework.http.converter.json.Jackson2ObjectMapperBuilder import org.springframework.security.test.context.support.WithMockUser @@ -22,34 +31,12 @@ import org.springframework.test.annotation.Rollback import org.springframework.test.context.ActiveProfiles import org.springframework.test.context.ContextConfiguration import org.springframework.transaction.annotation.Transactional - -import com.fasterxml.jackson.databind.ObjectMapper -import com.fasterxml.jackson.databind.SerializationFeature -import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule -import com.fasterxml.jackson.datatype.jsr310.deser.LocalDateTimeDeserializer - -import edu.internet2.tier.shibboleth.admin.ui.ShibbolethUiApplication -import edu.internet2.tier.shibboleth.admin.ui.configuration.CoreShibUiConfiguration -import edu.internet2.tier.shibboleth.admin.ui.configuration.CustomPropertiesConfiguration -import edu.internet2.tier.shibboleth.admin.ui.configuration.InternationalizationConfiguration -import edu.internet2.tier.shibboleth.admin.ui.configuration.SearchConfiguration -import edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor -import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.EntityDescriptorRepresentation -import edu.internet2.tier.shibboleth.admin.ui.security.model.Group -import edu.internet2.tier.shibboleth.admin.ui.security.model.OwnerType -import edu.internet2.tier.shibboleth.admin.ui.security.model.Ownership -import edu.internet2.tier.shibboleth.admin.ui.security.model.Role -import edu.internet2.tier.shibboleth.admin.ui.security.model.User -import edu.internet2.tier.shibboleth.admin.ui.security.model.listener.GroupUpdatedEntityListener -import edu.internet2.tier.shibboleth.admin.ui.security.model.listener.UserUpdatedEntityListener -import edu.internet2.tier.shibboleth.admin.ui.security.repository.GroupsRepository -import edu.internet2.tier.shibboleth.admin.ui.security.repository.OwnershipRepository -import edu.internet2.tier.shibboleth.admin.ui.security.repository.RoleRepository -import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository -import edu.internet2.tier.shibboleth.admin.ui.security.service.IGroupService -import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService import spock.lang.Specification +import javax.persistence.EntityManager +import java.time.LocalDateTime +import java.time.format.DateTimeFormatter + @DataJpaTest @ContextConfiguration(classes=[CoreShibUiConfiguration, CustomPropertiesConfiguration, LocalConfig]) @EnableJpaRepositories(basePackages = ["edu.internet2.tier.shibboleth.admin.ui"]) @@ -70,13 +57,18 @@ class UserServiceTests extends Specification { @Autowired RoleRepository roleRepository - + + @Autowired + ShibUIConfiguration shibUIConfiguration + @Autowired UserRepository userRepository @Autowired UserService userService - + + def users + @Transactional def setup() { userRepository.findAll().forEach { @@ -87,22 +79,217 @@ class UserServiceTests extends Specification { roleRepository.deleteAll() roleRepository.flush() groupService.clearAllForTesting() //leaves us just the admingroup - - def roles = [new Role().with { - name = 'ROLE_ADMIN' - it - }, new Role().with { - name = 'ROLE_USER' + + shibUIConfiguration.roles.each { it -> + def role = new Role(name: it) + roleRepository.saveAndFlush(role) + } + + if (userRepository.count() == 0) { + users = [new User().with { + username = 'admin' + password = '{noop}adminpass' + firstName = 'Joe' + lastName = 'Doe' + emailAddress = 'joe@institution.edu' + roles.add(roleRepository.findByName('ROLE_ADMIN').get()) + it + }, new User().with { + username = 'nonadmin' + password = '{noop}nonadminpass' + firstName = 'Peter' + lastName = 'Vandelay' + emailAddress = 'peter@institution.edu' + roles.add(roleRepository.findByName('ROLE_USER').get()) + it + }, new User().with { + username = 'robot' + password = '{noop}nonepass' + firstName = 'Bad' + lastName = 'robot' + emailAddress = 'badboy@institution.edu' + roles.add(roleRepository.findByName('ROLE_ENABLE').get()) + it + }, new User().with { + username = 'robot2' + password = '{noop}nonepass' + firstName = 'Bad2' + lastName = 'robot2' + emailAddress = 'badboy2@institution.edu' + roles.add(roleRepository.findByName('ROLE_ENABLE').get()) + it + }] + users.each { + it = userService.save(it) + } + } + } + + @WithMockUser(value = "admin", roles = ["ADMIN"]) + def "Validate isAuthorizedFor with admin user (always auth)"() { + when: "the object doesn't matter if user is admin" + def isAuth = userService.isAuthorizedFor(new Ownable(){ + String getObjectId() { return null } + OwnableType getOwnableType() { return null } + }); + + then: + isAuth + } + + @WithMockUser(value = "admin", roles = ["ADMIN"]) + def "Admin can activate"() { + when: "the object doesn't matter if user is admin" + def canEnable = userService.currentUserCanEnable(new IActivatable() { + ActivatableType getActivatableType() { return ActivatableType.ENTITY_DESCRIPTOR } + void setEnabled(Boolean enabled) { } + }); + + then: + canEnable + + when: "the object doesn't matter if user is admin" + canEnable = userService.currentUserCanEnable(new IActivatable() { + ActivatableType getActivatableType() { return ActivatableType.FILTER } + void setEnabled(Boolean enabled) { } + }); + + then: + canEnable + + when: "the object doesn't matter if user is admin" + canEnable = userService.currentUserCanEnable(new IActivatable() { + ActivatableType getActivatableType() { return ActivatableType.METADATA_RESOLVER } + void setEnabled(Boolean enabled) { } + }); + + then: + canEnable + } + + @WithMockUser(value = "nonadmin", roles = ["USER"]) + def "nonadmin cannot activate without enable role"() { + when: + def canEnable = userService.currentUserCanEnable(new IActivatable() { + ActivatableType getActivatableType() { return ActivatableType.ENTITY_DESCRIPTOR } + void setEnabled(Boolean enabled) { } + }); + + then: + !canEnable + + when: + canEnable = userService.currentUserCanEnable(new IActivatable() { + ActivatableType getActivatableType() { return ActivatableType.FILTER } + void setEnabled(Boolean enabled) { } + }); + + then: + !canEnable + + when: + canEnable = userService.currentUserCanEnable(new IActivatable() { + ActivatableType getActivatableType() { return ActivatableType.METADATA_RESOLVER } + void setEnabled(Boolean enabled) { } + }); + + then: + !canEnable + } + + @WithMockUser(value = "robot", roles = ["ENABLE"]) + def "nonadmin can activate with enable role"() { + given: + def ed = new EntityDescriptor().with { + it.idOfOwner = 'robot' it - }, new Role().with { - name = 'ROLE_NONE' + } + when: + def canEnable = userService.currentUserCanEnable(ed); + + then: + canEnable + + when: + canEnable = userService.currentUserCanEnable(new IActivatable() { + ActivatableType getActivatableType() { return ActivatableType.FILTER } + void setEnabled(Boolean enabled) { } + }); + + then: + canEnable + + when: + canEnable = userService.currentUserCanEnable(new IActivatable() { + ActivatableType getActivatableType() { return ActivatableType.METADATA_RESOLVER } + void setEnabled(Boolean enabled) { } + }); + + then: + canEnable + } + + @WithMockUser(value = "robot2", roles = ["ENABLE"]) + def "nonadmin cannot activate entity descriptor with enable role"() { + given: + def ed = new EntityDescriptor().with { + it.idOfOwner = 'robot' it - }] - roles.each { - roleRepository.save(it) - } + } + when: + def canEnable = userService.currentUserCanEnable(ed); + + then: + !canEnable + + when: + canEnable = userService.currentUserCanEnable(new IActivatable() { + ActivatableType getActivatableType() { return ActivatableType.FILTER } + void setEnabled(Boolean enabled) { } + }); + + then: + canEnable + + when: + canEnable = userService.currentUserCanEnable(new IActivatable() { + ActivatableType getActivatableType() { return ActivatableType.METADATA_RESOLVER } + void setEnabled(Boolean enabled) { } + }); + + then: + canEnable } - + + @WithMockUser(value = "nonadmin", roles = ["USER"]) + @Rollback + def "Validate isAuthorizedFor with non-admin user"() { + given: + ownershipRepository.saveAndFlush(new Ownership(new Owner() { + String getOwnerId() { return "nonadmin" } + OwnerType getOwnerType() { return OwnerType.GROUP } + }, new Ownable() { + String getObjectId() { return "foothing" } + OwnableType getOwnableType() { return OwnableType.ENTITY_DESCRIPTOR } + })) + when: + def isAuth = userService.isAuthorizedFor(new Ownable(){ + String getObjectId() { return "foothing" } + OwnableType getOwnableType() { return OwnableType.ENTITY_DESCRIPTOR } + }); + + then: + isAuth + } + + def "Double Check that shibUIConfiguration includes the enable role"() { + when: + def Optional role = roleRepository.findByName("ROLE_ENABLE") + + then: + role.isPresent() + } + @Rollback def "When creating user, user is set to the correct group"() { given: @@ -110,24 +297,24 @@ class UserServiceTests extends Specification { gb.setResourceId("testingGroupBBB") gb.setName("Group BBB") gb = groupService.createGroup(gb) - + Optional userRole = roleRepository.findByName("ROLE_USER") def User user = new User(username: "someUser", roles:[userRole.get()], password: "foo") user.setGroup(gb) - - when: + + when: def User result = userService.save(user) - + then: result.groupId == "testingGroupBBB" result.username == "someUser" result.userGroups.size() == 1 - + // Raw check that the DB is correct for ownership def Set users = ownershipRepository.findUsersByOwner(gb) users.size() == 1 users.getAt(0).ownedId == "someUser" - + // Validate that loading the group has the correct list as well Group g = groupService.find("testingGroupBBB"); g.ownedItems.size() == 1 @@ -140,31 +327,31 @@ class UserServiceTests extends Specification { ga.setResourceId("testingGroup") ga.setName("Group A") ga = groupService.createGroup(ga) - + Group gb = new Group(); gb.setResourceId("testingGroupBBB") gb.setName("Group BBB") gb = groupService.createGroup(gb) - + Optional userRole = roleRepository.findByName("ROLE_USER") def User user = new User(username: "someUser", roles:[userRole.get()], password: "foo") user.setGroup(gb) def User userInB = userService.save(user) - + when: userInB.setGroupId("testingGroup") // changing groups will happen by updating the user's groupid (from the ui) def User result = userService.save(userInB) - + then: result.groupId == "testingGroup" result.username == "someUser" result.userGroups.size() == 1 - + // Raw check that the DB is correct for ownership def Set users = ownershipRepository.findUsersByOwner(ga) users.size() == 1 users.getAt(0).ownedId == "someUser" - + // check db is correct for the previous group as well def Set users2 = ownershipRepository.findUsersByOwner(gb) users2.size() == 0 @@ -172,11 +359,11 @@ class UserServiceTests extends Specification { // Validate that loading the group has the correct list as well Group g = groupService.find("testingGroup"); g.ownedItems.size() == 1 - + Group g2 = groupService.find("testingGroupBBB"); g2.ownedItems.size() == 0 } - + @Rollback def "logically try to match user controller test causing headaches"() { given: @@ -184,26 +371,26 @@ class UserServiceTests extends Specification { ga.setResourceId("testingGroup") ga.setName("Group A") ga = groupService.createGroup(ga) - + Optional userRole = roleRepository.findByName("ROLE_USER") def User user = new User(username: "someUser", firstName: "Fred", lastName: "Flintstone", roles:[userRole.get()], password: "foo") user.setGroup(ga) userService.save(user) - + when: def User flintstoneUser = userRepository.findByUsername("someUser").get() flintstoneUser.setFirstName("Wilma") flintstoneUser.setGroupId("testingGroup") - + def User result = userService.save(flintstoneUser) - + then: result.groupId == "testingGroup" result.username == "someUser" result.userGroups.size() == 1 result.firstName == "Wilma" } - + @Rollback def "When creating user, user with multiple groups is saved correctly"() { given: @@ -211,12 +398,12 @@ class UserServiceTests extends Specification { ga.setResourceId("testingGroup") ga.setName("Group A") ga = groupService.createGroup(ga) - + Group gb = new Group(); gb.setResourceId("testingGroupBBB") gb.setName("Group BBB") gb = groupService.createGroup(gb) - + Optional userRole = roleRepository.findByName("ROLE_USER") User user = new User().with( { it.username = "someUser" @@ -224,41 +411,41 @@ class UserServiceTests extends Specification { it.password = "foo" it }) - + HashSet groups = new HashSet<>() groups.add(ga) groups.add(gb) user.setGroups(groups) - + when: def result = userService.save(user) - + then: result.userGroups.size() == 2 - + // Raw check that the DB is correct for ownership def Set users = ownershipRepository.findUsersByOwner(ga) users.size() == 1 users.getAt(0).ownedId == "someUser" - + def Set users2 = ownershipRepository.findUsersByOwner(gb) users2.size() == 1 users2.getAt(0).ownedId == "someUser" - + when: def userFromDb = userRepository.findById(result.id).get(); - + then: userFromDb.getUserGroups().size() == 2 - + when: Group gbUpdated = groupService.find("testingGroupBBB") - + then: gbUpdated.ownedItems.size() == 1 } - @org.springframework.boot.test.context.TestConfiguration + @TestConfiguration @Profile("local") static class LocalConfig { @Bean