Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CSP handler setting #401

Merged

Conversation

GertBurger
Copy link
Contributor

@GertBurger GertBurger commented Mar 10, 2024

Fixes #397

This PR adds the SAML_CSP_HANDLER setting so that users can specify how CSP should be updated, if at all.
The default behaviour of using django-csp remains the same.

tests/testprofiles/tests.py Outdated Show resolved Hide resolved
@GertBurger
Copy link
Contributor Author

@peppelinux How do you feel about the overall approach taken?

@peppelinux
Copy link
Member

Good feature,

however this must be refactored: https://github.com/IdentityPython/djangosaml2/pull/401/files#diff-fcaad0de48c174c47b13c1ff9c42a101f319429573505b1120123d31bfe791c8R220

we have to reduce the nested if statements, I'll try to give some hints If time will permit, otherwise please do your refactoring proposals

@GertBurger
Copy link
Contributor Author

Not too sure what you meant by "nested if statements" as there was only a single level. I do agree that there was too much logic bundled together and have split it up a bit.

I hope it is somewhat inline with what you were thinking.

@GertBurger GertBurger marked this pull request as ready for review March 13, 2024 10:19
@peppelinux
Copy link
Member

sorry for the late @GertBurger

we can open an issue saying that a a further analysis for a code refactor can be made as well.

at the current state the unit tests fails with py3.8 while djangosaml2 needs the support until the official eol of it and the decision of the idpy dev team.

could you propose any workaround in this PR?

@GertBurger
Copy link
Contributor Author

ah yes, cache was only introduced in Python 3.9 but it is a very thin wrapper around lru_cache. I've switched to the latter.

@peppelinux peppelinux merged commit 6dfbff3 into IdentityPython:master Apr 30, 2024
16 checks passed
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.

Disable notice regarding django-csp
2 participants