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

Pass provider options and query params to authorize_url #38

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ntilwalli
Copy link

@ntilwalli ntilwalli commented Nov 12, 2020

This PR integrate changes from #32 and #34, fixes tests as well as passes provider options to authorize_url!. This allows the request url to be configured since Twitter offers both an /oauth/authorize and /oauth/authenticate endpoint. The former endpoint requires the user goes through the app permission workflow each time, latter endpoint allows users to authenticate once and not have to authorize on subsequent login attemps until they revoke access. See StackOverflow discussion here.

@@ -28,6 +28,7 @@ defmodule UeberauthTwitter.Mixfile do
{:httpoison, "~> 1.0"},
{:oauther, "~> 1.1"},
{:ueberauth, "~> 0.6"},
{:hackney, "~> 1.16", override: true},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think hex does not let you publish a package when you use override: true in deps

@@ -23,19 +23,22 @@ _Note_: Sessions are required for this strategy.
```elixir
config :ueberauth, Ueberauth,
providers: [
twitter: {Ueberauth.Strategy.Twitter, []}
twitter: {Ueberauth.Strategy.Twitter, [authorize_url: "/oauth/authenticate"]}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it fix ** (Ueberauth.Strategy.Twitter.OAuth.ApiError) Could not authenticate you. (Code 32) ?

@benvp
Copy link

benvp commented Sep 1, 2022

Are there any plans on merging this? Going through the "I authorize the app to do so" flow every login is a little cumbersome.

@yordis
Copy link
Member

yordis commented Sep 2, 2022

@benvp not sure if @ntilwalli is still interested in continuing the work. I can't take the workload at the very moment, so please take the lead and I will assist you as much as I can with it

@benvp
Copy link

benvp commented Sep 2, 2022

Thanks for the reply. Will have a look and see how to bring this forward.

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

Successfully merging this pull request may close these issues.

4 participants