From 908cfc7dff0484ddde1f3ab6a0666fa135e4d59b Mon Sep 17 00:00:00 2001 From: Jamil Date: Wed, 27 Apr 2022 15:20:33 -0700 Subject: [PATCH] 522/allow disabling of config creation (#559) * Checkpoint * Optionally hide device mgmt buttons --- apps/fz_http/lib/fz_http/devices/device.ex | 18 +++++++- .../device_live/unprivileged/index.html.heex | 12 ++--- .../device_live/unprivileged/show_live.ex | 4 +- .../templates/shared/show_device.html.heex | 30 +++++++------ .../lib/fz_http_web/views/device_view.ex | 5 +++ .../device_live/unprivileged/index_test.exs | 39 +++++++++++++--- .../device_live/unprivileged/show_test.exs | 44 +++++++++++++++++++ apps/fz_http/test/support/test_helpers.ex | 10 ++++- config/config.exs | 1 + config/releases.exs | 4 ++ docs/docs/reference/configuration-file.md | 3 +- .../cookbooks/firezone/attributes/default.rb | 4 ++ .../cookbooks/firezone/libraries/config.rb | 1 + 13 files changed, 145 insertions(+), 30 deletions(-) create mode 100644 apps/fz_http/test/fz_http_web/live/device_live/unprivileged/show_test.exs diff --git a/apps/fz_http/lib/fz_http/devices/device.ex b/apps/fz_http/lib/fz_http/devices/device.ex index 080bdf695..33d20e813 100644 --- a/apps/fz_http/lib/fz_http/devices/device.ex +++ b/apps/fz_http/lib/fz_http/devices/device.ex @@ -18,7 +18,7 @@ defmodule FzHttp.Devices.Device do import FzHttp.Queries.INET - alias FzHttp.{Devices, Users.User} + alias FzHttp.{Devices, Users, Users.User} @description_max_length 2048 @@ -58,6 +58,7 @@ defmodule FzHttp.Devices.Device do |> put_next_ip(:ipv6) |> shared_changeset() |> validate_max_devices() + |> validate_device_management() end def update_changeset(device, attrs) do @@ -154,6 +155,21 @@ defmodule FzHttp.Devices.Device do end end + defp validate_device_management(changeset) do + user_id = changeset.changes.user_id || changeset.data.user_id + + if Users.get_user!(user_id).role == :admin || + Application.fetch_env!(:fz_http, :allow_unprivileged_device_management) do + changeset + else + add_error( + changeset, + :base, + "Must be an administrator to manage devices." + ) + end + end + defp validate_omitted_if_site(changeset, fields) when is_list(fields) do validate_omitted(changeset, filter_site_fields(changeset, fields, use_site: true)) end diff --git a/apps/fz_http/lib/fz_http_web/live/device_live/unprivileged/index.html.heex b/apps/fz_http/lib/fz_http_web/live/device_live/unprivileged/index.html.heex index 4ded9bdc6..18784d420 100644 --- a/apps/fz_http/lib/fz_http_web/live/device_live/unprivileged/index.html.heex +++ b/apps/fz_http/lib/fz_http_web/live/device_live/unprivileged/index.html.heex @@ -60,11 +60,13 @@ <% end %> -
- <%= live_patch(to: Routes.device_unprivileged_index_path(@socket, :new), class: "button") do %> - Add Device - <% end %> -
+ <%= if FzHttpWeb.DeviceView.can_manage_devices?(@current_user) do %> +
+ <%= live_patch(to: Routes.device_unprivileged_index_path(@socket, :new), class: "button") do %> + Add Device + <% end %> +
+ <% end %>

VPN Session

diff --git a/apps/fz_http/lib/fz_http_web/live/device_live/unprivileged/show_live.ex b/apps/fz_http/lib/fz_http_web/live/device_live/unprivileged/show_live.ex index 325be440d..7e858d10a 100644 --- a/apps/fz_http/lib/fz_http_web/live/device_live/unprivileged/show_live.ex +++ b/apps/fz_http/lib/fz_http_web/live/device_live/unprivileged/show_live.ex @@ -43,7 +43,9 @@ defmodule FzHttpWeb.DeviceLive.Unprivileged.Show do end def delete_device(device, socket) do - if socket.assigns.current_user.id == device.user_id do + if socket.assigns.current_user.id == device.user_id && + (has_role?(socket.assigns.current_user, :admin) || + Application.fetch_env!(:fz_http, :allow_unprivileged_device_management)) do Devices.delete_device(device) else {:not_authorized} diff --git a/apps/fz_http/lib/fz_http_web/templates/shared/show_device.html.heex b/apps/fz_http/lib/fz_http_web/templates/shared/show_device.html.heex index 17ace8ad9..e4bcea126 100644 --- a/apps/fz_http/lib/fz_http_web/templates/shared/show_device.html.heex +++ b/apps/fz_http/lib/fz_http_web/templates/shared/show_device.html.heex @@ -6,18 +6,20 @@ <%= render(FzHttpWeb.SharedView, "device_details.html", assigns) %> -
-

- Danger Zone -

+<%= if FzHttpWeb.DeviceView.can_manage_devices?(@current_user) do %> +
+

+ Danger Zone +

- -
+ +
+<% end %> diff --git a/apps/fz_http/lib/fz_http_web/views/device_view.ex b/apps/fz_http/lib/fz_http_web/views/device_view.ex index caf8bb4dc..fb01265a2 100644 --- a/apps/fz_http/lib/fz_http_web/views/device_view.ex +++ b/apps/fz_http/lib/fz_http_web/views/device_view.ex @@ -1,3 +1,8 @@ defmodule FzHttpWeb.DeviceView do use FzHttpWeb, :view + + def can_manage_devices?(user) do + has_role?(user, :admin) || + Application.fetch_env!(:fz_http, :allow_unprivileged_device_management) + end end diff --git a/apps/fz_http/test/fz_http_web/live/device_live/unprivileged/index_test.exs b/apps/fz_http/test/fz_http_web/live/device_live/unprivileged/index_test.exs index b39ec73ee..7d09bd43d 100644 --- a/apps/fz_http/test/fz_http_web/live/device_live/unprivileged/index_test.exs +++ b/apps/fz_http/test/fz_http_web/live/device_live/unprivileged/index_test.exs @@ -1,13 +1,14 @@ defmodule FzHttpWeb.DeviceLive.Unprivileged.IndexTest do use FzHttpWeb.ConnCase, async: false - # alias FzHttp.{Devices, Devices.Device} - describe "authenticated/device list" do - setup :create_devices + test "includes the device name in the list", %{ + unprivileged_user: user, + unprivileged_conn: conn + } do + {:ok, devices: devices} = create_devices(user_id: user.id) - test "includes the device name in the list", %{admin_conn: conn, devices: devices} do - path = Routes.device_admin_index_path(conn, :index) + path = Routes.device_unprivileged_index_path(conn, :index) {:ok, _view, html} = live(conn, path) for device <- devices do @@ -25,6 +26,32 @@ defmodule FzHttpWeb.DeviceLive.Unprivileged.IndexTest do end end + describe "authenticated device management disabled" do + setup do + restore_env(:allow_unprivileged_device_management, false, &on_exit/1) + end + + test "omits Add Device button", %{unprivileged_conn: conn} do + path = Routes.device_unprivileged_index_path(conn, :index) + {:ok, _view, html} = live(conn, path) + + refute html =~ "Add Device" + end + + test "prevents creating a device", %{unprivileged_conn: conn} do + path = Routes.device_unprivileged_index_path(conn, :new) + {:ok, view, _html} = live(conn, path) + + new_view = + view + |> element("#create-device") + |> render_submit(%{"device" => %{"public_key" => "test-pubkey", "name" => "test-tunnel"}}) + + assert new_view =~ "Must be an administrator to manage devices." + refute new_view =~ "Device added!" + end + end + describe "authenticated/creates device" do test "shows new form", %{unprivileged_conn: conn} do path = Routes.device_unprivileged_index_path(conn, :index) @@ -43,7 +70,7 @@ defmodule FzHttpWeb.DeviceLive.Unprivileged.IndexTest do """ end - test "creates tunnel", %{unprivileged_conn: conn} do + test "creates device", %{unprivileged_conn: conn} do path = Routes.device_unprivileged_index_path(conn, :new) {:ok, view, _html} = live(conn, path) diff --git a/apps/fz_http/test/fz_http_web/live/device_live/unprivileged/show_test.exs b/apps/fz_http/test/fz_http_web/live/device_live/unprivileged/show_test.exs new file mode 100644 index 000000000..3431efaf2 --- /dev/null +++ b/apps/fz_http/test/fz_http_web/live/device_live/unprivileged/show_test.exs @@ -0,0 +1,44 @@ +defmodule FzHttpWeb.DeviceLive.Unprivileged.ShowTest do + use FzHttpWeb.ConnCase, async: true + + describe "unauthenticated" do + setup :create_device + + @tag :unauthed + test "mount redirects to session path", %{unauthed_conn: conn, device: device} do + path = Routes.device_admin_show_path(conn, :show, device) + expected_path = Routes.root_path(conn, :index) + assert {:error, {:redirect, %{to: ^expected_path}}} = live(conn, path) + end + end + + describe "authenticated; device management disabled" do + test "prevents deleting a device; doesn't show button", %{ + unprivileged_user: user, + unprivileged_conn: conn + } do + {:ok, device: device} = create_device(user_id: user.id) + restore_env(:allow_unprivileged_device_management, false, &on_exit/1) + + path = Routes.device_unprivileged_show_path(conn, :show, device) + {:ok, _view, html} = live(conn, path) + + refute html =~ "Delete Device" + end + end + + # XXX: Revisit this when RBAC is more fleshed out. Admins can now view other admins' devices. + # describe "authenticated as other user" do + # setup [:create_device, :create_other_user_device] + # + # test "mount redirects to session path", %{ + # admin_conn: conn, + # device: _device, + # other_device: other_device + # } do + # path = Routes.device_admin_show_path(conn, :show, other_device) + # expected_path = Routes.auth_path(conn, :request) + # assert {:error, {:redirect, %{to: ^expected_path}}} = live(conn, path) + # end + # end +end diff --git a/apps/fz_http/test/support/test_helpers.ex b/apps/fz_http/test/support/test_helpers.ex index 149f372a6..edf458bd7 100644 --- a/apps/fz_http/test/support/test_helpers.ex +++ b/apps/fz_http/test/support/test_helpers.ex @@ -76,8 +76,14 @@ defmodule FzHttp.TestHelpers do {:ok, devices: devices} end - def create_user(_) do - user = UsersFixtures.user() + def create_user(tags) do + user = + if tags[:unprivileged] do + UsersFixtures.user(%{role: :unprivileged}) + else + UsersFixtures.user() + end + {:ok, user: user} end diff --git a/config/config.exs b/config/config.exs index fd410fe6d..d224f7d86 100644 --- a/config/config.exs +++ b/config/config.exs @@ -49,6 +49,7 @@ config :fz_http, FzHttpWeb.Authentication, config :fz_http, sandbox: true, + allow_unprivileged_device_management: true, telemetry_id: "543aae08-5a2b-428d-b704-2956dd3f5a57", wireguard_endpoint: nil, wireguard_dns: "1.1.1.1, 1.0.0.1", diff --git a/config/releases.exs b/config/releases.exs index 9f0c5b63d..63700aee9 100644 --- a/config/releases.exs +++ b/config/releases.exs @@ -40,6 +40,9 @@ telemetry_id = System.fetch_env!("TELEMETRY_ID") guardian_secret_key = System.fetch_env!("GUARDIAN_SECRET_KEY") external_url = System.fetch_env!("EXTERNAL_URL") +allow_unprivileged_device_management = + FzString.to_boolean(System.fetch_env!("ALLOW_UNPRIVILEGED_DEVICE_MANAGEMENT")) + # Local auth local_auth_enabled = FzString.to_boolean(System.fetch_env!("LOCAL_AUTH_ENABLED")) @@ -154,6 +157,7 @@ config :fz_http, FzHttpWeb.Authentication, secret_key: guardian_secret_key config :fz_http, + allow_unprivileged_device_management: allow_unprivileged_device_management, max_devices_per_user: max_devices_per_user, local_auth_enabled: local_auth_enabled, okta_auth_enabled: okta_auth_enabled, diff --git a/docs/docs/reference/configuration-file.md b/docs/docs/reference/configuration-file.md index 2b835a604..33fd8867e 100644 --- a/docs/docs/reference/configuration-file.md +++ b/docs/docs/reference/configuration-file.md @@ -26,6 +26,8 @@ Shown below is a complete listing of the configuration options available in | `default['firezone']['user']` | Name of unprivileged Linux user most services and files will belong to. | `'firezone'` | | `default['firezone']['group']` | Name of Linux group most services and files will belong to. | `'firezone'` | | `default['firezone']['admin_email']` | Email address for initial Firezone user. | `"firezone@localhost"` | +| `default['firezone']['max_devices_per_user']` | Maximum number of devices a user can have. | `10` | +| `default['firezone']['allow_unprivileged_device_management']` | Allows non-admin users to create and manage devices. | `true` | | `default['firezone']['egress_interface']` | Interface name where tunneled traffic will exit. If nil, the default route interface will be used. | `nil` | | `default['firezone']['fips_enabled']` | Enable or disable OpenSSL FIPs mode. | `nil` | | `default['firezone']['logging']['enabled']` | Enable or disable logging across Firezone. Set to `false` to disable logging entirely. | `true` | @@ -143,7 +145,6 @@ Shown below is a complete listing of the configuration options available in | `default['firezone']['wireguard']['ipv6']['enabled']` | Enable or disable IPv6 for WireGuard network. | `true` | | `default['firezone']['wireguard']['ipv6']['network']` | WireGuard network IPv6 address pool. | `'fd00::3:2:0/120'` | | `default['firezone']['wireguard']['ipv6']['address']` | WireGuard interface IPv6 address. Must be within IPv6 address pool. | `'fd00::3:2:1'` | -| `default['firezone']['wireguard']['max_devices_per_user']` | Maximum number of devices a user can have. | `10` | | `default['firezone']['runit']['svlogd_bin']` | Runit svlogd bin location. | `"#{node['firezone']['install_directory']}/embedded/bin/svlogd"` | | `default['firezone']['ssl']['directory']` | SSL directory for storing generated certs. | `'/var/opt/firezone/ssl'` | | `default['firezone']['ssl']['enabled']` | Enable or disable SSL for nginx. | `true` | diff --git a/omnibus/cookbooks/firezone/attributes/default.rb b/omnibus/cookbooks/firezone/attributes/default.rb index 91320f3f5..86e28b24d 100644 --- a/omnibus/cookbooks/firezone/attributes/default.rb +++ b/omnibus/cookbooks/firezone/attributes/default.rb @@ -41,6 +41,10 @@ default['firezone']['admin_email'] = 'firezone@localhost' # Default: 10 default['firezone']['max_devices_per_user'] = 10 +# Allow users to create (and download) their own devices. Set to false +# if you only want administrators to create and manage devices. +default['firezone']['allow_unprivileged_device_management'] = true + default['firezone']['config_directory'] = '/etc/firezone' default['firezone']['install_directory'] = '/opt/firezone' default['firezone']['app_directory'] = "#{node['firezone']['install_directory']}/embedded/service/firezone" diff --git a/omnibus/cookbooks/firezone/libraries/config.rb b/omnibus/cookbooks/firezone/libraries/config.rb index f866d2914..263a7550f 100644 --- a/omnibus/cookbooks/firezone/libraries/config.rb +++ b/omnibus/cookbooks/firezone/libraries/config.rb @@ -243,6 +243,7 @@ class Firezone 'WIREGUARD_IPV6_NETWORK' => attributes['wireguard']['ipv6']['network'], 'WIREGUARD_IPV6_ADDRESS' => attributes['wireguard']['ipv6']['address'], 'MAX_DEVICES_PER_USER' => attributes['max_devices_per_user'].to_s, + 'ALLOW_UNPRIVILEGED_DEVICE_MANAGEMENT' => attributes['allow_unprivileged_device_management'].to_s, # Allow env var to override config 'TELEMETRY_ENABLED' => ENV.fetch('TELEMETRY_ENABLED', attributes['telemetry']['enabled'] == false ? 'false' : 'true'),