undefined method `new_session_path` for omniauth_callbacks controller #1390

Closed
riffraff opened this Issue Oct 17, 2011 · 32 comments

Comments

Projects
None yet
@riffraff

this error appeared in our codebase since an upgrade from 1.3.4 to 1.4.8, and it seems somewhat related to similar errors reported recently. It consistently happens whenever an authorization is denied by the user and we end up on the failure side of the omniauth controller. Our code in callbacks controller is pretty much identical to that in the docs/wiki and worked fine in the past.
We also see the error when hitting the recovery password page, similarly to an already reported and closed issue.

In our case the route code looks like

devise_for :users , :controllers => { 
  :omniauth_callbacks => "omniauth_callbacks" ,
  :registrations => 'registrations',
  :sessions =>'sessions'
}

and the quick fix was to simply add

def new_session_path *args 
  new_user_session_path *args
end

to our ApplicationController

@kevinrutherford

This comment has been minimized.

Show comment
Hide comment
@kevinrutherford

kevinrutherford Oct 17, 2011

I also have this error in my app since an upgrade to 1.4.8. It occurs whenever a user requests a password reset email or a re-send of confirmation instructions. We're using rails 3.0.3.

I also have this error in my app since an upgrade to 1.4.8. It occurs whenever a user requests a password reset email or a re-send of confirmation instructions. We're using rails 3.0.3.

@jdxcode

This comment has been minimized.

Show comment
Hide comment
@jdxcode

jdxcode Oct 17, 2011

+1 received this error as well going from 1.4.7 to 1.4.8

jdxcode commented Oct 17, 2011

+1 received this error as well going from 1.4.7 to 1.4.8

@krisleech

This comment has been minimized.

Show comment
Hide comment
@krisleech

krisleech Oct 17, 2011

I can confirm that downgrading to 1.4.7 fixes this issue.

Rails 3.0.3, basic Devise setup

I can confirm that downgrading to 1.4.7 fixes this issue.

Rails 3.0.3, basic Devise setup

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Oct 18, 2011

Member

This should be fixed in master and v1.4 branches. Please confirm and I will gladly release 1.4.9. Thanks!

Member

josevalim commented Oct 18, 2011

This should be fixed in master and v1.4 branches. Please confirm and I will gladly release 1.4.9. Thanks!

@josevalim josevalim closed this Oct 18, 2011

@riffraff

This comment has been minimized.

Show comment
Hide comment
@riffraff

riffraff Oct 18, 2011

yep seems to work for me, fixing the error both in password recovery and omniauth failure.

yep seems to work for me, fixing the error both in password recovery and omniauth failure.

@krisleech

This comment has been minimized.

Show comment
Hide comment
@krisleech

krisleech Oct 18, 2011

Thanks, I can also confirm HEAD fixes this for Rails 3.0.3 / JRuby.

Thanks, I can also confirm HEAD fixes this for Rails 3.0.3 / JRuby.

@patbenatar

This comment has been minimized.

Show comment
Hide comment
@patbenatar

patbenatar Jan 20, 2012

This issue seems to have regressed in 1.5.3 with omniauth 1.0.1

This issue seems to have regressed in 1.5.3 with omniauth 1.0.1

@rebo

This comment has been minimized.

Show comment
Hide comment
@rebo

rebo Apr 26, 2012

Yes I am getting this issue again

rebo commented Apr 26, 2012

Yes I am getting this issue again

@krzkrzkrz

This comment has been minimized.

Show comment
Hide comment
@krzkrzkrz

krzkrzkrz May 2, 2012

Same, needs to be reopened

Same, needs to be reopened

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim May 2, 2012

Member

Can someone provide a way to reproduce the issue? Otherwise, there is no way we can fix it.

Member

josevalim commented May 2, 2012

Can someone provide a way to reproduce the issue? Otherwise, there is no way we can fix it.

@josevalim josevalim reopened this May 2, 2012

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim May 6, 2012

Member

I am closing this again for the same reason: lack of ways to reproduce the issue. Thanks.

Member

josevalim commented May 6, 2012

I am closing this again for the same reason: lack of ways to reproduce the issue. Thanks.

