-
Notifications
You must be signed in to change notification settings - Fork 270
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
Change settle endpoint to use POST instead of GET #1303
base: master
Are you sure you want to change the base?
Conversation
This is necessary to avoid XSS. State-changing actions should never be done with a GET.
@TomRoussel FYI, if you are interested to review. There were two security issues:
Tests are failing on purpose because I added a test for this second issue. I'm not yet sure how to fix it, I would prefer if we reuse existing checks (they are done in You should not feel bad about it, we're quite happy to finally have this feature thanks to your dedication. Let's fix this together :) |
Sure, I'm happy to help get this in a better state! I'll have a look at this later this week. Intuitively, it feels like it should be possible to think of something that reuses the existing checks for this usecase as well. There's no scheme that immediately pops in my head, but to be honest my experience with WTForms is limited to this project. |
No comments on the proposed changes. About the second issue: could we create a |
This is necessary to avoid XSS. State-changing actions should never be done with a GET.