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

Csrf hack #401

Merged
merged 7 commits into from
Nov 9, 2020
Merged

Csrf hack #401

merged 7 commits into from
Nov 9, 2020

Conversation

riatzukiza
Copy link

This pull request changes...

List expected changes

FUNCTIONALITY

Describe the functionality
Addresses Issue #

TO TEST

List the steps to test the PR

This pull request is ready to merge when...

  • Meets acceptance criteria for the issue
  • Code is reviewed by someone other than the original author
  • Code is tested
  • Experience is approved by UX
  • Meets a11y checklist
  • Meets Raft's Manual QASP Checklist🔒 (only applicable for PR to main HHS)
  • Documentation is updated
    • OpenAPI
    • Readme
    • Entity Relationship Diagram (ERD)

@riatzukiza riatzukiza marked this pull request as ready for review November 9, 2020 14:39
carltonsmith
carltonsmith reviewed Nov 9, 2020
View reviewed changes
tdrs-backend/tdpservice/users/api/authorization_check.py
@@ -26,11 +27,14 @@ def get(self, request, *args, **kwargs):
auth_params = {
"authenticated": True,
"user": serializer.data,
"csrf": csrf.get_token(request),
Copy link

Choose a reason for hiding this comment

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

Are we sure this is secure?

Copy link
Author

Choose a reason for hiding this comment

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

Should be, this value was already to be exposed to the client in a cookie. This wouldn't change the overall availability of this value from what it was before, though I'll be looking into it.

@riatzukiza
Attempting to fix
4931a11 
hack
@riatzukiza
Allow X-CSRFToken
567e570 
Fix header typo
@riatzukiza
update
e85f72f 
Only use cookie jar if not testing
@riatzukiza
All tests pass
cbe3ab6
@riatzukiza
Cleaned lint
4e7c430
@riatzukiza
Change how we get csrf token
64f6068
@riatzukiza
fix lint
987da47 
Rename import


rename import


rename


remove system environmet
carltonsmith
carltonsmith approved these changes Nov 9, 2020
View reviewed changes
Copy link

@carltonsmith carltonsmith left a comment

Choose a reason for hiding this comment

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

This is all set

@carltonsmith carltonsmith merged commit b80ac89 into 231-submit-form Nov 9, 2020
@andrew-jameson andrew-jameson deleted the csrf-hack branch July 27, 2021 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carltonsmith carltonsmith carltonsmith approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@riatzukiza @carltonsmith