@josevalim josevalim closed this May 6, 2012

@jeperkins4

This comment has been minimized.

Show comment
Hide comment
@jeperkins4

jeperkins4 Oct 4, 2012

I came across this issue in an app that I am migrating from ruby 1.8.7/rails 3.0.7 to 1.9.3/3.2.8. Make sure the following is not in your config/application.rb:

config.middleware.insert_before(Warden::Manager, Rack::OpenID)

I came across this issue in an app that I am migrating from ruby 1.8.7/rails 3.0.7 to 1.9.3/3.2.8. Make sure the following is not in your config/application.rb:

config.middleware.insert_before(Warden::Manager, Rack::OpenID)

@myitcv

This comment has been minimized.

Show comment
Hide comment
@myitcv

myitcv Feb 25, 2013

I encountered this whilst using Devise as confirmable only. It occurs for me when a user tries to confirm an email address using a token that has expired. Clearly it's necessary to define a route for this eventuality - so just to confirm, is defining a route like:

match "a/b", :via => [:get, :put], :as => "new_session"

the 'correct' way to define the route?

myitcv commented Feb 25, 2013

I encountered this whilst using Devise as confirmable only. It occurs for me when a user tries to confirm an email address using a token that has expired. Clearly it's necessary to define a route for this eventuality - so just to confirm, is defining a route like:

match "a/b", :via => [:get, :put], :as => "new_session"

the 'correct' way to define the route?

@twelve17

This comment has been minimized.

Show comment
Hide comment
@twelve17

twelve17 Jul 23, 2013

I ran into this today, but it was a narrow use case:

I'm writing an engine that uses devise version 2.2.4 plus omniauth-oauth2, and includes a custom subclass of Devise::OmniauthCallbacksController. I am only using :omniauthable in my User model.

The error only happens in the test/dummy app, which itself extends the engine's OmniauthCallbacksController (to implement a stub method), but not in a 'real' app that is using the gem in the same fashion. I'm in the process of trying to figure out if there is a notable difference in between this app and the test dummy app's configurations.

I'm running an integration test in the engine with the dummy app. I put a breakpoint in Devise.generate_helpers!, and see the new_session_path being generated, but for whatever reason, it's not available in the custom OmniauthCallbacksController subclass in the dummy app. class.ancestors in that controller does show the Devise::Controllers::UrlHelpers being included, so I must be missing something. I'll keep digging.

I ran into this today, but it was a narrow use case:

I'm writing an engine that uses devise version 2.2.4 plus omniauth-oauth2, and includes a custom subclass of Devise::OmniauthCallbacksController. I am only using :omniauthable in my User model.

The error only happens in the test/dummy app, which itself extends the engine's OmniauthCallbacksController (to implement a stub method), but not in a 'real' app that is using the gem in the same fashion. I'm in the process of trying to figure out if there is a notable difference in between this app and the test dummy app's configurations.

I'm running an integration test in the engine with the dummy app. I put a breakpoint in Devise.generate_helpers!, and see the new_session_path being generated, but for whatever reason, it's not available in the custom OmniauthCallbacksController subclass in the dummy app. class.ancestors in that controller does show the Devise::Controllers::UrlHelpers being included, so I must be missing something. I'll keep digging.

@RKushnir

This comment has been minimized.

Show comment
Hide comment
@RKushnir

RKushnir Sep 22, 2013

In the docs it says

If you are using ONLY omniauth authentication, you need to define a route named new_user_session (if not defined, root will be used)

but in fact it raises the aforementioned error and doesn't use the root.
I'm on a brand new Rails4 app with gems

devise (3.1.0)
omniauth (1.1.4)
omniauth-linkedin-oauth2 (0.1.1)
omniauth-oauth2 (1.1.1)

In the docs it says

If you are using ONLY omniauth authentication, you need to define a route named new_user_session (if not defined, root will be used)

but in fact it raises the aforementioned error and doesn't use the root.
I'm on a brand new Rails4 app with gems

devise (3.1.0)
omniauth (1.1.4)
omniauth-linkedin-oauth2 (0.1.1)
omniauth-oauth2 (1.1.1)
@printercu

