Hyrax: Adding existing works to existing collection fails

Created on 12 Jun 2017 · 36Comments · Source: samvera/hyrax

Descriptive summary

Given I have a work owned by me
And I have a collection owned by me
When I visit my dashboard
And select my work
And click "Add to Collection"
And select my collection
And click "Update Collection"
Then I should not see ActionController::InvalidAuthenticityToken

Expected behavior

My work should be added to my collection

Actual behavior

Started PUT "/collections/g158bh28p?locale=en" for 127.0.0.1 at 2017-06-12 10:21:54 -0400
Processing by Hyrax::CollectionsController#update as HTML
  Parameters: {"authenticity_token"=>"MdqIljoqgvEyFCRiXnKmGPbxMu8y7ykUlMWUamXLLqMkoLky/vAbpSiLprweauguBwQVCIeAYKTCEYrFzmPVLg==", "batch_document_ids"=>["5m60qr88b"], "collection"=>{"members"=>"add"}, "locale"=>"en", "id"=>"g158bh28p"}
Can't verify CSRF token authenticity.
Completed 422 Unprocessable Entity in 1ms (ActiveRecord: 0.0ms)



ActionController::InvalidAuthenticityToken - ActionController::InvalidAuthenticityToken:
  actionpack (5.1.1) lib/action_controller/metal/request_forgery_protection.rb:195:in `handle_unverified_request'
  actionpack (5.1.1) lib/action_controller/metal/request_forgery_protection.rb:227:in `handle_unverified_request'
  devise (4.3.0) lib/devise/controllers/helpers.rb:253:in `handle_unverified_request'
  actionpack (5.1.1) lib/action_controller/metal/request_forgery_protection.rb:222:in `verify_authenticity_token'
  activesupport (5.1.1) lib/active_support/callbacks.rb:413:in `block in make_lambda'
  activesupport (5.1.1) lib/active_support/callbacks.rb:197:in `block (2 levels) in halting'
  actionpack (5.1.1) lib/abstract_controller/callbacks.rb:12:in `block (2 levels) in <module:Callbacks>'
  activesupport (5.1.1) lib/active_support/callbacks.rb:198:in `block in halting'
  activesupport (5.1.1) lib/active_support/callbacks.rb:507:in `block in invoke_before'
  activesupport (5.1.1) lib/active_support/callbacks.rb:507:in `invoke_before'
  activesupport (5.1.1) lib/active_support/callbacks.rb:130:in `run_callbacks'
  actionpack (5.1.1) lib/abstract_controller/callbacks.rb:19:in `process_action'
  actionpack (5.1.1) lib/action_controller/metal/rescue.rb:20:in `process_action'
  actionpack (5.1.1) lib/action_controller/metal/instrumentation.rb:32:in `block in process_action'
  activesupport (5.1.1) lib/active_support/notifications.rb:166:in `block in instrument'
  activesupport (5.1.1) lib/active_support/notifications/instrumenter.rb:21:in `instrument'
  activesupport (5.1.1) lib/active_support/notifications.rb:166:in `instrument'
  actionpack (5.1.1) lib/action_controller/metal/instrumentation.rb:30:in `process_action'
  actionpack (5.1.1) lib/action_controller/metal/params_wrapper.rb:252:in `process_action'
  activerecord (5.1.1) lib/active_record/railties/controller_runtime.rb:22:in `process_action'
  actionpack (5.1.1) lib/abstract_controller/base.rb:124:in `process'
  actionview (5.1.1) lib/action_view/rendering.rb:30:in `process'
  actionpack (5.1.1) lib/action_controller/metal.rb:189:in `dispatch'
  actionpack (5.1.1) lib/action_controller/metal.rb:253:in `dispatch'
  actionpack (5.1.1) lib/action_dispatch/routing/route_set.rb:49:in `dispatch'
  actionpack (5.1.1) lib/action_dispatch/routing/route_set.rb:31:in `serve'
  actionpack (5.1.1) lib/action_dispatch/journey/router.rb:46:in `block in serve'
  actionpack (5.1.1) lib/action_dispatch/journey/router.rb:33:in `serve'
  actionpack (5.1.1) lib/action_dispatch/routing/route_set.rb:832:in `call'
  railties (5.1.1) lib/rails/engine.rb:522:in `call'
  railties (5.1.1) lib/rails/railtie.rb:185:in `method_missing'
  actionpack (5.1.1) lib/action_dispatch/routing/mapper.rb:17:in `block in <class:Constraints>'
  actionpack (5.1.1) lib/action_dispatch/routing/mapper.rb:46:in `serve'
  actionpack (5.1.1) lib/action_dispatch/journey/router.rb:46:in `block in serve'
  actionpack (5.1.1) lib/action_dispatch/journey/router.rb:33:in `serve'
  actionpack (5.1.1) lib/action_dispatch/routing/route_set.rb:832:in `call'
  warden (1.2.7) lib/warden/manager.rb:36:in `block in call'
  warden (1.2.7) lib/warden/manager.rb:35:in `call'
  rack (2.0.3) lib/rack/etag.rb:25:in `call'
  rack (2.0.3) lib/rack/conditional_get.rb:38:in `call'
  rack (2.0.3) lib/rack/head.rb:12:in `call'
  rack (2.0.3) lib/rack/session/abstract/id.rb:232:in `context'
  rack (2.0.3) lib/rack/session/abstract/id.rb:226:in `call'
  actionpack (5.1.1) lib/action_dispatch/middleware/cookies.rb:613:in `call'
  active-fedora (11.2.0) lib/active_fedora/ldp_cache.rb:26:in `call'
  flipflop (2.3.1) lib/flipflop/feature_cache.rb:12:in `call'
  activerecord (5.1.1) lib/active_record/migration.rb:556:in `call'
  actionpack (5.1.1) lib/action_dispatch/middleware/callbacks.rb:26:in `block in call'
  activesupport (5.1.1) lib/active_support/callbacks.rb:97:in `run_callbacks'
  actionpack (5.1.1) lib/action_dispatch/middleware/callbacks.rb:24:in `call'
  actionpack (5.1.1) lib/action_dispatch/middleware/executor.rb:12:in `call'
  better_errors (2.1.1) lib/better_errors/middleware.rb:84:in `protected_app_call'
  better_errors (2.1.1) lib/better_errors/middleware.rb:79:in `better_errors_call'
  better_errors (2.1.1) lib/better_errors/middleware.rb:57:in `call'
  actionpack (5.1.1) lib/action_dispatch/middleware/debug_exceptions.rb:59:in `call'
  actionpack (5.1.1) lib/action_dispatch/middleware/show_exceptions.rb:31:in `call'
  railties (5.1.1) lib/rails/rack/logger.rb:36:in `call_app'
  railties (5.1.1) lib/rails/rack/logger.rb:24:in `block in call'
  activesupport (5.1.1) lib/active_support/tagged_logging.rb:69:in `block in tagged'
  activesupport (5.1.1) lib/active_support/tagged_logging.rb:26:in `tagged'
  activesupport (5.1.1) lib/active_support/tagged_logging.rb:69:in `tagged'
  railties (5.1.1) lib/rails/rack/logger.rb:24:in `call'
  sprockets-rails (3.2.0) lib/sprockets/rails/quiet_assets.rb:13:in `call'
  actionpack (5.1.1) lib/action_dispatch/middleware/remote_ip.rb:79:in `call'
  actionpack (5.1.1) lib/action_dispatch/middleware/request_id.rb:25:in `call'
  rack (2.0.3) lib/rack/method_override.rb:22:in `call'
  rack (2.0.3) lib/rack/runtime.rb:22:in `call'
  activesupport (5.1.1) lib/active_support/cache/strategy/local_cache_middleware.rb:27:in `call'
  actionpack (5.1.1) lib/action_dispatch/middleware/executor.rb:12:in `call'
  actionpack (5.1.1) lib/action_dispatch/middleware/static.rb:125:in `call'
  rack (2.0.3) lib/rack/sendfile.rb:111:in `call'
  railties (5.1.1) lib/rails/engine.rb:522:in `call'
  puma (3.9.1) lib/puma/configuration.rb:224:in `call'
  puma (3.9.1) lib/puma/server.rb:602:in `handle_request'
  puma (3.9.1) lib/puma/server.rb:435:in `process_client'
  puma (3.9.1) lib/puma/server.rb:299:in `block in run'
  puma (3.9.1) lib/puma/thread_pool.rb:120:in `block in spawn_thread'

Steps to reproduce the behavior

Create a work and collection
From the user dashboard, select the work and add it to the collection

backport needed bug

Source

awead

All 36 comments

I was unable to duplicate this error. What version of Hyrax are you testing?

jcoyne on 12 Jun 2017

@jcoyne latest on master as of last week. Let me try re-generating and see what happens.

awead on 12 Jun 2017

@jcoyne no, I'm still getting it. I've regenerated the application, and used two different browsers.

awead on 12 Jun 2017

@awead are you seeing this with the test app? or with scholarsphere/another app? If the latter, can you share the contents of config/initializers/hyrax.rb?

mjgiarlo on 12 Jun 2017

(I ask because of https://github.com/samvera-labs/hyku/pull/980)

mjgiarlo on 12 Jun 2017

@mjgiarlo Hyrax test application generated in the hyrax repo.

awead on 12 Jun 2017

I'm getting this too, but only in development mode.

jcoyne on 13 Jun 2017

@awead could this be a turbolinks bug? This is an interesting case where it's trying to submit an authenticity token for a button_to form, that is different from the authenticity token in the header <meta> tag.

Are we tracking https://github.com/rails/rails/issues/24257 ?

jcoyne on 15 Jun 2017

I did a bunch more testing on this today and confirmed the bug in RAILS_ENV=production.

The button next to the Add Collection button works. Here are the two buttons: https://github.com/samvera/hyrax/blob/e2bf67b936aadd25269c3d85d5e016657edae430/app/views/hyrax/collections/_form_for_select_collection.html.erb#L34-L35

Only the button on line 34 renders with its own new authenticity token, possibly because of method: :put?

When I 1) disable Turbolinks, and 2) change :put to :patch for that button, it works as expected. I am not sure why. (Meanwhile, this also works fine in Hyku, which uses the same code here and has turbolinks turned on.)

mjgiarlo on 18 Jul 2017

@mjgiarlo

its own new authenticity token

because button_to makes a new form with an authenticity token.

jcoyne on 18 Jul 2017

OK, after more testing, I got some stuff wrong earlier.

First, turbolinks doesn't seem to be related.

As a test, I'm rendering the button twice now, once with method: :put and once with method: :patch. And the :put vs. :patch is misleading. Here's what happens: whichever button I click first (whether put or patch) fails. Whichever one I click second works, even if it's the same one. @jcoyne earlier said that a reload might fix the behavior, and while I couldn't get a reload to do the trick, submitting the form once then back-buttoning seems to make things work (which makes it sound turbolinks-related? ugh.).

It's not the case that :patch is OK and :put is not like I suggested earlier.

mjgiarlo on 18 Jul 2017

The method I was using to remove turbolinks was incomplete (removing it from application.js and then recompiling assets). When I remove it from the application's Gemfile and bundle (also removing it from application.js), this error no longer happens.

Worth saying, again, that this is not an issue we've seen in Hyku despite it using the same version of Rails (5.1.2) and Turbolinks (5.0.1) that I've been testing with a vanilla Hyrax test app today.

mjgiarlo on 19 Jul 2017

👍1

@awead @elrayle @jcoyne @jrochkind :point_up:

mjgiarlo on 19 Jul 2017

I wonder if it's due to the version of jquery that is used?

jcoyne on 19 Jul 2017

@jcoyne Ah, perhaps. Hyku uses jquery3. I'll test that out.

mjgiarlo on 19 Jul 2017

@jcoyne Changing //= require jquery to //= require jquery3 and then cleaning/clobbering/precompiling assets does not seem to account for the difference between Hyku and the test app, unfortunately.

mjgiarlo on 19 Jul 2017

FYI... when I pinned rails to 5.0.3, the problem at least appears to have gone away.

elrayle on 19 Jul 2017

Unclear to me what the proposed fix is for Hyrax. Can somebody clarify? Do we want to bump the version of rails required? Version of jquery required? Do something w/ turbolinks?

atz on 16 Aug 2017

@atz Unclear. Needs more analysis to figure out what the real culprit is. Removing turbolinks seems like a suboptimal solution to me.

mjgiarlo on 16 Aug 2017

@mjgiarlo Since the problem seems to be limited to this page, we could add data-no-turbolink for just it. I'm not much in favor of figuring out the real cause as much as figuring out the real fix.

atz on 17 Aug 2017

@atz Yes, that is most definitely worth a shot. If that fixes it, ship it. Inclined to have a go?

mjgiarlo on 17 Aug 2017

Hi, I think this problem is caused by the Form's CSRF authenticity_token lagging the Page's authenticity_token. I'm not quite sure why that is, though I've found that if you reload the page then the tokens do match again.

I've written a simple javascript which fires when the user clicks on the _update collection_ button and compares the form's token with the page's token. If they do not match, then it updates the form token before submission. This is definitely a work-around for the problem, rather than a fix. But if anyone else is fighting this issue, it might help:


Blacklight.onLoad(function () {

    // This javascript modifies the collection form's CSRF token if it does not match the page's CSRF token.
    // It is a work-around for issue https://github.com/samvera/hyrax/issues/1191

    $('[data-behavior="updates-collection"]').on('click', function() {
        var form = $(this).closest("form");
        var form_authenticity_token = form.find(":input[name='authenticity_token']");
        var header_authenticity_token = $("meta[name='csrf-token']").attr('content');
        if (form_authenticity_token.attr('value') != header_authenticity_token) {
            form_authenticity_token.attr('value', header_authenticity_token);
        }
    });
});

martyn-w on 30 Aug 2017

I wonder if only occurs when: Rails.application.config.action_controller.per_form_csrf_tokens = false (this is in config/initializers/new_framework_defaults.rb on rails 5). Has anyone checke this?

jcoyne on 22 Sep 2017

Thanks for the tip, @jcoyne! I don't believe anyone's checked this. If this is the case, we should ask folks how they'd feel about twiddling this value.

mjgiarlo on 22 Sep 2017

Where are we on this issue? I'm asking because we still keep stumbling across this in the collections-sprint branch. We've added turbolinks: false in a number of places, but it would be good to have a general solution.

elrayle on 20 Oct 2017

@elrayle Where we are:

@jcoyne's last comment may provide a clue as to what's causing the root behavior. Until someone looks into the prior point or finds another fix, we've been disabling turbolinks on inbound links. See these two PRs, which you will want to apply to other, similar links in the collections-sprint branch: https://github.com/samvera/hyrax/pull/1605/files and https://github.com/samvera/hyrax/pull/1614/files

mjgiarlo on 20 Oct 2017

I took another run at this but having no luck.

I tried turning on/off each of these configs in all possible combinations. When the errors occur, I can confirm that value of these settings are as expected. But they seem to have no impact.

# Avoid ActionController::InvalidAuthenticityToken errors
Rails.application.config.action_controller.per_form_csrf_tokens = false
Rails.application.config.action_controller.forgery_protection_origin_check = false
# Rails.application.config.action_controller.allow_forgery_protection = false

From https://github.com/plataformatec/devise/pull/4033/files

For Rails 5, note that protect_from_forgery is no longer prepended to the before_action chain, so if you have set authenticate_user before protect_from_forgery, your request will result in "Can't verify CSRF token authenticity." To resolve this, either change the order in which you call them, or use protect_from_forgery prepend: true.

I tried updating my_controller.rb with several options set on protect_from_forgery...

    # protect_from_forgery               # forwards to Dashboard with msg: 'You are already signed in.'
    # protect_from_forgery prepend: true # forwards to Dashboard with msg: 'You are already signed in.'
    # protect_from_forgery prepend: true, with: :exception # gets the error
    protect_from_forgery with: :exception                  # gets the error.  NOTE:  This is the one most recommended.

    before_action :authenticate_user!

Presumably prepend: true is not needed as long as protect_from_forgery comes before the before_action :authenticate_user! statement. But I tested with and without it just in case.

I tried skipping...

    skip_before_action :verify_authenticity_token       # gets the error

Places I found interesting information...

elrayle on 2 Nov 2017

@jcoyne pointed to this github issue as relevant to this conversation...

https://github.com/rails/rails/issues/22807

elrayle on 13 Nov 2017

I may have figured out what's happening in the case of batch adding works to a collection. It relates to turbolinks and jquery-ujs.

It appears the default setting is to generate a CSRF token for each form, i.e. Rails.application.config.action_controller.per_form_csrf_tokens = true. This contradicts what we see when a page loads without turbolinks, the tokens in the forms and the global token in the meta tag match. As it turns out jquery-ujs executes $.rails.refreshCSRFTokens() on page loads which copies the token from the meta tag to the form inputs. When a page loads with turbolinks the JS that copies the token is not executed. So this how we end up with different tokens in the HTML.

Normally, this would not be a problem because all of the CSRF tokens that were loaded are valid. Here's the interesting part, in this particular case the token was generated for the action /dashboard/collections/collection_replace_id, but the actual action depends on the selected collection. Selecting a collection modifies the action path, thus invalidating the token. Using the global token (the one defined in the meta tag) works because it is not specific to the action.

With that being said, it seems like setting Rails.application.config.action_controller.per_form_csrf_tokens = false would solve the problem.

jonathandixon on 21 Dec 2017

🎉1

@jonathandixon Wow, great find! This looks like the same: https://github.com/rails/jquery-ujs/issues/456

And see the workaround suggested here: https://github.com/rails/jquery-ujs/issues/456#issuecomment-183728902

mjgiarlo on 21 Dec 2017

@jonathandixon see also @elrayle's message above that suggests setting per_form_csrf_tokens to false does not seem to work.

mjgiarlo on 21 Dec 2017

I can confirm that setting per_form_csrf_tokens = false does not force the same token for all forms on the page in question.

jonathandixon on 21 Dec 2017

My last comment is incorrect. per_form_csrf_tokens = false is honored but tokens are masked (https://github.com/rails/rails/blob/e88e6cea2113ce3e54410cbd8c2da92b86f83d2b/actionpack/lib/action_controller/metal/request_forgery_protection.rb#L307).

Which put's me back at square 1 in trying to figure this out.

jonathandixon on 21 Dec 2017

@jonathandixon I haven't had time to dig in, but have you looked at this workaround? https://github.com/rails/jquery-ujs/issues/456#issuecomment-183728902

mjgiarlo on 21 Dec 2017

@mjgiarlo I only tried running $.rails.refreshCSRFTokens() in the developer console, which does work. I have a feeling something like this would work

$(document).on('turbolinks:load', function() {
  $.rails.refreshCSRFTokens();
});

I will try to do some more testing after the holidays.

jonathandixon on 22 Dec 2017

The scenario in this issue has been fixed for a while. And we now have a PR that seems to fix the general issue of turbolinks raising the invalid authenticity tokens exception. See PR #2875.

I think we can close this one when the PR is merged. Any objections? @awead @jcoyne @mjgiarlo @no-reply @jonathandixon @atz @martyn-w

elrayle on 1 Apr 2018

Was this page helpful?

0 / 5 - 0 ratings

Related issues

Batch upload fails when arrived at through the Work Form

younga3 · 6Comments

Allow for work types to be in a namespace

carolyncole · 5Comments

Implement operations functionality and UI

mjgiarlo · 6Comments

Add registered users group to dropdown list of groups with whom a collection can be shared

hannahfrost · 7Comments

Don't attempt to display ungenerated derivatives

revgum · 6Comments