janko / rodauth-rails

Rails integration for Rodauth authentication framework
https://github.com/jeremyevans/rodauth
MIT License
604 stars 40 forks source link

Double prefix happening during omniauth flow? #304

Closed ianrandmckenzie closed 5 months ago

ianrandmckenzie commented 5 months ago

Hi, I've been searching through my code, the rodauth repo, and this repo and cannot seem to figure out where this issue is occurring.

I am using omniauth-reddit with Rodauth and Rodauth Rails:

class RodauthApp < Rodauth::Rails::App
  configure RodauthMain

  configure do
    prefix '/user'
  end

  plugin :rodauth do
    accounts_table :end_users
  end

  route do |r|
    rodauth.load_memory # autologin remembered users
    if r.path.start_with?('/user/')
      r.rodauth # route rodauth requests
    end
  end
end

When I remove the prefix (and comment out the path.start_with? conditional), everything works as expected. However, when I prefix /user/, the callback phase fails with Reddit citing an invalid callback URL. When I look at the URL request on this page and convert the percent encoded characters back to normal, the url looks like this: http://localhost:3000/user/user/auth/reddit/callback

Confusingly, if I throw these in the view, they are as they should be:

rodauth.omniauth_request_path(:reddit) # /user/auth/reddit
rodauth.omniauth_callback_path(:reddit) # /user/auth/reddit/callback

Is there anything that immediately rings a bell or is this a PBCAC?

Thanks!

janko commented 5 months ago

Are you using OmniAuth 2? Is the reddit strategy compatible with it?

ianrandmckenzie commented 5 months ago

Hi Janko,

Yes, I am not sure, I've been out of the oauth game for a few years. Here's what I've got for gems under the hood: omniauth (2.1.0), omniauth-oauth2 (1.8.0), omniauth-rails_csrf_protection (1.0.1), rodauth-omniauth (0.3.4), and lastly I am using an omniauth-reddit strategy but I am not sure if my definition of 'compatible' is the same as yours since you might be asking me if /user is getting prefixed twice due to an out-of-date reddit omniauth strategy?

Sorry, that last sentence might sound sarcastic or something but it is a genuine question! haha

Everything works correctly without prefixing which to me says it is compatible?

ianrandmckenzie commented 5 months ago

BTW I have a functional workaround which is to add to my routes:

# config/routes.rb
get '/user/user/auth/reddit/callback', to: 'rodauth#omniauth'
janko commented 5 months ago

OmniAuth 2.0 changed the way its handles SCRIPT_NAME, which is set by rodauth-omniauth when using a path prefix. Previously, OmniAuth::Strategy#callback_path didn't include script name, so when strategies used full_host + script_name + callback_path for the redirect URL, that was correct.

However, OmniAuth 2.0 changed OmniAuth::Strategy#callback_path to include the script name. This meant that strategies that still called full_host + script_name + callback_path were now duplicating the script name. I think this is why the prefix is duplicated in your case.

Theoretically, any strategy that uses a recent version of omniauth-oauth2 should be good to go. But omniauth-reddit uses a fork of omniauth-oauth2 (?), which is for sure not up-to-date with OmniAuth 2.0. So, while omniauth-reddit might work without a prefix, it's still technically not compatible with OmniAuth 2.0, which causes issues with the prefix.

You might be able to monkey-patch it in a way that prefix is not duplicated:

class OmniAuth::Strategies::Reddit
  def callback_url
    options[:redirect_uri] || full_host + callback_path + query_string
  end
end

I will close this issue, since it's not a bug in rodauth-omniauth.