-
Notifications
You must be signed in to change notification settings - Fork 118
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
Comments
Here's what I ended up doing... I made a plug that packs up the query params into the state property which persists between Usage For now I have hardcoded the fields to 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 |
Nice! Do you think this is something that should go into Ueberauth and be available to all the OAuth 2 strategies? |
@hassox I, for one, want this functionality in Ueberauth. |
@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. |
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 |
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. |
@aphillipo not sure about all oauth providers but I didn't have any issues with both Google and Facebook so far |
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 |
I want to mock the whole flow but I haven't done it yet. (You missed out, Elixir London was great!) |
Gonna try to implement both oauth2 request state and session state, I trust the sessions more than the state, need something to fallback on. |
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? |
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/auth/user_from_auth.ex |
Loosely related to #125 |
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:
/auth/google
/auth/google?access_token=access_token_abc
BUT also passing along the auth details of the currently logged in user./auth/google/callback
, tie the account back to the original user.. meaning I have access toaccess_token_abc
Should I encode the info in the
state
OAuth property? Should I cache oauth state => user_id? Or something else entirely?Thanks!
The text was updated successfully, but these errors were encountered: