Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenID Connect Login #426

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
14 changes: 14 additions & 0 deletions lib/ret/oauth_token.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions lib/ret/remote_oidc_token.ex
Original file line number Diff line number Diff line change
@@ -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?
netpro2k marked this conversation as resolved.
Show resolved Hide resolved
{:ok, Application.get_env(:ret, mod)[:verification_key] |> JOSE.JWK.from_pem()} |> IO.inspect()
end
end
185 changes: 185 additions & 0 deletions lib/ret_web/channels/oidc_auth_channel.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
defmodule RetWeb.OIDCAuthChannel do
@moduledoc "Ret Web Channel for OpenID Connect Authentication"

use RetWeb, :channel
import Canada, only: [can?: 2]

alias Ret.{Statix, Account, OAuthToken, RemoteOIDCToken}

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")
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the intention with this metric? Doesn't seem particularly useful on its own.

{: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"
netpro2k marked this conversation as resolved.
Show resolved Hide resolved

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
netpro2k marked this conversation as resolved.
Show resolved Hide resolved
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},
netpro2k marked this conversation as resolved.
Show resolved Hide resolved
%{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
netpro2k marked this conversation as resolved.
Show resolved Hide resolved
{: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_tokens(oauth_code) do
Copy link
Contributor

Choose a reason for hiding this comment

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

defp?

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
# "#{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

# Only send creddentials back down to the original socket that started the request
Copy link
Contributor

Choose a reason for hiding this comment

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

typo

Suggested change
# 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},
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(nil))
account = identifier_hash |> Account.account_for_login_identifier_hash(account_creation_enabled)
credentials = account |> Account.credentials_for_account()
Copy link
Contributor

Choose a reason for hiding this comment

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

Took me a while to think out the implications of account creation being disabled. I guess the expectation is that users are still able to access the site/room, but are not actually "logged in" if site-wide account creation is disabled?
Might want to explain that out here in a comment.


broadcast!(socket, "auth_credentials", %{
credentials: credentials,
user_info: user_info,
verification_info: verification_info
})
end
end
1 change: 1 addition & 0 deletions lib/ret_web/channels/session_socket.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down