devise icon indicating copy to clipboard operation
devise copied to clipboard

Fixing compatibility with Rails 7 and API mode

Open morenocarullo opened this issue 3 years ago • 12 comments

This PR addresses this issue https://github.com/heartcombo/devise/issues/5443 .

The test coverage for it is not ideal yet, as we are working to find a good way to have API-mode tests properly integrated (quick hack of forcing it to true does not work).

morenocarullo avatar Feb 25 '22 13:02 morenocarullo

Hey @carlosantoniodasilva , this is a first part of the work to allow API mode to work as well. See https://github.com/heartcombo/devise/issues/5443.

morenocarullo avatar Feb 25 '22 13:02 morenocarullo

Why the hard lock to Rails 7+ ? The PR looks like it will work with Rails 6 without a hitch.

hms avatar Mar 03 '22 18:03 hms

Why the hard lock to Rails 7+ ? The PR looks like it will work with Rails 6 without a hitch.

you are right: but in Rails 6 it would not have any effect because rack.session will be a valid one.

morenocarullo avatar Mar 04 '22 04:03 morenocarullo

I think this is a good hack as bandaid. but I don't think this is a good solution. Some users may be using it in API mode and with sessions enabled. I think it would be broken from those users.

DisabledSessionError is useful information. because we can realize an implicit dependency on the "rack.session"

I think the problem is that devise implicitly depends on session. following lines ...

  • https://github.com/heartcombo/devise/blob/v4.8.1/lib/devise/controllers/sign_in_out.rb#L48
  • https://github.com/heartcombo/devise/blob/v4.8.1/lib/devise/controllers/sign_in_out.rb#L68
  • https://github.com/heartcombo/devise/blob/v4.8.1/lib/devise/test/controller_helpers.rb#L81
  • https://github.com/heartcombo/devise/blob/v4.8.1/lib/devise/test/controller_helpers.rb#L96

How about providing an option called ignore_session? like this

Devise.setup do |config|
  config.ignore_session = true # default false
end

As a side note, there seems to be a similar discussion on warden's side. https://github.com/wardencommunity/warden/pull/201

dnpp73 avatar Mar 04 '22 09:03 dnpp73

I think this is a good hack as bandaid. but I don't think this is a good solution. Some users may be using it in API mode and with sessions enabled. I think it would be broken from those users.

DisabledSessionError is useful information. because we can realize an implicit dependency on the "rack.session"

I think the problem is that devise implicitly depends on session. following lines ...

* https://github.com/heartcombo/devise/blob/v4.8.1/lib/devise/controllers/sign_in_out.rb#L48

* https://github.com/heartcombo/devise/blob/v4.8.1/lib/devise/controllers/sign_in_out.rb#L68

* https://github.com/heartcombo/devise/blob/v4.8.1/lib/devise/test/controller_helpers.rb#L81

* https://github.com/heartcombo/devise/blob/v4.8.1/lib/devise/test/controller_helpers.rb#L96

How about providing an option called ignore_session? like this

Devise.setup do |config|
  config.ignore_session = true # default false
end

As a side note, there seems to be a similar discussion on warden's side. wardencommunity/warden#201

I agree with you. I believe that, before beginning a journey to the "right thing" it would be great to have some blessing from the mantainers, or it will result in a lot of wasted work.

morenocarullo avatar Mar 04 '22 10:03 morenocarullo

@morenocarullo I forked and merged in my Devise version for an API ( Rails 7, API Only, JWT Auth). Works like a charm, thanks for your insight.

ghost avatar Mar 19 '22 17:03 ghost

Getting

NoMethodError (undefined method `destroy' for {}:Devise::Controllers::Rails7ApiMode::FakeRackSession):

when trying to sign out. Any thoughts?

alhajrahmoun avatar Mar 19 '22 21:03 alhajrahmoun

Didn't work on rails 7.0.2.3 for me. Works only if comment "load_for_write" line in actionpack's gem at lib/action_dispatch/request/session.rb

      # Writes given value to given key of the session.
      def []=(key, value)
        # load_for_write!
        @delegate[key.to_s] = value
      end

I don't know if this is correct in any way.

module ActionDispatch
  class Request
    class Session
      def []=(key, value)
        @delegate[key.to_s] = value
      end
    end
  end
end

is it safe??

internet-exploder avatar Mar 20 '22 04:03 internet-exploder

Worked for me. However I'm getting the following error when running rspec. What am I missing?

      LoadError:
        cannot load such file -- devise/controllers/rails7_api_mode

heyapricot avatar Mar 22 '22 16:03 heyapricot

Is this likely to be merged any time soon? Or has anyone discovered an alternative for dealing with the problem this PR solves?

justinshaw avatar Nov 06 '22 00:11 justinshaw

Any news?

alexjoseps avatar Nov 26 '22 19:11 alexjoseps

any updates on merging this solution or providing another one?

graveetone avatar Apr 19 '23 14:04 graveetone