search

jeffwils / grails-spring-security-saml

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

Exclude example metadata and keystore from build #40

Closed irstevenson closed 5 years ago

irstevenson commented 6 years ago

There are a couple of additional files in grails-app/conf/security which should be excluded from the built product. They are:

These are only for testing AFAIK (so can stay in the package), but are not intended for production use and so shouldn't be in the released plugin.

The fix should probably just be adding another exclude to the jar block in build.gradle:

https://github.com/jeffwils/grails-spring-security-saml/blob/ba8a71186c806c65915b6806e82e7b9543096204/build.gradle#L149-L151

valentingoebel commented 6 years ago

Removal of those files will probably cause the default configuration to fail. So we will need more validation code in SpringSecuritySamlGrailsPlugin.groovy.

The JKSKeyManager and FilesystemMetadataProvider sections need to check if the files exists and notify the user.

I also think it's time to cleanup the FilesystemMetadataProvider code and put it into a seperate function. It's roughly 30 lines of not so beautiful code and it is duplicated twice inside the doWithSpring function.

irstevenson commented 6 years ago

Indeed, definitely some more config validation should be part of it. We could possibly centralise this - i.e. have a validateConfig( conf ) method called at the start after we check active state. I might do that starting with #39 .

And yes, I agree, there's a lot of potential refactoring in doWithSpring(). I'll raise a ticket for that second bit.

(Also edited my initial comment: As I had two idp-local.xml where one was meant to be sp.xml - which it now is.)

irstevenson commented 6 years ago

Note the main issue here is solved with #54

:+1:

Can close once merged.

valentingoebel commented 5 years ago

This has been merged.