Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Too hard to preserve POST params prior to request phase #975

Open
aspiers opened this issue Nov 10, 2019 · 6 comments
Open

Too hard to preserve POST params prior to request phase #975

aspiers opened this issue Nov 10, 2019 · 6 comments

Comments

@aspiers
Copy link

aspiers commented Nov 10, 2019

Currently params provided to the request phase via the query string in the GET request are stored as session["omniauth.params"]:

session['omniauth.params'] = request.GET

and then retrieved and made available in the callback phase as request.env["omniauth.params"]:

@env['omniauth.params'] = session.delete('omniauth.params') || {}

This useful feature, whilst mentioned in third-party blogs like here and here, is still lacking official documentation as mentioned in issue #909. That's unfortunate, but there is a further issue caused by a combination of two facts:

  1. This only works when the request phase is triggered with GET.
  2. When mitigating CVE-2015-9284 via the advice given in https://github.com/omniauth/omniauth/wiki/Resolving-CVE-2015-9284, it is necessary to trigger the request phase via POST, since allowing GET /auth/:provider is unsafe.

So when attempting to write code which uses omniauth safely, there is no obvious way to preserve custom application-specific parameters which are passed at login time.

By looking at lib/omniauth/strategy.rb I managed to figure out that I could do something like:

Rails.application.config.middleware.use OmniAuth::Builder do
  provider :facebook, ENV['FACEBOOK_KEY'], ENV['FACEBOOK_SECRET']
  provider :twitter, ENV['TWITTER_KEY'], ENV['TWITTER_SECRET']

  before_request_phase do |env|
    # Prior to login, save user parameters to session so that we can
    # retrieve them after authentication in order to update the user's
    # preferences.
    #
    # This API is basically undocumented and was figured from reading
    # omniauth's strategy.rb.
    env["rack.session"]["user_params"] = env["rack.request.form_hash"]["user"]
  end
end

Then later on after authentication has succeeded and the callback redirects to my users_controller.rb, I can access these custom parameters via session[:user_params]. However it does not seem good that developers should have to assume such knowledge about the internals of the middleware in order to write code which safely preserves parameters across the various HTTP requests in the authentication flow. Therefore I suggest that something similar to omniauth.params is implemented for POST requests - it could be called omniauth.form_params, for instance.

And of course, all of this should be clearly documented, and explained in https://github.com/omniauth/omniauth/wiki/Resolving-CVE-2015-9284 too :-)

@gingerlime
Copy link

Also wondered about the same thing, and the search led me here. Is there any willingness to support this from the maintainers? are PRs for this going to be considered? or is it a dead-end?

@miguelperez
Copy link

I am using this strategy https://github.com/stevenkaras/omniauth-mailchimp and it does not work like that.
Basically, anything I add to the session in the request phase is lost when I am in the callback controller.

This is using rails 6.0.3

  provider :mailchimp, ENV['MAILCHIMP_CLIENT_ID'], ENV['MAILCHIMP_CLIENT_SECRET'], setup: lambda { |env|
    if env["omniauth.strategy"].on_request_path?
      env['rack.session']['foo'] = 'foo'
    end
  }

  before_request_phase do |env|
    # Prior to login, save user parameters to session so that we can
    # retrieve them after authentication in order to update the user's
    # preferences.
    #
    # This API is basically undocumented and was figured from reading
    # omniauth's strategy.rb.
    env["rack.session"]["user_params"] = 'something!!!'
  end
end

Then in the callback controller

def create
    puts session[:foo]
    puts session[:user_params]
    render plain: 'ok'
  end

Nothing is printed. Any Idea what could be happening here?

@BobbyMcWho
Copy link
Member

Also wondered about the same thing, and the search led me here. Is there any willingness to support this from the maintainers? are PRs for this going to be considered? or is it a dead-end?

I would need to look into any potential ramifications of the changes being discussed, and overall community need

@BobbyMcWho
Copy link
Member

I am using this strategy https://github.com/stevenkaras/omniauth-mailchimp and it does not work like that.

Basically, anything I add to the session in the request phase is lost when I am in the callback controller.

This is using rails 6.0.3

  provider :mailchimp, ENV['MAILCHIMP_CLIENT_ID'], ENV['MAILCHIMP_CLIENT_SECRET'], setup: lambda { |env|

    if env["omniauth.strategy"].on_request_path?

      env['rack.session']['foo'] = 'foo'

    end

  }



  before_request_phase do |env|

    # Prior to login, save user parameters to session so that we can

    # retrieve them after authentication in order to update the user's

    # preferences.

    #

    # This API is basically undocumented and was figured from reading

    # omniauth's strategy.rb.

    env["rack.session"]["user_params"] = 'something!!!'

  end

end

Then in the callback controller

def create

    puts session[:foo]

    puts session[:user_params]

    render plain: 'ok'

  end

Nothing is printed. Any Idea what could be happening here?

I'm not guaranteeing this works, but have you tried accessing with string keys?

@miguelperez
Copy link

I'm not guaranteeing this works, but have you tried accessing with string keys?

Yes, I tried that, but still does not work.

@sshock
Copy link

sshock commented Jun 29, 2021

I also ran into this problem after switching from GET /auth/:provider to POST /auth/:provider.

I was able to workaround it by looking at ::Rack::Utils.parse_query(URI(request.env["omniauth.origin"]).query)["myparam"] instead of request.env["omniauth.params"]["myparam"].

I also tried the strategy above of setting env["rack.session"]["myparam"] = env.dig("rack.request.form_hash", "myparam") in the before_request_phase, and that also worked.

If there is a better way, please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants