-
Notifications
You must be signed in to change notification settings - Fork 144
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
Make sure djangosaml2 works in csp-enabled applications too (fix #391) #392
Merged
peppelinux
merged 7 commits into
IdentityPython:master
from
prauscher:issue-391-add-csp
Dec 27, 2023
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
93b7c09
Make sure djangosaml2 works in csp-enabled applications too (fix #391)
prauscher fa6ea1b
Merge branch 'IdentityPython:master' into issue-391-add-csp
prauscher decef2f
Bump version to 1.9.0
prauscher 8f545f1
add draft for security considerations
prauscher 933ab51
Update docs/source/contents/security.md
prauscher 0b33457
Update docs/source/contents/security.md
prauscher 1ae2905
Add warning and documentation iff django-csp can not be found
prauscher File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -78,6 +78,26 @@ | |||||||||||
|
||||||||||||
logger = logging.getLogger("djangosaml2") | ||||||||||||
|
||||||||||||
# Update Content-Security-Policy headers for POST-Bindings | ||||||||||||
try: | ||||||||||||
from csp.decorators import csp_update | ||||||||||||
except ModuleNotFoundError: | ||||||||||||
# If csp is not installed, do not update fields as Content-Security-Policy | ||||||||||||
# is not used | ||||||||||||
Comment on lines
+85
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
def saml2_csp_update(view): | ||||||||||||
return view | ||||||||||||
|
||||||||||||
logger.warning("django-csp could not be found, not updating Content-Security-Policy. Please " | ||||||||||||
"make sure CSP is configured at least by httpd or setup django-csp. See " | ||||||||||||
"https://djangosaml2.readthedocs.io/contents/security.html#content-security-policy" | ||||||||||||
" for more information") | ||||||||||||
else: | ||||||||||||
# script-src 'unsafe-inline' to autosubmit forms, | ||||||||||||
# form-action https: to send data to IdPs | ||||||||||||
saml2_csp_update = csp_update( | ||||||||||||
SCRIPT_SRC=["'unsafe-inline'"], FORM_ACTION=["https:"] | ||||||||||||
) | ||||||||||||
|
||||||||||||
|
||||||||||||
def _set_subject_id(session, subject_id): | ||||||||||||
session["_saml2_subject_id"] = code(subject_id) | ||||||||||||
|
@@ -123,6 +143,7 @@ def get_state_client(self, request: HttpRequest): | |||||||||||
return state, client | ||||||||||||
|
||||||||||||
|
||||||||||||
@method_decorator(saml2_csp_update, name='dispatch') | ||||||||||||
class LoginView(SPConfigMixin, View): | ||||||||||||
"""SAML Authorization Request initiator. | ||||||||||||
|
||||||||||||
|
@@ -671,6 +692,7 @@ def get(self, request, *args, **kwargs): | |||||||||||
) | ||||||||||||
|
||||||||||||
|
||||||||||||
@method_decorator(saml2_csp_update, name='dispatch') | ||||||||||||
class LogoutInitView(LoginRequiredMixin, SPConfigMixin, View): | ||||||||||||
"""SAML Logout Request initiator | ||||||||||||
|
||||||||||||
|
@@ -749,7 +771,7 @@ def handle_unsupported_slo_exception(self, request, exception, *args, **kwargs): | |||||||||||
return HttpResponseRedirect(getattr(settings, "LOGOUT_REDIRECT_URL", "/")) | ||||||||||||
|
||||||||||||
|
||||||||||||
@method_decorator(csrf_exempt, name="dispatch") | ||||||||||||
@method_decorator([saml2_csp_update, csrf_exempt], name="dispatch") | ||||||||||||
class LogoutView(SPConfigMixin, View): | ||||||||||||
"""SAML Logout Response endpoint | ||||||||||||
|
||||||||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
Introduction | ||
============ | ||
|
||
Authentication and Authorization are quite security relevant topics on its own. | ||
Make sure you understand SAML2 and its implications, specifically the | ||
separation of duties between Service Provider (SP) and Identity Provider (IdP): | ||
this library aims to support a Service Provider in getting authenticated with | ||
with one or more Identity Provider. | ||
|
||
Communication between SP and IdP is routed via the Browser, eliminating the | ||
need for direct communication between SP and IdP. However, for security the use | ||
of cryptographic signatures (both while sending and receiving messages) must be | ||
examined and the private keys in use must be kept closely guarded. | ||
|
||
Content Security Policy | ||
======================= | ||
|
||
When using POST-Bindings, the Browser is presented with a small HTML-Form for | ||
every redirect (both Login and Logout), which is sent using JavaScript and | ||
sends the Data to the selected IdP. If your application uses technices such as | ||
Content Security Policy, this might affect the calls. Since Version 1.9.0 | ||
djangosaml2 will detect if django-csp is installed and update the Content | ||
Security Policy accordingly. | ||
|
||
[Content Security Policy](https://content-security-policy.com/) is an important | ||
HTTP-Extension to prevent User Input or other harmful sources from manipulating | ||
application data. Usage is strongly advised, see | ||
[OWASP Control](https://owasp.org/www-community/controls/Content_Security_Policy). | ||
|
||
To enable CSP with [django-csp](https://django-csp.readthedocs.io/), simply | ||
follow their [installation](https://django-csp.readthedocs.io/en/latest/installation.html) | ||
and [configuration](https://django-csp.readthedocs.io/en/latest/configuration.html) | ||
guides: djangosaml2 will automatically blend in and update the headers for | ||
POST-bindings, so you must not include exceptions for djangosaml2 in your | ||
global configuration. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add a
logger.warning
here. It would be appreciated to make aware the deployers that they may haev security risks (if CSP is not configured at least in the httpd)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I polished up the security page and added a warning, please check if I am missing important information :)