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.

Sign up for GitHub

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

Jump to bottom

Handle client side flow for google auth. #35

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

Handle client side flow for google auth. #35

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions lib/ueberauth/strategy/google.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,16 @@ defmodule Ueberauth.Strategy.Google do
end
end

@doc """
Handles the callback for Google client side flow.
"""
def handle_callback!(%Plug.Conn{params: %{"token" => token}} = conn) do
Copy link
Member

Choose a reason for hiding this comment

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

@hassox could we add the parameter as binary as well? We technically don't need the Conn so if I add the logic of the oauth callbacks outside of an phoenix project I dont need to carry on the Conn everywhere.

The same for other strategies

Copy link
Member

Choose a reason for hiding this comment

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

@yordis I'm not sure I follow when you say binary. Are you asking that this match be changed from %Plug.Conn{params: %{"token" => token}} to %{params: %{"token" => token}}?

Copy link
Member

Choose a reason for hiding this comment

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

@blakedietz could you make this change based on @yordis's comment below?

Copy link

Choose a reason for hiding this comment

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

@rohanpujaris the param that holds the JWT is called credential I think: https://developers.google.com/identity/gsi/web/reference/js-reference#credential

fetch_user(conn, OAuth2.AccessToken.new(token))
end

@doc false
def handle_callback!(conn) do
set_errors!(conn, [error("missing_code", "No code received")])
set_errors!(conn, [error("missing_code_or_token", "No code or token received")])
end

@doc false
Expand Down Expand Up @@ -124,7 +131,7 @@ defmodule Ueberauth.Strategy.Google do
resp = Ueberauth.Strategy.Google.OAuth.get(token, path)

case resp do
{:ok, %OAuth2.Response{status_code: 401, body: _body}} ->
Copy link
Member

Choose a reason for hiding this comment

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

Does a 401 here return with an :error in the tuple? I think when I was testing it it still responded with an :ok

Copy link
Author

@rohanpujaris rohanpujaris Jun 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@hassox Yep, it returned an :error when I tested it. I will retest it and confirm.

Copy link
Author

@rohanpujaris rohanpujaris Jun 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@hassox I have verified the response again. It returns with and :error. Below is the returned response.

{:error,
 %OAuth2.Response{body: %{"error" => "invalid_token",
    "error_description" => "Invalid Credentials"},
  headers: [{"vary", "X-Origin"},
   {"www-authenticate",
    "Bearer realm=\"https://accounts.google.com/\", error=invalid_token"},
   {"content-type", "application/json; charset=UTF-8"},
   {"date", "Wed, 14 Jun 2017 06:47:11 GMT"},
   {"expires", "Wed, 14 Jun 2017 06:47:11 GMT"},
   {"cache-control", "private, max-age=0"},
   {"x-content-type-options", "nosniff"}, {"x-frame-options", "SAMEORIGIN"},
   {"x-xss-protection", "1; mode=block"}, {"server", "GSE"},
   {"alt-svc", "quic=\":443\"; ma=2592000; v=\"38,37,36,35\""},
   {"accept-ranges", "none"}, {"vary", "Origin,Accept-Encoding"},
   {"transfer-encoding", "chunked"}], status_code: 401}}

Copy link
Member

Choose a reason for hiding this comment

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

The latest oauth2 version has changed so that any successful HTTP status codes (200..399) returns {:ok, resp} and all others returns {:error, resp}.

{:error, %OAuth2.Response{status_code: 401, body: _body}} ->
set_errors!(conn, [error("token", "unauthorized")])
{:ok, %OAuth2.Response{status_code: status_code, body: user}} when status_code in 200..399 ->
put_private(conn, :google_user, user)
Expand Down