[SHIBUI-1058]

Updates based on PR feedback.
TIER · Jan 23, 2019 · 63df45e · 63df45e
1 parent 6785119
commit 63df45e
Show file tree

Hide file tree

Showing 7 changed files with 17 additions and 21 deletions.
diff --git a/...u/internet2/tier/shibboleth/admin/ui/configuration/JsonSchemaComponentsConfiguration.java b/...u/internet2/tier/shibboleth/admin/ui/configuration/JsonSchemaComponentsConfiguration.java
@@ -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()

diff --git a/...in/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java b/...in/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java
@@ -120,7 +120,7 @@ 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()))) {
                 // Verify we're the only one attempting to update the EntityDescriptor
                 if (edRepresentation.getVersion() != existingEd.hashCode()) {
                     return new ResponseEntity<Void>(HttpStatus.CONFLICT);
@@ -154,11 +154,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()));
             }

diff --git a/...in/java/edu/internet2/tier/shibboleth/admin/ui/repository/EntityDescriptorRepository.java b/...in/java/edu/internet2/tier/shibboleth/admin/ui/repository/EntityDescriptorRepository.java
@@ -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);
 }
diff --git a/...edu/internet2/tier/shibboleth/admin/ui/scheduled/EntityDescriptorFilesScheduledTasks.java b/...edu/internet2/tier/shibboleth/admin/ui/scheduled/EntityDescriptorFilesScheduledTasks.java
@@ -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)) {
@@ -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());

diff --git a/...nd/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java b/...nd/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java
@@ -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();

diff --git a/.../edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy b/.../edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy
@@ -24,16 +24,12 @@ import org.springframework.data.jpa.repository.config.EnableJpaRepositories
 import org.springframework.security.core.Authentication
 import org.springframework.security.core.context.SecurityContext
 import org.springframework.security.core.context.SecurityContextHolder
-import org.springframework.security.web.context.HttpSessionSecurityContextRepository
 import org.springframework.test.context.ContextConfiguration
-import org.springframework.test.web.servlet.result.MockMvcResultHandlers
 import org.springframework.test.web.servlet.setup.MockMvcBuilders
 import org.springframework.web.client.RestTemplate
 import spock.lang.Specification
 import spock.lang.Subject
 
-import javax.servlet.http.HttpSession
-import java.security.Principal
 import java.time.LocalDateTime
 
 import static org.hamcrest.CoreMatchers.containsString
@@ -105,7 +101,7 @@ class EntityDescriptorControllerTests extends Specification {
 
         then:
         //One call to the repo expected
-        1 * entityDescriptorRepository.findAllByCustomQueryAndStream() >> emptyRecordsFromRepository
+        1 * entityDescriptorRepository.findAllStreamByCustomQuery() >> emptyRecordsFromRepository
         result.andExpect(expectedHttpResponseStatus)
                 .andExpect(content().contentType(expectedResponseContentType))
                 .andExpect(content().json(expectedEmptyListResponseBody))
@@ -152,7 +148,7 @@ class EntityDescriptorControllerTests extends Specification {
 
         then:
         //One call to the repo expected
-        1 * entityDescriptorRepository.findAllByCustomQueryAndStream() >> oneRecordFromRepository
+        1 * entityDescriptorRepository.findAllStreamByCustomQuery() >> oneRecordFromRepository
         result.andExpect(expectedHttpResponseStatus)
                 .andExpect(content().contentType(expectedResponseContentType))
                 .andExpect(content().json(expectedOneRecordListResponseBody, true))
@@ -223,7 +219,7 @@ class EntityDescriptorControllerTests extends Specification {
 
         then:
         //One call to the repo expected
-        1 * entityDescriptorRepository.findAllByCustomQueryAndStream() >> twoRecordsFromRepository
+        1 * entityDescriptorRepository.findAllStreamByCustomQuery() >> twoRecordsFromRepository
         result.andExpect(expectedHttpResponseStatus)
                 .andExpect(content().contentType(expectedResponseContentType))
                 .andExpect(content().json(expectedTwoRecordsListResponseBody, true))
@@ -272,14 +268,15 @@ class EntityDescriptorControllerTests extends Specification {
 
         then:
         //One call to the repo expected
-        1 * entityDescriptorRepository.findAllByCreatedBy('someUser') >> oneRecordFromRepository
+        1 * entityDescriptorRepository.findAllStreamByCreatedBy('someUser') >> oneRecordFromRepository
         result.andExpect(expectedHttpResponseStatus)
                 .andExpect(content().contentType(expectedResponseContentType))
                 .andExpect(content().json(expectedOneRecordListResponseBody, true))
     }
 
     def 'POST /EntityDescriptor and successfully create new record'() {
         given:
+        prepareUser('admin', 'ROLE_ADMIN')
         def expectedCreationDate = '2017-10-23T11:11:11'
         def expectedEntityId = 'https://shib'
         def expectedSpName = 'sp1'
@@ -394,6 +391,7 @@ class EntityDescriptorControllerTests extends Specification {
 
     def 'GET /EntityDescriptor/{resourceId} non-existent'() {
         given:
+        prepareUser('admin', 'ROLE_ADMIN')
         def providedResourceId = 'uuid-1'
 
         when:

diff --git a/...ernet2/tier/shibboleth/admin/ui/scheduled/EntityDescriptorFilesScheduledTasksTests.groovy b/...ernet2/tier/shibboleth/admin/ui/scheduled/EntityDescriptorFilesScheduledTasksTests.groovy
@@ -89,7 +89,7 @@ class EntityDescriptorFilesScheduledTasksTests extends Specification {
             }
             it
         })
-        1 * entityDescriptorRepository.findAllByServiceEnabled(true) >> [entityDescriptor].stream()
+        1 * entityDescriptorRepository.findAllStreamByServiceEnabled(true) >> [entityDescriptor].stream()
 
         when:
         if (directory.exists()) {
@@ -137,7 +137,7 @@ class EntityDescriptorFilesScheduledTasksTests extends Specification {
         def file = new File(directory, randomGenerator.randomId() + ".xml")
         file.text = "Delete me!"
 
-        1 * entityDescriptorRepository.findAllByServiceEnabled(true) >> [entityDescriptor].stream()
+        1 * entityDescriptorRepository.findAllStreamByServiceEnabled(true) >> [entityDescriptor].stream()
 
         when:
         entityDescriptorFilesScheduledTasks.removeDanglingEntityDescriptorFiles()