Conversation
| if not SESSION_COOKIE_SECURE: | ||
| # Needed on dev | ||
| SAML_SESSION_COOKIE_SAMESITE = 'Lax' |
There was a problem hiding this comment.
So this is the fix about forgetting sessions I talked about.
If this is not here, the SAML library doesn't set it's session cookie in local dev environments. (Because by default this requires the session-cookie secure flag to be set).
I would recommend apps to set something similar
|
Oh, btw, the new version of DjangoSAML2 is compatible with Django CSP < 4 as well. So the mandatory upgrade should not break apps with a < 4 constraint |
miggol
left a comment
There was a problem hiding this comment.
Requirements borked as usual
Otherwise looks good to me! Sensible defaults
dev/requirements.txt
Outdated
| build==1.2.2.post1 | ||
| # via pip-tools | ||
| certifi==2025.1.31 | ||
| cdh-django-core[all,core,dev,docs,federated-auth,files,mail,recommended,rest,vue] @ file:///home/ty/projects/web/django-shared-core |
There was a problem hiding this comment.
--find-links file:///../. cdh-django-core[all,core,dev,docs,federated-auth,files,mail,recommended,rest,vue]
Works for me here. But it's still that annoying there's no way to get this generated from requirements.in
There was a problem hiding this comment.
I've removed the offending line; it isn't required anyways...
The problem with the absolute path seems to stem from some internal pip logic. I know poetry can actually deal with this, but switching to that might be a bit extreme...
As promised, I looked into the possible Django-CSP and DjangoSAML2 issues. Turns out, nothing is broken!
So this PR is limited to a few minor changes related to all this: