Skip to content

Conversation

BrennanPaciorek
Copy link
Contributor

Description

  • What is being changed? Modify ansible_base.authentication.session.SessionAuthentication to temporarily patch CSRF_TRUSTED_ORIGINS with the setting retrieved from ansible_base.lib.utils.settings.get_setting
  • Why is this change needed? To support dynamic definitions of the CSRF_TRUSTED_ORIGIN setting, provided the correct ANSIBLE_BASE_SETTINGS_FUNCTION
  • How does this change address the issue? By having SessionAuthentication temporarily patch CSRF_TRUSTED_ORIGINS for the duration of csrf enforcement

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Test update
  • Refactoring (no functional changes)
  • Development environment change
  • Configuration change

Self-Review Checklist

  • I have performed a self-review of my code
  • I have added relevant comments to complex code sections
  • I have updated documentation where needed
  • I have considered the security impact of these changes
  • I have considered performance implications
  • I have thought about error handling and edge cases
  • I have tested the changes in my local environment

Testing Instructions

Prerequisites

Run your django app with one worker for simplicity (we cannot guarantee this in testing situations, so this is only for manual verification)

Steps to Test

  1. Modify your ANSIBLE_BASE_SETTINGS_FUNCTION to return a different CSRF_TRUSTED_ORIGINS
  2. Make a curl POST request, changing your Origin header to match an element of your alternate CSRF_TRUSTED_ORIGINS

Expected Results

  • CSRF_TRUSTED_ORIGINS django setting remains unchanged
  • CSRF origin checking does not fail

TheRealHaoLiu and others added 2 commits July 7, 2025 18:20
… use get_setting

- Add AnsibleBaseCsrfViewMiddleware that reads CSRF_TRUSTED_ORIGINS using
  ansible_base.lib.utils.settings.get_setting instead of directly from Django settings
- Override all three cached properties (csrf_trusted_origins_hosts, allowed_origins_exact,
  allowed_origin_subdomains) to use get_setting for dynamic configuration
- Add AnsibleBaseCSRFCheck class that inherits from AnsibleBaseCsrfViewMiddleware
- Modify SessionAuthentication.enforce_csrf to use AnsibleBaseCSRFCheck instead of
  Django's default CSRFCheck
- Add comprehensive tests for both middleware and session authentication CSRF functionality
- Enables CSRF_TRUSTED_ORIGINS to be dynamically loaded from various sources via
  ANSIBLE_BASE_SETTINGS_FUNCTION while maintaining backward compatibility
@BrennanPaciorek
Copy link
Contributor Author

I'll see how SonarCloud handles this one before I add some testing.

Copy link
Member

@john-westcott-iv john-westcott-iv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels so much cleaner.

@AlanCoding
Copy link
Member

Is this a rework of #742? Ping @TheRealHaoLiu

Copy link

github-actions bot commented Jul 8, 2025

DVCS PR Check Results:

PR appears valid (JIRA key(s) found)

Copy link

sonarqubecloud bot commented Jul 8, 2025

@john-westcott-iv john-westcott-iv merged commit 2c18ec8 into ansible:devel Jul 8, 2025
13 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.

4 participants