search

jeffwils / grails-spring-security-saml

Grails Spring Security SAML2.0 Plugin for Grails 3
8 stars 24 forks source link

application.yml Validation #15

Closed valentingoebel closed 6 years ago

valentingoebel commented 6 years ago

Failing to adhere to the expected configuration format causes "byzantine" error messages that are very hard to understand.

As an example I've taken out this snippet from the application.yml

providers:
    - ping: 'security/idp-local.xml'
groovy.lang.MissingMethodException: No signature of method: org.grails.plugin.springsecurity.saml.SpringSecuritySamlGrailsPlugin$_doWithSpring_closure1$_closure7.doCall() is applicable for argument types: (java.util.LinkedHashMap) values: [[ping:security/idp-local.xml]]
Possible solutions: doCall(java.lang.Object, java.lang.Object), findAll(), findAll(), isCase(java.lang.Object), isCase(java.lang.Object)
        at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:256)

The cause of the error is that providers should be specified as a map, not a list but that is not obvious from the actual error message. It just says doCall wasn't found.

The application.yml should look like this. (no dash)

providers:
    ping: 'security/idp-local.xml'

I'm sure there are more properties than just providers that also fail in non obvious ways when configured incorrectly.

irstevenson commented 6 years ago

Yes, as a new user of this plugin I tripped over this left right and center. It seemed to me you almost needed all of the config or else.

So as a simple starter, I was going to add a comment in the doco to that effect - i.e. all config must be present.

But also looking at the init processing the config there are things that could definitely be done to assist developers with configuration issues - e.g. log messages.

valentingoebel commented 6 years ago

When I tried to add a new property "saml.metadata.timeout". I realised that the plugin doesn't actually load the default configuration.

// SpringSecurityUtils.loadSecondaryConfig 'DefaultSamlSecurityConfig'

If we migrate the default configuration to plugin.yml I hope we will see less crashes.

valentingoebel commented 6 years ago

Ok I can confirm now that this has worked with autoCreate. I will create a PR soon.

valentingoebel commented 6 years ago

https://github.com/jeffwils/grails-spring-security-saml/pull/19

irstevenson commented 6 years ago

Closing due to merging of PR.