Middleware stack mangled by 1.4.0

[inopinatus]

Updating to 1.4.0 broke my application due to this commit which installed middleware a standard Rails application already has, including cookie middleware which it then forcefully configures.

As a result, Rails now returns two session cookies in the HTTP response headers, and any cookie configuration you had previously gets thrown away.

In my case, this caused every app user to be logged out.

[yskkin]

For me, this cause CSRF exception for existing user.

[klyonrad]

Updating from 1.3.0 to 1.4.0 broke my test suite too

[JIucToyxuu]

For me, this commit override my session store from :mongoid_store to :cookie_store.

[thomasbalsloev]

The commit causing this problem seems very little thought through.

Forcefully (as opposed to "optional") overriding global rails application settings in a gem is a very bad practice in my view.
Furthermore this change is apparently introduced due to a special case where Rails is used in API mode: #2919
A very narrow foundation to base such a far reaching decision on.

I can't believe it is your policy to only allow projects using session cookie store to use your excellent gem?

Best regards,
Thomas.

[inopinatus]

It's possible the author expected that the middleware change would only apply to requests that are handled by the rails_admin engine, and did not realise that middleware changes necessarily apply to all requests coming through Rack.

I think all the middleware insertion should be configurable, because other than Rack::Pjax, all those added by rails_admin are already present in a standard rails app and are therefore being duplicated, as can clearly be seen in the output of rails middleware.

This overriding of cookie configuration is only the most egregious example.

Perhaps the middleware insertion belongs in the application's generated config/initializers/rails_admin.rb where it can be developer-managed, and not in the engine.rb where it is hard-coded.

[yskkin]

@inopinatus Agreed, but moving initializer to generator may break once fixed problem #2919 again for existing app without changing configuration manually.
Do you think it is OK?
If so, I will try to rewrite PR #3049.

[nassredean]

Hey all, just chiming in to say that we also noticed this causing issues in production when we realized that the amount of data in redis was dropping as we started to hit the TTL for those sessions.

Worth mentioning that we do something slightly non standard in our routes file: mount RailsAdmin::Engine => '/admin', :as => 'rails_admin' wherein we mount the RailsAdmin::Engine to a specific route, though it sounds like people who are not doing this are experiencing the same problem.

[mshibuya]

Sorry for the issue, I've just pushed a fix for this.
RailsAdmin initializer no longer mangles middleware stack, it just urge users to fix if required middlewares are not added.

Please try with the latest master and give me feedback.

gem 'rails_admin', github: 'sferik/rails_admin'

If this is okay for everyone, I'll push another gem release.

[thomasbalsloev]

Hi @mshibuya

Seems to work nicely 👍
Thank you!

[inopinatus]

Hi @mshibuya

Looks good here as well. Thanks for the fix!