search

psu-libraries / psulib_blacklight

Penn State University Libraries' Blacklight Catalog
Apache License 2.0
10 stars 3 forks source link

when you click to bookmark it works, when you "unbookmark" it errors #71

Closed banukutlu closed 5 years ago

banukutlu commented 6 years ago

User does a search
User clicks a checkbox to bookmark
User clicks the same checkbox to un-bookmark An alert message is displayed saying "error"

The application log looks like this:

Started DELETE "/bookmarks/10023235" for 127.0.0.1 at 2018-10-31 09:47:13 -0400
Processing by BookmarksController#destroy as JSON
  Parameters: {"utf8"=>"✓", "authenticity_token"=>"2oNI6L6OmHe90HTjoH4ZcQmASQCpuuR3wwKRMW8VVUytQE+hwo3vVfbd58zcQdlWv2XbtELma+fbT7ZMTzEnfQ==", "id"=>"10023235"}
Can't verify CSRF token authenticity.
Completed 422 Unprocessable Entity in 1ms (ActiveRecord: 0.0ms)

ActionController::InvalidAuthenticityToken (ActionController::InvalidAuthenticityToken):

This doesn't happen in the demo blacklight and I am not finding it in the issue queue on the blacklight main repo.

stemming from #64 connected to #91

banukutlu commented 6 years ago

In GitLab by @cdm32 on Jul 20, 2018, 15:33

mentioned in commit 09d622bbfc7355917b188490778667131d4ced00

banukutlu commented 6 years ago

In GitLab by @ful105 on Aug 24, 2018, 14:15

when you click to bookmark it works, when you "unbookmark" it errors

banukutlu commented 6 years ago

Are bookmarks gonna be a feature?

cdmo commented 5 years ago

The answer to https://github.com/psu-libraries/psulib_blacklight/issues/71#issuecomment-427932562 is yes (via @ruthtillman at a meeting yesterday) - adding here for posterity

cdmo commented 5 years ago

If you read the docs, it says you need a user implementation for bookmarks to work, fwiw

ruthtillman commented 5 years ago

@cdmo so basically without user accounts it doesn't have a necessary component to clear? That makes sense.

cdmo commented 5 years ago

But it should allow a guest to bookmark. I think this is actually a bug with Blacklight and Rails 5.2... more to come

cdmo commented 5 years ago

Yeah, this is a bug with rails 5.2 and blacklight

See https://github.com/nsarno/knock/issues/208#issuecomment-373022274

I'll report back to the community

When I create a new bookmark controller and add skip_before_action :verify_authenticity_token, the error goes away

cdmo commented 5 years ago

There's more information available when you view the response to the XHR request in chrome:

ActionController::InvalidAuthenticityToken in BookmarksController#destroy

