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

valid_site? fails on 2.0.0 #72

Open
joelturnbull opened this issue Dec 11, 2018 · 7 comments
Open

valid_site? fails on 2.0.0 #72

joelturnbull opened this issue Dec 11, 2018 · 7 comments

Comments

@joelturnbull
Copy link

2.0.0 broke the shopify omniauth feature in my app and I had to downgrade back to 1.2.1. As I've seen in some other threads, the valid_site? method is returning false because of the change to how the shop value is accessed through the session. It seems like a lot of people are using this gem in conjunction with shopify_app, which I'm not. I'm using Devise and the devise initializer instead of the omniauth initializer to provide the keys and set the scopes we want to request.

When I inspect the values incoming to omniauth/strategies/shopify.rb#valid_site? I find my shop value in strategy.session['omniauth.params']['shop'] and nothing in strategy.session['shopify.omniauth_params']. I'm not sure what is setting this session value or how to tweak it in my code to provide it in the way the that 2.0.0 change expects. Any insight? Thanks

@tylerball
Copy link
Contributor

Hi Joe,

A bit of background: there was a lot of confusion in our codebase around this, shopify_app's docs were updated to recommend using session for storing the shop, but the example app and this library was not. I've tried to make this more consistent everywhere.

All we do in shopify_app is set session['shopify.omniauth_params'] to contain the shop given and pick it up in the default setup method here. I would double check that you aren't overriding that setup method in an initializer anywhere as that's what we used to do in the shopify_app generator, but if you're seeing the shop populated in that session var this package should be picking it up properly.

@uurcank
Copy link

uurcank commented Dec 16, 2018

I think 2.0 released assuming everybody is using the shopify_app gem. This currently does not work for a regular omniauth request.

Reverting back to 1.2.1 fixes the problem

@sumitbirla
Copy link

Is there any update or workaround on this issue? I had to revert to 1.2.1 as well, however even that seems to have stopped working in Safari today.

@uurcank
Copy link

uurcank commented Jul 30, 2019

Safari issue might be related to this

#43 Users signing up gets redircted to /auth/failure?message=csrf_detected&strategy=shopify

@troex
Copy link

troex commented Sep 7, 2019

I'm using sinatra and ran into similar issue when upgraded, just added

session['shopify.omniauth_params'] = { shop: params['shop'] }

to the route which receives install redirect

clupprich added a commit to clupprich/omniauth-shopify-oauth2 that referenced this issue Oct 2, 2019
clupprich added a commit to clupprich/omniauth-shopify-oauth2 that referenced this issue Oct 5, 2019
@clupprich
Copy link
Contributor

clupprich commented Oct 5, 2019

Looking at source code of OmniAuth::Strategy, the session['omniauth.params'] is set in the request phase only after the setup phase has run. That means that we can't use session['omniauth.params'] to extract the shop param we gonna need.

Instead, we need to read the shop param from the Rack env, for example:

option :setup, proc { |env|
  request = Rack::Request.new(env)
  site = if request.params['shop']
    "https://#{request.params['shop']}"
  else
    ''
  end

  env['omniauth.strategy'].options[:client_options][:site] = site
}

That's basically what the shopify_app does, too.

I'll create a PR in the next couple of days.

Edit: Just realized that this is basically the code in v1.2.1

@eni9889
Copy link

eni9889 commented Mar 13, 2020

To anyone else having this issue I had to update my OmniAuth config to include this:

  provider :shopify,
           ENV['SHOPIFY_API_KEY'],
           ENV['SHOPIFY_API_SECRET'],
           scope: ...,
           setup:         ->(env) {
             request                                                  = Rack::Request.new(env)
             site                                                     = if request.params['shop']
                                                                          "https://#{request.params['shop']}"
                                                                        else
                                                                          ''
                                                                        end
             env['omniauth.strategy'].options[:client_options][:site] = site
           }

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

7 participants