Skip to content

Commit

Permalink
Merged in feature/shibui-1021 (pull request #504)
Browse files Browse the repository at this point in the history
Remove defaults from settings/code/ui

Approved-by: Jonathan Johnson
Approved-by: Bill Smith
  • Loading branch information
chasegawa authored and Jonathan Johnson committed Aug 4, 2021
2 parents 51a1207 + 7dac3fd commit 37bdabc
Show file tree
Hide file tree
Showing 24 changed files with 139 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -346,12 +346,12 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService {
'xsi:type': 'FilesystemMetadataProvider',
metadataFile: resolver.metadataFile,

requireValidMetadata: !resolver.requireValidMetadata ?: null,
failFastInitialization: !resolver.failFastInitialization ?: null,
requireValidMetadata: resolver.requireValidMetadata,
failFastInitialization: resolver.failFastInitialization,
sortKey: resolver.sortKey,
criterionPredicateRegistryRef: resolver.criterionPredicateRegistryRef,
useDefaultPredicateRegistry: !resolver.useDefaultPredicateRegistry ?: null,
satisfyAnyPredicates: resolver.satisfyAnyPredicates ?: null,
useDefaultPredicateRegistry: resolver.useDefaultPredicateRegistry,
satisfyAnyPredicates: resolver.satisfyAnyPredicates,

parserPoolRef: resolver.reloadableMetadataResolverAttributes?.parserPoolRef,
minRefreshDelay: resolver.reloadableMetadataResolverAttributes?.minRefreshDelay,
Expand All @@ -368,12 +368,12 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService {
void constructXmlNodeForResolver(DynamicHttpMetadataResolver resolver, def markupBuilderDelegate, Closure childNodes) {
markupBuilderDelegate.MetadataProvider(id: resolver.xmlId,
'xsi:type': 'DynamicHTTPMetadataProvider',
requireValidMetadata: !resolver.requireValidMetadata ?: null,
failFastInitialization: !resolver.failFastInitialization ?: null,
requireValidMetadata: resolver.requireValidMetadata,
failFastInitialization: resolver.failFastInitialization,
sortKey: resolver.sortKey,
criterionPredicateRegistryRef: resolver.criterionPredicateRegistryRef,
useDefaultPredicateRegistry: !resolver.useDefaultPredicateRegistry ?: null,
satisfyAnyPredicates: resolver.satisfyAnyPredicates ?: null,
useDefaultPredicateRegistry: resolver.useDefaultPredicateRegistry,
satisfyAnyPredicates: resolver.satisfyAnyPredicates,
parserPoolRef: resolver.dynamicMetadataResolverAttributes?.parserPoolRef,
taskTimerRef: resolver.dynamicMetadataResolverAttributes?.taskTimerRef,
refreshDelayFactor: resolver.dynamicMetadataResolverAttributes?.refreshDelayFactor,
Expand Down Expand Up @@ -447,14 +447,14 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService {
'xsi:type': 'FileBackedHTTPMetadataProvider',
backingFile: resolver.backingFile,
metadataURL: resolver.metadataURL,
initializeFromBackupFile: !resolver.initializeFromBackupFile ?: null,
initializeFromBackupFile: resolver.initializeFromBackupFile,
backupFileInitNextRefreshDelay: resolver.backupFileInitNextRefreshDelay,
requireValidMetadata: !resolver.requireValidMetadata ?: null,
failFastInitialization: !resolver.failFastInitialization ?: null,
requireValidMetadata: resolver.requireValidMetadata,
failFastInitialization: resolver.failFastInitialization,
sortKey: resolver.sortKey,
criterionPredicateRegistryRef: resolver.criterionPredicateRegistryRef,
useDefaultPredicateRegistry: !resolver.useDefaultPredicateRegistry ?: null,
satisfyAnyPredicates: resolver.satisfyAnyPredicates ?: null,
useDefaultPredicateRegistry: resolver.useDefaultPredicateRegistry,
satisfyAnyPredicates: resolver.satisfyAnyPredicates,

parserPoolRef: resolver.reloadableMetadataResolverAttributes?.parserPoolRef,
minRefreshDelay: resolver.reloadableMetadataResolverAttributes?.minRefreshDelay,
Expand Down Expand Up @@ -490,12 +490,12 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService {

id: resolver.xmlId,
'xsi:type': 'LocalDynamicMetadataProvider',
requireValidMetadata: !resolver.requireValidMetadata ?: null,
failFastInitialization: !resolver.failFastInitialization ?: null,
requireValidMetadata: resolver.requireValidMetadata,
failFastInitialization: resolver.failFastInitialization,
sortKey: resolver.sortKey,
criterionPredicateRegistryRef: resolver.criterionPredicateRegistryRef,
useDefaultPredicateRegistry: !resolver.useDefaultPredicateRegistry ?: null,
satisfyAnyPredicates: resolver.satisfyAnyPredicates ?: null,
useDefaultPredicateRegistry: resolver.useDefaultPredicateRegistry,
satisfyAnyPredicates: resolver.satisfyAnyPredicates,
parserPoolRef: resolver.dynamicMetadataResolverAttributes?.parserPoolRef,
taskTimerRef: resolver.dynamicMetadataResolverAttributes?.taskTimerRef,
refreshDelayFactor: resolver.dynamicMetadataResolverAttributes?.refreshDelayFactor,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ private static URI getResourceUriFor(MetadataResolver resolver) {

private void doResolverInitialization(MetadataResolver persistedResolver) throws
ComponentInitializationException, ResolverException, IOException {
if (persistedResolver.getDoInitialization()) {
if (persistedResolver.getDoInitialization() != null && persistedResolver.getDoInitialization()) {
org.opensaml.saml.metadata.resolver.MetadataResolver openSamlRepresentation = null;
try {
openSamlRepresentation = metadataResolverConverterService.convertToOpenSamlRepresentation(persistedResolver);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public NameIdFormatFilter() {
type = "NameIDFormat";
}

private Boolean removeExistingFormats = false;
private Boolean removeExistingFormats;

@ElementCollection
@OrderColumn
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ public class DynamicHttpMetadataResolver extends MetadataResolver {
public DynamicHttpMetadataResolver() {
type = "DynamicHttpMetadataResolver";
this.httpMetadataResolverAttributes = new HttpMetadataResolverAttributes();
this.httpMetadataResolverAttributes.setConnectionRequestTimeout(DEFAULT_TIMEOUT);
this.httpMetadataResolverAttributes.setConnectionTimeout(DEFAULT_TIMEOUT);
this.httpMetadataResolverAttributes.setSocketTimeout(DEFAULT_TIMEOUT);
this.dynamicMetadataResolverAttributes = new DynamicMetadataResolverAttributes();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public FileBackedHttpMetadataResolver() {

private String backingFile;

private Boolean initializeFromBackupFile = true;
private Boolean initializeFromBackupFile;

private String backupFileInitNextRefreshDelay;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
public class FilesystemMetadataResolver extends MetadataResolver {
public FilesystemMetadataResolver() {
type = "FilesystemMetadataResolver";
this.setDoInitialization(false);
}

private String metadataFile;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public class HttpMetadataResolverAttributes {

private String socketTimeout;

private Boolean disregardTLSCertificate = false;
private Boolean disregardTLSCertificate;

private String tlsTrustEngineRef;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,21 +58,21 @@ public class MetadataResolver extends AbstractAuditable {
@Column(unique = true)
private String xmlId;

private Boolean enabled = true;
private Boolean enabled;

private Boolean requireValidMetadata = true;
private Boolean requireValidMetadata;

private Boolean failFastInitialization = true;
private Boolean failFastInitialization;

private Integer sortKey;

private String criterionPredicateRegistryRef;

private Boolean useDefaultPredicateRegistry = true;
private Boolean useDefaultPredicateRegistry;

private Boolean satisfyAnyPredicates = false;
private Boolean satisfyAnyPredicates;

private Boolean doInitialization = true;
private Boolean doInitialization;

@JsonIgnore
private Long versionModifiedTimestamp;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,29 +35,29 @@ public class OpenSamlFileBackedHTTPMetadataResolver extends FileBackedHTTPMetada

public OpenSamlFileBackedHTTPMetadataResolver(ParserPool parserPool,
IndexWriter indexWriter,
FileBackedHttpMetadataResolver sourceResolver) throws ResolverException {
FileBackedHttpMetadataResolver sourceResolver) throws ResolverException {
super(HttpClients.createMinimal(), sourceResolver.getMetadataURL(), sourceResolver.getBackingFile());
this.indexWriter = indexWriter;
this.sourceResolver = sourceResolver;
this.delegate = new OpenSamlMetadataResolverDelegate();

this.setId(sourceResolver.getResourceId());

OpenSamlMetadataResolverConstructorHelper.updateOpenSamlMetadataResolverFromHttpMetadataResolverAttributes(
this, sourceResolver.getHttpMetadataResolverAttributes());
OpenSamlMetadataResolverConstructorHelper.updateOpenSamlMetadataResolverFromReloadableMetadataResolverAttributes(
this, sourceResolver.getReloadableMetadataResolverAttributes(), parserPool);
OpenSamlMetadataResolverConstructorHelper.updateOpenSamlMetadataResolverFromHttpMetadataResolverAttributes(this,
sourceResolver.getHttpMetadataResolverAttributes());
OpenSamlMetadataResolverConstructorHelper.updateOpenSamlMetadataResolverFromReloadableMetadataResolverAttributes(this,
sourceResolver.getReloadableMetadataResolverAttributes(), parserPool);

this.setBackupFile(placeholderResolverService()
.resolveValueFromPossibleTokenPlaceholder(sourceResolver.getBackingFile()));
this.setBackupFile(placeholderResolverService().resolveValueFromPossibleTokenPlaceholder(sourceResolver.getBackingFile()));
this.setBackupFileInitNextRefreshDelay(toMillis(placeholderResolverService()
.resolveValueFromPossibleTokenPlaceholder(sourceResolver.getBackupFileInitNextRefreshDelay())));

this.setInitializeFromBackupFile(sourceResolver.getInitializeFromBackupFile());
.resolveValueFromPossibleTokenPlaceholder(sourceResolver.getBackupFileInitNextRefreshDelay())));
if (sourceResolver.getInitializeFromBackupFile() != null) {
this.setInitializeFromBackupFile(sourceResolver.getInitializeFromBackupFile());
}

this.setMetadataFilter(new MetadataFilterChain());

//TODO: Where does this get set in OpenSAML land?
// TODO: Where does this get set in OpenSAML land?
// sourceResolver.getMetadataURL();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class MetadataFiltersControllerIntegrationTests extends Specification {
createRequestHttpEntityFor { JsonOutput.toJson(existingFilterMap) }, String)

then:
updatedResultFromPUT.statusCode.value() == 200
updatedResultFromPUT.statusCode.value() == 200
}

def "PUT EntityAttributesFilter and update it"() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,25 @@ class MetadataResolversControllerIntegrationTests extends Specification {
'ResourceBacked' | _
'Filesystem' | _
}

@DirtiesContext
def "SHIBUI-1992 - error creating FileBackedHTTPMetadata"() {
def resolver = new FileBackedHttpMetadataResolver().with {
it.name = 'FBHMR'
it.xmlId = '1'
it.backingFile = 'tmp/foo'
it.metadataURL = 'https://nexus.microsoftonline-p.com/federationmetadata/saml20/federationmetadata.xml'
it.backupFileInitNextRefreshDelay = 'PT4H'
it.enabled = Boolean.FALSE
it
}

when:
def result = this.restTemplate.postForEntity(BASE_URI, createRequestHttpEntityFor { mapper.writeValueAsString(resolver) }, String)

then:
result.statusCodeValue == 201
}

@Unroll
def "PUT concrete MetadataResolver of type #resolverType with updated changes -> /api/MetadataResolvers/{resourceId}"(String resolverType) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ class EntityDescriptorTest extends Specification {
metadataURL: 'https://idp.unicon.net/idp/shibboleth',
backingFile: "%{idp.home}/metadata/test.xml",
reloadableMetadataResolverAttributes: new ReloadableMetadataResolverAttributes(),
httpMetadataResolverAttributes: new HttpMetadataResolverAttributes()
httpMetadataResolverAttributes: new HttpMetadataResolverAttributes(),
initializeFromBackupFile: Boolean.TRUE
)
).with {
it.initialize()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ class JPAMetadataResolverServiceImplTests extends Specification {
it.content = 'http://mdq-beta.incommon.org/global'
it
}
it.enabled = Boolean.TRUE
it
}
metadataResolverRepository.save(resolver)
Expand All @@ -411,20 +412,38 @@ class JPAMetadataResolverServiceImplTests extends Specification {
}

@DirtiesContext(methodMode = DirtiesContext.MethodMode.AFTER_METHOD)
def 'test namespace protection in nonURL resolver'() {
def 'test namespace protection in nonURL resolver with resolver setting enabled=true'() {
setup:
shibUIConfiguration.protectedAttributeNamespaces = ['http://shibboleth.net/ns/profiles']
def resolver = new LocalDynamicMetadataResolver().with {
it.xmlId = 'LocalDynamic'
it.sourceDirectory = '/tmp'
it.enabled = Boolean.TRUE
it
}

when:
metadataResolverRepository.save(resolver)

then:
generatedXmlIsTheSameAsExpectedXml('/conf/1059.xml', metadataResolverService.generateConfiguration())
generatedXmlIsTheSameAsExpectedXml('/conf/1059-enabled.xml', metadataResolverService.generateConfiguration())
}

@DirtiesContext(methodMode = DirtiesContext.MethodMode.AFTER_METHOD)
def 'test namespace protection in nonURL resolver with resolver setting enabled not set'() {
setup:
shibUIConfiguration.protectedAttributeNamespaces = ['http://shibboleth.net/ns/profiles']
def resolver = new LocalDynamicMetadataResolver().with {
it.xmlId = 'LocalDynamic'
it.sourceDirectory = '/tmp'
it
}

when:
metadataResolverRepository.save(resolver)

then:
generatedXmlIsTheSameAsExpectedXml('/conf/1059-disabled.xml', metadataResolverService.generateConfiguration())
}

@Ignore('there is a bug in org.opensaml.saml.metadata.resolver.filter.impl.EntityAttributesFilter.applyFilter')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,20 @@ package edu.internet2.tier.shibboleth.admin.ui.util

import edu.internet2.tier.shibboleth.admin.ui.security.model.User
import groovy.xml.XmlUtil
import junit.framework.Assert
import javax.xml.transform.Source;
import javax.xml.transform.Transformer
import javax.xml.transform.TransformerException
import javax.xml.transform.TransformerFactory
import javax.xml.transform.dom.DOMSource
import javax.xml.transform.stream.StreamResult

import org.apache.commons.lang.StringUtils
import org.springframework.security.core.context.SecurityContextHolder
import org.w3c.dom.Document
import org.xmlunit.builder.DiffBuilder
import org.xmlunit.builder.Input
import org.xmlunit.builder.Input.Builder

/**
* @author Bill Smith (wsmith@unicon.net)
Expand All @@ -29,15 +38,27 @@ class TestHelpers {
}

static void generatedXmlIsTheSameAsExpectedXml(String expectedXmlResource, Document generatedXml) {
assert !DiffBuilder.compare(Input.fromStream(TestHelpers.getResourceAsStream(expectedXmlResource)))
.withTest(Input.fromDocument(generatedXml))
def Builder builder = Input.fromDocument(generatedXml)
def Source source = builder.build()
def myDiff = DiffBuilder.compare(Input.fromStream(TestHelpers.getResourceAsStream(expectedXmlResource)))
.withTest(builder)
.withAttributeFilter({attribute -> !attribute.name.equals("sourceDirectory")})
.ignoreComments()
.ignoreWhitespace()
.build()
.hasDifferences()
System.out.println("@@@ \n" + getString(source) + "\n")
Assert.assertFalse(myDiff.toString(), myDiff.hasDifferences());
}

public static String getString(DOMSource domSource) throws TransformerException {
StringWriter writer = new StringWriter();
StreamResult result = new StreamResult(writer);
TransformerFactory tf = TransformerFactory.newInstance();
Transformer transformer = tf.newTransformer();
transformer.transform(domSource, result);
return writer.toString();
}

static String XmlDocumentToString(Document document) {
return XmlUtil.serialize(document.documentElement)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,7 @@ class TestObjectGenerator {
it.name = 'FilesystemMetadata'
it.xmlId = 'FilesystemMetadata'
it.metadataFile = 'metadata/metadata.xml'
it.doInitialization = Boolean.FALSE // Removed the default setting, added back to keep tests the same.

it.reloadableMetadataResolverAttributes = new ReloadableMetadataResolverAttributes().with {
it
Expand All @@ -527,6 +528,9 @@ class TestObjectGenerator {
it.maxRefreshDelay = 'P1D'
it
}
// Changes in MetadataResolver (removing defaults), so adding back those settings here.
it.enabled = Boolean.TRUE
it.doInitialization = Boolean.TRUE
it
}
}
Expand All @@ -543,6 +547,9 @@ class TestObjectGenerator {
it.content = 'content'
it
}
// Changes in MetadataResolver (removing defaults), so adding back those settings here.
it.enabled = Boolean.TRUE
it.doInitialization = Boolean.TRUE
it
}
}
Expand All @@ -556,6 +563,9 @@ class TestObjectGenerator {
it.dynamicMetadataResolverAttributes = new DynamicMetadataResolverAttributes().with {
it
}
// Changes in MetadataResolver (removing defaults), so adding back those settings here.
it.enabled = Boolean.TRUE
it.doInitialization = Boolean.TRUE
it
}
}
Expand All @@ -573,6 +583,9 @@ class TestObjectGenerator {
it.reloadableMetadataResolverAttributes = new ReloadableMetadataResolverAttributes().with {
it
}
// Changes in MetadataResolver (removing defaults), so adding back those settings here.
it.enabled = Boolean.TRUE
it.doInitialization = Boolean.TRUE
it
}
}
Expand All @@ -589,6 +602,9 @@ class TestObjectGenerator {
it.refreshDelayFactor = 0.3
it
}
// Changes in MetadataResolver (removing defaults), so adding back those settings here.
it.enabled = Boolean.TRUE
it.doInitialization = Boolean.TRUE
it
}
}
Expand Down
Loading

0 comments on commit 37bdabc

Please sign in to comment.