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

Best practice for connecting multiple accounts #15

Open
venkatd opened this issue Mar 10, 2016 · 13 comments
Open

Best practice for connecting multiple accounts #15

venkatd opened this issue Mar 10, 2016 · 13 comments

Comments

@venkatd
Copy link

venkatd commented Mar 10, 2016

First of all, thanks for the great library.

I was wondering if there's a best practice for allowing a user to connect multiple accounts. For example:

  • Login with Google by navigating to /auth/google
  • Create user record abc in the database and store the auth google details
  • Login with GitHub by navigating to /auth/google?access_token=access_token_abc BUT also passing along the auth details of the currently logged in user.
  • Within /auth/google/callback, tie the account back to the original user.. meaning I have access to access_token_abc

Should I encode the info in the state OAuth property? Should I cache oauth state => user_id? Or something else entirely?

Thanks!

@venkatd
Copy link
Author

venkatd commented Mar 16, 2016

Here's what I ended up doing...

I made a plug that packs up the query params into the state property which persists between auth/github and auth/github/callback

Usage

For now I have hardcoded the fields to @packable_fields, but if we want to preserve a user_id and other details, we can pack those into the state as well. I opted to go this route instead of caching a state to details map so I could avoid dealing with process state.

  pipeline :auth do
    plug Turtle.AuthPlug
  end

  scope "/auth", Turtle do
    pipe_through :auth
    get "/:provider", AuthController, :request
    get "/:provider/callback", AuthController, :callback
  end
defmodule Turtle.AuthPlug do
  import Plug.Conn

  @packable_params ["redirect_uri"]

  def init(options), do: options

  def call(conn, _opts) do
    conn = fetch_query_params(conn)
    params = conn.params

    new_params = case params do
      %{"code" => _, "state" => _} ->
        decode(params)
      %{"state" => _} ->
        encode(params)
      _ -> params
    end

    update_params(conn, new_params)
  end

  defp encode(params) do
    param_subset = Map.take(params, ["state" | @packable_params])
    packed_state = __MODULE__.Packer.pack(param_subset)
    params
      |> Map.drop(@packable_params)
      |> Map.put("state", packed_state)
  end

  defp decode(params = %{"state" => packed_state}) do
    unpacked_params = __MODULE__.Packer.unpack(packed_state)
    Map.merge(params, unpacked_params)
  end
  defp decode(params), do: params

  defp update_params(conn, new_params) do
    put_in(conn.params, new_params)
  end

  defmodule Packer do
    def pack(params) when is_map(params) do
      signed = JOSE.JWT.sign(jwk, jws, params)

      {_, encoded_state} = JOSE.JWS.compact(signed)

      encoded_state
    end

    def unpack(state) do
      {true, jwt, _} = JOSE.JWT.verify(jwk, state)
      jwt.fields
    end

    defp jwk do
       %{"kty" => "oct", "k" => secret_key}
    end

    defp jws do
      %{"alg" => "HS256"}
    end

    defp secret_key do
      env = Application.get_env(:turtle, Turtle.AuthPlug.Packer)
      env[:secret_key] |> :base64url.encode
    end
  end

end

@hassox
Copy link
Member

hassox commented Mar 20, 2016

Nice! Do you think this is something that should go into Ueberauth and be available to all the OAuth 2 strategies?

@develop7
Copy link

@hassox I, for one, want this functionality in Ueberauth.

@venkatd
Copy link
Author

venkatd commented Mar 27, 2016

@hassox yes I think it should as connecting multiple accounts is fairly common.

Would be good to get some feedback on improving the current implementation... I've been using Elixir ~2 months so might not be doing things idiomatically.

@stevedomin
Copy link

I've implemented something similar, I guess it would be nice to have it built into Ueberauth.

I've loosely followed this draft: https://tools.ietf.org/html/draft-bradley-oauth-jwt-encoded-state-03

@aphillipo
Copy link

aphillipo commented Sep 19, 2016

Is state available in all oauth providers? I read on stackoverflow somewhere that facebook had broken theirs, but I'd need to check for myself...

I'm just going to use the session and delete the redirect path on errors or successes and have a sensible default.

@stevedomin
Copy link

@aphillipo not sure about all oauth providers but I didn't have any issues with both Google and Facebook so far

@aphillipo
Copy link

Gah, I've built this using sessions... might switch over then. @stevedomin how are you writing tests for say a Facebook or Google login - do you test the whole flow and mock facebook, say, or just the bits you can? Sorry that I won't see you at Elixir London I'm in Japan :-D

@stevedomin
Copy link

I want to mock the whole flow but I haven't done it yet.

(You missed out, Elixir London was great!)

@mikeni
Copy link

mikeni commented Jan 16, 2017

Gonna try to implement both oauth2 request state and session state, I trust the sessions more than the state, need something to fallback on.

@hassox
Copy link
Member

hassox commented May 24, 2017

Hey folks. Sorry this issue has dragged on for a bit. I'd like to get it tidied up.

I'm not sure I'm following the approach here now I look back on it. If you want to tie multiple accounts together to a single user resource I'm not sure ueberauth needs to really get involved does it?

In your call back controller function, you will have the result of the callback, and the session already, so if your user is logged in you can just use the verify session + load resource guardian plug (or similar) to load the current user into your callback controller (just don't use ensure authenticated). That will give you the current user if there is one. You can use that to tie it together. What am I missing?

@aphillipo
Copy link

aphillipo commented May 24, 2017

I think the discussion descended into redirect from oauth2 providers and maintaining state. I could probably get a pull request done tonight (Japanese time)?

Also I already have the ability to connect multiple accounts with Ueberauth and Guardian - @hassox you wrote the example my code is based upon! Thanks!

https://github.com/hassox/phoenix_guardian/blob/ueberauth-guardian/web/controllers/auth_controller.ex

https://github.com/hassox/phoenix_guardian/blob/ueberauth-guardian/web/auth/user_from_auth.ex

@yordis yordis added this to the v1.1 milestone Dec 24, 2018
@Hanspagh
Copy link
Contributor

Loosely related to #125

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants