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

Commit 7d5ae39 breaks AuthCode Strategy for GitHub #64

Closed
smetana opened this issue Aug 22, 2016 · 9 comments
Closed

Commit 7d5ae39 breaks AuthCode Strategy for GitHub #64

smetana opened this issue Aug 22, 2016 · 9 comments

Comments

@smetana
Copy link

smetana commented Aug 22, 2016

The issue is in commit 7d5ae39

Removing client_secret from params makes GitHub to return "incorrect_client_credentials"

As per https://developer.github.com/v3/oauth/#web-application-flow client_secret is required to exchange authorization_code for access_token

@scrogson
Copy link
Member

scrogson commented Aug 22, 2016

Hi @smetana, the referenced commit was to make the AuthCode strategy compliant with the OAuth 2.0 spec. If GitHub requires that you send the client_secret along with the request, then it would need to be added manually.

AuthCode.get_token!(client, client_secret: client.client_secret, code: "...")

That should do the trick.

@smetana
Copy link
Author

smetana commented Aug 22, 2016

It does. Maybe it should be noticed in README because current strategy example does not work.

@scrogson
Copy link
Member

👍

@yang-wei
Copy link

I can't get this work.
Following the example:

## strategy callback

  def get_token(client, params, headers) do
    params = params |> Keyword.merge([client_secret: client.client_secret])
    # [code: "***", client_secret: "***"]
    client
    |> put_header("accept", "application/json")
    |> OAuth2.Strategy.AuthCode.get_token(params, headers)
  end

@yang-wei
Copy link

Ok so i get the token from github but I failed when I request the api:

  def callback(conn, %{"code" => code}) do

    client = GitHub.get_token!(code: code)

    user = get_user!(client) # failed here

    conn
    |> put_session(:current_user, user)
    |> redirect(to: "/")

  end

  defp get_user!(client) do
    case OAuth2.Client.get(client, "/user") do
      {:ok, %{status_code: 401, body: body}} ->
        IO.inspect body  # <-- get 401 here
        raise("Unauthorized token")
      {:ok, %{status_code: status_code, body: user}} when status_code in [200..399] ->
        user
      {:error, %{reason: reason}} ->
        Logger.error("Error: #{inspect reason}")
    end
  end

@masonforest
Copy link

@yang-wei try adding

params = params |> Keyword.merge([client_secret: client.client_secret])

to get_token! instead of get_token like I did here. Does that work?

@yang-wei
Copy link

yang-wei commented Sep 3, 2016

I gave up and implemented it by myself.
btw nice pr @masonforest

@binarytemple
Copy link

@yang-wei care to share a link to your implementation?

@yang-wei
Copy link

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