Skip to content

Conversation

@rhythmdesai404
Copy link

  1. Enabled CSRF Protection

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 22, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@rhythmdesai404
Copy link
Author

/easycla

1 similar comment
@rhythmdesai404
Copy link
Author

/easycla

@ericmustin ericmustin changed the base branch from main to add_defensiveness_on_callables October 22, 2024 21:22
@ericmustin ericmustin changed the base branch from add_defensiveness_on_callables to main October 22, 2024 21:22
@ericmustin
Copy link
Contributor

Misclick, sorry about that. Is this PR ready for review?

Comment on lines +10 to +11
# Enable CSRF Protection
protect_from_forgery with: :exception
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @rhythmdesai404! Since I haven't heard back from you after Eric's comment earlier, I'm assuming this is ready for review.

I'm curious about why we need to turn this on. This is test code, and shouldn't impact the user. Why do we need to enable it here?

Copy link
Author

Choose a reason for hiding this comment

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

Hi @kaylareopelle! Great question. Even though it's test code, enabling CSRF protection is important because it helps ensure that the tests accurately reflect the security measures in place for the actual application. By turning it on, we:
Simulate Real Conditions: Make sure the test environment mimics the real application environment, including its security features.
Consistency: Maintain consistency in how the application handles requests, whether in testing or in production.
In short, enabling CSRF protection in test code helps ensure that your application is secure and behaves as expected in all scenarios.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the context, @rhythmdesai404! That makes sense to me. If you'd still like to see this change, please reopen the PR and we'll give it a look.

@github-actions
Copy link
Contributor

👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this pull request will be closed eventually by the stale bot

@github-actions github-actions bot added the stale Marks an issue/PR stale label Nov 29, 2024
@github-actions github-actions bot closed this Dec 29, 2024
Copy link
Author

@rhythmdesai404 rhythmdesai404 left a comment

Choose a reason for hiding this comment

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

Enable CSRF protection to ensure test environment security

This change simulates real application conditions, catches potential issues early,

and maintains consistency in request handling across all environments.

protect_from_forgery with: :exception

Comment on lines +10 to +11
# Enable CSRF Protection
protect_from_forgery with: :exception
Copy link
Author

Choose a reason for hiding this comment

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

Hi @kaylareopelle! Great question. Even though it's test code, enabling CSRF protection is important because it helps ensure that the tests accurately reflect the security measures in place for the actual application. By turning it on, we:
Simulate Real Conditions: Make sure the test environment mimics the real application environment, including its security features.
Consistency: Maintain consistency in how the application handles requests, whether in testing or in production.
In short, enabling CSRF protection in test code helps ensure that your application is secure and behaves as expected in all scenarios.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale Marks an issue/PR stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants