Skip to content

Conversation

@lvanbuiten
Copy link

Pull request containing changes to make this app/module Content Security Policy(CSP) compliant, by removing all inline styling and scripting by moving it to separate (static) CSS/JavaScript files.
The onclick and onsubmit events are added in the JavaScript files upon document ready hook.
Dynamic content for scripting in mostly passed through by json_script, with one exception for MFA.html which is done by data-attributes.

I've thoroughly tested FIDO2, TOTP and Recovery Codes since I have everything setup for that. For the other methods I stepped through the flows and compared it against the original example app and noticed no differences.

PR is to branch v3.1 since this included changes of #53 (delete key).

Credits: Based the inspiration and some of the techniques of AndreasDickow

Lex van Buiten added 4 commits April 18, 2025 10:57
+ fix reference to `sb-admin.min.css` in `logout.html`
Fix HTML inline `onclick` events and `onsubmit` for forms
@mkalioby
Copy link
Owner

Thanks, and welcome to the Contributors.

I'll test it soon and get back to you.

@lvanbuiten
Copy link
Author

Hey @mkalioby, did you perhaps got any time to test the changes? If you need anything, just let me know! ;)

@mkalioby
Copy link
Owner

I didn't have time so far, I'll try my best during the weekend.

@mkalioby
Copy link
Owner

mkalioby commented May 2, 2025

@lvanbuiten Please excuse my ignorance.

I think we shall do something in example code, so we show the users how to enforce CSP. As for me, I don't really know what so I change in my project to enforce CSP and make sure it really works.

Thanks for the great work.

@lvanbuiten
Copy link
Author

@mkalioby No worries at all! It's neither my specialty, but something we have enabled in our django application and therefore would like contribute in order to not lower our enforcement.

I'll try my best and with my best knowledge explain CSP and the possible impact of django-mfa. Additionally I can try and modify the example code to enforce CSP, but maybe your view changes with the info below.

CSP explained

Content Security Policy (CSP) is a feature that helps to prevent or minimize the risk of certain types of security threats. It consists of a series of instructions from a website to a browser, which instruct the browser to place restrictions on the things that the code comprising the site is allowed to do. Content Security Policy(CSP)

With CSP rules in place, depending on the configuration of course, you are not allowed to serve inline CSS and Javascript within HTML files. The reason is, that someone could perform a man-in-the-middle attack and serve malicious content. The browser can validate the origin with CSS and Javascript files and match them against the provide CSP rules. If the origin doesn't match any of the rules, the browser will block the content and therefor doesn't serve it to the client's browser.

Impact for django-mfa (Usage)

In general there doesn't changes anything for the usage of django-mfa. The code shouldn't impact any of it functionalities or look-and-feel of the views. In short I only moved the code to the 'correct' places and used json_script to solve any dynamic variable/content usages in scripts.

The owner of the django application, so like me, chooses to enable and enforce CSP rules. We use Django-CSP, current version 3.8, to archive that goal.

With the following changes you can, in theory, enable CSP in your application.
In requirements.txt add django-csp~=3.8
In the settings module:

INSTALLED_APPS = (
    # ...
    "csp",
    # ...
)

MIDDLEWARE = (
    # ...
    "csp.middleware.CSPMiddleware",
    # ...
)

CSP = ("'self'", f'*.{DNS_NAME}')
CSP_DEFAULT_SRC = CSP
CSP_IMG_SRC = CSP + ('data:', 'blob:')
CSP_FONT_SRC = CSP + ('data:',)
CSP_SCRIPT_SRC = CSP
CSP_STYLE_SRC = CSP
CSP_CONNECT_SRC = CSP
CSP_FORM_ACTION = CSP

When violating these rules, with django-mfa2==3.1b1 your browser will output errors something like these:
image
This is the FIDO2 auth flow, where it doesn't ask to touch the hardware key since the code isn't supplied to the client.

Impact for django-mfa (Maintenance / Contribution)

For contributing or maintaining django-mfa it requires somewhat more awareness to prevent braking and/or introducing new CSP-rule violations in the (near) future.

@lvanbuiten
Copy link
Author

Hey @mkalioby,

Did you perhaps had any chance to proceed testing the changes or can i help you in any way?

@mkalioby
Copy link
Owner

Hello @lvanbuiten

I'm still working on it.

@mkalioby
Copy link
Owner

Thanks @lvanbuiten , all good.
Moved to #97 and RC1 is released. Please check.

@mkalioby mkalioby closed this May 16, 2025
@mkalioby
Copy link
Owner

@lvanbuiten
Copy link
Author

LGTM!

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