ActionController::InvalidAuthenticityToken
Extracted source (around line #211):

#209 
#210         def handle_unverified_request
*211           raise ActionController::InvalidAuthenticityToken
#212         end
#213       end
#214     end

Extracted source (around line #243):

#241 
#242       def handle_unverified_request # :doc:
*243         forgery_protection_strategy.new(self).handle_unverified_request
#244       end
#245 
#246       #:nodoc:

Extracted source (around line #255):

#253       # clear run strategies and remove cached variables.
#254       def handle_unverified_request
*255         super # call the default behaviour which resets/nullifies/raises
#256         request.env["devise.skip_storage"] = true
#257         sign_out_all_scopes(false)
#258       end

Rails.root: /Users/cdm32/Projects/ruby/psulib_blacklight

Framework Trace
actionpack (5.2.1) lib/action_controller/metal/request_forgery_protection.rb:211:in `handle_unverified_request'
actionpack (5.2.1) lib/action_controller/metal/request_forgery_protection.rb:243:in `handle_unverified_request'
devise (4.5.0) lib/devise/controllers/helpers.rb:255:in `handle_unverified_request'
actionpack (5.2.1) lib/action_controller/metal/request_forgery_protection.rb:238:in `verify_authenticity_token'
activesupport (5.2.1) lib/active_support/callbacks.rb:426:in `block in make_lambda'
activesupport (5.2.1) lib/active_support/callbacks.rb:198:in `block (2 levels) in halting'
actionpack (5.2.1) lib/abstract_controller/callbacks.rb:34:in `block (2 levels) in <module:Callbacks>'
activesupport (5.2.1) lib/active_support/callbacks.rb:199:in `block in halting'
activesupport (5.2.1) lib/active_support/callbacks.rb:513:in `block in invoke_before'
activesupport (5.2.1) lib/active_support/callbacks.rb:513:in `each'
activesupport (5.2.1) lib/active_support/callbacks.rb:513:in `invoke_before'
activesupport (5.2.1) lib/active_support/callbacks.rb:131:in `run_callbacks'
actionpack (5.2.1) lib/abstract_controller/callbacks.rb:41:in `process_action'
actionpack (5.2.1) lib/action_controller/metal/rescue.rb:22:in `process_action'
actionpack (5.2.1) lib/action_controller/metal/instrumentation.rb:34:in `block in process_action'
activesupport (5.2.1) lib/active_support/notifications.rb:168:in `block in instrument'
activesupport (5.2.1) lib/active_support/notifications/instrumenter.rb:23:in `instrument'
activesupport (5.2.1) lib/active_support/notifications.rb:168:in `instrument'
actionpack (5.2.1) lib/action_controller/metal/instrumentation.rb:32:in `process_action'
actionpack (5.2.1) lib/action_controller/metal/params_wrapper.rb:256:in `process_action'
activerecord (5.2.1) lib/active_record/railties/controller_runtime.rb:24:in `process_action'
actionpack (5.2.1) lib/abstract_controller/base.rb:134:in `process'
actionview (5.2.1) lib/action_view/rendering.rb:32:in `process'
actionpack (5.2.1) lib/action_controller/metal.rb:191:in `dispatch'
actionpack (5.2.1) lib/action_controller/metal.rb:252:in `dispatch'
actionpack (5.2.1) lib/action_dispatch/routing/route_set.rb:52:in `dispatch'
actionpack (5.2.1) lib/action_dispatch/routing/route_set.rb:34:in `serve'
actionpack (5.2.1) lib/action_dispatch/journey/router.rb:52:in `block in serve'
actionpack (5.2.1) lib/action_dispatch/journey/router.rb:35:in `each'
actionpack (5.2.1) lib/action_dispatch/journey/router.rb:35:in `serve'
actionpack (5.2.1) lib/action_dispatch/routing/route_set.rb:840: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 `catch'
warden (1.2.7) lib/warden/manager.rb:35:in `call'
rack (2.0.5) lib/rack/tempfile_reaper.rb:15:in `call'
rack (2.0.5) lib/rack/etag.rb:25:in `call'
rack (2.0.5) lib/rack/conditional_get.rb:38:in `call'
rack (2.0.5) lib/rack/head.rb:12:in `call'
actionpack (5.2.1) lib/action_dispatch/http/content_security_policy.rb:18:in `call'
rack (2.0.5) lib/rack/session/abstract/id.rb:232:in `context'
rack (2.0.5) lib/rack/session/abstract/id.rb:226:in `call'
actionpack (5.2.1) lib/action_dispatch/middleware/cookies.rb:670:in `call'
activerecord (5.2.1) lib/active_record/migration.rb:559:in `call'
actionpack (5.2.1) lib/action_dispatch/middleware/callbacks.rb:28:in `block in call'
activesupport (5.2.1) lib/active_support/callbacks.rb:98:in `run_callbacks'
actionpack (5.2.1) lib/action_dispatch/middleware/callbacks.rb:26:in `call'
actionpack (5.2.1) lib/action_dispatch/middleware/executor.rb:14:in `call'
actionpack (5.2.1) lib/action_dispatch/middleware/debug_exceptions.rb:61:in `call'
web-console (3.7.0) lib/web_console/middleware.rb:135:in `call_app'
web-console (3.7.0) lib/web_console/middleware.rb:30:in `block in call'
web-console (3.7.0) lib/web_console/middleware.rb:20:in `catch'
web-console (3.7.0) lib/web_console/middleware.rb:20:in `call'
actionpack (5.2.1) lib/action_dispatch/middleware/show_exceptions.rb:33:in `call'
railties (5.2.1) lib/rails/rack/logger.rb:38:in `call_app'
railties (5.2.1) lib/rails/rack/logger.rb:26:in `block in call'
activesupport (5.2.1) lib/active_support/tagged_logging.rb:71:in `block in tagged'
activesupport (5.2.1) lib/active_support/tagged_logging.rb:28:in `tagged'
activesupport (5.2.1) lib/active_support/tagged_logging.rb:71:in `tagged'
railties (5.2.1) lib/rails/rack/logger.rb:26:in `call'
sprockets-rails (3.2.1) lib/sprockets/rails/quiet_assets.rb:13:in `call'
actionpack (5.2.1) lib/action_dispatch/middleware/remote_ip.rb:81:in `call'
actionpack (5.2.1) lib/action_dispatch/middleware/request_id.rb:27:in `call'
rack (2.0.5) lib/rack/method_override.rb:22:in `call'
rack (2.0.5) lib/rack/runtime.rb:22:in `call'
activesupport (5.2.1) lib/active_support/cache/strategy/local_cache_middleware.rb:29:in `call'
actionpack (5.2.1) lib/action_dispatch/middleware/executor.rb:14:in `call'
actionpack (5.2.1) lib/action_dispatch/middleware/static.rb:127:in `call'
rack (2.0.5) lib/rack/sendfile.rb:111:in `call'
webpacker (3.5.5) lib/webpacker/dev_server_proxy.rb:22:in `perform_request'
rack-proxy (0.6.5) lib/rack/proxy.rb:57:in `call'
railties (5.2.1) lib/rails/engine.rb:524:in `call'
puma (3.12.0) lib/puma/configuration.rb:225:in `call'
puma (3.12.0) lib/puma/server.rb:658:in `handle_request'
puma (3.12.0) lib/puma/server.rb:472:in `process_client'
puma (3.12.0) lib/puma/server.rb:332:in `block in run'
puma (3.12.0) lib/puma/thread_pool.rb:133:in `block in spawn_thread'

Full Trace
actionpack (5.2.1) lib/action_controller/metal/request_forgery_protection.rb:211:in `handle_unverified_request'
actionpack (5.2.1) lib/action_controller/metal/request_forgery_protection.rb:243:in `handle_unverified_request'
devise (4.5.0) lib/devise/controllers/helpers.rb:255:in `handle_unverified_request'
actionpack (5.2.1) lib/action_controller/metal/request_forgery_protection.rb:238:in `verify_authenticity_token'
activesupport (5.2.1) lib/active_support/callbacks.rb:426:in `block in make_lambda'
activesupport (5.2.1) lib/active_support/callbacks.rb:198:in `block (2 levels) in halting'
actionpack (5.2.1) lib/abstract_controller/callbacks.rb:34:in `block (2 levels) in <module:Callbacks>'
activesupport (5.2.1) lib/active_support/callbacks.rb:199:in `block in halting'
activesupport (5.2.1) lib/active_support/callbacks.rb:513:in `block in invoke_before'
activesupport (5.2.1) lib/active_support/callbacks.rb:513:in `each'
activesupport (5.2.1) lib/active_support/callbacks.rb:513:in `invoke_before'
activesupport (5.2.1) lib/active_support/callbacks.rb:131:in `run_callbacks'
actionpack (5.2.1) lib/abstract_controller/callbacks.rb:41:in `process_action'
actionpack (5.2.1) lib/action_controller/metal/rescue.rb:22:in `process_action'
actionpack (5.2.1) lib/action_controller/metal/instrumentation.rb:34:in `block in process_action'
activesupport (5.2.1) lib/active_support/notifications.rb:168:in `block in instrument'
activesupport (5.2.1) lib/active_support/notifications/instrumenter.rb:23:in `instrument'
activesupport (5.2.1) lib/active_support/notifications.rb:168:in `instrument'
actionpack (5.2.1) lib/action_controller/metal/instrumentation.rb:32:in `process_action'
actionpack (5.2.1) lib/action_controller/metal/params_wrapper.rb:256:in `process_action'
activerecord (5.2.1) lib/active_record/railties/controller_runtime.rb:24:in `process_action'
actionpack (5.2.1) lib/abstract_controller/base.rb:134:in `process'
actionview (5.2.1) lib/action_view/rendering.rb:32:in `process'
actionpack (5.2.1) lib/action_controller/metal.rb:191:in `dispatch'
actionpack (5.2.1) lib/action_controller/metal.rb:252:in `dispatch'
actionpack (5.2.1) lib/action_dispatch/routing/route_set.rb:52:in `dispatch'
actionpack (5.2.1) lib/action_dispatch/routing/route_set.rb:34:in `serve'
actionpack (5.2.1) lib/action_dispatch/journey/router.rb:52:in `block in serve'
actionpack (5.2.1) lib/action_dispatch/journey/router.rb:35:in `each'
actionpack (5.2.1) lib/action_dispatch/journey/router.rb:35:in `serve'
actionpack (5.2.1) lib/action_dispatch/routing/route_set.rb:840: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 `catch'
warden (1.2.7) lib/warden/manager.rb:35:in `call'
rack (2.0.5) lib/rack/tempfile_reaper.rb:15:in `call'
rack (2.0.5) lib/rack/etag.rb:25:in `call'
rack (2.0.5) lib/rack/conditional_get.rb:38:in `call'
rack (2.0.5) lib/rack/head.rb:12:in `call'
actionpack (5.2.1) lib/action_dispatch/http/content_security_policy.rb:18:in `call'
rack (2.0.5) lib/rack/session/abstract/id.rb:232:in `context'
rack (2.0.5) lib/rack/session/abstract/id.rb:226:in `call'
actionpack (5.2.1) lib/action_dispatch/middleware/cookies.rb:670:in `call'
activerecord (5.2.1) lib/active_record/migration.rb:559:in `call'
actionpack (5.2.1) lib/action_dispatch/middleware/callbacks.rb:28:in `block in call'
activesupport (5.2.1) lib/active_support/callbacks.rb:98:in `run_callbacks'
actionpack (5.2.1) lib/action_dispatch/middleware/callbacks.rb:26:in `call'
actionpack (5.2.1) lib/action_dispatch/middleware/executor.rb:14:in `call'
actionpack (5.2.1) lib/action_dispatch/middleware/debug_exceptions.rb:61:in `call'
web-console (3.7.0) lib/web_console/middleware.rb:135:in `call_app'
web-console (3.7.0) lib/web_console/middleware.rb:30:in `block in call'
web-console (3.7.0) lib/web_console/middleware.rb:20:in `catch'
web-console (3.7.0) lib/web_console/middleware.rb:20:in `call'
actionpack (5.2.1) lib/action_dispatch/middleware/show_exceptions.rb:33:in `call'
railties (5.2.1) lib/rails/rack/logger.rb:38:in `call_app'
railties (5.2.1) lib/rails/rack/logger.rb:26:in `block in call'
activesupport (5.2.1) lib/active_support/tagged_logging.rb:71:in `block in tagged'
activesupport (5.2.1) lib/active_support/tagged_logging.rb:28:in `tagged'
activesupport (5.2.1) lib/active_support/tagged_logging.rb:71:in `tagged'
railties (5.2.1) lib/rails/rack/logger.rb:26:in `call'
sprockets-rails (3.2.1) lib/sprockets/rails/quiet_assets.rb:13:in `call'
actionpack (5.2.1) lib/action_dispatch/middleware/remote_ip.rb:81:in `call'
actionpack (5.2.1) lib/action_dispatch/middleware/request_id.rb:27:in `call'
rack (2.0.5) lib/rack/method_override.rb:22:in `call'
rack (2.0.5) lib/rack/runtime.rb:22:in `call'
activesupport (5.2.1) lib/active_support/cache/strategy/local_cache_middleware.rb:29:in `call'
actionpack (5.2.1) lib/action_dispatch/middleware/executor.rb:14:in `call'
actionpack (5.2.1) lib/action_dispatch/middleware/static.rb:127:in `call'
rack (2.0.5) lib/rack/sendfile.rb:111:in `call'
webpacker (3.5.5) lib/webpacker/dev_server_proxy.rb:22:in `perform_request'
rack-proxy (0.6.5) lib/rack/proxy.rb:57:in `call'
railties (5.2.1) lib/rails/engine.rb:524:in `call'
puma (3.12.0) lib/puma/configuration.rb:225:in `call'
puma (3.12.0) lib/puma/server.rb:658:in `handle_request'
puma (3.12.0) lib/puma/server.rb:472:in `process_client'
puma (3.12.0) lib/puma/server.rb:332:in `block in run'
puma (3.12.0) lib/puma/thread_pool.rb:133:in `block in spawn_thread'

Request parameters
{"utf8"=>"✓",
 "_method"=>"delete",
 "authenticity_token"=>"gicBgGhvbw4ITHtl1H0wAIM5PLL8//dIG+e4UGi1XOy8AYU0iuCERPZPmsSoQ95ZmtwPDiGFdZRa0RCEuJ42IA==",
 "id"=>"10299491"}

Session dump
_csrf_token: "RWLCZESLeBG7kZ3PbjRmSpLU4x+uMEYrLpb5BzLd9Tg="
guest_user_id: "guest_GrYUsQJWEaLjw55NTa12_1540998932_1@example.com"
history: [32]
search: {"id"=>32}
session_id: "d17ddba47c58b31372abf735e2cd0e2b"

Env dump
GATEWAY_INTERFACE: "CGI/1.2"
HTTP_ACCEPT: "application/json, text/javascript, */*; q=0.01"
HTTP_ACCEPT_ENCODING: "gzip, deflate, br"
HTTP_ACCEPT_LANGUAGE: "en-US,en;q=0.9"
HTTP_ORIGIN: "http://localhost:3000"
HTTP_VERSION: "HTTP/1.1"
ORIGINAL_SCRIPT_NAME: ""
REMOTE_ADDR: "::1"
SERVER_NAME: "localhost"
SERVER_PROTOCOL: "HTTP/1.1"

Response headers
None
cdmo commented 5 years ago

Found in griddler too, https://github.com/thoughtbot/griddler/issues/297. Seems like at least some in the community are just turning this off.