Skip to content

Commit

Permalink
merge
Browse files Browse the repository at this point in the history
  • Loading branch information
rmathis committed Jan 29, 2019
2 parents ad86788 + fda8924 commit bd867f2
Show file tree
Hide file tree
Showing 52 changed files with 866 additions and 3,908 deletions.
1 change: 1 addition & 0 deletions backend/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ dependencies {
compile 'io.springfox:springfox-swagger-ui:2.9.2'

testCompile "org.springframework.boot:spring-boot-starter-test"
testCompile "org.springframework.security:spring-security-test"
testCompile "org.spockframework:spock-core:1.1-groovy-2.4"
testCompile "org.spockframework:spock-spring:1.1-groovy-2.4"
testCompile "org.xmlunit:xmlunit-core:2.5.1"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package edu.internet2.tier.shibboleth.admin.ui.configuration;

import org.springframework.context.annotation.Configuration;
import org.springframework.security.config.annotation.method.configuration.EnableGlobalMethodSecurity;
import org.springframework.security.config.annotation.method.configuration.GlobalMethodSecurityConfiguration;

/**
* @author Bill Smith (wsmith@unicon.net)
*/
@Configuration
@EnableGlobalMethodSecurity(
prePostEnabled = true,
securedEnabled = true,
jsr250Enabled = true)
public class EndpointSecurityConfiguration extends GlobalMethodSecurityConfiguration {
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,6 @@ public class JsonSchemaComponentsConfiguration {
@Setter
private String nameIdFormatFilterUiSchemaLocation = "classpath:nameid-filter.schema.json";

@Autowired
UserRepository userRepository;

@Bean
public JsonSchemaResourceLocationRegistry jsonSchemaResourceLocationRegistry(ResourceLoader resourceLoader, ObjectMapper jacksonMapper) {
return JsonSchemaResourceLocationRegistry.inMemory()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ public void initRestTemplate() {
public ResponseEntity<?> create(@RequestBody EntityDescriptorRepresentation edRepresentation) {
final String entityId = edRepresentation.getEntityId();

ResponseEntity<?> entityDescriptorEnablingDeniedResponse = entityDescriptorEnablePermissionsCheck(edRepresentation.isServiceEnabled());
if (entityDescriptorEnablingDeniedResponse != null) {
return entityDescriptorEnablingDeniedResponse;
}

ResponseEntity<?> existingEntityDescriptorConflictResponse = existingEntityDescriptorCheck(entityId);
if (existingEntityDescriptorConflictResponse != null) {
return existingEntityDescriptorConflictResponse;
Expand Down Expand Up @@ -113,7 +118,12 @@ public ResponseEntity<?> update(@RequestBody EntityDescriptorRepresentation edRe
if (existingEd == null) {
return ResponseEntity.notFound().build();
} else {
if (currentUser.getRole().equals("ROLE_ADMIN") || currentUser.getUsername().equals(existingEd.getCreatedBy())) {
if (currentUser != null && (currentUser.getRole().equals("ROLE_ADMIN") || currentUser.getUsername().equals(existingEd.getCreatedBy()))) {
ResponseEntity<?> entityDescriptorEnablingDeniedResponse = entityDescriptorEnablePermissionsCheck(edRepresentation.isServiceEnabled());
if (entityDescriptorEnablingDeniedResponse != null) {
return entityDescriptorEnablingDeniedResponse;
}

// Verify we're the only one attempting to update the EntityDescriptor
if (edRepresentation.getVersion() != existingEd.hashCode()) {
return new ResponseEntity<Void>(HttpStatus.CONFLICT);
Expand Down Expand Up @@ -142,11 +152,11 @@ public ResponseEntity<?> getAll() {
User currentUser = userService.getCurrentUser();
if (currentUser != null) {
if (currentUser.getRole().equals("ROLE_ADMIN")) {
return ResponseEntity.ok(entityDescriptorRepository.findAllByCustomQueryAndStream()
return ResponseEntity.ok(entityDescriptorRepository.findAllStreamByCustomQuery()
.map(ed -> entityDescriptorService.createRepresentationFromDescriptor(ed))
.collect(Collectors.toList()));
} else {
return ResponseEntity.ok(entityDescriptorRepository.findAllByCreatedBy(currentUser.getUsername())
return ResponseEntity.ok(entityDescriptorRepository.findAllStreamByCreatedBy(currentUser.getUsername())
.map(ed -> entityDescriptorService.createRepresentationFromDescriptor(ed))
.collect(Collectors.toList()));
}
Expand Down Expand Up @@ -211,6 +221,17 @@ private ResponseEntity<?> existingEntityDescriptorCheck(String entityId) {
return null;
}

private ResponseEntity<?> entityDescriptorEnablePermissionsCheck(boolean serviceEnabled) {
User user = userService.getCurrentUser();
if (user != null) {
if (serviceEnabled && !user.getRole().equals("ROLE_ADMIN")) {
return ResponseEntity.status(HttpStatus.FORBIDDEN)
.body(new ErrorResponse(HttpStatus.FORBIDDEN, "You do not have the permissions necessary to enable this service."));
}
}
return null;
}

private ResponseEntity<?> handleUploadingEntityDescriptorXml(byte[] rawXmlBytes, String spName) throws Exception {
final EntityDescriptor ed = EntityDescriptor.class.cast(openSamlObjects.unmarshalFromXml(rawXmlBytes));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ public interface EntityDescriptorRepository extends CrudRepository<EntityDescrip

EntityDescriptor findByResourceId(String resourceId);

Stream<EntityDescriptor> findAllByServiceEnabled(boolean serviceEnabled);
Stream<EntityDescriptor> findAllStreamByServiceEnabled(boolean serviceEnabled);

@Query("select e from EntityDescriptor e")
Stream<EntityDescriptor> findAllByCustomQueryAndStream();
Stream<EntityDescriptor> findAllStreamByCustomQuery();

Stream<EntityDescriptor> findAllByCreatedBy(String createdBy);
Stream<EntityDescriptor> findAllStreamByCreatedBy(String createdBy);
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public EntityDescriptorFilesScheduledTasks(String metadataDirName,
@Scheduled(fixedRateString = "${shibui.taskRunRate:30000}")
@Transactional(readOnly = true)
public void generateEntityDescriptorFiles() throws MarshallingException {
this.entityDescriptorRepository.findAllByServiceEnabled(true)
this.entityDescriptorRepository.findAllStreamByServiceEnabled(true)
.forEach(ed -> {
Path targetFilePath = targetFilePathFor(toSha1HexString(ed.getEntityID()));
if (Files.exists(targetFilePath)) {
Expand Down Expand Up @@ -91,7 +91,7 @@ public void removeDanglingEntityDescriptorFiles() {
.map(it -> it.substring(0, it.indexOf(".")))
.collect(toSet());

Set<String> enabledEidsSha1Hashes = this.entityDescriptorRepository.findAllByServiceEnabled(true)
Set<String> enabledEidsSha1Hashes = this.entityDescriptorRepository.findAllStreamByServiceEnabled(true)
.map(EntityDescriptor::getEntityID)
.map(EntityDescriptorFilesScheduledTasks::toSha1HexString)
.collect(toSet());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.slf4j.LoggerFactory;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.security.crypto.bcrypt.BCrypt;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.bind.annotation.DeleteMapping;
Expand All @@ -22,6 +23,7 @@
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.client.HttpClientErrorException;

import java.security.Principal;
import java.util.List;
import java.util.Optional;

Expand All @@ -48,6 +50,7 @@ public UsersController(UserRepository userRepository, RoleRepository roleReposit
this.userService = userService;
}

@PreAuthorize("hasRole('ADMIN')")
@Transactional(readOnly = true)
@GetMapping
public List<User> getAll() {
Expand All @@ -56,21 +59,19 @@ public List<User> getAll() {

@Transactional(readOnly = true)
@GetMapping("/current")
public ResponseEntity<?> getCurrentUser() {
User user = userService.getCurrentUser();
if (user != null) {
return ResponseEntity.ok(user);
} else {
return ResponseEntity.notFound().build();
}
public User getCurrentUser(Principal principal) {
// TODO: fix this
return userService.getCurrentUser();
}

@PreAuthorize("hasRole('ADMIN')")
@Transactional(readOnly = true)
@GetMapping("/{username}")
public ResponseEntity<?> getOne(@PathVariable String username) {
return ResponseEntity.ok(findUserOrThrowHttp404(username));
}

@PreAuthorize("hasRole('ADMIN')")
@Transactional
@DeleteMapping("/{username}")
public ResponseEntity<?> deleteOne(@PathVariable String username) {
Expand All @@ -79,6 +80,7 @@ public ResponseEntity<?> deleteOne(@PathVariable String username) {
return ResponseEntity.noContent().build();
}

@PreAuthorize("hasRole('ADMIN')")
@Transactional
@PostMapping
ResponseEntity<?> saveOne(@RequestBody User user) {
Expand All @@ -96,6 +98,7 @@ ResponseEntity<?> saveOne(@RequestBody User user) {
return ResponseEntity.ok(savedUser);
}

@PreAuthorize("hasRole('ADMIN')")
@Transactional
@PatchMapping("/{username}")
ResponseEntity<?> updateOne(@PathVariable(value = "username") String username, @RequestBody User user) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public void updateUserRole(User user) {
}

public User getCurrentUser() {
//TODO: Consider returning an Optional here
User user = null;
if (SecurityContextHolder.getContext() != null && SecurityContextHolder.getContext().getAuthentication() != null) {
String principal = (String) SecurityContextHolder.getContext().getAuthentication().getPrincipal();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import edu.internet2.tier.shibboleth.admin.ui.security.model.User;
import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository;
import org.apache.commons.lang.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.context.support.ResourceBundleMessageSource;
Expand Down Expand Up @@ -72,6 +73,6 @@ public String[] getSystemAdminEmailAddresses() {
logger.warn("No users with ROLE_ADMIN were found! Check your configuration!");
systemAdmins = new HashSet<>();
}
return systemAdmins.stream().map(User::getEmailAddress).distinct().toArray(String[]::new);
return systemAdmins.stream().filter(user -> StringUtils.isNotBlank(user.getEmailAddress())).map(User::getEmailAddress).distinct().toArray(String[]::new);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -119,14 +119,12 @@
"description": "tooltip.refresh-delay-factor",
"type": "string",
"widget": {
"id": "number",
"step": 0.01
"id": "string",
"help": "message.real-number"
},
"placeholder": "label.real-number",
"minimum": 0,
"maximum": 1,
"default": null,
"pattern": "^([0]*(\\.[0-9]+)?|[0]*\\.[0-9]*[1-9][0-9]*)$"
"default": "",
"pattern": "^(?:([0]*(\\.[0-9]+)?|[0]*\\.[0-9]*[1-9][0-9]*)|)$"
},
"minCacheDuration": {
"title": "label.min-cache-duration",
Expand Down Expand Up @@ -599,8 +597,7 @@
"certificateFile": {
"title": "label.certificate-file",
"description": "tooltip.certificate-file",
"type": "string",
"default": ""
"type": "string"
}
},
"anyOf": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,12 @@
"description": "tooltip.refresh-delay-factor",
"type": "string",
"widget": {
"id": "number",
"step": 0.01
"id": "string",
"help": "message.real-number"
},
"placeholder": "label.real-number",
"minimum": 0,
"maximum": 1,
"default": null,
"pattern": "^([0]*(\\.[0-9]+)?|[0]*\\.[0-9]*[1-9][0-9]*)$"
"default": "",
"pattern": "^(?:([0]*(\\.[0-9]+)?|[0]*\\.[0-9]*[1-9][0-9]*)|)$"
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions backend/src/main/resources/i18n/messages_en.properties
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ message.name-must-be-unique=Name must be unique.
message.uri-valid-format=URI must be valid format.
message.id-unique=ID must be unique.
message.array-items-must-be-unique=Items in list must be unique.
message.real-number=Optional. If using a value, must be a real number between 0-1.

message.org-name-required=Organization Name is required.
message.org-displayName-required=Organization Name is required.
Expand All @@ -401,6 +402,7 @@ message.org-incomplete=These three fields must all be entered if any single fiel
message.type-required=Missing required property: Type
message.match-required=Missing required property: Match
message.value-required=Missing required property: Value
message.required=Missing required property.

message.conflict=Conflict
message.data-version-contention=Data Version Contention
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,12 @@
"description": "tooltip.refresh-delay-factor",
"type": "string",
"widget": {
"id": "number",
"step": 0.01
"id": "string",
"help": "message.real-number"
},
"placeholder": "label.real-number",
"minimum": 0,
"maximum": 1,
"default": null,
"pattern": "^([0]*(\\.[0-9]+)?|[0]*\\.[0-9]*[1-9][0-9]*)$"
"default": "",
"pattern": "^(?:([0]*(\\.[0-9]+)?|[0]*\\.[0-9]*[1-9][0-9]*)|)$"
},
"minCacheDuration": {
"title": "label.min-cache-duration",
Expand All @@ -87,6 +85,7 @@
"PT30M",
"PT1H",
"PT4H",
"PT8H",
"PT12H",
"PT24H"
]
Expand All @@ -109,6 +108,7 @@
"PT30M",
"PT1H",
"PT4H",
"PT8H",
"PT12H",
"PT24H"
]
Expand All @@ -131,6 +131,7 @@
"PT30M",
"PT1H",
"PT4H",
"PT8H",
"PT12H",
"PT24H"
]
Expand Down Expand Up @@ -176,6 +177,7 @@
"PT30M",
"PT1H",
"PT4H",
"PT8H",
"PT12H",
"PT24H"
]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
package edu.internet2.tier.shibboleth.admin.ui.controller

import edu.internet2.tier.shibboleth.admin.ui.configuration.CoreShibUiConfiguration
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.configuration.TestConfiguration
import edu.internet2.tier.shibboleth.admin.ui.opensaml.OpenSamlObjects
import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService
import edu.internet2.tier.shibboleth.admin.ui.service.JPAEntityDescriptorServiceImpl
import edu.internet2.tier.shibboleth.admin.ui.service.JPAEntityServiceImpl
import net.shibboleth.ext.spring.resource.ResourceHelper
import org.opensaml.saml.metadata.resolver.impl.ResourceBackedMetadataResolver
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.core.io.ClassPathResource
import org.springframework.data.jpa.repository.config.EnableJpaRepositories
import org.springframework.http.MediaType
import org.springframework.test.context.ContextConfiguration
import org.springframework.test.web.servlet.setup.MockMvcBuilders
import spock.lang.Specification
import spock.lang.Subject
Expand All @@ -15,6 +25,10 @@ import static org.springframework.test.web.servlet.request.MockMvcRequestBuilder
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status

@DataJpaTest
@ContextConfiguration(classes=[CoreShibUiConfiguration, SearchConfiguration, TestConfiguration, InternationalizationConfiguration])
@EnableJpaRepositories(basePackages = ["edu.internet2.tier.shibboleth.admin.ui"])
@EntityScan("edu.internet2.tier.shibboleth.admin.ui")
class EntitiesControllerTests extends Specification {
def openSamlObjects = new OpenSamlObjects().with {
init()
Expand All @@ -30,10 +44,13 @@ class EntitiesControllerTests extends Specification {
it
}

@Autowired
UserService userService

@Subject
def controller = new EntitiesController(
openSamlObjects: openSamlObjects,
entityDescriptorService: new JPAEntityDescriptorServiceImpl(openSamlObjects, new JPAEntityServiceImpl(openSamlObjects)),
entityDescriptorService: new JPAEntityDescriptorServiceImpl(openSamlObjects, new JPAEntityServiceImpl(openSamlObjects), userService),
metadataResolver: metadataResolver
)

Expand Down
Loading

0 comments on commit bd867f2

Please sign in to comment.