From 4df2c3e8239481b2be6a9f7c43def39494482a1d Mon Sep 17 00:00:00 2001 From: netpro2k Date: Tue, 27 Oct 2020 20:19:15 -0700 Subject: [PATCH 01/11] First pass at a basic OIDC flow --- lib/ret/oauth_token.ex | 14 ++ lib/ret_web/channels/oidc_auth_channel.ex | 202 ++++++++++++++++++++++ lib/ret_web/channels/session_socket.ex | 1 + 3 files changed, 217 insertions(+) create mode 100644 lib/ret_web/channels/oidc_auth_channel.ex diff --git a/lib/ret/oauth_token.ex b/lib/ret/oauth_token.ex index a23f765be..af0557601 100644 --- a/lib/ret/oauth_token.ex +++ b/lib/ret/oauth_token.ex @@ -36,6 +36,20 @@ defmodule Ret.OAuthToken do token end + + def token_for_oidc_request(topic_key, session_id) do + {:ok, token, _claims} = + Ret.OAuthToken.encode_and_sign( + # OAuthTokens do not have a resource associated with them + nil, + %{topic_key: topic_key, session_id: session_id, aud: :ret_oidc}, + allowed_algos: ["HS512"], + ttl: {10, :minutes}, + allowed_drift: 60 * 1000 + ) + + token + end end defmodule Ret.OAuthTokenSecretFetcher do diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex new file mode 100644 index 000000000..f89038c60 --- /dev/null +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -0,0 +1,202 @@ +defmodule RetWeb.OIDCAuthChannel do + @moduledoc "Ret Web Channel for OIDC Authentication" + + use RetWeb, :channel + import Canada, only: [can?: 2] + + alias Ret.{Statix, Account, OAuthToken} + + intercept(["auth_credentials"]) + + def join("oidc:" <> _topic_key, _payload, socket) do + # Expire channel in 5 minutes + Process.send_after(self(), :channel_expired, 60 * 1000 * 5) + + # Rate limit joins to reduce attack surface + :timer.sleep(500) + + Statix.increment("ret.channels.oidc.joins.ok") + {:ok, %{session_id: socket.assigns.session_id}, socket} + end + + defp module_config(key) do + Application.get_env(:ret, __MODULE__)[key] + end + + defp get_redirect_uri(), do: RetWeb.Endpoint.url() <> "/verify" + + defp get_authorize_url(state, nonce) do + "#{module_config(:endpoint)}authorize?" <> + URI.encode_query(%{ + response_type: "code", + response_mode: "query", + client_id: module_config(:client_id), + scope: module_config(:scopes), + state: state, + nonce: nonce, + redirect_uri: get_redirect_uri() + }) + end + + def handle_in("auth_request", _payload, socket) do + if Map.get(socket.assigns, :nonce) do + {:reply, {:error, "Already started an auth request on this session"}, socket} + else + "oidc:" <> topic_key = socket.topic + oidc_state = Ret.OAuthToken.token_for_oidc_request(topic_key, socket.assigns.session_id) + nonce = SecureRandom.uuid() + authorize_url = get_authorize_url(oidc_state, nonce) + + socket = socket |> assign(:nonce, nonce) + + IO.inspect("Started oauth flow with oidc_state #{oidc_state}, authorize_url: #{authorize_url}") + {:reply, {:ok, %{authorize_url: authorize_url}}, socket} + end + end + + def handle_in("auth_verified", %{"token" => code, "payload" => state}, socket) do + Process.send_after(self(), :close_channel, 1000 * 5) + + IO.inspect("Verify OIDC auth!!!!!") + + # Slow down token guessing + :timer.sleep(500) + + "oidc:" <> expected_topic_key = socket.topic + + # TODO since we already have session_id secured by a token on the other end using a JWT for state may be overkill + case OAuthToken.decode_and_verify(state) do + {:ok, + %{ + "topic_key" => topic_key, + "session_id" => session_id, + "aud" => "ret_oidc" + }} + when topic_key == expected_topic_key -> + %{ + "access_token" => access_token, + "id_token" => raw_id_token + } = fetch_oidc_access_token(code) + + # TODO lookup pubkey by kid in header + %JOSE.JWS{fields: %{"kid" => kid}} = JOSE.JWT.peek_protected(raw_id_token) + IO.inspect(kid) + + pub_key = module_config(:verification_key) |> JOSE.JWK.from_pem() + + case JOSE.JWT.verify_strict(pub_key, module_config(:allowed_algos), raw_id_token) + |> IO.inspect() do + {true, + %JOSE.JWT{ + fields: %{ + "aud" => _aud, + "nonce" => nonce, + "preferred_username" => remote_username, + "sub" => remote_user_id + } + }, _jws} -> + # TODO we may want to verify some more fields like issuer and expiration time + + # %{"sub" => remote_user_id, "preferred_username" => remote_username} = + # fetch_oidc_user_info(access_token) |> IO.inspect() + + broadcast_credentials_and_payload( + remote_user_id, + %{email: remote_username}, + %{session_id: session_id, nonce: nonce}, + socket + ) + + {:reply, :ok, socket} + + {false, _jwt, _jws} -> + {:reply, {:error, %{msg: "invalid OIDC token from endpoint"}}, socket} + + {:error, _} -> + {:reply, {:error, %{msg: "error verifying token"}}, socket} + end + + # TODO we may want to be less specific about errors + {:ok, _} -> + {:reply, {:error, %{msg: "Invalid topic key"}}, socket} + + {:error, error} -> + {:reply, {:error, error}, socket} + end + end + + def fetch_oidc_access_token(oauth_code) do + body = { + :form, + [ + client_id: module_config(:client_id), + client_secret: module_config(:client_secret), + grant_type: "authorization_code", + redirect_uri: get_redirect_uri(), + code: oauth_code, + scope: module_config(:scopes) + ] + } + + # todo handle error response + "#{module_config(:endpoint)}token" + |> Ret.HttpUtils.retry_post_until_success(body, [{"content-type", "application/x-www-form-urlencoded"}]) + |> Map.get(:body) + |> Poison.decode!() + end + + # def fetch_oidc_user_info(access_token) do + # "#{module_config(:endpoint)}userinfo" + # |> Ret.HttpUtils.retry_get_until_success([{"authorization", "Bearer #{access_token}"}]) + # |> Map.get(:body) + # |> Poison.decode!() + # |> IO.inspect() + # end + + def handle_in(_event, _payload, socket) do + {:noreply, socket} + end + + def handle_info(:close_channel, socket) do + GenServer.cast(self(), :close) + {:noreply, socket} + end + + def handle_info(:channel_expired, socket) do + GenServer.cast(self(), :close) + {:noreply, socket} + end + + def handle_out( + "auth_credentials" = event, + %{credentials: credentials, user_info: user_info, verification_info: verification_info}, + socket + ) do + Process.send_after(self(), :close_channel, 1000 * 5) + IO.inspect("checking creds") + IO.inspect(socket) + IO.inspect(verification_info) + + if Map.get(socket.assigns, :session_id) == Map.get(verification_info, :session_id) and + Map.get(socket.assigns, :nonce) == Map.get(verification_info, :nonce) do + IO.inspect("sending creds") + push(socket, event, %{credentials: credentials, user_info: user_info}) + end + + {:noreply, socket} + end + + defp broadcast_credentials_and_payload(nil, _user_info, _verification_info, _socket), do: nil + + defp broadcast_credentials_and_payload(identifier_hash, user_info, verification_info, socket) do + account_creation_enabled = can?(nil, create_account()) + account = identifier_hash |> Account.account_for_login_identifier_hash(account_creation_enabled) + credentials = account |> Account.credentials_for_account() + + broadcast!(socket, "auth_credentials", %{ + credentials: credentials, + user_info: user_info, + verification_info: verification_info + }) + end +end diff --git a/lib/ret_web/channels/session_socket.ex b/lib/ret_web/channels/session_socket.ex index f2efb92d4..a99bdc8c8 100644 --- a/lib/ret_web/channels/session_socket.ex +++ b/lib/ret_web/channels/session_socket.ex @@ -5,6 +5,7 @@ defmodule RetWeb.SessionSocket do channel("hub:*", RetWeb.HubChannel) channel("link:*", RetWeb.LinkChannel) channel("auth:*", RetWeb.AuthChannel) + channel("oidc:*", RetWeb.OIDCAuthChannel) def id(socket) do "session:#{socket.assigns.session_id}" From 4251e2fca23c8fba6f2a014cb62017073b6b14fd Mon Sep 17 00:00:00 2001 From: netpro2k Date: Wed, 28 Oct 2020 19:18:05 -0700 Subject: [PATCH 02/11] Cleanup id token handling --- lib/ret/remote_oidc_token.ex | 25 ++++ lib/ret_web/channels/oidc_auth_channel.ex | 133 ++++++++++------------ 2 files changed, 83 insertions(+), 75 deletions(-) create mode 100644 lib/ret/remote_oidc_token.ex diff --git a/lib/ret/remote_oidc_token.ex b/lib/ret/remote_oidc_token.ex new file mode 100644 index 000000000..3661ff5eb --- /dev/null +++ b/lib/ret/remote_oidc_token.ex @@ -0,0 +1,25 @@ +defmodule Ret.RemoteOIDCToken do + @moduledoc """ + This represents an OpenID Connect token returned from a remote service. + The public keys will be configured by an admin for a particular setup. + """ + use Guardian, + otp_app: :ret, + secret_fetcher: Ret.RemoteOIDCTokenSecretsFetcher + + def subject_for_token(_, _), do: {:ok, nil} + def resource_from_claims(_), do: {:ok, nil} +end + +defmodule Ret.RemoteOIDCTokenSecretsFetcher do + def fetch_signing_secret(_mod, _opts) do + {:error, :not_implemented} + end + + def fetch_verifying_secret(mod, token_headers, _opts) do + IO.inspect(token_headers) + + # TODO use KID to look up the key. Do we want to bake it into the config at setup time? Fetch and cache? Should it be optional? + {:ok, Application.get_env(:ret, mod)[:verification_key] |> JOSE.JWK.from_pem()} |> IO.inspect() + end +end diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index f89038c60..d4e5f06c9 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -1,10 +1,10 @@ defmodule RetWeb.OIDCAuthChannel do - @moduledoc "Ret Web Channel for OIDC Authentication" + @moduledoc "Ret Web Channel for OpenID Connect Authentication" use RetWeb, :channel import Canada, only: [can?: 2] - alias Ret.{Statix, Account, OAuthToken} + alias Ret.{Statix, Account, OAuthToken, RemoteOIDCToken} intercept(["auth_credentials"]) @@ -65,84 +65,66 @@ defmodule RetWeb.OIDCAuthChannel do "oidc:" <> expected_topic_key = socket.topic # TODO since we already have session_id secured by a token on the other end using a JWT for state may be overkill - case OAuthToken.decode_and_verify(state) do - {:ok, - %{ - "topic_key" => topic_key, - "session_id" => session_id, - "aud" => "ret_oidc" - }} - when topic_key == expected_topic_key -> - %{ - "access_token" => access_token, - "id_token" => raw_id_token - } = fetch_oidc_access_token(code) - - # TODO lookup pubkey by kid in header - %JOSE.JWS{fields: %{"kid" => kid}} = JOSE.JWT.peek_protected(raw_id_token) - IO.inspect(kid) - - pub_key = module_config(:verification_key) |> JOSE.JWK.from_pem() - - case JOSE.JWT.verify_strict(pub_key, module_config(:allowed_algos), raw_id_token) - |> IO.inspect() do - {true, - %JOSE.JWT{ - fields: %{ - "aud" => _aud, - "nonce" => nonce, - "preferred_username" => remote_username, - "sub" => remote_user_id - } - }, _jws} -> - # TODO we may want to verify some more fields like issuer and expiration time - - # %{"sub" => remote_user_id, "preferred_username" => remote_username} = - # fetch_oidc_user_info(access_token) |> IO.inspect() - - broadcast_credentials_and_payload( - remote_user_id, - %{email: remote_username}, - %{session_id: session_id, nonce: nonce}, - socket - ) - - {:reply, :ok, socket} - - {false, _jwt, _jws} -> - {:reply, {:error, %{msg: "invalid OIDC token from endpoint"}}, socket} - - {:error, _} -> - {:reply, {:error, %{msg: "error verifying token"}}, socket} - end - - # TODO we may want to be less specific about errors - {:ok, _} -> - {:reply, {:error, %{msg: "Invalid topic key"}}, socket} + with {:ok, + %{ + "topic_key" => topic_key, + "session_id" => session_id, + "aud" => "ret_oidc" + }} + when topic_key == expected_topic_key <- OAuthToken.decode_and_verify(state), + {:ok, + %{ + "access_token" => access_token, + "id_token" => raw_id_token + }} <- fetch_oidc_tokens(code), + {:ok, + %{ + "aud" => _aud, + "nonce" => nonce, + "preferred_username" => remote_username, + "sub" => remote_user_id + }} <- RemoteOIDCToken.decode_and_verify(raw_id_token) do + # TODO we may want to verify some more fields like issuer and expiration time + + broadcast_credentials_and_payload( + remote_user_id, + %{email: remote_username}, + %{session_id: session_id, nonce: nonce}, + socket + ) + {:reply, :ok, socket} + else + # TODO we may want to be less specific about errors and or immediatly disconnect to prevent abuse {:error, error} -> + # GenServer.cast(self(), :close) {:reply, {:error, error}, socket} + + v -> + # GenServer.cast(self(), :close) + IO.inspect(v) + {:reply, {:error, %{msg: "error fetching or verifying token"}}, socket} end end - def fetch_oidc_access_token(oauth_code) do - body = { - :form, - [ - client_id: module_config(:client_id), - client_secret: module_config(:client_secret), - grant_type: "authorization_code", - redirect_uri: get_redirect_uri(), - code: oauth_code, - scope: module_config(:scopes) - ] - } - - # todo handle error response - "#{module_config(:endpoint)}token" - |> Ret.HttpUtils.retry_post_until_success(body, [{"content-type", "application/x-www-form-urlencoded"}]) - |> Map.get(:body) - |> Poison.decode!() + def fetch_oidc_tokens(oauth_code) do + body = + {:form, + [ + client_id: module_config(:client_id), + client_secret: module_config(:client_secret), + grant_type: "authorization_code", + redirect_uri: get_redirect_uri(), + code: oauth_code, + scope: module_config(:scopes) + ]} + + headers = [{"content-type", "application/x-www-form-urlencoded"}] + + case Ret.HttpUtils.retry_post_until_success("#{module_config(:endpoint)}token", body, headers) do + %HTTPoison.Response{body: body} -> body |> Poison.decode() + _ -> {:error, "Failed to fetch tokens"} + end end # def fetch_oidc_user_info(access_token) do @@ -167,6 +149,7 @@ defmodule RetWeb.OIDCAuthChannel do {:noreply, socket} end + # Only send creddentials back down to the original socket that started the request def handle_out( "auth_credentials" = event, %{credentials: credentials, user_info: user_info, verification_info: verification_info}, @@ -189,7 +172,7 @@ defmodule RetWeb.OIDCAuthChannel do defp broadcast_credentials_and_payload(nil, _user_info, _verification_info, _socket), do: nil defp broadcast_credentials_and_payload(identifier_hash, user_info, verification_info, socket) do - account_creation_enabled = can?(nil, create_account()) + account_creation_enabled = can?(nil, create_account(nil)) account = identifier_hash |> Account.account_for_login_identifier_hash(account_creation_enabled) credentials = account |> Account.credentials_for_account() From 6b103554c4dbf39c94b2786ba68599b349293188 Mon Sep 17 00:00:00 2001 From: netpro2k Date: Thu, 12 Nov 2020 17:23:01 -0800 Subject: [PATCH 03/11] Handle using multiple public keys for OIDC --- lib/ret/remote_oidc_token.ex | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/ret/remote_oidc_token.ex b/lib/ret/remote_oidc_token.ex index 3661ff5eb..5fae1ede2 100644 --- a/lib/ret/remote_oidc_token.ex +++ b/lib/ret/remote_oidc_token.ex @@ -1,7 +1,7 @@ defmodule Ret.RemoteOIDCToken do @moduledoc """ This represents an OpenID Connect token returned from a remote service. - The public keys will be configured by an admin for a particular setup. + These tokens are never created locally, only ever provided externally and verified locally. """ use Guardian, otp_app: :ret, @@ -12,14 +12,27 @@ defmodule Ret.RemoteOIDCToken do end defmodule Ret.RemoteOIDCTokenSecretsFetcher do + @moduledoc """ + This represents the public keys for an OpenID Connect endpoint used to verify tokens. + The public keys will be configured by an admin for a particular setup. These can not be used for signing. + """ + def fetch_signing_secret(_mod, _opts) do {:error, :not_implemented} end - def fetch_verifying_secret(mod, token_headers, _opts) do - IO.inspect(token_headers) + def fetch_verifying_secret(mod, %{"kid" => kid, "typ" => "JWT"}, _opts) do + # TODO implement read through cache that hits discovery endpoint instead of hardcoding keys in config + case Application.get_env(:ret, mod)[:verification_jwks] + |> Poison.decode!() + |> Map.get("keys") + |> Enum.find(&(Map.get(&1, "kid") == kid)) do + nil -> {:error, :invalid_key_id} + key -> {:ok, key |> JOSE.JWK.from_map()} + end + end - # TODO use KID to look up the key. Do we want to bake it into the config at setup time? Fetch and cache? Should it be optional? - {:ok, Application.get_env(:ret, mod)[:verification_key] |> JOSE.JWK.from_pem()} |> IO.inspect() + def fetch_verifying_secret(_mod, _token_headers_, _optss) do + {:error, :invalid_token} end end From b4a595147650fde893a6316ff15034127a59b579 Mon Sep 17 00:00:00 2001 From: netpro2k Date: Thu, 12 Nov 2020 17:25:09 -0800 Subject: [PATCH 04/11] Don't assume name of authorize/token endponts --- lib/ret_web/channels/oidc_auth_channel.ex | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index d4e5f06c9..c12570cf1 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -26,7 +26,7 @@ defmodule RetWeb.OIDCAuthChannel do defp get_redirect_uri(), do: RetWeb.Endpoint.url() <> "/verify" defp get_authorize_url(state, nonce) do - "#{module_config(:endpoint)}authorize?" <> + "#{module_config(:auth_endpoint)}?" <> URI.encode_query(%{ response_type: "code", response_mode: "query", @@ -121,14 +121,14 @@ defmodule RetWeb.OIDCAuthChannel do headers = [{"content-type", "application/x-www-form-urlencoded"}] - case Ret.HttpUtils.retry_post_until_success("#{module_config(:endpoint)}token", body, headers) do + case Ret.HttpUtils.retry_post_until_success("#{module_config(:token_endpoint)}", body, headers) do %HTTPoison.Response{body: body} -> body |> Poison.decode() _ -> {:error, "Failed to fetch tokens"} end end # def fetch_oidc_user_info(access_token) do - # "#{module_config(:endpoint)}userinfo" + # "#{module_config(:userinfo_endpoint)}" # |> Ret.HttpUtils.retry_get_until_success([{"authorization", "Bearer #{access_token}"}]) # |> Map.get(:body) # |> Poison.decode!() From 9239a476c5c5353388f99901bc1e2f5f8c720aaf Mon Sep 17 00:00:00 2001 From: netpro2k Date: Thu, 12 Nov 2020 17:26:49 -0800 Subject: [PATCH 05/11] Better displayname handling --- lib/ret_web/channels/oidc_auth_channel.ex | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index c12570cf1..646dc1620 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -81,14 +81,20 @@ defmodule RetWeb.OIDCAuthChannel do %{ "aud" => _aud, "nonce" => nonce, - "preferred_username" => remote_username, "sub" => remote_user_id - }} <- RemoteOIDCToken.decode_and_verify(raw_id_token) do + } = id_token} <- RemoteOIDCToken.decode_and_verify(raw_id_token) do # TODO we may want to verify some more fields like issuer and expiration time + displayname = + id_token + |> Map.get( + "preferred_username", + id_token |> Map.get("name", remote_user_id) + ) + broadcast_credentials_and_payload( remote_user_id, - %{email: remote_username}, + %{displayName: displayname}, %{session_id: session_id, nonce: nonce}, socket ) @@ -98,12 +104,12 @@ defmodule RetWeb.OIDCAuthChannel do # TODO we may want to be less specific about errors and or immediatly disconnect to prevent abuse {:error, error} -> # GenServer.cast(self(), :close) - {:reply, {:error, error}, socket} + {:reply, {:error, %{message: error}}, socket} v -> # GenServer.cast(self(), :close) IO.inspect(v) - {:reply, {:error, %{msg: "error fetching or verifying token"}}, socket} + {:reply, {:error, %{message: "error fetching or verifying token"}}, socket} end end From 59e3edc1a2b836cd47d4856cd14e63d4e56718b5 Mon Sep 17 00:00:00 2001 From: netpro2k Date: Tue, 17 Nov 2020 18:50:39 -0800 Subject: [PATCH 06/11] Add habitat config for OIDC --- habitat/config/config.toml | 15 +++++++++++++++ lib/ret/remote_oidc_token.ex | 4 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/habitat/config/config.toml b/habitat/config/config.toml index e937f483c..45d98a293 100644 --- a/habitat/config/config.toml +++ b/habitat/config/config.toml @@ -299,6 +299,21 @@ realm = "{{ cfg.turn.realm }}" public_tls_ports = "{{ cfg.turn.public_tls_ports }}" {{/if}} +{{#if cfg.oidc }} +{{#with cfg.oidc }} +[ret."Elixir.RetWeb.OIDCAuthChannel"] +{{ toToml auth_endpoint }} +{{ toToml token_endpoint }} +{{ toToml scopes }} +{{ toToml client_id }} +{{ toToml client_secret }} + +[ret."Elixir.Ret.RemoteOIDCToken"] +{{ toToml allowed_algos }} +{{ toToml verification_secret_jwk_set }} +{{/with}} +{{/if}} + [web_push_encryption.vapid_details] subject = "{{ cfg.web_push.subject }}" public_key = "{{ cfg.web_push.public_key }}" diff --git a/lib/ret/remote_oidc_token.ex b/lib/ret/remote_oidc_token.ex index 5fae1ede2..64140ff16 100644 --- a/lib/ret/remote_oidc_token.ex +++ b/lib/ret/remote_oidc_token.ex @@ -22,8 +22,8 @@ defmodule Ret.RemoteOIDCTokenSecretsFetcher do end def fetch_verifying_secret(mod, %{"kid" => kid, "typ" => "JWT"}, _opts) do - # TODO implement read through cache that hits discovery endpoint instead of hardcoding keys in config - case Application.get_env(:ret, mod)[:verification_jwks] + # TODO implement read through cache that hits discovery endpoint instead of hardcoding keys in config as per https://openid.net/specs/openid-connect-core-1_0.html#RotateSigKeys + case Application.get_env(:ret, mod)[:verification_secret_jwk_set] |> Poison.decode!() |> Map.get("keys") |> Enum.find(&(Map.get(&1, "kid") == kid)) do From 5f507f543399eddfb01f98faf1b014e1932dc629 Mon Sep 17 00:00:00 2001 From: netpro2k Date: Fri, 20 Nov 2020 17:04:29 -0800 Subject: [PATCH 07/11] Fix escaping and formatting in habitat config --- habitat/config/config.toml | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/habitat/config/config.toml b/habitat/config/config.toml index 45d98a293..7c6394842 100644 --- a/habitat/config/config.toml +++ b/habitat/config/config.toml @@ -302,15 +302,17 @@ public_tls_ports = "{{ cfg.turn.public_tls_ports }}" {{#if cfg.oidc }} {{#with cfg.oidc }} [ret."Elixir.RetWeb.OIDCAuthChannel"] -{{ toToml auth_endpoint }} -{{ toToml token_endpoint }} -{{ toToml scopes }} -{{ toToml client_id }} -{{ toToml client_secret }} +auth_endpoint = {{ toToml auth_endpoint }} +token_endpoint = {{ toToml token_endpoint }} +scopes = {{ toToml scopes }} +client_id = {{ toToml client_id }} +client_secret = {{ toToml client_secret }} [ret."Elixir.Ret.RemoteOIDCToken"] -{{ toToml allowed_algos }} -{{ toToml verification_secret_jwk_set }} +allowed_algos = {{ toToml allowed_algos }} +verification_secret_jwk_set = """ +{{ verification_secret_jwk_set }} +""" {{/with}} {{/if}} From 888555bc3926dbee1db88bdb6d296f396d7e3a4d Mon Sep 17 00:00:00 2001 From: netpro2k Date: Tue, 1 Dec 2020 17:00:39 -0800 Subject: [PATCH 08/11] Delint --- lib/ret_web/channels/oidc_auth_channel.ex | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index 646dc1620..655d7a8e3 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -74,7 +74,7 @@ defmodule RetWeb.OIDCAuthChannel do when topic_key == expected_topic_key <- OAuthToken.decode_and_verify(state), {:ok, %{ - "access_token" => access_token, + "access_token" => _access_token, "id_token" => raw_id_token }} <- fetch_oidc_tokens(code), {:ok, @@ -113,6 +113,10 @@ defmodule RetWeb.OIDCAuthChannel do end end + def handle_in(_event, _payload, socket) do + {:noreply, socket} + end + def fetch_oidc_tokens(oauth_code) do body = {:form, @@ -141,10 +145,6 @@ defmodule RetWeb.OIDCAuthChannel do # |> IO.inspect() # end - def handle_in(_event, _payload, socket) do - {:noreply, socket} - end - def handle_info(:close_channel, socket) do GenServer.cast(self(), :close) {:noreply, socket} From 1a6339c397a9dcdedb172b2f524fbfbb19ed28b2 Mon Sep 17 00:00:00 2001 From: netpro2k Date: Tue, 1 Dec 2020 17:05:44 -0800 Subject: [PATCH 09/11] Cleanup dead code and remove debugging --- lib/ret_web/channels/oidc_auth_channel.ex | 27 ++++------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index 655d7a8e3..b7985d66b 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -49,7 +49,6 @@ defmodule RetWeb.OIDCAuthChannel do socket = socket |> assign(:nonce, nonce) - IO.inspect("Started oauth flow with oidc_state #{oidc_state}, authorize_url: #{authorize_url}") {:reply, {:ok, %{authorize_url: authorize_url}}, socket} end end @@ -57,14 +56,11 @@ defmodule RetWeb.OIDCAuthChannel do def handle_in("auth_verified", %{"token" => code, "payload" => state}, socket) do Process.send_after(self(), :close_channel, 1000 * 5) - IO.inspect("Verify OIDC auth!!!!!") - - # Slow down token guessing + # Slow down any brute force attacks :timer.sleep(500) "oidc:" <> expected_topic_key = socket.topic - # TODO since we already have session_id secured by a token on the other end using a JWT for state may be overkill with {:ok, %{ "topic_key" => topic_key, @@ -101,14 +97,11 @@ defmodule RetWeb.OIDCAuthChannel do {:reply, :ok, socket} else - # TODO we may want to be less specific about errors and or immediatly disconnect to prevent abuse - {:error, error} -> - # GenServer.cast(self(), :close) - {:reply, {:error, %{message: error}}, socket} + # intentionally not exposing the nature of the error, can uncomment this to return more details to the client + # {:error, error} -> + # {:reply, {:error, %{message: error}}, socket} v -> - # GenServer.cast(self(), :close) - IO.inspect(v) {:reply, {:error, %{message: "error fetching or verifying token"}}, socket} end end @@ -137,14 +130,6 @@ defmodule RetWeb.OIDCAuthChannel do end end - # def fetch_oidc_user_info(access_token) do - # "#{module_config(:userinfo_endpoint)}" - # |> Ret.HttpUtils.retry_get_until_success([{"authorization", "Bearer #{access_token}"}]) - # |> Map.get(:body) - # |> Poison.decode!() - # |> IO.inspect() - # end - def handle_info(:close_channel, socket) do GenServer.cast(self(), :close) {:noreply, socket} @@ -162,13 +147,9 @@ defmodule RetWeb.OIDCAuthChannel do socket ) do Process.send_after(self(), :close_channel, 1000 * 5) - IO.inspect("checking creds") - IO.inspect(socket) - IO.inspect(verification_info) if Map.get(socket.assigns, :session_id) == Map.get(verification_info, :session_id) and Map.get(socket.assigns, :nonce) == Map.get(verification_info, :nonce) do - IO.inspect("sending creds") push(socket, event, %{credentials: credentials, user_info: user_info}) end From a645f247c9d39a139ac13ae9f2b2a9337c38e8eb Mon Sep 17 00:00:00 2001 From: netpro2k Date: Tue, 1 Dec 2020 17:15:22 -0800 Subject: [PATCH 10/11] delint --- lib/ret_web/channels/oidc_auth_channel.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index b7985d66b..04773fc5f 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -101,7 +101,7 @@ defmodule RetWeb.OIDCAuthChannel do # {:error, error} -> # {:reply, {:error, %{message: error}}, socket} - v -> + _ -> {:reply, {:error, %{message: "error fetching or verifying token"}}, socket} end end From 60aa6d28e30b72513845afbd8c27360b9b39b958 Mon Sep 17 00:00:00 2001 From: netpro2k Date: Tue, 1 Dec 2020 17:18:25 -0800 Subject: [PATCH 11/11] No need to make scopes configurable right now --- lib/ret_web/channels/oidc_auth_channel.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index 04773fc5f..45dc8e5da 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -31,7 +31,7 @@ defmodule RetWeb.OIDCAuthChannel do response_type: "code", response_mode: "query", client_id: module_config(:client_id), - scope: module_config(:scopes), + scope: "openid profile", state: state, nonce: nonce, redirect_uri: get_redirect_uri() @@ -119,7 +119,7 @@ defmodule RetWeb.OIDCAuthChannel do grant_type: "authorization_code", redirect_uri: get_redirect_uri(), code: oauth_code, - scope: module_config(:scopes) + scope: "openid profile" ]} headers = [{"content-type", "application/x-www-form-urlencoded"}]