This comment has been minimized.

Show comment
Hide comment
@printercu

printercu Apr 13, 2014

@josevalim, got same issue on 3.2.4. Faced this problem when omniauth provider returns error and https://github.com/plataformatec/devise/blob/master/app/controllers/devise/omniauth_callbacks_controller.rb#L28 is called.

I use only

class User < ActiveRecord::Base
  devise :omniauthable, :rememberable, :trackable
  # ...
end

so seems like new_session_path haven't been generated.

I have routes for new_user_session_path but it doesn't help:

  devise_scope :user do
    get '/session/new(.:format)'        =>  'devise/sessions#new',      as: :new_user_session
    get '/session/sign_out(.:format)'   =>  'devise/sessions#destroy',  as: :destroy_user_session
  end

This fix works for me:

class ApplicationController < ActionController::Base
  # ...
  def new_session_path(scope)
    new_user_session_path
  end
end

Should we reopen issue?

@josevalim, got same issue on 3.2.4. Faced this problem when omniauth provider returns error and https://github.com/plataformatec/devise/blob/master/app/controllers/devise/omniauth_callbacks_controller.rb#L28 is called.

I use only

class User < ActiveRecord::Base
  devise :omniauthable, :rememberable, :trackable
  # ...
end

so seems like new_session_path haven't been generated.

I have routes for new_user_session_path but it doesn't help:

  devise_scope :user do
    get '/session/new(.:format)'        =>  'devise/sessions#new',      as: :new_user_session
    get '/session/sign_out(.:format)'   =>  'devise/sessions#destroy',  as: :destroy_user_session
  end

This fix works for me:

class ApplicationController < ActionController::Base
  # ...
  def new_session_path(scope)
    new_user_session_path
  end
end

Should we reopen issue?

@josevalim josevalim reopened this Apr 13, 2014

@laurocaetano

This comment has been minimized.

Show comment
Hide comment
@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Apr 16, 2014

Member

@laurocaetano we just generate those routes depending on the devise functionality you are using:

routes ||= begin
mappings = Devise.mappings.values.map(&:used_helpers).flatten.uniq
Devise::URL_HELPERS.slice(*mappings)
end

I am not sure what is the best way to proceed here. Maybe a respond_to? check with a proper error message? Or maybe leave it as is and say new_session_path or after_omniauth_failure_path_for must be redefined. We just need to agree on something.

Member

josevalim commented Apr 16, 2014

@laurocaetano we just generate those routes depending on the devise functionality you are using:

routes ||= begin
mappings = Devise.mappings.values.map(&:used_helpers).flatten.uniq
Devise::URL_HELPERS.slice(*mappings)
end

I am not sure what is the best way to proceed here. Maybe a respond_to? check with a proper error message? Or maybe leave it as is and say new_session_path or after_omniauth_failure_path_for must be redefined. We just need to agree on something.

@printercu

This comment has been minimized.

Show comment
Hide comment
@printercu

printercu Apr 17, 2014

Мауbe just create this helper if :omniauthable is used? There is notice in documentation that one should add new_user_session route manually if uses :omniauthable without :database_authenticatable. So the route should be present, we only need a helper.

Мауbe just create this helper if :omniauthable is used? There is notice in documentation that one should add new_user_session route manually if uses :omniauthable without :database_authenticatable. So the route should be present, we only need a helper.

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Apr 17, 2014

Member

@printercu yeah, that is likely a better option!

Member

josevalim commented Apr 17, 2014

@printercu yeah, that is likely a better option!

@laurocaetano

This comment has been minimized.

Show comment
Hide comment
@laurocaetano

laurocaetano Apr 22, 2014

Contributor

I got something like https://gist.github.com/laurocaetano/55cf2edd5c08881030c6, but I'm not familiar with this part of the code and maybe I'm missing something.

Contributor

laurocaetano commented Apr 22, 2014

I got something like https://gist.github.com/laurocaetano/55cf2edd5c08881030c6, but I'm not familiar with this part of the code and maybe I'm missing something.

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Apr 23, 2014

Member

