I am finally in the last steps of integrating this into the SAML server we're using, which happens to be ADFS.
There are two extra options in the security configurations that seems to be needed for ADFS, see https://github.com/onelogin/php-saml/blob/e54e4e2decc764c4693e6c00b4107c..., settings `signatureAlgorithm` and `lowercaseUrlencoding`
Looking forward, we should probably want to do a loose integration for all the advanced options? maybe third_party_settings or just a textfield with configuration?
Attached a patch that takes care of exposing these two options.
Also added a couple of extra changes: converted array to short form and removed a IDE warning on the constructor of SamlauthConfigureForm, happy to move those to separate changes if you prefer it that way.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 2887207-1.patch | 26.37 KB | pcambra |
Comments
Comment #2
pcambraComment #3
roderikThanks! This looks good.
However I did not commit the changes to samlauth.authentication.yml - and verified that without these, the configForm / reformatConfig() will cause the library's defaults to be used.
I'm not sure about the loose integration. I like having the descriptions below all the form elements to guide the user. If the number of configurable upstream options does not grow too much, I think we can stick to explicitly configuring everything.
If it keeps growing, though... then yeah, a textarea for "additional options" might be useful to at least have support for options that this module hasn't implemented yet. Dunno.
The extra changes... I was waiting for a good time to do them. This patch seems a good time. (I doesn't add a lot of logic so I doubt it will be a burden to code readers.) So thanks!
(Rebased and added one code comment in the config screen.)
Comment #5
roderik(vanity tagging - awesome sprint room here; more people should come.)