Merge branch 'master' of bitbucket.org:unicon/shib-idp-ui into bugfix…

…/SHIBUI-1015

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

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;

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,32 +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);
-
-        //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);
-        }
+        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) {
@@ -143,32 +138,21 @@ 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);
-
-        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);
-        }
+        doResolverInitialization(persistedResolver);
 
         return ResponseEntity.ok(persistedResolver);
     }
 
     @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;
@@ -181,4 +165,17 @@ private static URI getResourceUriFor(MetadataResolver resolver) {
                 .build()
                 .toUri();
     }
+
+    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) {
+                throw new MetadataFileNotFoundException("message.file-doesnt-exist");
+            }
+            OpenSamlChainingMetadataResolverUtil.updateChainingMetadataResolver((OpenSamlChainingMetadataResolver) chainingMetadataResolver, openSamlRepresentation);
+        }
+    }
 }

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.hibernate.exception.ConstraintViolationException;
@@ -12,6 +12,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.BAD_REQUEST;
 import static org.springframework.http.HttpStatus.NOT_FOUND;
 
@@ -53,4 +54,10 @@ public final ResponseEntity<ErrorResponse> handleAllOtherExceptions(Exception ex
         ErrorResponse errorResponse = new ErrorResponse("400", ex.getLocalizedMessage());
         return new ResponseEntity<>(errorResponse, HttpStatus.BAD_REQUEST);
     }
+
+    @ExceptionHandler(MetadataFileNotFoundException.class)
+    public final ResponseEntity<ErrorResponse> metadataFileNotFoundHandler(MetadataFileNotFoundException ex) {
+        ErrorResponse errorResponse = new ErrorResponse(INTERNAL_SERVER_ERROR.toString(), ex.getLocalizedMessage());
+        return new ResponseEntity<>(errorResponse, INTERNAL_SERVER_ERROR);
+    }
 }

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);
+    }
+}

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(),

backend/src/main/resources/i18n/messages.properties

@@ -398,6 +398,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.
 message.database-constraint=There was a database constraint problem processing the request. Check the request to ensure that fields that must be unique are truly unique.
 
 tooltip.entity-id=Entity ID

backend/src/main/resources/i18n/messages_en.properties

@@ -404,6 +404,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.
 message.database-constraint=There was a database constraint problem processing the request. Check the request to ensure that fields that must be unique are truly unique.
 
 tooltip.entity-id=Entity ID

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"() {

ui/src/app/metadata/domain/service/resolver.service.ts

@@ -45,15 +45,19 @@ export class ResolverService {
         return this.http.post<MetadataResolver>(`${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<MetadataResolver> {
         let body = `metadataUrl=${url}`;
         return this.http.post<MetadataResolver>(`${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<string> {

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<providerActions.AddResolverSuccess>(ResolverCollectionActionTypes.ADD_RESOLVER_SUCCESS),
         map(action => action.payload),
         map(provider => {
-            console.log(provider);
             return new draftActions.RemoveDraftRequest(provider);
         })
     );
 
+    @Effect()
+    addResolverFailNotification$ = this.actions$.pipe(
+        ofType<providerActions.AddResolverFail>(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<providerActions.UploadResolverRequest>(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,14 +159,16 @@ 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)))
                 )
         )
     );
 
     constructor(
         private descriptorService: ResolverService,
         private actions$: Actions,
-        private router: Router
+        private router: Router,
+        private store: Store<fromRoot.State>,
+        private i18nService: I18nService
     ) { }
 } /* istanbul ignore next */