Integrate SAML2 basic authentication - uses pysaml2

[muthusuba]

a) The actual call to the pysaml2 for auth/verification calls xmlsec binary (it seems). So, some optimizations around it might be good.
b) Tested only with Redirect based SAML2 auth
c) It requires the following (example) configuration in homeserver.yaml
saml2_config:
config_path: "/home/.../sp_conf.py"
idp_redirect_url: "http://..../idp"
d) sp_conf.py is something like: https://github.com/rohe/pysaml2/blob/master/example/sp-repoze/sp_conf.py.example
https://pythonhosted.org/pysaml2/howto/config.html

[matrixbot]

Can one of the admins verify this patch?

[erikjohnston]

matrixbot: ok to test

[erikjohnston]

I would prefer if saml2 was disabled by default, since not everyone is going to have saml. One way to do this would be to include the commented out config. It would also be nice to include some comments about what the config is for and links to documentation.

[erikjohnston]

Thanks for the pull request!

There are a few style warnings that should be easy to fix:

synapse/rest/client/v1/login.py:23:1: F401 'cgi' imported but unused
synapse/rest/client/v1/login.py:27:1: F401 'BINDING_HTTP_REDIRECT' imported but unused
synapse/rest/client/v1/login.py:29:1: F401 'create_metadata_string' imported but unused
synapse/rest/client/v1/login.py:32:1: F401 'ServiceError' imported but unused
synapse/rest/client/v1/login.py:33:1: F401 'Extensions' imported but unused
synapse/rest/client/v1/login.py:34:1: F401 'SPCertEnc' imported but unused
synapse/rest/client/v1/login.py:35:1: F401 'rndstr' imported but unused
synapse/rest/client/v1/login.py:140:29: E126 continuation line over-indented for hanging indent

Would it also make sense to allow/support multiple saml backends?

[erikjohnston]

We should only be advertising saml flows if we have saml enabled.

[muthusuba]

Thank you for the quick review!

a) Made saml2 optional
b) I thought I ran pep8 on login.py :( the style warnings should hopefully be fixed
c) logger moved to top of the file.

[erikjohnston]

Thanks, it looks good. not sure why jenkins is complaining though, it doesn't fail pep8 on my box.

matrixbot: test this please.

[erikjohnston]

Oh, that's because @ara4n broke the tests on develop.

[erikjohnston]

Ooops, I've reverted this PR since I've realised this hasn't been "signed off"

Could you just confirm you agree to license this code as per https://github.com/matrix-org/synapse/blob/master/CONTRIBUTING.rst#sign-off ? (And if you could also open another PR for this branch that would be great).

[muthusuba]

Should I sign-off every commit or would it be ok to add one on the top?

[erikjohnston]

One at the top is fine :)

On Thu, 9 Jul 2015 18:22 Muthu Subramanian notifications@github.com wrote:

Should I sign-off every commit or would it be ok to add one on the top?

—
Reply to this email directly or view it on GitHub
#200 (comment).

[muthusuba]

I just realized that https://github.com/matrix-org/synapse/blob/master/CONTRIBUTING.rst#sign-off says even a comment in the pull request is fine (?) So, here it is:

Signed-off-by: Muthu Subramanian muthu.subramanian.karunanidhi@ericsson.com

[muthusuba]

Resubmitted: #201

[ptman]

some documentation for saml support would be great