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.

Sign up for GitHub

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

Jump to bottom

csrfmiddlewaretoken send but not used #1581

Open
bwbroersma opened this issue Dec 4, 2024 · 2 comments · May be fixed by #1582
Open

csrfmiddlewaretoken send but not used #1581

bwbroersma opened this issue Dec 4, 2024 · 2 comments · May be fixed by #1582

Comments

@bwbroersma
Copy link
Collaborator

bwbroersma commented Dec 4, 2024 •
edited
Loading

The CSRF token is used:

Internet.nl/interface/templates/base.html

Line 59 in ca80957

{% csrf_token %}

However "django.middleware.csrf.CsrfViewMiddleware" is missing:

Internet.nl/internetnl/settings.py

Lines 175 to 187 in ca80957

MIDDLEWARE = [
# serves static files in production
"whitenoise.middleware.WhiteNoiseMiddleware",
"django_statsd.middleware.GraphiteRequestTimingMiddleware",
"django_statsd.middleware.GraphiteMiddleware",
"django_hosts.middleware.HostsRequestMiddleware",
"django.contrib.sessions.middleware.SessionMiddleware",
"django.middleware.common.CommonMiddleware",
"django.contrib.auth.middleware.AuthenticationMiddleware",
"django.contrib.messages.middleware.MessageMiddleware",
"django_hosts.middleware.HostsResponseMiddleware",
"internetnl.custom_middlewares.ActivateTranslationMiddleware",
]

This is needed according to step 1 of the documentation.

However for /change_language/ case, where the result is only a Location redirect, I don't see any CRSF possibility.
bwbroersma added a commit to bwbroersma/Internet.nl that referenced this issue Dec 4, 2024
@bwbroersma
Remove not needed CSRF token
86480ae 
For `/change_language/` the result is only a Location redirect (no session/cookie state), therefore there is no need for a CRSF token.
Fix internetstandards#1581.

Signed-off-by: Benjamin W. Broersma <[email protected]>
@bwbroersma bwbroersma linked a pull request Dec 4, 2024 that will close this issue
Open
@mxsasha
Copy link
Collaborator

mxsasha commented Dec 5, 2024

This has me wondering what we use sessions for. Because if we use sessions, is there maybe something worth protecting with CSRF?
As we have no logins and no private data behind them, the only CSRF case I can think of is triggering someone's browser to run a test. Which the "attacker" already could anyways. We'd just attribute it to the wrong client - information which we barely process anyways.

@bwbroersma
Copy link
Collaborator Author

A text search on session makes me think it's not in use. I would say let's remove it and check if the CI tests is still okay. Would be great if another dependency can be dropped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove not needed CSRF token bwbroersma/Internet.nl
2 participants
@mxsasha @bwbroersma