Skip to content

Commit

Permalink
SHIBUI-1988/1991
Browse files Browse the repository at this point in the history
Default GROUP handling and some user owner checks removed
  • Loading branch information
chasegawa committed Jul 4, 2021
1 parent 95a1529 commit 0721b0a
Show file tree
Hide file tree
Showing 13 changed files with 76 additions and 111 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ public class EntityDescriptor extends AbstractDescriptor implements org.opensaml
@JoinColumn(name = "group_resource_id")
@EqualsAndHashCode.Exclude
@Setter
@Getter
@Audited(targetAuditMode = RelationTargetAuditMode.NOT_AUDITED)
private Group group;

Expand Down Expand Up @@ -145,6 +144,10 @@ public IDPSSODescriptor getIDPSSODescriptor(String s) {
.orElse(null);
}

public Group getGroup() {
return group == null ? Group.DEFAULT_GROUP : group;
}

@Transient
public Optional<SPSSODescriptor> getOptionalSPSSODescriptor() {
return this.getOptionalSPSSODescriptor("");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;

import edu.internet2.tier.shibboleth.admin.ui.security.model.Group;
import lombok.Getter;
import lombok.Setter;

Expand Down Expand Up @@ -34,7 +35,6 @@ public class EntityDescriptorRepresentation implements Serializable {
@NotNull
private String entityId;

@Getter
@Setter
private String groupId;

Expand Down Expand Up @@ -109,6 +109,10 @@ public String getId() {
return id;
}

public String getGroupId() {
return groupId == null ? Group.DEFAULT_GROUP.getResourceId() : groupId;
}

public List<LogoutEndpointRepresentation> getLogoutEndpoints() {
return this.getLogoutEndpoints(false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public interface EntityDescriptorRepository extends JpaRepository<EntityDescript
"where e.createdBy = u.username and e.serviceEnabled = false and r.name in ('ROLE_USER', 'ROLE_NONE')")
Stream<EntityDescriptor> findAllDisabledAndNotOwnedByAdmin();

Stream<EntityDescriptor> findAllStreamByCreatedBy(String createdBy);
// Stream<EntityDescriptor> findAllStreamByCreatedBy(String createdBy);

Stream<EntityDescriptor> findAllStreamByGroup_resourceIdOrCreatedBy(String resourceId, String username);
Stream<EntityDescriptor> findAllStreamByGroup_resourceId(String resourceId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import javax.persistence.FetchType;
import javax.persistence.Id;
import javax.persistence.OneToMany;
import javax.persistence.Transient;

import org.hibernate.envers.Audited;
import org.hibernate.envers.RelationTargetAuditMode;
Expand All @@ -22,6 +23,10 @@
@Entity(name = "user_groups")
@Data
public class Group {
@Transient
@JsonIgnore
public static Group DEFAULT_GROUP;

@Column(name = "group_description", nullable = true)
String description;

Expand All @@ -41,4 +46,7 @@ public class Group {
@JsonIgnore
@EqualsAndHashCode.Exclude
Set<EntityDescriptor> entityDescriptors;

@Column(name = "default_group", nullable = false)
boolean defaultGroup = false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
@ToString(exclude = "roles")
@Table(name = "USERS")
public class User extends AbstractAuditable {

private String emailAddress;

private String firstName;
Expand Down Expand Up @@ -70,9 +69,13 @@ public class User extends AbstractAuditable {
@Column(nullable = false, unique = true)
private String username;

public Group getGroup() {
return group == null ? Group.DEFAULT_GROUP : group;
}

public String getGroupId() {
if (groupId == null && group != null) {
groupId = group.getResourceId();
if (groupId == null && getGroup() != null) {
groupId = getGroup().getResourceId();
}
return groupId;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,6 @@ public interface GroupsRepository extends JpaRepository<Group, String> {

@SuppressWarnings("unchecked")
Group save(Group group);

Group findByDefaultGroupTrue();
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package edu.internet2.tier.shibboleth.admin.ui.security.service;

import java.util.List;
import java.util.UUID;

import javax.annotation.PostConstruct;
import javax.transaction.Transactional;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;
Expand Down Expand Up @@ -59,4 +63,16 @@ public Group updateGroup(Group group) throws EntityNotFoundException {
return repo.save(group);
}

@PostConstruct
@Transactional
private void ensureDefaultGroupExists() {
Group g = repo.findByDefaultGroupTrue();
if (g == null) {
g = new Group();
g.setDefaultGroup(true);
g.setName("DEFAULT-GROUP");
g = repo.save(g);
}
Group.DEFAULT_GROUP = g;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ public UserAccess getCurrentUserAccess() {
if (user.getGroup() != null) {
return UserAccess.GROUP;
}
else {
return UserAccess.OWNER;
}
return UserAccess.NONE;
}

public boolean isAuthorizedFor(String objectCreatedBy, Group objectGroup) {
Expand All @@ -66,10 +64,7 @@ public boolean isAuthorizedFor(String objectCreatedBy, String objectGroupResourc
return true;
case GROUP:
User currentUser = getCurrentUser();
return objectCreatedBy.equals(currentUser.getUsername()) || groupId.equals(currentUser.getGroupId());
case OWNER:
User cu = getCurrentUser();
return objectCreatedBy.equals(cu.getUsername());
return groupId.equals(currentUser.getGroupId());
default:
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public EntityDescriptorRepresentation createRepresentationFromDescriptor(org.ope
representation.setVersion(ed.hashCode());
representation.setCreatedBy(ed.getCreatedBy());
representation.setCurrent(ed.isCurrent());
representation.setGroupId(ed.getGroup() != null ? ed.getGroup().getResourceId() : null);
representation.setGroupId(ed.getGroup() != null ? ed.getGroup().getResourceId() : Group.DEFAULT_GROUP.getResourceId());

if (ed.getSPSSODescriptor("") != null && ed.getSPSSODescriptor("").getSupportedProtocols().size() > 0) {
ServiceProviderSsoDescriptorRepresentation serviceProviderSsoDescriptorRepresentation = representation.getServiceProviderSsoDescriptor(true);
Expand Down Expand Up @@ -414,11 +414,11 @@ public List<EntityDescriptorRepresentation> getAllRepresentationsBasedOnUserAcce
User user = userService.getCurrentUser();
Group group = user.getGroup();
return entityDescriptorRepository
.findAllStreamByGroup_resourceIdOrCreatedBy(group == null ? null : group.getResourceId(), user.getUsername())
.map(ed -> createRepresentationFromDescriptor(ed)).collect(Collectors.toList());
case OWNER:
return entityDescriptorRepository.findAllStreamByCreatedBy(userService.getCurrentUser().getUsername())
.findAllStreamByGroup_resourceId(group.getResourceId())
.map(ed -> createRepresentationFromDescriptor(ed)).collect(Collectors.toList());
// case OWNER:
// return entityDescriptorRepository.findAllStreamByCreatedBy(userService.getCurrentUser().getUsername())
// .map(ed -> createRepresentationFromDescriptor(ed)).collect(Collectors.toList());
default:
throw new ForbiddenException();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class EntitiesControllerTests extends Specification {
// .andExpect(header().exists(HttpHeaders.ETAG)) // MUST HAVE - is done by filter, so skipped for test
.andExpect(header().exists(HttpHeaders.LAST_MODIFIED))
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(content().json(expectedBody, false))
.andExpect(jsonPath('$.entityId', is("http://test.scaldingspoon.org/test1")))
}

def 'GET /entities/http%3A%2F%2Ftest.scaldingspoon.org%2Ftest1'() {
Expand Down Expand Up @@ -189,7 +189,7 @@ class EntitiesControllerTests extends Specification {
// .andExpect(header().exists(HttpHeaders.ETAG)) // MUST HAVE - is done by filter, so skipped for test
.andExpect(header().exists(HttpHeaders.LAST_MODIFIED))
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(content().json(expectedBody, false))
.andExpect(jsonPath('$.entityId').value("http://test.scaldingspoon.org/test1"))
}

def 'GET /api/entities/http%3A%2F%2Ftest.scaldingspoon.org%2Ftest1 XML'() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException
import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException
import edu.internet2.tier.shibboleth.admin.ui.opensaml.OpenSamlObjects
import edu.internet2.tier.shibboleth.admin.ui.repository.EntityDescriptorRepository
import edu.internet2.tier.shibboleth.admin.ui.security.model.Group
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
Expand Down Expand Up @@ -106,6 +107,11 @@ class EntityDescriptorControllerTests extends Specification {

securityContext.getAuthentication() >> authentication
SecurityContextHolder.setContext(securityContext)
Group defGroup = new Group();
defGroup.setResourceId("testingGroup");
defGroup.setName("For Tests");
defGroup.setDefaultGroup(true);
Group.DEFAULT_GROUP = defGroup;
}

def 'GET /EntityDescriptors with empty repository as admin'() {
Expand Down Expand Up @@ -138,7 +144,7 @@ class EntityDescriptorControllerTests extends Specification {
def role = 'ROLE_ADMIN'
authentication.getName() >> username
userRepository.findByUsername(username) >> TestHelpers.generateOptionalUser(username, role)
groupService.find(null) >> null //"groupId": null
groupService.find("testingGroup") >> Group.DEFAULT_GROUP
def expectedCreationDate = '2017-10-23T11:11:11'
def entityDescriptor = new EntityDescriptor(resourceId: 'uuid-1', entityID: 'eid1', serviceProviderName: 'sp1', serviceEnabled: true,
createdDate: LocalDateTime.parse(expectedCreationDate))
Expand All @@ -162,7 +168,7 @@ class EntityDescriptorControllerTests extends Specification {
"version": $version,
"createdBy": null,
"current": false,
"groupId": null
"groupId": "testingGroup"
}
]
"""
Expand All @@ -189,7 +195,7 @@ class EntityDescriptorControllerTests extends Specification {
def role = 'ROLE_ADMIN'
authentication.getName() >> username
userRepository.findByUsername(username) >> TestHelpers.generateOptionalUser(username, role)
groupService.find(null) >> null //"groupId": null
groupService.find("testingGroup") >> Group.DEFAULT_GROUP
def expectedCreationDate = '2017-10-23T11:11:11'
def entityDescriptorOne = new EntityDescriptor(resourceId: 'uuid-1', entityID: 'eid1', serviceProviderName: 'sp1',
serviceEnabled: true,
Expand Down Expand Up @@ -218,7 +224,7 @@ class EntityDescriptorControllerTests extends Specification {
"version": $versionOne,
"createdBy": null,
"current": false,
"groupId": null
"groupId": testingGroup
},
{
"id": "uuid-2",
Expand All @@ -236,7 +242,7 @@ class EntityDescriptorControllerTests extends Specification {
"version": $versionTwo,
"createdBy": null,
"current": false,
"groupId": null
"groupId": testingGroup
}
]
"""
Expand All @@ -254,61 +260,8 @@ class EntityDescriptorControllerTests extends Specification {
.andExpect(content().contentType(expectedResponseContentType))
.andExpect(content().json(expectedTwoRecordsListResponseBody, true))

}


//todo review
def 'GET /EntityDescriptors with 1 record in repository as user returns only that user\'s records'() {
given:
def username = 'someUser'
def role = 'ROLE_USER'
authentication.getName() >> username
userRepository.findByUsername(username) >> TestHelpers.generateOptionalUser(username, role)
groupService.find(null) >> null
def expectedCreationDate = '2017-10-23T11:11:11'
def entityDescriptorOne = new EntityDescriptor(resourceId: 'uuid-1', entityID: 'eid1', serviceProviderName: 'sp1',
serviceEnabled: true,
createdDate: LocalDateTime.parse(expectedCreationDate),
createdBy: 'someUser')
def versionOne = entityDescriptorOne.hashCode()
def oneRecordFromRepository = [entityDescriptorOne].stream()
def expectedOneRecordListResponseBody = """
[
{
"id": "uuid-1",
"serviceProviderName": "sp1",
"entityId": "eid1",
"serviceEnabled": true,
"createdDate": "$expectedCreationDate",
"modifiedDate": null,
"organization": {},
"contacts": null,
"serviceProviderSsoDescriptor": null,
"logoutEndpoints": null,
"securityInfo": null,
"assertionConsumerServices": null,
"version": $versionOne,
"createdBy": "someUser",
"current": false,
"groupId": null
}
]
"""

def expectedResponseContentType = APPLICATION_JSON
def expectedHttpResponseStatus = status().isOk()

when:
def result = mockMvc.perform(get('/api/EntityDescriptors'))

then:
//One call to the repo expected
1 * entityDescriptorRepository.findAllStreamByCreatedBy('someUser') >> oneRecordFromRepository
result.andExpect(expectedHttpResponseStatus)
.andExpect(content().contentType(expectedResponseContentType))
.andExpect(content().json(expectedOneRecordListResponseBody, true))
}

}

//todo review
def 'POST /EntityDescriptor and successfully create new record'() {
given:
Expand Down Expand Up @@ -362,7 +315,7 @@ class EntityDescriptorControllerTests extends Specification {
"version": $version,
"createdBy": null,
"current": false,
"groupId": null
"groupId": "testingGroup"
}
"""

Expand Down Expand Up @@ -523,7 +476,7 @@ class EntityDescriptorControllerTests extends Specification {
"version": $version,
"createdBy": null,
"current": false,
"groupId": null
"groupId": "testingGroup"
}
"""

Expand Down Expand Up @@ -576,7 +529,7 @@ class EntityDescriptorControllerTests extends Specification {
"version": $version,
"createdBy": "someUser",
"current": false,
"groupId": null
"groupId": "testingGroup"
}
"""

Expand All @@ -588,8 +541,7 @@ class EntityDescriptorControllerTests extends Specification {
1 * entityDescriptorRepository.findByResourceId(providedResourceId) >> entityDescriptor


result.andExpect(status().isOk())
.andExpect(content().json(expectedJsonBody, true))
result.andExpect(status().isOk()).andExpect(content().json(expectedJsonBody, true))
}

def 'GET /EntityDescriptor/{resourceId} existing, owned by some other user'() {
Expand Down Expand Up @@ -674,22 +626,20 @@ class EntityDescriptorControllerTests extends Specification {
entityDescriptor.setElementLocalName("EntityDescriptor")
entityDescriptor.setNamespacePrefix("md")
entityDescriptor.setNamespaceURI("urn:oasis:names:tc:SAML:2.0:metadata")
entityDescriptor.setGroup(Group.DEFAULT_GROUP)
def expectedXML = """<?xml version="1.0" encoding="UTF-8"?>
<md:EntityDescriptor
xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" entityID="$expectedEntityId"/>"""
when:
def result = mockMvc.perform(get("/api/EntityDescriptor/$providedResourceId")
.accept(APPLICATION_XML))
def result = mockMvc.perform(get("/api/EntityDescriptor/$providedResourceId").accept(APPLICATION_XML))
then:
//EntityDescriptor found
1 * entityDescriptorRepository.findByResourceId(providedResourceId) >> entityDescriptor
result.andExpect(status().isOk())
.andExpect(content().xml(expectedXML))
result.andExpect(status().isOk()).andExpect(content().xml(expectedXML))
}
def 'GET /EntityDescriptor/{resourceId} existing (xml), other user-owned'() {
Expand Down
Loading

0 comments on commit 0721b0a

Please sign in to comment.