fix(security): enforce CSRF protection on REST API endpoints#1885
Open
snowbird91 wants to merge 1 commit intotjcsl:devfrom
Open
fix(security): enforce CSRF protection on REST API endpoints#1885snowbird91 wants to merge 1 commit intotjcsl:devfrom
snowbird91 wants to merge 1 commit intotjcsl:devfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a CSRF vulnerability in the REST API by removing a custom session authentication class that disabled CSRF checks and switching to DRF’s built-in SessionAuthentication, which enforces CSRF validation for session-authenticated requests.
Changes:
- Removed
CsrfExemptSessionAuthentication(which made CSRF enforcement a no-op) from the API authentication module. - Updated DRF
DEFAULT_AUTHENTICATION_CLASSESto userest_framework.authentication.SessionAuthenticationinstead of the CSRF-exempt custom class.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| intranet/settings/init.py | Switches DRF default session auth to built-in SessionAuthentication to enforce CSRF on session-based API requests. |
| intranet/apps/api/authentication.py | Removes the CSRF-exempt session authentication implementation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
616
to
620
| "DEFAULT_AUTHENTICATION_CLASSES": ( | ||
| # "intranet.apps.api.authentication.ApiBasicAuthentication", # Disabled for security | ||
| "intranet.apps.api.authentication.CsrfExemptSessionAuthentication", # exempts CSRF checking on API | ||
| "rest_framework.authentication.SessionAuthentication", | ||
| "oauth2_provider.contrib.rest_framework.OAuth2Authentication", | ||
| ), |
snowbird91
force-pushed
the
fix/enforce-csrf-on-api
branch
from
8c349a7 to
e5a1584
Compare
snowbird91
force-pushed
the
fix/enforce-csrf-on-api
branch
from
e5a1584 to
1367e11
Compare
|
coverage: 79.214% (-0.05%) from 79.264% — snowbird91:fix/enforce-csrf-on-api into tjcsl:dev |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
remove CsrfExemptSessionAuthentication which disabled CSRF checking for all session-auth API requests. replaced with DRF's built in SessionAuthentication which enforces CSRF validation.
Proposed changes
Brief description of rationale
CsrfExemptSessionAuthentication overrides enforce_csrf() to be a no-op, meaning any session-authenticated API request bypasses CSRF protection. a malicious webpage could make API calls on behalf of a logged-in user by exploiting their session cookie. replacing with DRF's built in SessionAuthentication enforces CSRF validation which fixes this vulnerability.