@laurocaetano that seems to be the way to go. @printercu could you please give it a try?

Member

josevalim commented Apr 23, 2014

@laurocaetano that seems to be the way to go. @printercu could you please give it a try?

@printercu

This comment has been minimized.

Show comment
Hide comment
@printercu

printercu Apr 23, 2014

I can't test its behaviour with omniauth now, but i've got this in console:

2.1.1 :001 > Sessions::OmniauthCallbacksController.instance_method(:new_session_path)
NameError: undefined method `new_session_path' for class `Sessions::OmniauthCallbacksController'
    from (irb):1:in `instance_method'
    from (irb):1
    from /Users/max/.rvm/gems/ruby-2.1.1/gems/railties-4.1.0/lib/rails/commands/console.rb:90:in `start'
    from /Users/max/.rvm/gems/ruby-2.1.1/gems/railties-4.1.0/lib/rails/commands/console.rb:9:in `start'
    from /Users/max/.rvm/gems/ruby-2.1.1/gems/railties-4.1.0/lib/rails/commands/commands_tasks.rb:69:in `console'
    from /Users/max/.rvm/gems/ruby-2.1.1/gems/railties-4.1.0/lib/rails/commands/commands_tasks.rb:40:in `run_command!'
    from /Users/max/.rvm/gems/ruby-2.1.1/gems/railties-4.1.0/lib/rails/commands.rb:17:in `<top (required)>'
    from bin/rails:4:in `require'
    from bin/rails:4:in `<main>'
2.1.1 :002 > Sessions::OmniauthCallbacksController.instance_method(:omniauth_callback_path).source_location
 => ["/Users/max/.rvm/gems/ruby-2.1.1/gems/devise-3.2.4/lib/devise/omniauth/url_helpers.rb", 12] 

I tried this, it works in console, but i don't know how) Is it expected behaviour? I'll try it later on server.

# d.add_module :omniauthable, controller: :omniauth_callbacks,  route: {session: [:new]}

2.1.1 :001 > Sessions::OmniauthCallbacksController.instance_method(:new_session_path).source_location
 => ["/Users/max/.rvm/gems/ruby-2.1.1/gems/devise-3.2.4/lib/devise/controllers/url_helpers.rb", 48] 
2.1.1 :002 > Sessions::OmniauthCallbacksController.instance_method(:omniauth_callback_path).source_location
 => ["/Users/max/.rvm/gems/ruby-2.1.1/gems/devise-3.2.4/lib/devise/omniauth/url_helpers.rb", 12] 

Update. I've got error undefined method 'user_omniauth_authorize_path'.

I can't test its behaviour with omniauth now, but i've got this in console:

2.1.1 :001 > Sessions::OmniauthCallbacksController.instance_method(:new_session_path)
NameError: undefined method `new_session_path' for class `Sessions::OmniauthCallbacksController'
    from (irb):1:in `instance_method'
    from (irb):1
    from /Users/max/.rvm/gems/ruby-2.1.1/gems/railties-4.1.0/lib/rails/commands/console.rb:90:in `start'
    from /Users/max/.rvm/gems/ruby-2.1.1/gems/railties-4.1.0/lib/rails/commands/console.rb:9:in `start'
    from /Users/max/.rvm/gems/ruby-2.1.1/gems/railties-4.1.0/lib/rails/commands/commands_tasks.rb:69:in `console'
    from /Users/max/.rvm/gems/ruby-2.1.1/gems/railties-4.1.0/lib/rails/commands/commands_tasks.rb:40:in `run_command!'
    from /Users/max/.rvm/gems/ruby-2.1.1/gems/railties-4.1.0/lib/rails/commands.rb:17:in `<top (required)>'
    from bin/rails:4:in `require'
    from bin/rails:4:in `<main>'
2.1.1 :002 > Sessions::OmniauthCallbacksController.instance_method(:omniauth_callback_path).source_location
 => ["/Users/max/.rvm/gems/ruby-2.1.1/gems/devise-3.2.4/lib/devise/omniauth/url_helpers.rb", 12] 

I tried this, it works in console, but i don't know how) Is it expected behaviour? I'll try it later on server.

