Missing X-CSRF-Token Header on AJAX POST Request with File Upload

[mvastola]

I'm trying to trace either some sort of conflict on my end (my app is rather complex, albeit not really JS-wise) or a bug in jquery-ujs which I have tentatively traced to the nonBlankFileInputs variable. (That is the bug seems to occur, if this if block is true: https://github.com/rails/jquery-ujs/blob/master/src/rails.js#L477)

Basically, I have a form that sends correct AJAX requests (correct meaning including the X-CSRF-Token header) as long as, and only as long as, its file input field left empty. As soon as a file input field is selected the header is no longer sent (and I think the request is also no longer handled via AJAX).

Some quirky things about this form that may be contributing to this bug are

• I use the simple_form gem instead of the default form_for tags.
• The form itself is loaded by AJAX due to user input, so it isn't present on the page at load time, in case that is causing some event not to bind to it.
• The form loads in a modal box javascript library (called bootbox) which has its own button design, so the way I ended up implementing the submit action is by placing a display: none submit button within the <form> tags and then triggering it with javascript from the bootbox code with $('div.bootbox div.bootbox-body').find('input[type=submit]').first().click();. This has worked perfectly so far though.

Any advice on how I can trace this problem further would be greatly appreciated.

[rafaelfranca]

It is the expected behavior. If the form has file inputs it will fallback to standard POST, so it will not need to send the header.

[mvastola]

But how does the csrf token from the meta tags get included then? And why am I  getting an invalid authenticity token error then?

[rafaelfranca]

The form usually include the authenticity token. Check if it is being included in the form.

[mvastola]

You mean as a hidden input? No. It isn't.

[rafaelfranca]

Yes, the hidden input. So that is why it is not working.

[mvastola]

Iiinteresting.. thanks a ton!

[mvastola]

@rafaelfranca so doing some more investigation into this, I seem to have figured out the source of the problem here.

