Sync users with OIDC provider (#605)

* tmp [skip ci]

* add connection [skip ci]

* implement periodic refresh

* format [skip ci]

* no need to guard

* make credo happy

* remove filter

* updates

* extract component

* prevent changing admin's connection permission

* add to show as well

* show connections

* hide oidc connections section if nothing is there

* add test

* leave site id out for now

may not need it if we create different users for different sites, even
for the same email

* move delay randomization to manager

* add test

* address comments

* re-enable user when sign-ed via oidc

* fix test

* remove unused alias

* fix tests

* update tests

* fix re-enable

* add tests

* Update vpn_connection_component_test.exs

* revert formats

* tests

* remove alias

* remove special case for admins

* remove auto re-enable
This commit is contained in:
Po Chen
2022-05-26 15:43:09 +10:00
committed by GitHub
parent 8f96cdf818
commit 3cbc18d0d6
25 changed files with 525 additions and 47 deletions

View File

@@ -36,9 +36,11 @@ defmodule FzHttp.Application do
{Phoenix.PubSub, name: FzHttp.PubSub},
FzHttpWeb.Presence,
FzHttp.ConnectivityCheckService,
FzHttp.VpnSessionScheduler
FzHttp.VpnSessionScheduler,
{OpenIDConnect.Worker, openid_connect_providers},
{DynamicSupervisor, name: FzHttp.RefresherSupervisor, strategy: :one_for_one},
FzHttp.OIDC.RefreshManager
]
|> append_if(openid_connect_providers, {OpenIDConnect.Worker, openid_connect_providers})
end
defp children(:test) do
@@ -51,8 +53,4 @@ defmodule FzHttp.Application do
FzHttpWeb.Presence
]
end
defp append_if(list, condition, item) do
if condition, do: list ++ [item], else: list
end
end

View File

@@ -88,7 +88,7 @@ defmodule FzHttp.Devices do
preload: :user
)
|> Enum.filter(fn device ->
device.user.role == :admin || !Users.vpn_session_expired?(device.user, vpn_duration)
!device.user.disabled_at && !Users.vpn_session_expired?(device.user, vpn_duration)
end)
|> Enum.map(fn device ->
%{

View File

@@ -0,0 +1,36 @@
defmodule FzHttp.OIDC do
@moduledoc """
The OIDC context.
"""
import Ecto.Query, warn: false
alias FzHttp.{OIDC.Connection, Repo, Users.User}
def list_connections(%User{id: id}) do
Repo.all(from Connection, where: [user_id: ^id])
end
def get_connection!(user_id, provider) do
Repo.get_by!(Connection, user_id: user_id, provider: provider)
end
def get_connection(user_id, provider) do
Repo.get_by(Connection, user_id: user_id, provider: provider)
end
def create_connection(user_id, provider, refresh_token) do
%Connection{user_id: user_id}
|> Connection.changeset(%{provider: provider, refresh_token: refresh_token})
|> Repo.insert(
conflict_target: [:user_id, :provider],
on_conflict: {:replace, [:refresh_token]}
)
end
def update_connection(%Connection{} = connection, attrs) do
connection
|> Connection.changeset(attrs)
|> Repo.update()
end
end

View File

@@ -0,0 +1,25 @@
defmodule FzHttp.OIDC.Connection do
@moduledoc """
OIDC connections
"""
use Ecto.Schema
import Ecto.Changeset
schema "oidc_connections" do
field :provider, :string
field :refresh_response, :map
field :refresh_token, :string
field :refreshed_at, :utc_datetime_usec
field :user_id, :id
timestamps(type: :utc_datetime_usec)
end
@doc false
def changeset(connection, attrs) do
connection
|> cast(attrs, [:provider, :refresh_token, :refreshed_at, :refresh_response])
|> validate_required([:provider, :refresh_token])
end
end

View File

@@ -0,0 +1,51 @@
defmodule FzHttp.OIDC.RefreshManager do
@moduledoc """
Manager module for refreshing OIDC connections
"""
use GenServer, restart: :permanent
import Ecto.Query
alias FzHttp.{Repo, Users.User}
@spawn_interval 60 * 60 * 1000
@max_delay_after_spawn 15
def start_link(_) do
GenServer.start_link(__MODULE__, [])
end
def init(_) do
{:ok, [], {:continue, :schedule}}
end
def handle_continue(:schedule, state) do
spawn_refresher()
{:noreply, state}
end
def handle_info(:spawn_refresher, user_id) do
spawn_refresher()
{:noreply, user_id}
end
defp schedule do
Process.send_after(self(), :spawn_refresher, @spawn_interval)
end
defp spawn_refresher do
schedule()
from(u in User, where: is_nil(u.disabled_at))
|> Repo.all()
|> Enum.each(&do_spawn/1)
end
defp do_spawn(%{id: id} = _user) do
delay_after_spawn = Enum.random(1..@max_delay_after_spawn) * 1000
DynamicSupervisor.start_child(
FzHttp.RefresherSupervisor,
{FzHttp.OIDC.Refresher, {id, delay_after_spawn}}
)
end
end

View File

@@ -0,0 +1,78 @@
defmodule FzHttp.OIDC.Refresher do
@moduledoc """
Worker module for refreshing OIDC connections
"""
use GenServer, restart: :temporary
import Ecto.{Changeset, Query}
alias FzHttp.{OIDC, OIDC.Connection, Repo, Users}
require Logger
def start_link(init_opts) do
GenServer.start_link(__MODULE__, init_opts)
end
def init({user_id, delay}) do
{:ok, user_id, {:continue, {:delay, delay}}}
end
def handle_continue({:delay, delay}, user_id) do
Process.sleep(delay)
refresh(user_id)
end
def refresh(user_id) do
connections = Repo.all(from Connection, where: [user_id: ^user_id])
Enum.each(connections, &do_refresh(user_id, &1))
{:stop, :shutdown, user_id}
end
defp do_refresh(user_id, %{provider: provider, refresh_token: refresh_token} = conn) do
provider = String.to_existing_atom(provider)
Logger.info("Refreshing user\##{user_id} @ #{provider}...")
result =
openid_connect().fetch_tokens(
provider,
%{grant_type: "refresh_token", refresh_token: refresh_token}
)
refresh_response =
case result do
{:ok, refreshed} ->
refreshed
{:error, :fetch_tokens, %{body: body}} ->
%{error: body}
_ ->
%{error: "unknown error"}
end
OIDC.update_connection(conn, %{
refreshed_at: DateTime.utc_now(),
refresh_response: refresh_response
})
with %{error: _} <- refresh_response do
user = Users.get_user!(user_id)
Logger.info("Disabling user #{user.email} due to OIDC token refresh failure...")
user
|> change()
|> put_change(:disabled_at, DateTime.utc_now())
|> prepare_changes(fn changeset ->
FzHttp.Telemetry.disable_user(user, "oidc refresh failure")
FzHttpWeb.Endpoint.broadcast("users_socket:#{user.id}", "disconnect", %{})
changeset
end)
|> Repo.update!()
end
end
defp openid_connect do
Application.fetch_env!(:fz_http, :openid_connect)
end
end

View File

@@ -86,6 +86,14 @@ defmodule FzHttp.Telemetry do
)
end
def disable_user(user, reason) do
telemetry_module().capture(
"disable_user",
common_fields() ++
[user_email_hash: hash(user.email), reason: reason]
)
end
def fz_http_started do
telemetry_module().capture(
"fz_http_started",

View File

@@ -3,6 +3,7 @@ defmodule FzHttp.Users do
The Users context.
"""
import Ecto.Changeset
import Ecto.Query, warn: false
alias FzCommon.{FzCrypto, FzMap}
@@ -141,6 +142,16 @@ defmodule FzHttp.Users do
update_user(user, %{last_signed_in_at: DateTime.utc_now(), last_signed_in_method: method})
end
def enable_vpn_connection(user, %{provider: :identity}), do: user
def enable_vpn_connection(user, %{provider: :magic_link}), do: user
def enable_vpn_connection(user, %{provider: _oidc_provider}) do
user
|> change()
|> put_change(:disabled_at, nil)
|> Repo.update!()
end
@doc """
Returns DateTime that VPN sessions expire based on last_signed_in_at
and the security.require_auth_for_vpn_frequency setting.

View File

@@ -10,7 +10,7 @@ defmodule FzHttp.Users.User do
import Ecto.Changeset
import FzHttp.Users.PasswordHelpers
alias FzHttp.Devices.Device
alias FzHttp.{Devices.Device, OIDC.Connection}
schema "users" do
field :uuid, Ecto.UUID, autogenerate: true
@@ -21,6 +21,7 @@ defmodule FzHttp.Users.User do
field :password_hash, :string
field :sign_in_token, :string
field :sign_in_token_created_at, :utc_datetime_usec
field :disabled_at, :utc_datetime_usec
# VIRTUAL FIELDS
field :device_count, :integer, virtual: true
@@ -29,6 +30,7 @@ defmodule FzHttp.Users.User do
field :current_password, :string, virtual: true
has_many :devices, Device, on_delete: :delete_all
has_many :oidc_connections, Connection, on_delete: :delete_all
timestamps(type: :utc_datetime_usec)
end

View File

@@ -67,6 +67,11 @@ defmodule FzHttpWeb.AuthController do
{:ok, claims} <- openid_connect.verify(provider, tokens["id_token"]) do
case UserFromAuth.find_or_create(provider, claims) do
{:ok, user} ->
# only first-time connect will include refresh token
with %{"refresh_token" => refresh_token} <- tokens do
FzHttp.OIDC.create_connection(user.id, provider_key, refresh_token)
end
conn
|> configure_session(renew: true)
|> put_session(:live_socket_id, "users_socket:#{user.id}")

View File

@@ -20,6 +20,7 @@
<tr>
<th>Email</th>
<th>Devices</th>
<th>VPN Connection</th>
<th>Last Signed In</th>
<th>Last Signed In Method</th>
<th>Created</th>
@@ -33,6 +34,11 @@
<%= live_patch(user.email, to: Routes.user_show_path(@socket, :show, user)) %>
</td>
<td id={"user-#{user.id}-device-count"}><%= user.device_count %></td>
<td>
<.live_component id={"allowed-to-connect-#{user.id}"}
module={FzHttpWeb.UserLive.VPNConnectionComponent }
user={user} disabled={true} />
</td>
<td id={"user-#{user.id}-timestamp"}
data-timestamp={user.last_signed_in_at}
phx-hook="FormatTimestamp">

View File

@@ -40,6 +40,14 @@
<%= render FzHttpWeb.SharedView, "user_details.html", user: @user %>
</section>
<%= if length(@connections) > 0 do %>
<section class="section is-main-section">
<h4 class="title is-4">OIDC Connections</h4>
<%= render FzHttpWeb.SharedView, "oidc_connections_table.html", connections: @connections %>
</section>
<% end %>
<section class="section is-main-section">
<h4 class="title is-4">Devices</h4>
@@ -62,25 +70,31 @@
<section class="section is-main-section">
<h4 class="title is-4">Danger Zone</h4>
<button
class="button is-warning"
data-confirm={"Are you sure? #{mote_message(@user)}"}
phx-click={mote(@user)}
phx-value-user_id={@user.id}>
<span class="icon is-small">
<i class="fas fa-user-shield"></i>
</span>
<span class="is-capitalized"><%= mote(@user) %></span>
</button>
<.live_component id="allowed-to-connect"
module={FzHttpWeb.UserLive.VPNConnectionComponent }
user={@user} label="Permission to Connect VPN" />
<button
class="button is-danger"
data-confirm="Are you sure? This will permanently delete this user, all associated devices and instantly drop any active VPN sessions associated to this user."
phx-click="delete_user"
phx-value-user_id={@user.id}>
<span class="icon is-small">
<i class="fas fa-trash"></i>
</span>
<span>Delete User</span>
</button>
<div class="mt-4">
<button
class="button is-warning"
data-confirm={"Are you sure? #{mote_message(@user)}"}
phx-click={mote(@user)}
phx-value-user_id={@user.id}>
<span class="icon is-small">
<i class="fas fa-user-shield"></i>
</span>
<span class="is-capitalized"><%= mote(@user) %></span>
</button>
<button
class="button is-danger"
data-confirm="Are you sure? This will permanently delete this user, all associated devices and instantly drop any active VPN sessions associated to this user."
phx-click="delete_user"
phx-value-user_id={@user.id}>
<span class="icon is-small">
<i class="fas fa-trash"></i>
</span>
<span>Delete User</span>
</button>
</div>
</section>

View File

@@ -5,18 +5,20 @@ defmodule FzHttpWeb.UserLive.Show do
"""
use FzHttpWeb, :live_view
alias FzHttp.{Devices, Repo, Users}
alias FzHttp.{Devices, OIDC, Repo, Users}
alias FzHttpWeb.ErrorHelpers
@impl Phoenix.LiveView
def mount(%{"id" => user_id} = _params, _session, socket) do
user = Users.get_user!(user_id)
devices = Devices.list_devices(user)
connections = OIDC.list_connections(user)
{:ok,
socket
|> assign(:devices, devices)
|> assign(:device_config, socket.assigns[:device_config])
|> assign(:connections, connections)
|> assign(:user, user)
|> assign(:page_title, "Users")}
end

View File

@@ -0,0 +1,47 @@
defmodule FzHttpWeb.UserLive.VPNConnectionComponent do
@moduledoc """
Handles user form.
"""
use FzHttpWeb, :live_component
import Ecto.Changeset
alias FzHttp.Repo
@impl Phoenix.LiveComponent
def render(assigns) do
~H"""
<label>
<input type="checkbox" phx-target={@myself} phx-click="toggle_disabled_at"
data-confirm="Are you sure? This may affect this user's internet connectivity."
disabled={assigns[:disabled] || @user.role == :admin}
checked={!@user.disabled_at} value={if(@user.disabled_at, do: "on")} />
<%= assigns[:label] %>
</label>
"""
end
@impl Phoenix.LiveComponent
def handle_event("toggle_disabled_at", params, socket) do
to_disable = !params["value"]
user =
socket.assigns.user
|> change()
|> put_change(
:disabled_at,
if(to_disable, do: DateTime.utc_now(), else: nil)
)
|> prepare_changes(fn
%{changes: %{disabled_at: nil}} = changeset ->
changeset
%{data: user} = changeset ->
FzHttp.Telemetry.disable_user(user, "disabled by admin")
FzHttpWeb.Endpoint.broadcast("users_socket:#{user.id}", "disconnect", %{})
changeset
end)
|> Repo.update!()
{:noreply, assign(socket, :user, user)}
end
end

View File

@@ -7,15 +7,13 @@
Please sign in via one of the methods below.
</p>
<%= if @openid_connect_providers > 0 do %>
<%= for {provider, config} <- @openid_connect_providers do %>
<p>
<%= link(
"Sign in with #{Keyword.get(config, :label)}",
to: authorization_uri(@openid_connect, provider),
class: "button") %>
</p>
<% end %>
<%= for {provider, config} <- @openid_connect_providers do %>
<p>
<%= link(
"Sign in with #{config[:label]}",
to: authorization_uri(@openid_connect, provider),
class: "button") %>
</p>
<% end %>
<%= if @local_enabled do %>

View File

@@ -0,0 +1,26 @@
<table class="table is-bordered is-hoverable is-striped is-fullwidth">
<thead>
<tr>
<th>Provider</th>
<th>Refreshed At</th>
<th>Refresh Result</th>
</tr>
</thead>
<tbody>
<%= for conn <- @connections do %>
<tr>
<td>
<%= conn.provider %>
</td>
<td id={"connection-#{conn.id}-refreshed-at"} data-timestamp={conn.refreshed_at} phx-hook="FormatTimestamp">…</td>
<td>
<%= if match?(%{"error" => _}, conn.refresh_response) do %>
ERROR: <%= conn.refresh_response["error"] %>
<% else %>
OK
<% end %>
</td>
</tr>
<% end %>
</tbody>
</table>

View File

@@ -5,7 +5,9 @@ defmodule FzHttpWeb.RootView do
def authorization_uri(oidc, provider) do
params = %{
state: FzCrypto.rand_string()
state: FzCrypto.rand_string(),
# needed for google
access_type: "offline"
}
oidc.authorization_uri(provider, params)

View File

@@ -0,0 +1,17 @@
defmodule FzHttp.Repo.Migrations.CreateOidcConnections do
use Ecto.Migration
def change do
create table(:oidc_connections) do
add :provider, :string, null: false
add :refresh_token, :string
add :refreshed_at, :utc_datetime_usec
add :refresh_response, :map
add :user_id, references(:users, on_delete: :nothing), null: false
timestamps(type: :utc_datetime_usec)
end
create unique_index(:oidc_connections, [:user_id, :provider])
end
end

View File

@@ -0,0 +1,9 @@
defmodule FzHttp.Repo.Migrations.AddDisabledAtToUser do
use Ecto.Migration
def change do
alter table("users") do
add :disabled_at, :utc_datetime_usec
end
end
end

View File

@@ -0,0 +1,58 @@
defmodule FzHttp.OIDC.RefresherTest do
use FzHttp.DataCase, async: true
import Mox
alias FzHttp.{OIDC.Refresher, Repo}
setup :create_user
setup %{user: user} do
conn =
Repo.insert!(%FzHttp.OIDC.Connection{
user_id: user.id,
provider: "test",
refresh_token: "REFRESH_TOKEN"
})
{:ok, conn: conn}
end
describe "refresh failed" do
setup do
expect(OpenIDConnect.Mock, :fetch_tokens, fn _, _ ->
{:error, :fetch_tokens, "TEST_ERROR"}
end)
:ok
end
test "disable user", %{user: user, conn: conn} do
Refresher.refresh(user.id)
user = Repo.reload(user)
conn = Repo.reload(conn)
assert user.disabled_at
assert %{"error" => _} = conn.refresh_response
end
end
describe "refresh succeeded" do
setup do
expect(OpenIDConnect.Mock, :fetch_tokens, fn _, _ ->
{:ok, %{data: "success"}}
end)
:ok
end
test "does not change user", %{user: user, conn: conn} do
Refresher.refresh(user.id)
user = Repo.reload(user)
conn = Repo.reload(conn)
refute user.disabled_at
refute match?(%{"error" => _}, conn.refresh_response)
end
end
end

View File

@@ -1,7 +1,7 @@
defmodule FzHttp.UsersTest do
use FzHttp.DataCase, async: true
alias FzHttp.Users
alias FzHttp.{Repo, Users}
describe "consume_sign_in_token/1 valid token" do
setup [:create_user_with_valid_sign_in_token]
@@ -222,7 +222,7 @@ defmodule FzHttp.UsersTest do
end
describe "delete_user/1" do
setup [:create_user]
setup :create_user
test "raises Ecto.NoResultsError when a deleted user is fetched", %{user: user} do
Users.delete_user(user)
@@ -234,7 +234,7 @@ defmodule FzHttp.UsersTest do
end
describe "change_user/1" do
setup [:create_user]
setup :create_user
test "returns changeset", %{user: user} do
assert %Ecto.Changeset{} = Users.change_user(user)
@@ -246,4 +246,33 @@ defmodule FzHttp.UsersTest do
assert %Ecto.Changeset{} = Users.new_user()
end
end
describe "enable_vpn_connection/2" do
import Ecto.Changeset
setup :create_user
setup %{user: user} do
user = user |> change |> put_change(:disabled_at, DateTime.utc_now()) |> Repo.update!()
{:ok, user: user}
end
@tag :unprivileged
test "enable via OIDC", %{user: user} do
Users.enable_vpn_connection(user, %{provider: :oidc})
user = Repo.reload(user)
assert %{disabled_at: nil} = user
end
@tag :unprivileged
test "no change via password", %{user: user} do
Users.enable_vpn_connection(user, %{provider: :identity})
user = Repo.reload(user)
assert user.disabled_at
end
end
end

View File

@@ -614,4 +614,38 @@ defmodule FzHttpWeb.UserLive.ShowTest do
assert test_view =~ "should be at least 12 character(s)"
end
end
describe "disable/enable user" do
import Ecto.Changeset
alias FzHttp.Repo
test "enable user", %{admin_conn: conn, unprivileged_user: user} do
user = user |> change |> put_change(:disabled_at, DateTime.utc_now()) |> Repo.update!()
path = Routes.user_show_path(conn, :show, user.id)
{:ok, view, _html} = live(conn, path)
view
|> element("input[type=checkbox]")
|> render_click()
user = Repo.reload(user)
refute user.disabled_at
end
test "disable user", %{admin_conn: conn, unprivileged_user: user} do
path = Routes.user_show_path(conn, :show, user.id)
{:ok, view, _html} = live(conn, path)
view
|> element("input[type=checkbox]")
|> render_click()
user = Repo.reload(user)
assert user.disabled_at
end
end
end

View File

@@ -0,0 +1,22 @@
defmodule FzHttpWeb.UserLive.VPNConnectionComponentTest do
use FzHttpWeb.ConnCase, async: true
alias FzHttpWeb.UserLive.VPNConnectionComponent
describe "admin" do
setup :create_user
test "checkbox is disabled", %{user: user} do
assert render_component(VPNConnectionComponent, id: "1", user: user) =~ ~r"\bdisabled\b"
end
end
describe "unprivileged" do
setup :create_user
@tag :unprivileged
test "checkbox is not disabled", %{user: user} do
refute render_component(VPNConnectionComponent, id: "1", user: user) =~ ~r"\bdisabled\b"
end
end
end

View File

@@ -3,6 +3,6 @@ defmodule OpenIDConnect.MockBehaviour do
Mock Behaviour for OpenIDConnect so that we can use Mox
"""
@callback authorization_uri(any, map) :: String.t()
@callback fetch_tokens(any, map) :: {:ok, any} | {:error, any}
@callback verify(any, map) :: {:ok, any} | {:error, any}
@callback fetch_tokens(any, map) :: {:ok, any} | {:error, :fetch_tokens, any}
@callback verify(any, map) :: {:ok, any} | {:error, :verify, any}
end

View File

@@ -146,7 +146,9 @@ defmodule FzHttp.TestHelpers do
{:ok, user: user}
end
def create_users(%{count: count}) do
def create_users(opts) do
count = opts[:count] || 5
users =
Enum.map(1..count, fn i ->
UsersFixtures.user(%{email: "userlist#{i}@test"})
@@ -155,8 +157,6 @@ defmodule FzHttp.TestHelpers do
{:ok, users: users}
end
def create_users(_), do: create_users(%{count: 5})
def clear_users(_) do
{count, _result} = Repo.delete_all(User)
{:ok, count: count}