Skip to content

Conversation

@astoltz
Copy link

@astoltz astoltz commented Apr 6, 2016

The changes in 8c6047c broke SAML login under UWSGI. Specifically, the SAML session ID wasn't being saved, therefore a session cookie was not created. This prevented the verification of the SAMLResponse from succeeding.

@knaperek
Copy link
Collaborator

knaperek commented Apr 6, 2016

Any possible explanation of this? To me it seems it should work just fine according to the docs...

@astoltz
Copy link
Author

astoltz commented Apr 6, 2016

I'm not sure if UWSGI has anything to do with it. I thought it noteworthy because I couldn't reproduce the issue using runserver.

What I was actually seeing when tracking down the issue was the values from OutstandingQueriesCache.set() weren't getting passed to request.session. I'm not sure where this is supposed to be happening outside of DjangoSessionCacheAdapter.sync().

@astoltz
Copy link
Author

astoltz commented Apr 6, 2016

I should also mention I'm using Django 1.9.4.

@knaperek
Copy link
Collaborator

knaperek commented Apr 6, 2016

And with this commit reverted everything is working alright? What kind of Session Backend are you using?
It would be great if you managed to reproduce it and identify the real issue.

@astoltz
Copy link
Author

astoltz commented Apr 6, 2016

Yes, with the sync to session commands added back in, I can successfully authenticate to a SAML IDP. The session backend is the default Django database handler.

@astoltz
Copy link
Author

astoltz commented Apr 6, 2016

I think I need to keep digging. SAMLLogout isn't working for me now.

saml2.saml.NameID object at 0x7fb894098e10 is not JSON serializable

@astoltz
Copy link
Author

astoltz commented Apr 6, 2016

I was able to reproduce this using runserver.

Ubuntu 14.04LTS on Digital Ocean.

apt-get update -y
apt-get dist-upgrade -y
apt-get autoremove -y
reboot
apt-get install -y php5 python-dev python-virtualenv libffi-dev libssl-dev xmlsec1

cd `mktemp -d`
wget 'https://simplesamlphp.org/res/downloads/simplesamlphp-1.14.2.tar.gz'
tar -xzf simplesamlphp-1.14.2.tar.gz
mv simplesamlphp-1.14.2 /var/www/html/simplesaml/

vi /var/www/html/simplesaml/config/config.php
# Set secretsalt, auth.adminpassword, and enable.saml20-idp

cd /var/www/html/simplesaml/cert
openssl req -newkey rsa:2048 -new -x509 -days 3652 -nodes -out server.crt -keyout server.pem

vi /etc/apache2/sites-enabled/000-default.conf
# Alias /simplesaml /var/www/html/simplesaml/www

mkdir -p /var/www/html/startserver
cd /var/www/html/startserver
virtualenv .
. bin/activate
pip install Django==1.9.5
pip install djangosaml2==0.14.4
django-admin startproject startserver
cd /var/www/html/startserver/startserver/startserver
openssl req -newkey rsa:2048 -new -x509 -days 3652 -nodes -out mycert.pem -keyout mycert.key
cd ..
# Follow djangosaml2 setup steps
./manage.py migrate
./manage.py runserver 0.0.0.0:8000

You can see the issue at http://45.55.223.123:8000/saml2/login/. I didn't finish getting the setup done since I was able to reproduce this again.

Username: student
Password: studentpass

SimpleSAMLPHP directory: http://45.55.223.123/simplesaml.tgz
StartServer directory: http://45.55.223.123/startserver.tgz

@knaperek
Copy link
Collaborator

knaperek commented Apr 6, 2016

Thanks, I'll try to find some time to check this until the end of the week. In the meantime, could you please check if it also happens with Django 1.8?

@knaperek
Copy link
Collaborator

knaperek commented Apr 6, 2016

Oh, just did a quick check and I see UnsolicitedResponse at /saml2/acs/ error. Could you enable IdP initiated login and check if it still happens?

'allow_unsolicited': True in the sp config.

@knaperek
Copy link
Collaborator

Closing this as it is already resolved in PR #10. Anyway, I'd still welcome if somebody managed to trace down the root cause, as I believe this fix should (theoretically) not be needed :-)

@knaperek knaperek closed this Apr 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants