Fix Shib setting rules.

[halfninja]

I think ShibRequireSetting was a mental mixup between ShibRequireSession
and ShibRequestSetting. I've replaced it with separate options for both. The latter is
an array as you can set multiple different settings, while the former is a simple
On or Off value. Each item in the settings array should be the name and value of the setting,
e.g. ['requireSession false', 'applicationId myresource']

[halfninja]

Have just thought that it would make more sense for the settings to be a hash instead of an array.

[halfninja]

Alright, it now takes a hash.

[igalic]

@Aethylred ping

[Aethylred]

Right. It look like @halfninja is correct.

Using ShibRequestSetting is the current method of configuring mod_shib (as per the documentation) and uses the content settings

While ShibRequireSession is the old but still supported way, currently it is preferred that:

ShibRequireSession On

is replaced with

ShibRequestSetting requireSession On

Given that this module is unlikely to install a version of shibboleth old enough to not support ShibRequestSetting there might be some way of just adding or making sure {'requiredSession' => 'On'} is in the ShibRequestSetting hash if ShibRequireSession is not false.

This would have the advantage of helping users who're following old documentation (very common when implementing Shibboleth for the first time) find the declaration they're looking for, yet produce a vhost.conf that meets contemporary standards.

[Aethylred]

oh, and this would be a lot better if Puppet could validate the content of hashes.

hmm, this didn't require the tests to be updated? Ah, I think I've pointed out before that the tests do not confirm if parameters are being correctly inserted into the vhost file.

[igalic]

@halfninja do you think you could add rspec tests for this also?
oh.. we don't really have much in terms of "directories" spec tests :(

[halfninja]

I agree with your thoughts @Aethylred - shib_require_session=>On could quietly get turned into the newer request setting format. I have to do some other Puppet work before I can return to this, but I'll definitely look into that when I do.

Regarding validation, I think one could do a certain amount with the help of stdlib functions like has_key - might be too much work to check that every key was valid, but for ones we knew about we could validate their values.

Will also have a go at specs when I'm next working on it @igalic. Thanks.

[Aethylred]

@igalic I actually lodged a ticket on the lack of spec tests on the vhost file contents. It's an issue across the whole module and a Big Job: https://tickets.puppetlabs.com/browse/MODULES-1381

[igalic]

@mhaskel ^