I'm not sure if you'd like to reopen this issue or if I should make a new one (please advise), but this definitely seems to warrant further discussion/analysis.. Basically, this problem seems to be a case of both the action_view gem and the jquery-ujs gems each thinking that the other is handling the inclusion of the authenticity token when there is a file field in a form is submitted via AJAX. (See https://github.com/rails/rails/blob/621ed494f573c4e37c1f7d37cc8741cc4c502827/actionview/lib/action_view/helpers/form_tag_helper.rb#L20 on lines 20, 31 and 38 for further details.)

According to the source code comments there, a simple workaround would be to set config.action_view.embed_authenticity_token_in_remote_forms = true, but it appears this is intentionally disabled by default (and presumably delegated to this gem by the Rails team) to allow for the fragment caching of forms.

Might I propose solving this conflict by catching submit events for AJAX POST forms with file uploads and creating/updating a hidden authenticity_token input field (from the token in the meta tags) before letting the submission through?

If that route is not chosen, I really think there should at least be better warnings/documentation for this situation, since, AFAIK, there is currently none whatsoever, neither in Rails nor in this gem. (By "warnings", I mean that perhaps an error should be thrown in ActionView if a file_field in a form_for where :remote => true and !config.action_view.embed_authenticity_token_in_remote_forms. This raises the question though as to why this functionality can't just be implemented.)

Please let me know if I should also be creating a concurrent issue in the Rails repository.

[rafaelfranca]

I believe we should take in consideration rendering the hidden field if remote: true and file_field is present. Could you open a PR?

[mvastola]

I can certainly try my hand at it.. Should have one within a week (if not much sooner).

[mvastola]

Should this issue be reopened in the interim in case others have ideas/input?

[mvastola]

@rafaelfranca Wait, to clarify, when you say "rendering the hidden field", are you suggesting doing this at submit-time via JS in jquery-ujs (as I suggested) or inside ActionView?

[rafaelfranca]

Inside action view. It is the best place to know about this case.

[mvastola]

@rafaelfranca I have created issue https://github.com/rails/rails/issues/22807 in the rails repo to ensure that whatever PR I write, the repo owner is prepared to merge it.

Please comment there and share your perspective.

[mvastola]

@rafaelfranca

rails/rails#22807 doesn't have any replies yet, unfortunately, so I'm tempted to just submit a PR to this repo to implement the fix the way I mentioned above.

I know you said you think the fix belongs in actionview, but I'm wary of doing that since any implementation there would break fragment caching.. Is there any reason why you don't think the fix belongs in this gem?

[mvastola]

Workaround

FYI, this doesn't resolve this issue, but one workaround I've discovered is to include a javascript plugin that enables support for POST XHR requests with file inputs.

For example, if the following javascripts are required (in order):

• jquery
• jquery-ui
• jquery-ujs
• https://github.com/blueimp/jQuery-File-Upload

And then something the following code is run at page load:

$(document).on('ajax:aborted:file', 'form', function(event, nonBlankFileInputs) {
  $(this).fileupload();
  $(this).fileupload('send', {fileInput: nonBlankFileInputs});
  return false;
});

All POST requests marked as "remote" will be submitted via XHR requests, even if they have file inputs.

[lucaspiller]

It is the expected behavior. If the form has file inputs it will fallback to standard POST

@rafaelfranca Maybe this behaviour should be revisited as the latest browsers support HTML5 APIs to perform file uploads via AJAX, with XHR2 it even supports upload progress without any server-side changes. For non-supported browsers it can fallback to a POST as it does currently.

[aguynamedben]

Wow, I just ran into this, and it was painful to track down.

To me, it seems like jquery-ujs is responsible for all the JavaScript logic of remote: true, so before submitting the form, if the form has a file input, it should fetch the CSRF value from the <%= csrf_meta_tags %>, dynamically create a hidden element for the CSRF token on the form on-the-fly, and then submit the form.

This would necessatate changes to documentation to indicate that the security improvements of per-form CSRF tokens do not apply to forms with file inputs:

If you set config.action_controller.per_form_csrf_tokens = true, per-form CSRF tokens will be created for increased security, except in the case that a form inlcudes a file uploads (file_field) and remote: true. In that case, the page-level CSRF token will be used instead. This is due to limitations in jquery-ujs' ability to submit form with file inputs via JavaScript.

It seems like this approach:

• Still allows for fragment caching of forms (the hidden form element is created dynamically)
• Allows per-form CSRF tokens in most cases, with one exceptional case which is documented
• Keeps all the nuances of remote: true form submission in jquery-ujs

[aguynamedben]

Another workaround

An easy workaround is to add authenticity_token: true to your form_for:

<%= form_for @user, remote: true, authenticity_token: true do |f| %>
  <%= f.file_field :image %>
<% end %>

That workaround is easy if you're not fragment caching forms, and ignores the documentation's statement that you should only use the :authenticity_token to customize or hide the field:

Use only if you need to pass custom authenticity token string, or to not add authenticity_token field at all (by passing false).

Perhaps another way to "fix" this is to always include the authenticity token, and to instruct users who care about fragment caching to use authenticity_token: false on form_for. They're already having to perform nuanced coding around where to place cache statements, it seems reasonable that they should also have to remove CSRF tokens before caching.

[aguynamedben]

After reading this again very carefully, I see that @mvastola and mine's proposed solution for jquery-ujs is the exact same. 👏 @mvastola

[rafaelfranca]

I like the solution proposed here https://github.com/rails/jquery-ujs/issues/451#issuecomment-258547646. Could you work on it?

[mvastola]

Considering this solution is (as @aguynamedben noted) identical to my original suggestion, it will hopefully come as a shock to no one that I concur with it. :smile:

@aguynamedben,

If you'd be willing to do this, that would be great. (Free time is hard to come by these days.) I was hoping to do this way back when I submitted it, but I was confused by @rafaelfranca's suggestion that the change be made within action_view and then I forgot about this when I didn't get clarification.

It's worth pointing out that I raised this issue before Rails 5 was released, and thus before per-form CSRF tokens were a thing. Given their advent and the security benefits they confer, I think it's worth factoring in a version of your another workaround into this patch, which would allow people who don't use fragment caching to reap the benefits of per-form CSRF tokens. I would make a tweak though: I'd include per-form CSRF tokens by default (for remote forms too) and automagically detect if the form_for tag is being fragment-cached (that is, if it's inside a cache block). If it is, then the form_for's :authenticity_token option should default to false and it would use the per-page CSRF token. The only difficulty I see is that you'd have to figure out some robust solution for the controller processing the submission to recognize that it should not accept the per-page CSRF token. (NB: This change would presumably be in action_view rather than jquery-ujs..)

@lucaspiller,

I agree that your suggestion should be implemented, but IMHO, that exceeds the scope of this issue. Having jquery-ujs implement HTML5/XHR2 support would still require using POST requests as a fallback in browsers that don't support the former, and the point of this issue is that this implementation is currently buggy (or, at the very least, unintuitive and not straightforward to implement). That said, I would whole-hardheartedly :+1: a PR that provided the compatibility with newer browser features that you suggest.