# d.add_module :omniauthable, controller: :omniauth_callbacks,  route: {session: [:new]}

2.1.1 :001 > Sessions::OmniauthCallbacksController.instance_method(:new_session_path).source_location
 => ["/Users/max/.rvm/gems/ruby-2.1.1/gems/devise-3.2.4/lib/devise/controllers/url_helpers.rb", 48] 
2.1.1 :002 > Sessions::OmniauthCallbacksController.instance_method(:omniauth_callback_path).source_location
 => ["/Users/max/.rvm/gems/ruby-2.1.1/gems/devise-3.2.4/lib/devise/omniauth/url_helpers.rb", 12] 

Update. I've got error undefined method 'user_omniauth_authorize_path'.

@laurocaetano

This comment has been minimized.

Show comment
Hide comment
@laurocaetano

laurocaetano Apr 24, 2014

Contributor

The solution I mentioned before won't work because add_module will only get the Hash's first key. https://github.com/plataformatec/devise/blob/master/lib/devise.rb#L383

@josevalim should we change the add_module method to accept more than one key?

Contributor

laurocaetano commented Apr 24, 2014

The solution I mentioned before won't work because add_module will only get the Hash's first key. https://github.com/plataformatec/devise/blob/master/lib/devise.rb#L383

@josevalim should we change the add_module method to accept more than one key?

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Apr 24, 2014

Member

Definitely!

Member

josevalim commented Apr 24, 2014

Definitely!

@laurocaetano

This comment has been minimized.

Show comment
Hide comment
@laurocaetano

laurocaetano Apr 24, 2014

Contributor

Well, it also won't work 😢, because it will only have access to the routes defined by the module -> https://github.com/plataformatec/devise/blob/master/lib/devise.rb#L392

Discussing with @ulissesalmeida we came up with some solutions:

  1. Just let this way and add a documentation to warn users that new_session_path(scope) must be defined.
  2. Add the documentation and raise an error saying that this helper must be defined.
  3. Rewrite how the routes and helpers work, so that it can accept the helpers defined in other modules.
  4. Make :omniauthable work without any external dependency, creating it's own view, action for failures and etc.

What you guys think about it?

cc @lucasmazza @carlosantoniodasilva

Contributor

laurocaetano commented Apr 24, 2014

Well, it also won't work 😢, because it will only have access to the routes defined by the module -> https://github.com/plataformatec/devise/blob/master/lib/devise.rb#L392

Discussing with @ulissesalmeida we came up with some solutions:

  1. Just let this way and add a documentation to warn users that new_session_path(scope) must be defined.
  2. Add the documentation and raise an error saying that this helper must be defined.
  3. Rewrite how the routes and helpers work, so that it can accept the helpers defined in other modules.
  4. Make :omniauthable work without any external dependency, creating it's own view, action for failures and etc.

What you guys think about it?

cc @lucasmazza @carlosantoniodasilva

@lucasmazza

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Apr 24, 2014

Contributor

Giving a quick look on this I would go with options 2 or/then 3. 😄

Contributor

lucasmazza commented Apr 24, 2014

Giving a quick look on this I would go with options 2 or/then 3. 😄

@printercu

This comment has been minimized.

Show comment
Hide comment

Vote for 1. There is also documentation for it https://github.com/plataformatec/devise/wiki/OmniAuth%3A-Overview#using-omniauth-without-other-authentications . Just add few lines there.

@laurocaetano

This comment has been minimized.

Show comment
Hide comment
@printercu

This comment has been minimized.

Show comment
Hide comment
@printercu

printercu Apr 25, 2014

👍 Thank you!

👍 Thank you!

@ulissesalmeida

This comment has been minimized.

Show comment
Hide comment
Member

ulissesalmeida commented Apr 25, 2014

👍

@laurocaetano

This comment has been minimized.

Show comment
Hide comment
@laurocaetano

laurocaetano Apr 25, 2014

Contributor

Thanks guys, I'm closing this for now.

❤️

Contributor

laurocaetano commented Apr 25, 2014

Thanks guys, I'm closing this for now.

❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment