search

simi / omniauth-facebook

Facebook OAuth2 Strategy for OmniAuth
https://simi.github.io/omniauth-facebook/
1.26k stars 403 forks source link

Remove duplicated script_name in callback_url #380

Closed tak1n closed 1 year ago

tak1n commented 2 years ago

First, thanks for providing and maintaining this gem, it helped us integrate with facebook and providing some products on top of it for quite some time. Lately, we found an issue with our setup. We have a rails application running behind a reverse proxy and as we use Sidekiq and want to keep using the Sidekiq Web UIs we set SCRIPT_NAME as env var.

This led to some weird behaviour with omniauth-facebook/omniauth though, the redirect_uri started to contain the SCRIPT_NAME part twice. When digging through the code we can see omniauth-facebook uses script_name to construct the callback_url in https://github.com/simi/omniauth-facebook/blob/master/lib/omniauth/strategies/facebook.rb#L88.

Taking a look at omniauth we can then see callback_path is as well constructed with script_name in it (https://github.com/omniauth/omniauth/blob/master/lib/omniauth/strategy.rb#L450). The fix to rely on the default values here while still being able to set SCRIPT_NAME would be to get rid of the usage in omniauth-facebook.

simi commented 1 year ago

Any chance to reflect this also in tests?

github-actions[bot] commented 1 year ago

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

tak1n commented 1 year ago

Any chance to reflect this also in tests?

Sry, missed that completely. Will add some tests this weekend.

tak1n commented 1 year ago

@simi adapted the tests accordingly. Had to update the mocha setup as well removed MultiJson usage. MultiJson is not declared as a dependency anywhere, I also decided against introducing it as a dependency as it seems to be in a somewhat limbo state in regards to maintenance (no ruby 3+ support, last commit in 2020).

simi commented 1 year ago

Thanks!