From 05149335b30d585e32bbec5a9b7e177efdfcd0f4 Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Tue, 13 Nov 2018 17:16:55 -0700 Subject: [PATCH 1/6] [SHIBUI-999] Added dependency on commons-io. Moved validation of json schema to before the body is read/used. Fixed a bug in relying party overrides creation from representation that would cause a boolean value to be persisted instead of the persistValue defined in the yaml. --- backend/build.gradle | 3 ++ ...sonSchemaValidatingControllerAdvice.groovy | 29 +++++++++++++++---- .../ui/service/JPAEntityServiceImpl.java | 5 ++-- 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/backend/build.gradle b/backend/build.gradle index f03f9f9ed..baba8fd6d 100644 --- a/backend/build.gradle +++ b/backend/build.gradle @@ -139,6 +139,9 @@ dependencies { //JSON schema validator compile 'org.sharegov:mjson:1.4.1' + + //Apache commons-io + compile group: 'commons-io', name: 'commons-io', version: '2.6' } def generatedSrcDir = new File(buildDir, 'generated/src/main/java') 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 712142172..72931e182 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,9 +1,13 @@ package edu.internet2.tier.shibboleth.admin.ui.jsonschema import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.EntityDescriptorRepresentation +import groovy.json.JsonBuilder import mjson.Json +import org.apache.commons.io.IOUtils +import org.apache.commons.io.input.CloseShieldInputStream import org.springframework.beans.factory.annotation.Autowired import org.springframework.core.MethodParameter +import org.springframework.http.HttpHeaders import org.springframework.http.HttpInputMessage import org.springframework.http.HttpStatus import org.springframework.http.ResponseEntity @@ -11,6 +15,7 @@ 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.AbstractMessageConverterMethodArgumentResolver import org.springframework.web.servlet.mvc.method.annotation.RequestBodyAdviceAdapter import javax.annotation.PostConstruct @@ -38,15 +43,29 @@ class RelyingPartyOverridesJsonSchemaValidatingControllerAdvice extends RequestB } @Override - Object afterBodyRead(Object body, HttpInputMessage inputMessage, MethodParameter parameter, Type targetType, Class> converterType) { - def relyingPartyOverrides = EntityDescriptorRepresentation.cast(body).relyingPartyOverrides - def relyingPartyOverridesJson = Json.make([relyingPartyOverrides: relyingPartyOverrides]) + HttpInputMessage beforeBodyRead(HttpInputMessage inputMessage, MethodParameter parameter, + Type targetType, Class> converterType) + throws IOException { + def baos = new ByteArrayOutputStream() + IOUtils.copy(inputMessage.body, baos) + def bytes = baos.toByteArray() def schema = Json.schema(this.jsonSchemaLocation.uri) - def validationResult = schema.validate(relyingPartyOverridesJson) + def stream = new ByteArrayInputStream(bytes) + def validationResult = schema.validate(Json.read(stream.getText())) if (!validationResult.at('ok')) { throw new JsonSchemaValidationFailedException(validationResult.at('errors').asList()) } - body + return new HttpInputMessage() { + @Override + InputStream getBody() throws IOException { + return new ByteArrayInputStream(bytes) + } + + @Override + HttpHeaders getHeaders() { + return inputMessage.getHeaders() + } + } } @ExceptionHandler(JsonSchemaValidationFailedException) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java index 31a8b28ae..dd4335dea 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java @@ -105,10 +105,11 @@ public List getAttributeListFromRelyingPartyOverridesRepresentation(M switch (ModelRepresentationConversions.AttributeTypes.valueOf(overrideProperty.getDisplayType().toUpperCase())) { case BOOLEAN: if (overrideProperty.getPersistType() != null && - !overrideProperty.getPersistType().equalsIgnoreCase("boolean")) { + !overrideProperty.getPersistType().equalsIgnoreCase("boolean") && + (Boolean) entry.getValue()) { list.add(attributeUtility.createAttributeWithStringValues(overrideProperty.getAttributeName(), overrideProperty.getAttributeFriendlyName(), - (String) entry.getValue())); + overrideProperty.getPersistValue())); } else { list.add(attributeUtility.createAttributeWithBooleanValue(overrideProperty.getAttributeName(), overrideProperty.getAttributeFriendlyName(), From 836880d28cceebbcb2d70e0a25b9e6b7d8039da7 Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Tue, 13 Nov 2018 17:18:22 -0700 Subject: [PATCH 2/6] [SHIBUI-999] Cleaned up imports. --- ...ngPartyOverridesJsonSchemaValidatingControllerAdvice.groovy | 3 --- 1 file changed, 3 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 72931e182..4cc5799cf 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,10 +1,8 @@ package edu.internet2.tier.shibboleth.admin.ui.jsonschema import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.EntityDescriptorRepresentation -import groovy.json.JsonBuilder import mjson.Json import org.apache.commons.io.IOUtils -import org.apache.commons.io.input.CloseShieldInputStream import org.springframework.beans.factory.annotation.Autowired import org.springframework.core.MethodParameter import org.springframework.http.HttpHeaders @@ -15,7 +13,6 @@ 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.AbstractMessageConverterMethodArgumentResolver import org.springframework.web.servlet.mvc.method.annotation.RequestBodyAdviceAdapter import javax.annotation.PostConstruct From 7833ae1754b69b990bd6a4031aac347bb44e27fb Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Tue, 13 Nov 2018 19:18:27 -0700 Subject: [PATCH 3/6] [SHIBUI-999] Added a fix for dealing with ambiguous types stored in the relying party overrides map. --- .../admin/ui/service/JPAEntityServiceImpl.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java index dd4335dea..0f96e0895 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java @@ -106,14 +106,20 @@ public List getAttributeListFromRelyingPartyOverridesRepresentation(M case BOOLEAN: if (overrideProperty.getPersistType() != null && !overrideProperty.getPersistType().equalsIgnoreCase("boolean") && - (Boolean) entry.getValue()) { + Boolean.valueOf((String)entry.getValue())) { list.add(attributeUtility.createAttributeWithStringValues(overrideProperty.getAttributeName(), overrideProperty.getAttributeFriendlyName(), overrideProperty.getPersistValue())); } else { - list.add(attributeUtility.createAttributeWithBooleanValue(overrideProperty.getAttributeName(), - overrideProperty.getAttributeFriendlyName(), - (Boolean) entry.getValue())); + if (entry.getValue() instanceof String) { + list.add(attributeUtility.createAttributeWithBooleanValue(overrideProperty.getAttributeName(), + overrideProperty.getAttributeFriendlyName(), + Boolean.valueOf((String) entry.getValue()))); + } else { + list.add(attributeUtility.createAttributeWithBooleanValue(overrideProperty.getAttributeName(), + overrideProperty.getAttributeFriendlyName(), + (Boolean) entry.getValue())); + } } break; case INTEGER: From 06fb86ea670f79941de0595a8811f50183aafdef Mon Sep 17 00:00:00 2001 From: Jj! Date: Wed, 14 Nov 2018 08:41:18 -0600 Subject: [PATCH 4/6] [SHIBUI-999] cleanup --- backend/build.gradle | 3 --- ...sonSchemaValidatingControllerAdvice.groovy | 22 +++++-------------- .../ui/service/JPAEntityServiceImpl.java | 2 +- 3 files changed, 7 insertions(+), 20 deletions(-) diff --git a/backend/build.gradle b/backend/build.gradle index baba8fd6d..f03f9f9ed 100644 --- a/backend/build.gradle +++ b/backend/build.gradle @@ -139,9 +139,6 @@ dependencies { //JSON schema validator compile 'org.sharegov:mjson:1.4.1' - - //Apache commons-io - compile group: 'commons-io', name: 'commons-io', version: '2.6' } def generatedSrcDir = new File(buildDir, 'generated/src/main/java') 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 4cc5799cf..934d7b1a7 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 @@ -2,10 +2,8 @@ package edu.internet2.tier.shibboleth.admin.ui.jsonschema import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.EntityDescriptorRepresentation import mjson.Json -import org.apache.commons.io.IOUtils import org.springframework.beans.factory.annotation.Autowired import org.springframework.core.MethodParameter -import org.springframework.http.HttpHeaders import org.springframework.http.HttpInputMessage import org.springframework.http.HttpStatus import org.springframework.http.ResponseEntity @@ -43,26 +41,18 @@ class RelyingPartyOverridesJsonSchemaValidatingControllerAdvice extends RequestB HttpInputMessage beforeBodyRead(HttpInputMessage inputMessage, MethodParameter parameter, Type targetType, Class> converterType) throws IOException { - def baos = new ByteArrayOutputStream() - IOUtils.copy(inputMessage.body, baos) - def bytes = baos.toByteArray() + def bytes = inputMessage.body.bytes def schema = Json.schema(this.jsonSchemaLocation.uri) + def stream = new ByteArrayInputStream(bytes) def validationResult = schema.validate(Json.read(stream.getText())) if (!validationResult.at('ok')) { throw new JsonSchemaValidationFailedException(validationResult.at('errors').asList()) } - return new HttpInputMessage() { - @Override - InputStream getBody() throws IOException { - return new ByteArrayInputStream(bytes) - } - - @Override - HttpHeaders getHeaders() { - return inputMessage.getHeaders() - } - } + return [ + getBody: { new ByteArrayInputStream(bytes) }, + getHeaders: { inputMessage.headers } + ] as HttpInputMessage } @ExceptionHandler(JsonSchemaValidationFailedException) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java index 0f96e0895..90cf86c5d 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java @@ -106,7 +106,7 @@ public List getAttributeListFromRelyingPartyOverridesRepresentation(M case BOOLEAN: if (overrideProperty.getPersistType() != null && !overrideProperty.getPersistType().equalsIgnoreCase("boolean") && - Boolean.valueOf((String)entry.getValue())) { + (Boolean)entry.getValue()) { list.add(attributeUtility.createAttributeWithStringValues(overrideProperty.getAttributeName(), overrideProperty.getAttributeFriendlyName(), overrideProperty.getPersistValue())); From c54b551ef3985e3b46c6d825071a5f1cd5b8d67b Mon Sep 17 00:00:00 2001 From: Jj! Date: Wed, 14 Nov 2018 09:07:42 -0600 Subject: [PATCH 5/6] [SHIBUI-999] revert to fix test --- .../tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java index 90cf86c5d..0f96e0895 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java @@ -106,7 +106,7 @@ public List getAttributeListFromRelyingPartyOverridesRepresentation(M case BOOLEAN: if (overrideProperty.getPersistType() != null && !overrideProperty.getPersistType().equalsIgnoreCase("boolean") && - (Boolean)entry.getValue()) { + Boolean.valueOf((String)entry.getValue())) { list.add(attributeUtility.createAttributeWithStringValues(overrideProperty.getAttributeName(), overrideProperty.getAttributeFriendlyName(), overrideProperty.getPersistValue())); From f849c917ce81fba925e5da05d1a9e2dd22eae561 Mon Sep 17 00:00:00 2001 From: Jj! Date: Wed, 14 Nov 2018 09:26:10 -0600 Subject: [PATCH 6/6] [SHIBUI-999] cleanup --- .../tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java index 0f96e0895..ae443a055 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java @@ -106,7 +106,7 @@ public List getAttributeListFromRelyingPartyOverridesRepresentation(M case BOOLEAN: if (overrideProperty.getPersistType() != null && !overrideProperty.getPersistType().equalsIgnoreCase("boolean") && - Boolean.valueOf((String)entry.getValue())) { + ((entry.getValue() instanceof Boolean && (Boolean)entry.getValue()) || Boolean.valueOf((String)entry.getValue()))) { list.add(attributeUtility.createAttributeWithStringValues(overrideProperty.getAttributeName(), overrideProperty.getAttributeFriendlyName(), overrideProperty.getPersistValue()));