NameIDFormat/NameIDPolicy Improvements

[bradenmacdonald]

Currently, python-saml has only one setting, "NameIDFormat" to control NameIDs. I have made a few changes:

A. For the SP, the NameIDFormat (string) setting is replaced with NameIDFormats (list), because the SAML spec says the NameIDFormat can be specified zero or more times (0,1,2,3,4, ...) and is intended to list all NameID formats accepted by the SP. As our own use case for SAML requires working with multiple IdPs, it is important that we can configure our metadata to list many accepted formats, or none at all. For backwards compatibility, the NameIDFormat (string) setting will still be accepted.
B. NameIDPolicyFormat and NameIDPolicyAllowCreate are two new settings to control the optional NameIDPolicy element in authn requests. If the old NameIDFormat setting is used, these will automatically be set to match. Otherwise, the new default is to not specify a NameIDPolicy at all, which is the default stated in the spec. I also put these policy settings in the idp settings, so that they can be changed per-provider for those working with multiple IdPs.

---

This is a contribution from edX, developed by OpenCraft as part of bringing integrated SAML support to the edX platform, via a new python-social-auth SAML backend.

[coveralls]

Coverage increased (+0.02%) to 97.92% when pulling 0583e37 on open-craft:edx2-nameid-changes into c44971e on onelogin:master.

[pitbulk]

@bradenmacdonald I intentionally implemented all the NameID stuff with this unique parameter at the SP.

I want to maintain OneLogin SAML toolkit simple, I really don't think that NameIDPolicyFormat and NameIDPolicyAllowCreate are really needed for the 95% of the IdPs, and selecting what NameID expect the SP with the NameIDFormat is ok.

I know that NameIDPolicy is optional, but some ADFS settings required it, so I added it.

[bradenmacdonald]

@pitbulk Ok, fair enough. We haven't actually needed the fancier NameID configuration yet, but it seemed like something we may need.

What do you think about doing one of these:

1. Keep the single NameIDFormat setting as it is now, but if it is not set, or set to None, then don't include any <NameIDFormat> in the metadata, and don't include any PolicyFormat/AllowCreate in the authnrequest.

or

2. Change the default from PERSISTENT to UNSPECIFIED (Persistent as a default causes problems with IdPs that don't provide persistent nameIDs - both of the IdPs I tested with did not work when PERSISTENT was used).

If you don't want to do either of those, I can still work around it but I will need to go back and make further changes to python-social-auth. Thanks for your help with everything so far.

[pitbulk]

Agree with 2). In fact, if you see the default settings of the demo, already you can find the
NAMEID_UNSPECIFIED as default:
https://github.com/onelogin/python-saml/blob/master/demo-flask/saml/settings.json#L14
https://github.com/onelogin/python-saml/blob/master/demo-django/saml/settings.json#L14

but let's change that

[pitbulk]

Done:
3a5847b

[bradenmacdonald]

That was fast! Thanks so much :)

Would you be able to release a new version of python-saml now? We'd love to switch to a versioned release on PyPI.

I'm going to test out that latest commit with my python-social-auth setup and IdPs, and if it's all working I'll just close this PR.

[pitbulk]

@bradenmacdonald I need to review if there are more issues/PRs that could be easily added, but I plan to do a new release today or tomorrow.

[pitbulk]

And sorry for the delay... maintain the php-saml, python-saml, ruby-saml and all the SAML plugins is not easy :(
Also I need to release the python3-saml toolkit soon...

[bradenmacdonald]

@pitbulk No worries, I understand! This is an open source project after all. We're just glad that you're making this software available and have been able to accept these contributions so we can all share the same codebase.

If you're able to do a new release today or tomorrow, that would be awesome, and we should be able to fit it in to our own upcoming release of edx-platform, which has a cutoff around the end of this week.

I have tested this with the new default, and it seems to work well with our IdPs so far. I'm going to close this PR. Cheers, and thanks again!

[pitbulk]

Nice to see how edx-platform keep growing.. I worked in a similar project long time ago:
https://github.com/OpenMOOC

[bradenmacdonald]

Ah, very cool :)