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.

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

Add configuration to require POST only authentication requsts #493

Conversation

sultaniman
Copy link
Contributor

@sultaniman sultaniman commented Sep 1, 2023

Proposed changes

This PR introduces a new configuration and authentication behaviour when it is active and will require POST request during authentication

  • Configuration key is SOCIAL_AUTH_REQUIRE_POST: bool,
  • Error response if request method is not POST.

Types of changes

Please check the type of change your PR introduces:

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (PEP8, lint, formatting, renaming, etc)
  • Refactoring (no functional changes, no api changes)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Build related changes (build process, tests runner, etc)
  • Other (please describe):

Related issues and discussions

Checklist

Put an x in the boxes that apply. You can also fill these out after creating
the PR. If you're unsure about any of them, don't hesitate to ask. We're here to
help! This is simply a reminder of what we are going to look for before merging
your code.

  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works

@sultaniman sultaniman marked this pull request as ready for review September 1, 2023 15:16
@codecov
Copy link

codecov bot commented Sep 1, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.07% 🎉

Comparison is base (4b4b019) 93.99% compared to head (c180450) 94.07%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #493      +/-   ##
==========================================
+ Coverage   93.99%   94.07%   +0.07%     
==========================================
  Files          39       39              
  Lines        1116     1131      +15     
  Branches      129      133       +4     
==========================================
+ Hits         1049     1064      +15     
  Misses         43       43              
  Partials       24       24              
Flag Coverage Δ
unittests 94.07% <100.00%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
social_django/utils.py 97.56% <100.00%> (+0.78%) ⬆️
social_django/views.py 100.00% <100.00%> (ø)
tests/test_views.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nijel nijel linked an issue Sep 4, 2023 that may be closed by this pull request
@sultaniman sultaniman force-pushed the feature/require-post-only-auth-requests branch from 0d9f10b to 59f1e42 Compare September 6, 2023 09:08
@nijel nijel merged commit a5137ed into python-social-auth:master Sep 7, 2023
8 checks passed
@nijel
Copy link
Member

nijel commented Sep 7, 2023

Merged, thanks for your contribution!

@nijel nijel self-assigned this Sep 7, 2023
@sultaniman
Copy link
Contributor Author

sultaniman commented Sep 7, 2023

@nijel I will also create a PR to update Django specific configuration documentation.
Thanks for reviews.

@sultaniman sultaniman deleted the feature/require-post-only-auth-requests branch September 7, 2023 08:19
sultaniman added a commit to sultaniman/social-docs that referenced this pull request Sep 7, 2023
To reflect Django configuration changes related to python-social-auth/social-app-django#493 this PR adds notes about using `SOCIAL_AUTH_REQUIRE_POST` configuration
@sultaniman
Copy link
Contributor Author

@nijel I updated docs and here is the PR

nijel pushed a commit to python-social-auth/social-docs that referenced this pull request Sep 7, 2023
To reflect Django configuration changes related to python-social-auth/social-app-django#493 this PR adds notes about using `SOCIAL_AUTH_REQUIRE_POST` configuration
@nijel nijel linked an issue Sep 7, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Requiring POST method by default CSRF on login
2 participants