From e9e75caad661e616570258219d54e338718cd002 Mon Sep 17 00:00:00 2001 From: Jodie Muramoto Date: Wed, 28 Nov 2018 16:03:37 -0700 Subject: [PATCH 01/11] SHIBUI-1005: Updated metadata provider name label, missing time selections, updated validation numbers for refresh delay factor, provider enable info text, corrected dynamic attributes, moved enabled checkbox outside gray fieldset; --- backend/src/main/resources/i18n/messages.properties | 7 ++++--- .../src/main/resources/i18n/messages_en.properties | 7 ++++--- .../local-dynamic-metadata-provider.schema.json | 2 +- .../src/test/resources/i18n/messages_es.properties | 7 ++++--- .../provider/model/local-dynamic.provider.form.ts | 12 ++++++------ .../schema/provider/metadata-provider.schema.json | 6 +++--- 6 files changed, 22 insertions(+), 19 deletions(-) diff --git a/backend/src/main/resources/i18n/messages.properties b/backend/src/main/resources/i18n/messages.properties index 87aaf7299..8ad97ae2b 100644 --- a/backend/src/main/resources/i18n/messages.properties +++ b/backend/src/main/resources/i18n/messages.properties @@ -293,7 +293,7 @@ label.metadata-provider-type=Metadata Provider Type label.metadata-provider-name=Metadata Provider Name label.select-metadata-type=Select a metadata provider type label.metadata-provider-status=Metadata Provider Status -label.enable-provider-upon-saving=Enable Metadata Provider upon saving? +label.enable-provider-upon-saving=Enable Metadata Provider? label.certificate-type=Type label.metadata-file=Metadata File @@ -313,6 +313,7 @@ label.select-metadata-provider-type=Select Metadata Provider Type label.filter-list=Filter List label.common-attributes=Common Attributes label.reloading-attributes=Reloading Attributes +label.dynamic-attributes=Dynamic Attributes label.metadata-filter-plugins=Metadata Filter Plugins label.advanced-settings=Advanced Settings label.edit-metadata-provider=Edit Metadata Provider @@ -432,7 +433,7 @@ tooltip.http-caching-directory=If httpCaching='file', this attribute specifies w tooltip.http-max-cache-entries=The maximum number of responses written to cache. This attribute is incompatible with httpClientRef. tooltip.max-cache-entry-size=The maximum response body size that may be cached, in bytes. This attribute is incompatible with httpClientRef. -tooltip.metadata-provider-name=Metadata Provider Name +tooltip.metadata-provider-name=Metadata Provider Name (for display on the Dashboard only) tooltip.metadata-provider-type=Metadata Provider Type tooltip.xml-id=Identifier for logging, identification for command line reload, etc. tooltip.metadata-url=Identifier for logging, identification for command line reload, etc. @@ -443,7 +444,7 @@ tooltip.require-valid-metadata=Whether candidate metadata found by the resolver tooltip.fail-fast-init=Whether to fail initialization of the underlying MetadataResolverService (and possibly the IdP as a whole) if the initialization of a metadata provider fails. When false, the IdP may start, and will continue to attempt to reload valid metadata if configured to do so, but operations that require valid metadata will fail until it does. tooltip.use-default-predicate-reg=Flag which determines whether the default CriterionPredicateRegistry will be used if a custom one is not supplied explicitly. tooltip.satisfy-any-predicates=Flag which determines whether predicates used in filtering are connected by a logical 'OR' (true) or by logical 'AND' (false). -tooltip.enable-provider-upon-saving=Enable Metadata Provider upon saving? +tooltip.enable-provider-upon-saving=If checkbox is clicked, the metadata provider is enabled for integration with the IdP tooltip.max-validity-interval=Defines the window within which the metadata is valid. tooltip.require-signed-root=If true, this fails to load metadata with no signature on the root XML element. diff --git a/backend/src/main/resources/i18n/messages_en.properties b/backend/src/main/resources/i18n/messages_en.properties index 984f3c21c..e8b3ec6bd 100644 --- a/backend/src/main/resources/i18n/messages_en.properties +++ b/backend/src/main/resources/i18n/messages_en.properties @@ -296,7 +296,7 @@ label.metadata-provider-type=Metadata Provider Type label.metadata-provider-name=Metadata Provider Name label.select-metadata-type=Select a metadata provider type label.metadata-provider-status=Metadata Provider Status -label.enable-provider-upon-saving=Enable Metadata Provider upon saving? +label.enable-provider-upon-saving=Enable Metadata Provider? label.certificate-type=Type label.metadata-file=Metadata File @@ -316,6 +316,7 @@ label.select-metadata-provider-type=Select Metadata Provider Type label.filter-list=Filter List label.common-attributes=Common Attributes label.reloading-attributes=Reloading Attributes +label.dynamic-attributes=Dynamic Attributes label.metadata-filter-plugins=Metadata Filter Plugins label.advanced-settings=Advanced Settings label.edit-metadata-provider=Edit Metadata Provider @@ -442,7 +443,7 @@ tooltip.http-caching-directory=If httpCaching='file', this attribute specifies w tooltip.http-max-cache-entries=The maximum number of responses written to cache. This attribute is incompatible with httpClientRef. tooltip.max-cache-entry-size=The maximum response body size that may be cached, in bytes. This attribute is incompatible with httpClientRef. -tooltip.metadata-provider-name=Metadata Provider Name +tooltip.metadata-provider-name=Metadata Provider Name (for display on the Dashboard only) tooltip.metadata-provider-type=Metadata Provider Type tooltip.xml-id=Identifier for logging, identification for command line reload, etc. tooltip.metadata-url=Identifier for logging, identification for command line reload, etc. @@ -454,7 +455,7 @@ tooltip.require-valid-metadata=Whether candidate metadata found by the resolver tooltip.fail-fast-init=Whether to fail initialization of the underlying MetadataResolverService (and possibly the IdP as a whole) if the initialization of a metadata provider fails. When false, the IdP may start, and will continue to attempt to reload valid metadata if configured to do so, but operations that require valid metadata will fail until it does. tooltip.use-default-predicate-reg=Flag which determines whether the default CriterionPredicateRegistry will be used if a custom one is not supplied explicitly. tooltip.satisfy-any-predicates=Flag which determines whether predicates used in filtering are connected by a logical 'OR' (true) or by logical 'AND' (false). -tooltip.enable-provider-upon-saving=Enable Metadata Provider upon saving? +tooltip.enable-provider-upon-saving=If checkbox is clicked, the metadata provider is enabled for integration with the IdP tooltip.max-validity-interval=Defines the window within which the metadata is valid. tooltip.require-signed-root=If true, this fails to load metadata with no signature on the root XML element. diff --git a/backend/src/main/resources/local-dynamic-metadata-provider.schema.json b/backend/src/main/resources/local-dynamic-metadata-provider.schema.json index 04aa496f9..bb98b8b67 100644 --- a/backend/src/main/resources/local-dynamic-metadata-provider.schema.json +++ b/backend/src/main/resources/local-dynamic-metadata-provider.schema.json @@ -8,7 +8,7 @@ ], "properties": { "name": { - "title": "label.metadata-provider-name", + "title": "label.metadata-provider-name-display-only", "description": "tooltip.metadata-provider-name", "type": "string", "widget": { diff --git a/backend/src/test/resources/i18n/messages_es.properties b/backend/src/test/resources/i18n/messages_es.properties index 241aa5c49..7899789c6 100644 --- a/backend/src/test/resources/i18n/messages_es.properties +++ b/backend/src/test/resources/i18n/messages_es.properties @@ -282,7 +282,7 @@ label.metadata-provider-type=(es) Metadata Provider Type label.metadata-provider-name=(es) Metadata Provider Name label.select-metadata-type=(es) Select a metadata provider type label.metadata-provider-status=(es) Metadata Provider Status -label.enable-provider-upon-saving=(es) Enable Metadata Provider upon saving? +label.enable-provider-upon-saving=(es) Enable Metadata Provider? label.enable-filter=(es) Enable Filter? label.required-valid-until=(es) Required Valid Until Filter @@ -299,6 +299,7 @@ label.select-metadata-provider-type=(es) Select Metadata Provider Type label.filter-list=(es) Filter List label.common-attributes=(es) Common Attributes label.reloading-attributes=(es) Reloading Attributes +label.dynamic-attributes=(es) Dynamic Attributes label.metadata-filter-plugins=(es) Metadata Filter Plugins label.advanced-settings=(es) Advanced Settings label.edit-metadata-provider=(es) Edit Metadata Provider @@ -417,7 +418,7 @@ tooltip.http-caching-directory=(es) If httpCaching=(es) 'file', this attribute s tooltip.http-max-cache-entries=(es) The maximum number of responses written to cache. This attribute is incompatible with httpClientRef. tooltip.max-cache-entry-size=(es) The maximum response body size that may be cached, in bytes. This attribute is incompatible with httpClientRef. -tooltip.metadata-provider-name=(es) Metadata Provider Name +tooltip.metadata-provider-name=(es) Metadata Provider Name (for display on the Dashboard only) tooltip.metadata-provider-type=(es) Metadata Provider Type tooltip.xml-id=(es) Identifier for logging, identification for command line reload, etc. tooltip.metadata-url=(es) Identifier for logging, identification for command line reload, etc. @@ -428,7 +429,7 @@ tooltip.require-valid-metadata=(es) Whether candidate metadata found by the reso tooltip.fail-fast-init=(es) Whether to fail initialization of the underlying MetadataResolverService (and possibly the IdP as a whole) if the initialization of a metadata provider fails. When false, the IdP may start, and will continue to attempt to reload valid metadata if configured to do so, but operations that require valid metadata will fail until it does. tooltip.use-default-predicate-reg=(es) Whether to fail initialization of the underlying MetadataResolverService (and possibly the IdP as a whole) if the initialization of a metadata provider fails. When false, the IdP may start, and will continue to attempt to reload valid metadata if configured to do so, but operations that require valid metadata will fail until it does. tooltip.satisfy-any-predicates=(es) Flag which determines whether predicates used in filtering are connected by a logical 'OR' (true) or by logical 'AND' (false). -tooltip.enable-provider-upon-saving=(es) Enable Metadata Provider upon saving? +tooltip.enable-provider-upon-saving=(es) If checkbox is clicked, the metadata provider is enabled for integration with the IdP tooltip.max-validity-interval=(es) Defines the window within which the metadata is valid. tooltip.require-signed-root=(es) If true, this fails to load metadata with no signature on the root XML element. diff --git a/ui/src/app/metadata/provider/model/local-dynamic.provider.form.ts b/ui/src/app/metadata/provider/model/local-dynamic.provider.form.ts index 4485409bc..d2c16cb6b 100644 --- a/ui/src/app/metadata/provider/model/local-dynamic.provider.form.ts +++ b/ui/src/app/metadata/provider/model/local-dynamic.provider.form.ts @@ -42,8 +42,8 @@ export const LocalDynamicMetadataProviderWizard: Wizard Date: Fri, 30 Nov 2018 10:30:01 -0700 Subject: [PATCH 02/11] [SHIBUI-925] Updated exception handling in overrides json schema validation to return an ErrorResponse object. Also fixed the method name of the handler. Updated controller to return an ErrorResponse object when a 409 is encountered. --- ...artyOverridesJsonSchemaValidatingControllerAdvice.groovy | 6 +++--- .../admin/ui/controller/EntityDescriptorController.java | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/jsonschema/RelyingPartyOverridesJsonSchemaValidatingControllerAdvice.groovy b/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/jsonschema/RelyingPartyOverridesJsonSchemaValidatingControllerAdvice.groovy index 934d7b1a7..e77097af6 100644 --- a/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/jsonschema/RelyingPartyOverridesJsonSchemaValidatingControllerAdvice.groovy +++ b/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/jsonschema/RelyingPartyOverridesJsonSchemaValidatingControllerAdvice.groovy @@ -1,5 +1,6 @@ package edu.internet2.tier.shibboleth.admin.ui.jsonschema +import edu.internet2.tier.shibboleth.admin.ui.controller.ErrorResponse import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.EntityDescriptorRepresentation import mjson.Json import org.springframework.beans.factory.annotation.Autowired @@ -10,7 +11,6 @@ import org.springframework.http.ResponseEntity import org.springframework.http.converter.HttpMessageConverter import org.springframework.web.bind.annotation.ControllerAdvice import org.springframework.web.bind.annotation.ExceptionHandler -import org.springframework.web.context.request.WebRequest import org.springframework.web.servlet.mvc.method.annotation.RequestBodyAdviceAdapter import javax.annotation.PostConstruct @@ -56,8 +56,8 @@ class RelyingPartyOverridesJsonSchemaValidatingControllerAdvice extends RequestB } @ExceptionHandler(JsonSchemaValidationFailedException) - final ResponseEntity handleUserNotFoundException(JsonSchemaValidationFailedException ex, WebRequest request) { - new ResponseEntity<>([errors: ex.errors], HttpStatus.BAD_REQUEST) + final ResponseEntity handleJsonSchemaValidationFailedException(JsonSchemaValidationFailedException ex) { + ResponseEntity.status(HttpStatus.BAD_REQUEST).body(new ErrorResponse("400", String.join('\n', ex.errors))) } @PostConstruct 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 afde9d244..c3e76983e 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 @@ -160,7 +160,7 @@ private ResponseEntity existingEntityDescriptorCheck(String entityId) { return ResponseEntity .status(HttpStatus.CONFLICT) .headers(headers) - .body(String.format("The entity descriptor with entity id [%s] already exists.", entityId)); + .body(new ErrorResponse(String.valueOf(HttpStatus.CONFLICT.value()), String.format("The entity descriptor with entity id [%s] already exists.", entityId))); } //No existing entity descriptor, which is an OK condition indicated by returning a null conflict response return null; From a350d33cae6a01d9984d652774a935dd6b920ceb Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Mon, 3 Dec 2018 14:29:34 -0700 Subject: [PATCH 03/11] [SHIBUI-1001] Modified resolver initialization code to move shared code into a method. PUT and POST now both behave the same way with the same error message when there is a file not found exception. --- .../MetadataResolversController.java | 34 +++++++++++-------- .../main/resources/i18n/messages.properties | 1 + .../resources/i18n/messages_en.properties | 1 + 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java index 4593d8bbf..59fed8e93 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java @@ -119,11 +119,9 @@ public ResponseEntity create(@RequestBody MetadataResolver newResolver) throw MetadataResolver persistedResolver = resolverRepository.save(newResolver); positionOrderContainerService.appendPositionOrderForNew(persistedResolver); - //TODO: currently, the update call might explode, but the save works.. in which case, the UI never gets - // n valid response. This operation is not atomic. Should we return an error here? - if (persistedResolver.getDoInitialization()) { - org.opensaml.saml.metadata.resolver.MetadataResolver openSamlRepresentation = metadataResolverConverterService.convertToOpenSamlRepresentation(persistedResolver); - OpenSamlChainingMetadataResolverUtil.updateChainingMetadataResolver((OpenSamlChainingMetadataResolver) chainingMetadataResolver, openSamlRepresentation); + ResponseEntity initializationError = doResolverInitialization(persistedResolver); + if (initializationError != null) { + return initializationError; } return ResponseEntity.created(getResourceUriFor(persistedResolver)).body(persistedResolver); @@ -151,15 +149,9 @@ public ResponseEntity update(@PathVariable String resourceId, @RequestBody Me MetadataResolver persistedResolver = resolverRepository.save(updatedResolver); - if (persistedResolver.getDoInitialization()) { - org.opensaml.saml.metadata.resolver.MetadataResolver openSamlRepresentation = null; - try { - openSamlRepresentation = metadataResolverConverterService.convertToOpenSamlRepresentation(persistedResolver); - } catch (FileNotFoundException e) { - return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body(new ErrorResponse(HttpStatus.INTERNAL_SERVER_ERROR.toString(), "label.file-doesnt-exist")); - } - OpenSamlChainingMetadataResolverUtil.updateChainingMetadataResolver((OpenSamlChainingMetadataResolver) chainingMetadataResolver, openSamlRepresentation); + ResponseEntity initializationError = doResolverInitialization(persistedResolver); + if (initializationError != null) { + return initializationError; } return ResponseEntity.ok(persistedResolver); @@ -181,4 +173,18 @@ private static URI getResourceUriFor(MetadataResolver resolver) { .build() .toUri(); } + + private ResponseEntity doResolverInitialization(MetadataResolver persistedResolver) throws ComponentInitializationException, ResolverException, IOException { + if (persistedResolver.getDoInitialization()) { + org.opensaml.saml.metadata.resolver.MetadataResolver openSamlRepresentation = null; + try { + openSamlRepresentation = metadataResolverConverterService.convertToOpenSamlRepresentation(persistedResolver); + } catch (FileNotFoundException e) { + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .body(new ErrorResponse(HttpStatus.INTERNAL_SERVER_ERROR.toString(), "message.file-doesnt-exist")); + } + OpenSamlChainingMetadataResolverUtil.updateChainingMetadataResolver((OpenSamlChainingMetadataResolver) chainingMetadataResolver, openSamlRepresentation); + } + return null; + } } diff --git a/backend/src/main/resources/i18n/messages.properties b/backend/src/main/resources/i18n/messages.properties index 31c4de9b8..672ae51f4 100644 --- a/backend/src/main/resources/i18n/messages.properties +++ b/backend/src/main/resources/i18n/messages.properties @@ -397,6 +397,7 @@ message.wizard-status=Step { index } of { length } message.entity-id-min-unique=You must add at least one entity id target and they must each be unique. message.required-for-scripts=Required for Scripts message.required-for-regex=Required for Regex +message.file-doesnt-exist=The requested file to be processed does not exist on the server. tooltip.entity-id=Entity ID tooltip.service-provider-name=Service Provider Name (Dashboard Display Only) diff --git a/backend/src/main/resources/i18n/messages_en.properties b/backend/src/main/resources/i18n/messages_en.properties index 01b5c18e2..c5eabc3ee 100644 --- a/backend/src/main/resources/i18n/messages_en.properties +++ b/backend/src/main/resources/i18n/messages_en.properties @@ -403,6 +403,7 @@ message.wizard-status=Step { index } of { length } message.entity-id-min-unique=You must add at least one entity id target and they must each be unique. message.required-for-scripts=Required for Scripts message.required-for-regex=Required for Regex +message.file-doesnt-exist=The requested file to be processed does not exist on the server. tooltip.entity-id=Entity ID tooltip.service-provider-name=Service Provider Name (Dashboard Display Only) From 3f14ef499128a49ecd933722b78b43c3680a344c Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Wed, 5 Dec 2018 08:21:33 -0500 Subject: [PATCH 04/11] SHIBUI-926 --- .../MetadataResolversController.java | 31 +++++++------------ .../support/RestControllersSupport.java | 9 +++++- .../MetadataFileNotFoundException.java | 14 +++++++++ .../MetadataResolverConverterServiceImpl.java | 2 +- 4 files changed, 34 insertions(+), 22 deletions(-) create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/exceptions/MetadataFileNotFoundException.java diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java index 59fed8e93..b9a7ab3f0 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java @@ -1,6 +1,7 @@ package edu.internet2.tier.shibboleth.admin.ui.controller; import com.fasterxml.jackson.databind.exc.InvalidTypeIdException; +import edu.internet2.tier.shibboleth.admin.ui.domain.exceptions.MetadataFileNotFoundException; import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver; import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolverValidationService; import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml.OpenSamlChainingMetadataResolver; @@ -105,30 +106,26 @@ public ResponseEntity getOne(@PathVariable String resourceId) { } @PostMapping("/MetadataResolvers") - @Transactional + @Transactional(rollbackFor = Exception.class) public ResponseEntity create(@RequestBody MetadataResolver newResolver) throws IOException, ResolverException, ComponentInitializationException { if (resolverRepository.findByName(newResolver.getName()) != null) { return ResponseEntity.status(HttpStatus.CONFLICT).build(); } ResponseEntity validationErrorResponse = validate(newResolver); - if(validationErrorResponse != null) { + if (validationErrorResponse != null) { return validationErrorResponse; } MetadataResolver persistedResolver = resolverRepository.save(newResolver); positionOrderContainerService.appendPositionOrderForNew(persistedResolver); - - ResponseEntity initializationError = doResolverInitialization(persistedResolver); - if (initializationError != null) { - return initializationError; - } + doResolverInitialization(persistedResolver); return ResponseEntity.created(getResourceUriFor(persistedResolver)).body(persistedResolver); } @PutMapping("/MetadataResolvers/{resourceId}") - @Transactional + @Transactional(rollbackFor = Exception.class) public ResponseEntity update(@PathVariable String resourceId, @RequestBody MetadataResolver updatedResolver) throws IOException, ResolverException, ComponentInitializationException { MetadataResolver existingResolver = resolverRepository.findByResourceId(resourceId); if (existingResolver == null) { @@ -141,18 +138,13 @@ public ResponseEntity update(@PathVariable String resourceId, @RequestBody Me } ResponseEntity validationErrorResponse = validate(updatedResolver); - if(validationErrorResponse != null) { + if (validationErrorResponse != null) { return validationErrorResponse; } updatedResolver.setAudId(existingResolver.getAudId()); - MetadataResolver persistedResolver = resolverRepository.save(updatedResolver); - - ResponseEntity initializationError = doResolverInitialization(persistedResolver); - if (initializationError != null) { - return initializationError; - } + doResolverInitialization(persistedResolver); return ResponseEntity.ok(persistedResolver); } @@ -160,7 +152,7 @@ public ResponseEntity update(@PathVariable String resourceId, @RequestBody Me @SuppressWarnings("Unchecked") private ResponseEntity validate(MetadataResolver metadataResolver) { ValidationResult validationResult = metadataResolverValidationService.validateIfNecessary(metadataResolver); - if(!validationResult.isValid()) { + if (!validationResult.isValid()) { return ResponseEntity.badRequest().body(validationResult.getErrorMessage()); } return null; @@ -174,17 +166,16 @@ private static URI getResourceUriFor(MetadataResolver resolver) { .toUri(); } - private ResponseEntity doResolverInitialization(MetadataResolver persistedResolver) throws ComponentInitializationException, ResolverException, IOException { + private void doResolverInitialization(MetadataResolver persistedResolver) throws + ComponentInitializationException, ResolverException, IOException { if (persistedResolver.getDoInitialization()) { org.opensaml.saml.metadata.resolver.MetadataResolver openSamlRepresentation = null; try { openSamlRepresentation = metadataResolverConverterService.convertToOpenSamlRepresentation(persistedResolver); } catch (FileNotFoundException e) { - return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body(new ErrorResponse(HttpStatus.INTERNAL_SERVER_ERROR.toString(), "message.file-doesnt-exist")); + throw new MetadataFileNotFoundException("message.file-doesnt-exist"); } OpenSamlChainingMetadataResolverUtil.updateChainingMetadataResolver((OpenSamlChainingMetadataResolver) chainingMetadataResolver, openSamlRepresentation); } - return null; } } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/support/RestControllersSupport.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/support/RestControllersSupport.java index 7dc1a9abe..1adb07d6d 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/support/RestControllersSupport.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/support/RestControllersSupport.java @@ -1,7 +1,7 @@ package edu.internet2.tier.shibboleth.admin.ui.controller.support; -import com.google.common.collect.ImmutableMap; import edu.internet2.tier.shibboleth.admin.ui.controller.ErrorResponse; +import edu.internet2.tier.shibboleth.admin.ui.domain.exceptions.MetadataFileNotFoundException; import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver; import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository; import org.springframework.beans.factory.annotation.Autowired; @@ -11,6 +11,7 @@ import org.springframework.web.bind.annotation.RestControllerAdvice; import org.springframework.web.client.HttpClientErrorException; +import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR; import static org.springframework.http.HttpStatus.NOT_FOUND; /** @@ -46,4 +47,10 @@ public final ResponseEntity handleAllOtherExceptions(Exception ex ErrorResponse errorResponse = new ErrorResponse("400", ex.getLocalizedMessage()); return new ResponseEntity<>(errorResponse, HttpStatus.BAD_REQUEST); } + + @ExceptionHandler(MetadataFileNotFoundException.class) + public final ResponseEntity metadataFileNotFoundHandler(MetadataFileNotFoundException ex) { + ErrorResponse errorResponse = new ErrorResponse(INTERNAL_SERVER_ERROR.toString(), ex.getLocalizedMessage()); + return new ResponseEntity<>(errorResponse, INTERNAL_SERVER_ERROR); + } } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/exceptions/MetadataFileNotFoundException.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/exceptions/MetadataFileNotFoundException.java new file mode 100644 index 000000000..bc3dd1493 --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/exceptions/MetadataFileNotFoundException.java @@ -0,0 +1,14 @@ +package edu.internet2.tier.shibboleth.admin.ui.domain.exceptions; + +/** + * Indicates that provided metadata file is not found. Thrown during opensaml API initialization code path, if there is + * such. Throwing such exception is useful in @Transactional context for atomic transaction rollbacks, etc. + * + * @author Dmitriy Kopylenko + */ +public class MetadataFileNotFoundException extends RuntimeException { + + public MetadataFileNotFoundException(String message) { + super(message); + } +} diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/MetadataResolverConverterServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/MetadataResolverConverterServiceImpl.java index ccb77164c..8088793a9 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/MetadataResolverConverterServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/MetadataResolverConverterServiceImpl.java @@ -64,7 +64,7 @@ private OpenSamlFilesystemMetadataResolver convertToOpenSamlRepresentation(Files IndexWriter indexWriter = indexWriterService.getIndexWriter(resolver.getResourceId()); File metadataFile = new File(resolver.getMetadataFile()); if (resolver.getDoInitialization() && !metadataFile.exists()) { - throw new FileNotFoundException("No file was found on the fileysystem for provided filename: " + resolver.getMetadataFile()); + throw new FileNotFoundException("No file was found on the file system for provided filename: " + resolver.getMetadataFile()); } OpenSamlFilesystemMetadataResolver openSamlResolver = new OpenSamlFilesystemMetadataResolver(openSamlObjects.getParserPool(), From 21265a41d4b0daf0cdbec3451e92582e83b64a3b Mon Sep 17 00:00:00 2001 From: Ryan Mathis Date: Thu, 6 Dec 2018 08:52:54 -0700 Subject: [PATCH 05/11] SHIBUI-925 Implemented fix for error messages on upload --- .../domain/service/resolver.service.ts | 8 +++-- .../resolver/effect/collection.effects.ts | 33 +++++++++++++++---- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/ui/src/app/metadata/domain/service/resolver.service.ts b/ui/src/app/metadata/domain/service/resolver.service.ts index f0c9f4b3c..4012f9f23 100644 --- a/ui/src/app/metadata/domain/service/resolver.service.ts +++ b/ui/src/app/metadata/domain/service/resolver.service.ts @@ -45,7 +45,9 @@ export class ResolverService { return this.http.post(`${this.base}${this.endpoint}`, xml, { headers: new HttpHeaders().set('Content-Type', 'application/xml'), params: new HttpParams().set('spName', name) - }); + }).pipe(catchError(error => { + return throwError({ errorCode: error.status, errorMessage: `Unable to upload file ... ${error.error}` }); + })); } createFromUrl(name: string, url: string): Observable { @@ -53,7 +55,9 @@ export class ResolverService { return this.http.post(`${this.base}${this.endpoint}`, body, { headers: new HttpHeaders().set('Content-Type', 'application/x-www-form-urlencoded'), params: new HttpParams().set('spName', name) - }); + }).pipe(catchError(error => { + return throwError({ errorCode: error.status, errorMessage: `Unable to upload file ... ${error.error}` }); + })); } preview(id: string): Observable { diff --git a/ui/src/app/metadata/resolver/effect/collection.effects.ts b/ui/src/app/metadata/resolver/effect/collection.effects.ts index d56110a7e..46d2d0712 100644 --- a/ui/src/app/metadata/resolver/effect/collection.effects.ts +++ b/ui/src/app/metadata/resolver/effect/collection.effects.ts @@ -1,15 +1,21 @@ import { Injectable } from '@angular/core'; import { Effect, Actions, ofType } from '@ngrx/effects'; import { Router } from '@angular/router'; - +import { Store } from '@ngrx/store'; import { of } from 'rxjs'; -import { map, catchError, switchMap, tap } from 'rxjs/operators'; +import { map, catchError, switchMap, tap, withLatestFrom } from 'rxjs/operators'; import * as providerActions from '../action/collection.action'; import * as draftActions from '../action/draft.action'; import { ResolverCollectionActionTypes } from '../action/collection.action'; import { ResolverService } from '../../domain/service/resolver.service'; import { removeNulls } from '../../../shared/util'; +import { AddNotification } from '../../../notification/action/notification.action'; +import { Notification, NotificationType } from '../../../notification/model/notification'; +import { I18nService } from '../../../i18n/service/i18n.service'; +import * as fromRoot from '../../../app.reducer'; +import * as fromI18n from '../../../i18n/reducer'; + /* istanbul ignore next */ @Injectable() @@ -112,11 +118,24 @@ export class ResolverCollectionEffects { ofType(ResolverCollectionActionTypes.ADD_RESOLVER_SUCCESS), map(action => action.payload), map(provider => { - console.log(provider); return new draftActions.RemoveDraftRequest(provider); }) ); + @Effect() + addResolverFailNotification$ = this.actions$.pipe( + ofType(ResolverCollectionActionTypes.ADD_RESOLVER_FAIL), + map(action => action.payload), + withLatestFrom(this.store.select(fromI18n.getMessages)), + map(([error, messages]) => new AddNotification( + new Notification( + NotificationType.Danger, + `${error.errorCode}: ${this.i18nService.translate(error.errorMessage, null, messages)}`, + 8000 + ) + )) + ); + @Effect() uploadResolverRequest$ = this.actions$.pipe( ofType(ResolverCollectionActionTypes.UPLOAD_RESOLVER_REQUEST), @@ -126,7 +145,7 @@ export class ResolverCollectionEffects { .upload(file.name, file.body) .pipe( map(p => new providerActions.AddResolverSuccess(p)), - catchError(() => of(new providerActions.AddResolverFail(file))) + catchError((error) => of(new providerActions.AddResolverFail(error))) ) ) ); @@ -140,7 +159,7 @@ export class ResolverCollectionEffects { .createFromUrl(file.name, file.url) .pipe( map(p => new providerActions.AddResolverSuccess(p)), - catchError(() => of(new providerActions.AddResolverFail(file))) + catchError((error) => of(new providerActions.AddResolverFail(error))) ) ) ); @@ -148,6 +167,8 @@ export class ResolverCollectionEffects { constructor( private descriptorService: ResolverService, private actions$: Actions, - private router: Router + private router: Router, + private store: Store, + private i18nService: I18nService ) { } } /* istanbul ignore next */ From 3590d8d0631f6ca26c005bd17a9557af4eeb98c6 Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Thu, 6 Dec 2018 09:49:36 -0700 Subject: [PATCH 06/11] [SHIBUI-925] Updated a test to reflect the change in error messaging. --- .../admin/ui/controller/EntityDescriptorControllerTests.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 122a349ae..b38941b74 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 @@ -505,7 +505,7 @@ class EntityDescriptorControllerTests extends Specification { then: result.andExpect(status().isConflict()) - .andExpect(content().string("The entity descriptor with entity id [http://test.scaldingspoon.org/test1] already exists.")) + .andExpect(content().string("{\"errorCode\":\"409\",\"errorMessage\":\"The entity descriptor with entity id [http://test.scaldingspoon.org/test1] already exists.\"}")) } def "POST /EntityDescriptor handles x-www-form-urlencoded happily"() { From 0c1304a8d16ed17f21ad5dcad6aa1697d0c8ca68 Mon Sep 17 00:00:00 2001 From: Jodie Muramoto Date: Thu, 6 Dec 2018 10:50:32 -0700 Subject: [PATCH 07/11] SHIBUI-1010: Removed redundant ARIA roles and attributes (if button is semantic, does not need the role of button, etc.); --- .../resources/local-dynamic-metadata-provider.schema.json | 2 +- ui/src/app/app.component.html | 3 +-- .../domain/component/wizard-summary.component.html | 6 +++--- .../metadata/filter/container/edit-filter.component.html | 2 +- .../metadata/filter/container/new-filter.component.html | 2 +- .../app/metadata/manager/container/manager.component.html | 2 +- .../provider/container/provider-wizard.component.html | 5 +++-- .../resolver/container/blank-resolver.component.html | 2 +- .../resolver/container/confirm-copy.component.html | 6 +++--- .../resolver/container/copy-resolver.component.html | 3 +-- .../resolver/container/new-resolver.component.html | 2 +- .../resolver/container/upload-resolver.component.html | 2 +- ui/src/app/shared/autocomplete/autocomplete.component.html | 1 - ui/src/app/shared/component/info-icon.component.html | 2 -- ui/src/app/wizard/component/wizard.component.html | 7 ++----- 15 files changed, 20 insertions(+), 27 deletions(-) diff --git a/backend/src/main/resources/local-dynamic-metadata-provider.schema.json b/backend/src/main/resources/local-dynamic-metadata-provider.schema.json index bb98b8b67..c76434258 100644 --- a/backend/src/main/resources/local-dynamic-metadata-provider.schema.json +++ b/backend/src/main/resources/local-dynamic-metadata-provider.schema.json @@ -9,7 +9,7 @@ "properties": { "name": { "title": "label.metadata-provider-name-display-only", - "description": "tooltip.metadata-provider-name", + "description": "tooltip.metadata-provider-name-display-only", "type": "string", "widget": { "id": "string", diff --git a/ui/src/app/app.component.html b/ui/src/app/app.component.html index f09ae298a..b7adb997b 100644 --- a/ui/src/app/app.component.html +++ b/ui/src/app/app.component.html @@ -14,7 +14,6 @@ - + diff --git a/ui/src/app/metadata/filter/container/edit-filter.component.html b/ui/src/app/metadata/filter/container/edit-filter.component.html index e42137ebf..a7771dbef 100644 --- a/ui/src/app/metadata/filter/container/edit-filter.component.html +++ b/ui/src/app/metadata/filter/container/edit-filter.component.html @@ -1,4 +1,4 @@ -
+
diff --git a/ui/src/app/metadata/filter/container/new-filter.component.html b/ui/src/app/metadata/filter/container/new-filter.component.html index e56dc4634..08633e938 100644 --- a/ui/src/app/metadata/filter/container/new-filter.component.html +++ b/ui/src/app/metadata/filter/container/new-filter.component.html @@ -1,4 +1,4 @@ -
+
diff --git a/ui/src/app/metadata/manager/container/manager.component.html b/ui/src/app/metadata/manager/container/manager.component.html index 2c354d7e3..7803d47e5 100644 --- a/ui/src/app/metadata/manager/container/manager.component.html +++ b/ui/src/app/metadata/manager/container/manager.component.html @@ -1,4 +1,4 @@ -
+
  • Metadata Sources diff --git a/ui/src/app/metadata/provider/container/provider-wizard.component.html b/ui/src/app/metadata/provider/container/provider-wizard.component.html index b1027711b..366bb70d5 100644 --- a/ui/src/app/metadata/provider/container/provider-wizard.component.html +++ b/ui/src/app/metadata/provider/container/provider-wizard.component.html @@ -7,11 +7,12 @@
-
+
+ (onSave)="save()" + role="navigation">
diff --git a/ui/src/app/metadata/resolver/container/blank-resolver.component.html b/ui/src/app/metadata/resolver/container/blank-resolver.component.html index 5229f61f1..70ee5fa19 100644 --- a/ui/src/app/metadata/resolver/container/blank-resolver.component.html +++ b/ui/src/app/metadata/resolver/container/blank-resolver.component.html @@ -9,7 +9,7 @@

  • -
  • -
  • -
  • -
  • -