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

Csrf support #411

Closed
wants to merge 1 commit into from
Closed

Csrf support #411

wants to merge 1 commit into from

Conversation

jonmchan
Copy link
Contributor

Adds CSRF Token to ApiDoc so that we can submit forms that have CSRF protection. This PR checks if CSRF is enabled, and if it, generates the proper CSRF token.

Requires #408 to be fixed and merged first.

@jonmchan
Copy link
Contributor Author

@willdurand - this one is ready too. I rebased it like you probably would have asked me. Now ApiDoc will work for CSRF protected forms!

Added CSRF support to ApiDoc for Form Endpoints

Adding displaying default value in Markdown

updating tests to work with new default value

fixing failing tests by disabling CSRF for regular tests

created tests for CSRF token provider
@willdurand
Copy link
Collaborator

I don't think it is a correct/expected behavior. Think about the sandbox as a client for your API. CSRF token, as is, can't work with APIs as it must be stateless.

@jonmchan
Copy link
Contributor Author

@willdurand - valid point, but would you consider the other use case where someone wants to use ApiDoc to rapidly test endpoints and don't want to navigate to all the different form pages to fill them all out? During an application I was building, QA did exactly this and used the ApiDoc page to rapidly test all the endpoints. Being forced to disable CSRF for testing seemed unnecessary.

My rationale behind arguing for this is that this completes Form type support. Also, for those who have CSRF disabled, nothing would be added anyways.

@jonmchan
Copy link
Contributor Author

jonmchan commented Aug 7, 2014

@willdurand ping? any opinion?

@jaytaph
Copy link
Contributor

jaytaph commented Sep 13, 2014

I have solved this by using an additional FormTypeExtension and is based on the FOSRestBundle version (https://github.com/FriendsOfSymfony/FOSRestBundle/blob/master/Form/Extension/DisableCSRFExtension.php)

Basically what it needs to do is check against whatever you want to check to see if you are creating a "normal" form which needs csrf-protection, or an form for API purposes, which doesn't need this.

if it finds that you are creating the form in the "API" context, it will just set csrf_protection option to false, which clears any csrf-additions that are added. When validating, there is nothing wrong, as the normal csrf protection doesn't kick in.

@jonmchan
Copy link
Contributor Author

@jaytaph - that's nice - but it still doesn't address the other use case that I am talking about in using ApiDocBundle as a rapid test interface where you can access all the forms without having to actually navigate and work with the real forms. It allows you to isolate and test thee functionality instead of having to go through the entire flow.

@willdurand
Copy link
Collaborator

Closing it for now as the master branch has changed a lot.. Feel free to re-submit this PR, sorry.

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.

None yet

3 participants