From e43056b29d4c820cb816e771ce338f005b57afd6 Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Wed, 23 Jan 2019 11:40:27 -0700 Subject: [PATCH] [SHIBUI-1058] Added checks to see if ROLE_USER attempted to set serviceEnabled=true. Added tests. --- .../EntityDescriptorController.java | 22 +++---- .../EntityDescriptorControllerTests.groovy | 66 +++++++++++++++++++ 2 files changed, 76 insertions(+), 12 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java index 8aecb529d..03cdf92b5 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java @@ -74,16 +74,16 @@ public void initRestTemplate() { public ResponseEntity create(@RequestBody EntityDescriptorRepresentation edRepresentation) { final String entityId = edRepresentation.getEntityId(); - ResponseEntity existingEntityDescriptorConflictResponse = existingEntityDescriptorCheck(entityId); - if (existingEntityDescriptorConflictResponse != null) { - return existingEntityDescriptorConflictResponse; - } - ResponseEntity entityDescriptorEnablingDeniedResponse = entityDescriptorEnablePermissionsCheck(edRepresentation.isServiceEnabled()); if (entityDescriptorEnablingDeniedResponse != null) { return entityDescriptorEnablingDeniedResponse; } + ResponseEntity existingEntityDescriptorConflictResponse = existingEntityDescriptorCheck(entityId); + if (existingEntityDescriptorConflictResponse != null) { + return existingEntityDescriptorConflictResponse; + } + EntityDescriptor ed = (EntityDescriptor) entityDescriptorService.createDescriptorFromRepresentation(edRepresentation); EntityDescriptor persistedEd = entityDescriptorRepository.save(ed); @@ -94,13 +94,11 @@ public ResponseEntity create(@RequestBody EntityDescriptorRepresentation edRe @PostMapping(value = "/EntityDescriptor", consumes = "application/xml") public ResponseEntity upload(@RequestBody byte[] entityDescriptorXml, @RequestParam String spName) throws Exception { - //TODO: Do we want security checks here? return handleUploadingEntityDescriptorXml(entityDescriptorXml, spName); } @PostMapping(value = "/EntityDescriptor", consumes = "application/x-www-form-urlencoded") public ResponseEntity upload(@RequestParam String metadataUrl, @RequestParam String spName) throws Exception { - //TODO: Do we want security checks here? try { byte[] xmlContents = this.restTemplate.getForObject(metadataUrl, byte[].class); return handleUploadingEntityDescriptorXml(xmlContents, spName); @@ -121,16 +119,16 @@ public ResponseEntity update(@RequestBody EntityDescriptorRepresentation edRe return ResponseEntity.notFound().build(); } else { 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(HttpStatus.CONFLICT); - } - 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(HttpStatus.CONFLICT); + } + EntityDescriptor updatedEd = EntityDescriptor.class.cast(entityDescriptorService.createDescriptorFromRepresentation(edRepresentation)); diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy index 3108ee01f..1a66ad333 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy @@ -354,6 +354,45 @@ class EntityDescriptorControllerTests extends Specification { } + def 'POST /EntityDescriptor as user disallows enabling'() { + given: + prepareUser('user', 'ROLE_USER') + def expectedEntityId = 'https://shib' + def expectedSpName = 'sp1' + + def postedJsonBody = """ + { + "serviceProviderName": "$expectedSpName", + "entityId": "$expectedEntityId", + "organization": null, + "serviceEnabled": true, + "createdDate": null, + "modifiedDate": null, + "organization": null, + "contacts": null, + "mdui": null, + "serviceProviderSsoDescriptor": null, + "logoutEndpoints": null, + "securityInfo": null, + "assertionConsumerServices": null, + "relyingPartyOverrides": null, + "attributeRelease": null + } + """ + + when: + def result = mockMvc.perform( + post('/api/EntityDescriptor') + .contentType(APPLICATION_JSON_UTF8) + .content(postedJsonBody)) + + then: + 0 * entityDescriptorRepository.findByEntityID(_) + 0 * entityDescriptorRepository.save(_) + + result.andExpect(status().isForbidden()) + } + def 'POST /EntityDescriptor record already exists'() { given: def expectedEntityId = 'eid1' @@ -840,6 +879,33 @@ class EntityDescriptorControllerTests extends Specification { .andExpect(content().json(JsonOutput.toJson(expectedJson), true)) } + def "PUT /EntityDescriptor disallows user from enabling"() { + given: + prepareUser('someUser', 'ROLE_USER') + def entityDescriptor = generator.buildEntityDescriptor() + entityDescriptor.serviceEnabled = false + def updatedEntityDescriptor = generator.buildEntityDescriptor() + updatedEntityDescriptor.serviceEnabled = true + updatedEntityDescriptor.resourceId = entityDescriptor.resourceId + def updatedEntityDescriptorRepresentation = service.createRepresentationFromDescriptor(updatedEntityDescriptor) + updatedEntityDescriptorRepresentation.version = entityDescriptor.hashCode() + def postedJsonBody = mapper.writeValueAsString(updatedEntityDescriptorRepresentation) + + def resourceId = entityDescriptor.resourceId + + 1 * entityDescriptorRepository.findByResourceId(resourceId) >> entityDescriptor + 0 * entityDescriptorRepository.save(_) >> updatedEntityDescriptor + + when: + def result = mockMvc.perform( + put("/api/EntityDescriptor/$resourceId") + .contentType(APPLICATION_JSON_UTF8) + .content(postedJsonBody)) + + then: + result.andExpect(status().isForbidden()) + } + def "PUT /EntityDescriptor denies the request if the PUTing user is not an ADMIN and not the createdBy user"() { given: prepareUser('randomUser', 'ROLE_USER')