From 4df2c3e8239481b2be6a9f7c43def39494482a1d Mon Sep 17 00:00:00 2001 From: netpro2k Date: Tue, 27 Oct 2020 20:19:15 -0700 Subject: [PATCH 01/34] 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/34] 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/34] 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/34] 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/34] 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/34] 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/34] 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/34] 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/34] 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/34] 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/34] 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"}] From a17b7b13f22a96ffe41efbdec16a43386546322b Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Fri, 14 Oct 2022 14:45:08 +0100 Subject: [PATCH 12/34] OIDC example credentials for local testing --- config/dev.exs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/config/dev.exs b/config/dev.exs index 03fb9b372..e0ea88d60 100644 --- a/config/dev.exs +++ b/config/dev.exs @@ -232,3 +232,18 @@ config :ret, Ret.Locking, config :ret, Ret.Repo.Migrations.AdminSchemaInit, postgrest_password: "password" config :ret, Ret.StatsJob, node_stats_enabled: false, node_gauges_enabled: false config :ret, Ret.Coturn, realm: "ret" + +# OIDC test server https://oidctest.wsweet.org/ + +config :ret, RetWeb.OIDCAuthChannel, + auth_endpoint: "https://oidctest.wsweet.org/oauth2/authorize", + token_endpoint: "https://oidctest.wsweet.org/oauth2/token", + scopes: "openid profile email", + client_id: "private", + client_secret: "tardis" + +config :ret, Ret.RemoteOIDCToken, + allowed_algos: ["RS256"], + verification_secret_jwk_set: """ + {"keys":[{"kty":"RSA","use":"sig","kid":"oidctest","e":"AQAB","n":"wsnaUGh8VkwaR4-ZdUduiDtE9BZ1WqhkvgDzOyTmjcmsuKg7y5WhqTSk3haSi2PStzel_4i6-_3QZbKoCgfBE8mi5yOjzkn3iW-EzKY2SdQbk-0p8OSrSgbArM5wnfXK80Vui_f_K78fd_Nu3_I7uvuUoml4LIJWAsNFbavjHuWJPe6vQB3-_HsxIgQEe9U79ir2ksgTj_BJOGCTOhXoCaPAXF5U4BGi9DAXXkx_RTxd3JkQMqyIUSE3biUnrJt7pFVkzaFa5xQ07TmYiHYPBj-iiG_WaA1emuPWbsZoDYKKRsutkmm9auJLA04FbvmSBvCvuBx9A27bzPXO-0Oez6E6DxXDM4386kJQjgrFC_YIR7VRe4KEB6ylnfIpp6BQD3wmVLqKyC6_I1MRFNoHWh-8jUnuXoI9sLrx6QwwtUtQT6cXjEaW_qeQPal0OMXsUKMRGK6dG47KAAsrjWkkkufIUSU9GUbdVfAm_CePu9fRBE4tkaZn1mXXqGmDMRFosspu6aGfiKF3G7GnkRroKHdaqX2NM0y1e0gSjFppmjE__fd2azigsvXR7c8wZreNs5ZwC6OWcfERB7fB4BML7CkPtvFmLvg81Se-q1WacXRIXMzNyhpkHQA4mkheG4WIyrj43WOgxI5iLLJkTeS0b2QkKzoa9_OQMBKUzFyVat0"}]} + """ From d1dc0e5c8ffeb842a65d4602c1b8d21ad9384337 Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Fri, 14 Oct 2022 14:46:49 +0100 Subject: [PATCH 13/34] Guardian needs explicit list of signing algorithms to check --- lib/ret_web/channels/oidc_auth_channel.ex | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index 45dc8e5da..ac1565459 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -61,6 +61,8 @@ defmodule RetWeb.OIDCAuthChannel do "oidc:" <> expected_topic_key = socket.topic + allowed_algos = Application.get_env(:ret, Ret.RemoteOIDCToken)[:allowed_algos] + with {:ok, %{ "topic_key" => topic_key, @@ -78,7 +80,7 @@ defmodule RetWeb.OIDCAuthChannel do "aud" => _aud, "nonce" => nonce, "sub" => remote_user_id - } = id_token} <- RemoteOIDCToken.decode_and_verify(raw_id_token) do + } = id_token} <- RemoteOIDCToken.decode_and_verify(raw_id_token, %{}, allowed_algos: allowed_algos) do # TODO we may want to verify some more fields like issuer and expiration time displayname = From 95105b09b56dc7961fc6e3603b4a734d44a234ab Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Fri, 14 Oct 2022 14:47:53 +0100 Subject: [PATCH 14/34] Function requires options to be supplied in keyword format --- lib/ret_web/channels/oidc_auth_channel.ex | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index ac1565459..fc7592fda 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -122,11 +122,13 @@ defmodule RetWeb.OIDCAuthChannel do redirect_uri: get_redirect_uri(), code: oauth_code, scope: "openid profile" - ]} + ]} - headers = [{"content-type", "application/x-www-form-urlencoded"}] + options = [ + headers: [{"content-type", "application/x-www-form-urlencoded"}] + ] - case Ret.HttpUtils.retry_post_until_success("#{module_config(:token_endpoint)}", body, headers) do + case Ret.HttpUtils.retry_post_until_success("#{module_config(:token_endpoint)}", body, options) do %HTTPoison.Response{body: body} -> body |> Poison.decode() _ -> {:error, "Failed to fetch tokens"} end From b8d729205fb6a985b4fae8cc1e8ea5ad2dc4970c Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Fri, 14 Oct 2022 14:48:08 +0100 Subject: [PATCH 15/34] Spelling typo --- 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 fc7592fda..d300a5ce4 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -144,7 +144,7 @@ defmodule RetWeb.OIDCAuthChannel do {:noreply, socket} end - # Only send creddentials back down to the original socket that started the request + # Only send credentials 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}, From 44eabd21273da1047bc5d536eaad3041315e7755 Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Fri, 14 Oct 2022 14:50:57 +0100 Subject: [PATCH 16/34] Log OIDC errors on the server side for investigation --- lib/ret_web/channels/oidc_auth_channel.ex | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index d300a5ce4..c6265f60e 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -99,11 +99,9 @@ defmodule RetWeb.OIDCAuthChannel do {:reply, :ok, socket} else - # 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} - - _ -> + {:error, error} -> + # Error message is very limited https://github.com/ueberauth/guardian/issues/711 + Logger.warn("OIDC error: #{inspect(error)}") {:reply, {:error, %{message: "error fetching or verifying token"}}, socket} end end From cf4510d18267fb85f32626cb605282a808ad1148 Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Fri, 14 Oct 2022 16:11:45 +0100 Subject: [PATCH 17/34] Missing require import --- lib/ret_web/channels/oidc_auth_channel.ex | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index c6265f60e..10bfafcbe 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -1,6 +1,8 @@ defmodule RetWeb.OIDCAuthChannel do @moduledoc "Ret Web Channel for OpenID Connect Authentication" + require Logger + use RetWeb, :channel import Canada, only: [can?: 2] From d96a6e08bb17316dfdeacbe86b6389dbc47b1e1e Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Mon, 17 Oct 2022 15:06:53 +0100 Subject: [PATCH 18/34] Passthrough UserInfo from OIDC --- config/dev.exs | 1 + habitat/config/config.toml | 1 + lib/ret_web/channels/oidc_auth_channel.ex | 22 +++++++++++----------- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/config/dev.exs b/config/dev.exs index e0ea88d60..5c880e2ed 100644 --- a/config/dev.exs +++ b/config/dev.exs @@ -238,6 +238,7 @@ config :ret, Ret.Coturn, realm: "ret" config :ret, RetWeb.OIDCAuthChannel, auth_endpoint: "https://oidctest.wsweet.org/oauth2/authorize", token_endpoint: "https://oidctest.wsweet.org/oauth2/token", + userinfo_endpoint: "https://oidctest.wsweet.org/oauth2/userinfo", scopes: "openid profile email", client_id: "private", client_secret: "tardis" diff --git a/habitat/config/config.toml b/habitat/config/config.toml index 8a214e5bf..78f215599 100644 --- a/habitat/config/config.toml +++ b/habitat/config/config.toml @@ -306,6 +306,7 @@ public_tls_ports = "{{ cfg.turn.public_tls_ports }}" [ret."Elixir.RetWeb.OIDCAuthChannel"] auth_endpoint = {{ toToml auth_endpoint }} token_endpoint = {{ toToml token_endpoint }} +userinfo_endpoint = {{ toToml userinfo_endpoint }} scopes = {{ toToml scopes }} client_id = {{ toToml client_id }} client_secret = {{ toToml client_secret }} diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index 10bfafcbe..cff0b9ec1 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -40,6 +40,13 @@ defmodule RetWeb.OIDCAuthChannel do }) end + def fetch_user_info(access_token) do + "#{module_config(:userinfo_endpoint)}" + |> Ret.HttpUtils.retry_get_until_success(headers: [{"authorization", "Bearer #{access_token}"}]) + |> Map.get(:body) + |> Poison.decode!() + 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} @@ -74,7 +81,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, @@ -85,16 +92,9 @@ defmodule RetWeb.OIDCAuthChannel do } = id_token} <- RemoteOIDCToken.decode_and_verify(raw_id_token, %{}, allowed_algos: allowed_algos) 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, - %{displayName: displayname}, + %{oidc: fetch_user_info(access_token)}, %{session_id: session_id, nonce: nonce}, socket ) @@ -102,7 +102,7 @@ defmodule RetWeb.OIDCAuthChannel do {:reply, :ok, socket} else {:error, error} -> - # Error message is very limited https://github.com/ueberauth/guardian/issues/711 + # Error messages from Guardian are very limited https://github.com/ueberauth/guardian/issues/711 Logger.warn("OIDC error: #{inspect(error)}") {:reply, {:error, %{message: "error fetching or verifying token"}}, socket} end @@ -121,7 +121,7 @@ defmodule RetWeb.OIDCAuthChannel do grant_type: "authorization_code", redirect_uri: get_redirect_uri(), code: oauth_code, - scope: "openid profile" + scope: "openid profile email roles" ]} options = [ From 8849a406cc69ca8fef72af34f91a51be4346a82b Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Wed, 19 Oct 2022 11:39:37 +0100 Subject: [PATCH 19/34] Encapsulated OIDC setup into single RemoteOIDCClient module and simplified required parameters --- config/dev.exs | 13 +--- habitat/config/config.toml | 12 +--- lib/ret/remote_oidc_client.ex | 85 +++++++++++++++++++++++ lib/ret/remote_oidc_token.ex | 6 +- lib/ret_web/channels/oidc_auth_channel.ex | 49 +++++++------ 5 files changed, 120 insertions(+), 45 deletions(-) create mode 100644 lib/ret/remote_oidc_client.ex diff --git a/config/dev.exs b/config/dev.exs index 5c880e2ed..d2690000f 100644 --- a/config/dev.exs +++ b/config/dev.exs @@ -234,17 +234,8 @@ config :ret, Ret.StatsJob, node_stats_enabled: false, node_gauges_enabled: false config :ret, Ret.Coturn, realm: "ret" # OIDC test server https://oidctest.wsweet.org/ - -config :ret, RetWeb.OIDCAuthChannel, - auth_endpoint: "https://oidctest.wsweet.org/oauth2/authorize", - token_endpoint: "https://oidctest.wsweet.org/oauth2/token", - userinfo_endpoint: "https://oidctest.wsweet.org/oauth2/userinfo", +config :ret, Ret.RemoteOIDCClient, + openid_configuration: "https://oidctest.wsweet.org/.well-known/openid-configuration", scopes: "openid profile email", client_id: "private", client_secret: "tardis" - -config :ret, Ret.RemoteOIDCToken, - allowed_algos: ["RS256"], - verification_secret_jwk_set: """ - {"keys":[{"kty":"RSA","use":"sig","kid":"oidctest","e":"AQAB","n":"wsnaUGh8VkwaR4-ZdUduiDtE9BZ1WqhkvgDzOyTmjcmsuKg7y5WhqTSk3haSi2PStzel_4i6-_3QZbKoCgfBE8mi5yOjzkn3iW-EzKY2SdQbk-0p8OSrSgbArM5wnfXK80Vui_f_K78fd_Nu3_I7uvuUoml4LIJWAsNFbavjHuWJPe6vQB3-_HsxIgQEe9U79ir2ksgTj_BJOGCTOhXoCaPAXF5U4BGi9DAXXkx_RTxd3JkQMqyIUSE3biUnrJt7pFVkzaFa5xQ07TmYiHYPBj-iiG_WaA1emuPWbsZoDYKKRsutkmm9auJLA04FbvmSBvCvuBx9A27bzPXO-0Oez6E6DxXDM4386kJQjgrFC_YIR7VRe4KEB6ylnfIpp6BQD3wmVLqKyC6_I1MRFNoHWh-8jUnuXoI9sLrx6QwwtUtQT6cXjEaW_qeQPal0OMXsUKMRGK6dG47KAAsrjWkkkufIUSU9GUbdVfAm_CePu9fRBE4tkaZn1mXXqGmDMRFosspu6aGfiKF3G7GnkRroKHdaqX2NM0y1e0gSjFppmjE__fd2azigsvXR7c8wZreNs5ZwC6OWcfERB7fB4BML7CkPtvFmLvg81Se-q1WacXRIXMzNyhpkHQA4mkheG4WIyrj43WOgxI5iLLJkTeS0b2QkKzoa9_OQMBKUzFyVat0"}]} - """ diff --git a/habitat/config/config.toml b/habitat/config/config.toml index 78f215599..dab56e7b1 100644 --- a/habitat/config/config.toml +++ b/habitat/config/config.toml @@ -303,19 +303,11 @@ public_tls_ports = "{{ cfg.turn.public_tls_ports }}" {{#if cfg.oidc }} {{#with cfg.oidc }} -[ret."Elixir.RetWeb.OIDCAuthChannel"] -auth_endpoint = {{ toToml auth_endpoint }} -token_endpoint = {{ toToml token_endpoint }} -userinfo_endpoint = {{ toToml userinfo_endpoint }} +[ret."Elixir.Ret.RemoteOIDCClient"] +openid_configuration = {{ toToml openid_configuration }} scopes = {{ toToml scopes }} client_id = {{ toToml client_id }} client_secret = {{ toToml client_secret }} - -[ret."Elixir.Ret.RemoteOIDCToken"] -allowed_algos = {{ toToml allowed_algos }} -verification_secret_jwk_set = """ -{{ verification_secret_jwk_set }} -""" {{/with}} {{/if}} diff --git a/lib/ret/remote_oidc_client.ex b/lib/ret/remote_oidc_client.ex new file mode 100644 index 000000000..1c67b0cc0 --- /dev/null +++ b/lib/ret/remote_oidc_client.ex @@ -0,0 +1,85 @@ +defmodule Ret.RemoteOIDCClient do + @moduledoc """ + This represents an OpenID client configured via the openid_configuration parameter, + which should point to a discovery endpoint https://openid.net/specs/openid-connect-discovery-1_0.html + Downloaded configuration files openid-configuration and jwks_uri are cached indefinately. + """ + + require Logger + + defp get_openid_configuration_uri() do + Application.get_env(:ret, __MODULE__)[:openid_configuration] + end + + defp download_openid_configuration() do + Logger.info("Downloading OIDC configuration from #{get_openid_configuration_uri()}") + result = get_openid_configuration_uri() + |> Ret.HttpUtils.retry_get_until_success + |> Map.get(:body) + |> Poison.decode!() + :persistent_term.put(:openid_configuration_cache, result) + Logger.info("Downloaded OIDC configuration: #{inspect(result)}") + result + end + + defp get_openid_configuration() do + :persistent_term.get(:openid_configuration_cache, nil) || download_openid_configuration() + end + + defp get_jwks_uri() do + get_openid_configuration() |> Map.get("jwks_uri") + end + + defp download_jwks() do + Logger.info("Downloading JWKS from #{get_jwks_uri()}") + result = get_jwks_uri() + |> Ret.HttpUtils.retry_get_until_success + |> Map.get(:body) + |> Poison.decode!() + result |> IO.inspect + :persistent_term.put(:openid_jwks_cache, result) + Logger.info("Downloaded JWKS: #{inspect(result)}") + result + end + + # Public methods + + def get_jwks() do + :persistent_term.get(:openid_jwks_cache, nil) || download_jwks() + end + + def get_auth_endpoint() do + get_openid_configuration() |> Map.get("authorization_endpoint") + end + + def get_token_endpoint() do + get_openid_configuration() |> Map.get("token_endpoint") + end + + def get_allowed_algos() do + get_openid_configuration() |> Map.get("id_token_signing_alg_values_supported") + end + + def get_userinfo_endpoint() do + # Optional in spec + get_openid_configuration() |> Map.get("userinfo_endpoint") + end + + def get_scopes_supported() do + # Optional in spec + get_openid_configuration() |> Map.get("scopes_supported") + end + + def get_scopes() do + Application.get_env(:ret, __MODULE__)[:scopes] + end + + def get_client_id() do + Application.get_env(:ret, __MODULE__)[:client_id] + end + + def get_client_secret() do + Application.get_env(:ret, __MODULE__)[:client_secret] + end + +end diff --git a/lib/ret/remote_oidc_token.ex b/lib/ret/remote_oidc_token.ex index 64140ff16..303a4bd90 100644 --- a/lib/ret/remote_oidc_token.ex +++ b/lib/ret/remote_oidc_token.ex @@ -22,9 +22,9 @@ 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 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!() + # TODO force cache to refresh when unknown kid found to support key rotation + # as per https://openid.net/specs/openid-connect-core-1_0.html#RotateSigKeys + case Ret.RemoteOIDCClient.get_jwks() |> Map.get("keys") |> Enum.find(&(Map.get(&1, "kid") == kid)) do nil -> {:error, :invalid_key_id} diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index cff0b9ec1..36fc2b351 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -6,10 +6,15 @@ defmodule RetWeb.OIDCAuthChannel do use RetWeb, :channel import Canada, only: [can?: 2] - alias Ret.{Statix, Account, OAuthToken, RemoteOIDCToken} + alias Ret.{Statix, Account, OAuthToken, RemoteOIDCClient, RemoteOIDCToken} intercept(["auth_credentials"]) + # Ref https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims + @standard_claims ["sub", "name", "given_name", "family_name", "middle_name", "nickname", + "preferred_username", "profile", "picture", "website", "email", "email_verified", "gender", + "birthdate", "zoneinfo", "locale", "phone_number", "phone_number_verified", "address", "updated_at"] + def join("oidc:" <> _topic_key, _payload, socket) do # Expire channel in 5 minutes Process.send_after(self(), :channel_expired, 60 * 1000 * 5) @@ -21,18 +26,14 @@ defmodule RetWeb.OIDCAuthChannel do {: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(:auth_endpoint)}?" <> + RemoteOIDCClient.get_auth_endpoint() <> "?" <> URI.encode_query(%{ response_type: "code", response_mode: "query", - client_id: module_config(:client_id), + client_id: RemoteOIDCClient.get_client_id(), scope: "openid profile", state: state, nonce: nonce, @@ -41,10 +42,14 @@ defmodule RetWeb.OIDCAuthChannel do end def fetch_user_info(access_token) do - "#{module_config(:userinfo_endpoint)}" - |> Ret.HttpUtils.retry_get_until_success(headers: [{"authorization", "Bearer #{access_token}"}]) - |> Map.get(:body) - |> Poison.decode!() + # user info endpoint is optional + case RemoteOIDCClient.get_userinfo_endpoint() do + nil -> nil + url -> url + |> Ret.HttpUtils.retry_get_until_success(headers: [{"authorization", "Bearer #{access_token}"}]) + |> Map.get(:body) + |> Poison.decode!() + end end def handle_in("auth_request", _payload, socket) do @@ -70,7 +75,7 @@ defmodule RetWeb.OIDCAuthChannel do "oidc:" <> expected_topic_key = socket.topic - allowed_algos = Application.get_env(:ret, Ret.RemoteOIDCToken)[:allowed_algos] + allowed_algos = ["RS256"] with {:ok, %{ @@ -90,11 +95,13 @@ defmodule RetWeb.OIDCAuthChannel do "nonce" => nonce, "sub" => remote_user_id } = id_token} <- RemoteOIDCToken.decode_and_verify(raw_id_token, %{}, allowed_algos: allowed_algos) do - # TODO we may want to verify some more fields like issuer and expiration time + + # The OIDC user info endpoint is optional so assume info will be in the id token instead and filter for just the standard claims + user_info = fetch_user_info(access_token) || :maps.filter(fn key, _val -> key in @standard_claims end, id_token) broadcast_credentials_and_payload( remote_user_id, - %{oidc: fetch_user_info(access_token)}, + %{oidc: user_info}, %{session_id: session_id, nonce: nonce}, socket ) @@ -116,19 +123,19 @@ defmodule RetWeb.OIDCAuthChannel 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: "openid profile email roles" + client_id: RemoteOIDCClient.get_client_id(), + client_secret: RemoteOIDCClient.get_client_secret(), + grant_type: "authorization_code", + redirect_uri: get_redirect_uri(), + code: oauth_code, + scope: RemoteOIDCClient.get_scopes() ]} options = [ headers: [{"content-type", "application/x-www-form-urlencoded"}] ] - case Ret.HttpUtils.retry_post_until_success("#{module_config(:token_endpoint)}", body, options) do + case Ret.HttpUtils.retry_post_until_success(RemoteOIDCClient.get_token_endpoint(), body, options) do %HTTPoison.Response{body: body} -> body |> Poison.decode() _ -> {:error, "Failed to fetch tokens"} end From d5d996ebed103b1ec7db94041fde8862d2705427 Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Wed, 19 Oct 2022 11:58:40 +0100 Subject: [PATCH 20/34] Definitive list of allowed algorithms for JSON signing --- lib/ret_web/channels/oidc_auth_channel.ex | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index 36fc2b351..b01744c1c 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -12,8 +12,12 @@ defmodule RetWeb.OIDCAuthChannel do # Ref https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims @standard_claims ["sub", "name", "given_name", "family_name", "middle_name", "nickname", - "preferred_username", "profile", "picture", "website", "email", "email_verified", "gender", - "birthdate", "zoneinfo", "locale", "phone_number", "phone_number_verified", "address", "updated_at"] + "preferred_username", "profile", "picture", "website", "email", "email_verified", "gender", + "birthdate", "zoneinfo", "locale", "phone_number", "phone_number_verified", "address", "updated_at"] + + # Intersection of possible values for JSON signing https://www.rfc-editor.org/rfc/rfc7518#section-3.1 + # and algorithms supported by JOSE https://hexdocs.pm/jose/JOSE.JWS.html#module-algorithms + @supported_algorithms ["HS256", "HS384", "HS512", "RS256", "RS384", "RS512", "ES256", "ES384", "ES512", "PS256", "PS384", "PS512"] def join("oidc:" <> _topic_key, _payload, socket) do # Expire channel in 5 minutes @@ -75,8 +79,6 @@ defmodule RetWeb.OIDCAuthChannel do "oidc:" <> expected_topic_key = socket.topic - allowed_algos = ["RS256"] - with {:ok, %{ "topic_key" => topic_key, @@ -94,7 +96,7 @@ defmodule RetWeb.OIDCAuthChannel do "aud" => _aud, "nonce" => nonce, "sub" => remote_user_id - } = id_token} <- RemoteOIDCToken.decode_and_verify(raw_id_token, %{}, allowed_algos: allowed_algos) do + } = id_token} <- RemoteOIDCToken.decode_and_verify(raw_id_token, %{}, allowed_algos: @supported_algorithms) do # The OIDC user info endpoint is optional so assume info will be in the id token instead and filter for just the standard claims user_info = fetch_user_info(access_token) || :maps.filter(fn key, _val -> key in @standard_claims end, id_token) From f9f2cb6902e434711c90176fb3d9772f866fef75 Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Wed, 19 Oct 2022 13:17:38 +0100 Subject: [PATCH 21/34] Use supplied scopes for fetching token --- 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 b01744c1c..175f085bd 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -38,7 +38,7 @@ defmodule RetWeb.OIDCAuthChannel do response_type: "code", response_mode: "query", client_id: RemoteOIDCClient.get_client_id(), - scope: "openid profile", + scope: RemoteOIDCClient.get_scopes(), state: state, nonce: nonce, redirect_uri: get_redirect_uri() From ef9f96341c99326ffce1783a1c58c05f1d6f302e Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Wed, 19 Oct 2022 13:21:59 +0100 Subject: [PATCH 22/34] Small change to test data --- config/dev.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/dev.exs b/config/dev.exs index d2690000f..0a19bb651 100644 --- a/config/dev.exs +++ b/config/dev.exs @@ -236,6 +236,6 @@ config :ret, Ret.Coturn, realm: "ret" # OIDC test server https://oidctest.wsweet.org/ config :ret, Ret.RemoteOIDCClient, openid_configuration: "https://oidctest.wsweet.org/.well-known/openid-configuration", - scopes: "openid profile email", + scopes: "openid profile email roles", client_id: "private", client_secret: "tardis" From 1f406f3f13c5599a66e0b7aa76606c972c745cad Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Wed, 19 Oct 2022 13:25:01 +0100 Subject: [PATCH 23/34] Statistic policy outside the scope of this PR --- lib/ret_web/channels/oidc_auth_channel.ex | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index 175f085bd..902c398f0 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -26,7 +26,6 @@ defmodule RetWeb.OIDCAuthChannel do # 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 From 5096e487ec9ccfe56f34307bfc2db208d7c74ce6 Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Wed, 19 Oct 2022 13:27:01 +0100 Subject: [PATCH 24/34] Make some functions private --- 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 902c398f0..dd91db874 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -44,7 +44,7 @@ defmodule RetWeb.OIDCAuthChannel do }) end - def fetch_user_info(access_token) do + defp fetch_user_info(access_token) do # user info endpoint is optional case RemoteOIDCClient.get_userinfo_endpoint() do nil -> nil @@ -120,7 +120,7 @@ defmodule RetWeb.OIDCAuthChannel do {:noreply, socket} end - def fetch_oidc_tokens(oauth_code) do + defp fetch_oidc_tokens(oauth_code) do body = {:form, [ From 664d93355738c1a4475b2e5bef58f127ece8e22c Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Wed, 19 Oct 2022 16:16:50 +0100 Subject: [PATCH 25/34] Comment update --- 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 dd91db874..665954390 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -97,7 +97,7 @@ defmodule RetWeb.OIDCAuthChannel do "sub" => remote_user_id } = id_token} <- RemoteOIDCToken.decode_and_verify(raw_id_token, %{}, allowed_algos: @supported_algorithms) do - # The OIDC user info endpoint is optional so assume info will be in the id token instead and filter for just the standard claims + # The OIDC user info endpoint is optional, so if it missing we assume info will be in the id token instead and filter for just the standard claims user_info = fetch_user_info(access_token) || :maps.filter(fn key, _val -> key in @standard_claims end, id_token) broadcast_credentials_and_payload( From d630e941306313b2bbda9a857f2e01e4b9c90236 Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Thu, 20 Oct 2022 12:08:26 +0100 Subject: [PATCH 26/34] Hashed identifier that is unique for each OIDC provider and won't clash with email logins --- lib/ret/remote_oidc_client.ex | 4 +--- lib/ret_web/channels/oidc_auth_channel.ex | 9 +++++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/ret/remote_oidc_client.ex b/lib/ret/remote_oidc_client.ex index 1c67b0cc0..51c7e0043 100644 --- a/lib/ret/remote_oidc_client.ex +++ b/lib/ret/remote_oidc_client.ex @@ -7,7 +7,7 @@ defmodule Ret.RemoteOIDCClient do require Logger - defp get_openid_configuration_uri() do + def get_openid_configuration_uri() do Application.get_env(:ret, __MODULE__)[:openid_configuration] end @@ -42,8 +42,6 @@ defmodule Ret.RemoteOIDCClient do result end - # Public methods - def get_jwks() do :persistent_term.get(:openid_jwks_cache, nil) || download_jwks() end diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index 665954390..c2c94cb47 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -97,11 +97,16 @@ defmodule RetWeb.OIDCAuthChannel do "sub" => remote_user_id } = id_token} <- RemoteOIDCToken.decode_and_verify(raw_id_token, %{}, allowed_algos: @supported_algorithms) do - # The OIDC user info endpoint is optional, so if it missing we assume info will be in the id token instead and filter for just the standard claims + # Searchable identifier is unique to the OIDC provider and user + identifier_hash = RemoteOIDCClient.get_openid_configuration_uri() <> "#" <> remote_user_id + |> Account.identifier_hash_for_email() + + # The OIDC user info endpoint is optional, so if it missing we assume info will be in the id token instead + # and filter for just the standard claims user_info = fetch_user_info(access_token) || :maps.filter(fn key, _val -> key in @standard_claims end, id_token) broadcast_credentials_and_payload( - remote_user_id, + identifier_hash, %{oidc: user_info}, %{session_id: session_id, nonce: nonce}, socket From 1adba56be43d670841c210c7e5b1162545fd691c Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Thu, 20 Oct 2022 16:18:58 +0100 Subject: [PATCH 27/34] Email account creation disabled in OIDC mode. The failure mode is as ungraceful as the existing path when disable_sign_up is true. --- lib/ret_web/channels/auth_channel.ex | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/ret_web/channels/auth_channel.ex b/lib/ret_web/channels/auth_channel.ex index de9e78f0d..68abf7f07 100644 --- a/lib/ret_web/channels/auth_channel.ex +++ b/lib/ret_web/channels/auth_channel.ex @@ -4,7 +4,7 @@ defmodule RetWeb.AuthChannel do use RetWeb, :channel import Canada, only: [can?: 2] - alias Ret.{Statix, LoginToken, Account, Crypto} + alias Ret.{Statix, LoginToken, Account, Crypto, AppConfig} intercept(["auth_credentials"]) @@ -25,8 +25,10 @@ defmodule RetWeb.AuthChannel do account = email |> Account.account_for_email() account_disabled = account && account.state == :disabled + # Accounts can only be created if the general setting is enabled and the server is not in OIDC mode + can_create_email_accounts = can?(nil, create_account(nil)) && !AppConfig.get_config_bool("auth|use_oidc") - if !account_disabled && (can?(nil, create_account(nil)) || !!account) do + if !account_disabled && (can_create_email_accounts || !!account) do # Create token + send email %LoginToken{token: token, payload_key: payload_key} = LoginToken.new_login_token_for_email(email) @@ -98,7 +100,9 @@ defmodule RetWeb.AuthChannel do defp broadcast_credentials_and_payload(nil, _payload, _socket), do: nil defp broadcast_credentials_and_payload(identifier_hash, payload, socket) do - account = identifier_hash |> Account.account_for_login_identifier_hash(can?(nil, create_account(nil))) + # Accounts can only be created if the general setting is enabled and the server is not in OIDC mode + can_create_email_accounts = can?(nil, create_account(nil)) && !AppConfig.get_config_bool("auth|use_oidc") + account = identifier_hash |> Account.account_for_login_identifier_hash(can_create_email_accounts) credentials = account |> Account.credentials_for_account() broadcast!(socket, "auth_credentials", %{credentials: credentials, payload: payload}) end From 996adea1df0b3623e91c59aef754c17aec96d293 Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Thu, 20 Oct 2022 16:32:52 +0100 Subject: [PATCH 28/34] Don't allow OIDC auth unless explicitly enabled --- lib/ret_web/channels/oidc_auth_channel.ex | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index c2c94cb47..87587ee25 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -6,7 +6,7 @@ defmodule RetWeb.OIDCAuthChannel do use RetWeb, :channel import Canada, only: [can?: 2] - alias Ret.{Statix, Account, OAuthToken, RemoteOIDCClient, RemoteOIDCToken} + alias Ret.{Statix, Account, OAuthToken, RemoteOIDCClient, RemoteOIDCToken, AppConfig} intercept(["auth_credentials"]) @@ -59,14 +59,18 @@ defmodule RetWeb.OIDCAuthChannel 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) - - {:reply, {:ok, %{authorize_url: authorize_url}}, socket} + if AppConfig.get_config_bool("auth|use_oidc") do + "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) + + {:reply, {:ok, %{authorize_url: authorize_url}}, socket} + else + {:reply, {:error, %{message: "OpenID Connect not enabled"}}, socket} + end end end From 2f7bfe0dccb535068f7476961729aa76a29cc5bb Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Fri, 21 Oct 2022 11:49:50 +0100 Subject: [PATCH 29/34] Lint suggestions --- lib/ret/remote_oidc_token.ex | 2 +- lib/ret_web/channels/oidc_auth_channel.ex | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ret/remote_oidc_token.ex b/lib/ret/remote_oidc_token.ex index 303a4bd90..e6f74b39c 100644 --- a/lib/ret/remote_oidc_token.ex +++ b/lib/ret/remote_oidc_token.ex @@ -21,7 +21,7 @@ defmodule Ret.RemoteOIDCTokenSecretsFetcher do {:error, :not_implemented} end - def fetch_verifying_secret(mod, %{"kid" => kid, "typ" => "JWT"}, _opts) do + def fetch_verifying_secret(_mod, %{"kid" => kid, "typ" => "JWT"}, _opts) do # TODO force cache to refresh when unknown kid found to support key rotation # as per https://openid.net/specs/openid-connect-core-1_0.html#RotateSigKeys case Ret.RemoteOIDCClient.get_jwks() diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index 87587ee25..2f1a9315b 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -6,7 +6,7 @@ defmodule RetWeb.OIDCAuthChannel do use RetWeb, :channel import Canada, only: [can?: 2] - alias Ret.{Statix, Account, OAuthToken, RemoteOIDCClient, RemoteOIDCToken, AppConfig} + alias Ret.{Account, OAuthToken, RemoteOIDCClient, RemoteOIDCToken, AppConfig} intercept(["auth_credentials"]) From 599cd705ca4fab75913eefb5a81da60bb2418f3f Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Mon, 24 Oct 2022 09:54:55 +0100 Subject: [PATCH 30/34] Make list of permitted claims more a configurable parameter --- config/dev.exs | 3 +++ config/prod.exs | 8 ++++++++ habitat/config/config.toml | 1 + lib/ret/remote_oidc_client.ex | 4 ++++ lib/ret_web/channels/oidc_auth_channel.ex | 8 ++------ 5 files changed, 18 insertions(+), 6 deletions(-) diff --git a/config/dev.exs b/config/dev.exs index 0a19bb651..521fb8194 100644 --- a/config/dev.exs +++ b/config/dev.exs @@ -237,5 +237,8 @@ config :ret, Ret.Coturn, realm: "ret" config :ret, Ret.RemoteOIDCClient, openid_configuration: "https://oidctest.wsweet.org/.well-known/openid-configuration", scopes: "openid profile email roles", + permitted_claims: ["sub", "name", "given_name", "family_name", "middle_name", "nickname", + "preferred_username", "profile", "picture", "website", "email", "email_verified", "gender", + "birthdate", "zoneinfo", "locale", "phone_number", "phone_number_verified", "address", "updated_at"], client_id: "private", client_secret: "tardis" diff --git a/config/prod.exs b/config/prod.exs index 2b7321166..353c83424 100644 --- a/config/prod.exs +++ b/config/prod.exs @@ -164,3 +164,11 @@ config :ret, Ret.StatsJob, node_stats_enabled: false, node_gauges_enabled: false # Default repo check and page check to off so for polycosm hosts database + s3 hits can go idle config :ret, RetWeb.HealthController, check_repo: false + +config :ret, Ret.RemoteOIDCClient, + # Conventional default scopes + scopes: "openid profile email", + # Standard claims https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims + permitted_claims: ["sub", "name", "given_name", "family_name", "middle_name", "nickname", + "preferred_username", "profile", "picture", "website", "email", "email_verified", "gender", + "birthdate", "zoneinfo", "locale", "phone_number", "phone_number_verified", "address", "updated_at"] diff --git a/habitat/config/config.toml b/habitat/config/config.toml index dab56e7b1..14b82b34d 100644 --- a/habitat/config/config.toml +++ b/habitat/config/config.toml @@ -306,6 +306,7 @@ public_tls_ports = "{{ cfg.turn.public_tls_ports }}" [ret."Elixir.Ret.RemoteOIDCClient"] openid_configuration = {{ toToml openid_configuration }} scopes = {{ toToml scopes }} +permitted_claims = {{ toToml permitted_claims }} client_id = {{ toToml client_id }} client_secret = {{ toToml client_secret }} {{/with}} diff --git a/lib/ret/remote_oidc_client.ex b/lib/ret/remote_oidc_client.ex index 51c7e0043..b8b0679a6 100644 --- a/lib/ret/remote_oidc_client.ex +++ b/lib/ret/remote_oidc_client.ex @@ -72,6 +72,10 @@ defmodule Ret.RemoteOIDCClient do Application.get_env(:ret, __MODULE__)[:scopes] end + def get_permitted_claims() do + Application.get_env(:ret, __MODULE__)[:permitted_claims] + end + def get_client_id() do Application.get_env(:ret, __MODULE__)[:client_id] end diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index 2f1a9315b..f6d9b5202 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -10,11 +10,6 @@ defmodule RetWeb.OIDCAuthChannel do intercept(["auth_credentials"]) - # Ref https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims - @standard_claims ["sub", "name", "given_name", "family_name", "middle_name", "nickname", - "preferred_username", "profile", "picture", "website", "email", "email_verified", "gender", - "birthdate", "zoneinfo", "locale", "phone_number", "phone_number_verified", "address", "updated_at"] - # Intersection of possible values for JSON signing https://www.rfc-editor.org/rfc/rfc7518#section-3.1 # and algorithms supported by JOSE https://hexdocs.pm/jose/JOSE.JWS.html#module-algorithms @supported_algorithms ["HS256", "HS384", "HS512", "RS256", "RS384", "RS512", "ES256", "ES384", "ES512", "PS256", "PS384", "PS512"] @@ -107,7 +102,8 @@ defmodule RetWeb.OIDCAuthChannel do # The OIDC user info endpoint is optional, so if it missing we assume info will be in the id token instead # and filter for just the standard claims - user_info = fetch_user_info(access_token) || :maps.filter(fn key, _val -> key in @standard_claims end, id_token) + permitted_claims = RemoteOIDCClient.get_permitted_claims() + user_info = fetch_user_info(access_token) || :maps.filter(fn key, _val -> key in permitted_claims end, id_token) broadcast_credentials_and_payload( identifier_hash, From 4009651a20dd86aa88064cbd426e38cccefda219 Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Mon, 24 Oct 2022 11:42:33 +0100 Subject: [PATCH 31/34] Fix filtering when UserInfo endpoint is active --- lib/ret_web/channels/oidc_auth_channel.ex | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/ret_web/channels/oidc_auth_channel.ex b/lib/ret_web/channels/oidc_auth_channel.ex index f6d9b5202..cbc500829 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -101,13 +101,14 @@ defmodule RetWeb.OIDCAuthChannel do |> Account.identifier_hash_for_email() # The OIDC user info endpoint is optional, so if it missing we assume info will be in the id token instead - # and filter for just the standard claims + # and filter for just the permitted claims + all_claims = fetch_user_info(access_token) || id_token permitted_claims = RemoteOIDCClient.get_permitted_claims() - user_info = fetch_user_info(access_token) || :maps.filter(fn key, _val -> key in permitted_claims end, id_token) + filtered_claims = :maps.filter(fn key, _val -> key in permitted_claims end, all_claims) broadcast_credentials_and_payload( identifier_hash, - %{oidc: user_info}, + %{oidc: filtered_claims}, %{session_id: session_id, nonce: nonce}, socket ) From f8e051a811a9272901fbcb61929362177c831969 Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Mon, 24 Oct 2022 11:48:20 +0100 Subject: [PATCH 32/34] More focused test parameters --- config/dev.exs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/config/dev.exs b/config/dev.exs index 521fb8194..d1bd08a29 100644 --- a/config/dev.exs +++ b/config/dev.exs @@ -237,8 +237,6 @@ config :ret, Ret.Coturn, realm: "ret" config :ret, Ret.RemoteOIDCClient, openid_configuration: "https://oidctest.wsweet.org/.well-known/openid-configuration", scopes: "openid profile email roles", - permitted_claims: ["sub", "name", "given_name", "family_name", "middle_name", "nickname", - "preferred_username", "profile", "picture", "website", "email", "email_verified", "gender", - "birthdate", "zoneinfo", "locale", "phone_number", "phone_number_verified", "address", "updated_at"], + permitted_claims: ["sub", "email", "name", "preferred_username", "roles"], client_id: "private", client_secret: "tardis" From ee7ccc27ed74b277d6019666328c509e44f4c29d Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Mon, 24 Oct 2022 11:59:50 +0100 Subject: [PATCH 33/34] Support for additional authentication parameters --- config/dev.exs | 3 ++- habitat/config/config.toml | 2 ++ lib/ret/remote_oidc_client.ex | 4 ++++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/config/dev.exs b/config/dev.exs index d1bd08a29..61dc57718 100644 --- a/config/dev.exs +++ b/config/dev.exs @@ -239,4 +239,5 @@ config :ret, Ret.RemoteOIDCClient, scopes: "openid profile email roles", permitted_claims: ["sub", "email", "name", "preferred_username", "roles"], client_id: "private", - client_secret: "tardis" + client_secret: "tardis", + additional_authorization_parameters: "&prompt=select_account" diff --git a/habitat/config/config.toml b/habitat/config/config.toml index 14b82b34d..e525ca37b 100644 --- a/habitat/config/config.toml +++ b/habitat/config/config.toml @@ -309,6 +309,8 @@ scopes = {{ toToml scopes }} permitted_claims = {{ toToml permitted_claims }} client_id = {{ toToml client_id }} client_secret = {{ toToml client_secret }} +additional_authorization_parameters = {{ toToml additional_authorization_parameters }} + {{/with}} {{/if}} diff --git a/lib/ret/remote_oidc_client.ex b/lib/ret/remote_oidc_client.ex index b8b0679a6..74cc4140d 100644 --- a/lib/ret/remote_oidc_client.ex +++ b/lib/ret/remote_oidc_client.ex @@ -84,4 +84,8 @@ defmodule Ret.RemoteOIDCClient do Application.get_env(:ret, __MODULE__)[:client_secret] end + def get_additional_authorization_parameters() do + Application.get_env(:ret, __MODULE__)[:additional_authorization_parameters] + end + end From 8b3795672f7c046984860184c4f68925f0c649cc Mon Sep 17 00:00:00 2001 From: Rupert Rawnsley Date: Mon, 24 Oct 2022 12:00:01 +0100 Subject: [PATCH 34/34] Missing from last checkin --- 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 cbc500829..3897a015a 100644 --- a/lib/ret_web/channels/oidc_auth_channel.ex +++ b/lib/ret_web/channels/oidc_auth_channel.ex @@ -36,7 +36,7 @@ defmodule RetWeb.OIDCAuthChannel do state: state, nonce: nonce, redirect_uri: get_redirect_uri() - }) + }) <> RemoteOIDCClient.get_additional_authorization_parameters() end defp fetch_user_info(access_token) do