From 218ad006af33cd4b532df2ca44ecdd71c83648c9 Mon Sep 17 00:00:00 2001 From: Andrew Dryga Date: Mon, 16 Jan 2023 13:04:59 -0600 Subject: [PATCH] Revisit Users context, cover UI with e2e tests and introduce first AuditLog features (#1267) 1. `auto_create_users` default value is removed. We want to avoid situations when admins integrate OIDC/SAML providers and don't expect anyone that has access to it to automatically gain access to VPN, which is especially critical for providers like Google Workspace, where all employees typically have access. 2. OpenID library was completely rewritten and a new version is integrated. It will allow async tests and better scales for the cloud version of the panel. 3. `Mox` was removed, we don't test modules by overriding them to prevent breaking changes that tests can't capture. 4. Deps are reordered and unused ones are removed. 5. Browser/e2e tests are added to ensure we won't break UI features in the future, allowing for front-end refactoring. 6. Users context was overhauled for better code clarity. --- .credo.exs | 3 +- .formatter.exs | 16 +- .github/workflows/test.yml | 136 ++++ apps/fz_common/.formatter.exs | 9 + apps/fz_common/lib/fz_crypto.ex | 11 +- apps/fz_common/mix.exs | 7 +- apps/fz_http/.formatter.exs | 18 + apps/fz_http/assets/js/live_view.js | 4 +- apps/fz_http/lib/fz_http.ex | 13 + apps/fz_http/lib/fz_http/application.ex | 12 +- apps/fz_http/lib/fz_http/configurations.ex | 40 +- .../fz_http/configurations/configuration.ex | 14 +- .../configuration/openid_connect_provider.ex | 21 +- .../configuration/saml_identity_provider.ex | 18 +- apps/fz_http/lib/fz_http/devices.ex | 4 +- apps/fz_http/lib/fz_http/devices/device.ex | 26 +- .../lib/fz_http/devices/device/query.ex | 16 +- apps/fz_http/lib/fz_http/events.ex | 4 +- apps/fz_http/lib/fz_http/oidc/refresher.ex | 31 +- apps/fz_http/lib/fz_http/oidc/start_proxy.ex | 55 -- apps/fz_http/lib/fz_http/oidc/supervisor.ex | 0 apps/fz_http/lib/fz_http/release.ex | 18 +- apps/fz_http/lib/fz_http/repo.ex | 20 +- apps/fz_http/lib/fz_http/telemetry.ex | 2 +- apps/fz_http/lib/fz_http/users.ex | 314 +++----- .../lib/fz_http/users/password_helpers.ex | 43 - apps/fz_http/lib/fz_http/users/user.ex | 123 +-- .../lib/fz_http/users/user/changeset.ex | 68 ++ apps/fz_http/lib/fz_http/users/user/query.ex | 45 ++ .../{validators/common.ex => validator.ex} | 96 ++- .../lib/fz_http/validators/openid_connect.ex | 21 - apps/fz_http/lib/fz_http/validators/saml.ex | 21 - .../fz_http_web/auth/html/authentication.ex | 20 +- .../fz_http_web/auth/json/authentication.ex | 2 +- .../channels/notification_channel.ex | 25 +- .../lib/fz_http_web/controller_helpers.ex | 4 + .../controllers/auth_controller.ex | 69 +- .../controllers/json/user_controller.ex | 32 +- .../controllers/user_controller.ex | 2 +- apps/fz_http/lib/fz_http_web/endpoint.ex | 3 +- .../live/device_live/admin/show_live.ex | 2 +- .../device_live/unprivileged/show_live.ex | 2 +- .../live/hooks/allow_ecto_sandbox.ex | 6 + .../live/mfa_live/register_steps_component.ex | 3 +- .../live/setting_live/account.html.heex | 2 +- .../setting_live/account_form_component.ex | 2 +- .../account_form_component.html.heex | 15 - .../live/setting_live/account_live.ex | 4 +- .../live/setting_live/security.html.heex | 4 + .../setting_live/show_api_token_component.ex | 2 +- .../live/user_live/form_component.ex | 2 +- .../live/user_live/form_component.html.heex | 5 +- .../fz_http_web/live/user_live/index_live.ex | 4 +- .../fz_http_web/live/user_live/show_live.ex | 8 +- .../user_live/vpn_connection_component.ex | 1 + apps/fz_http/lib/fz_http_web/live_helpers.ex | 4 +- .../lib/fz_http_web/mailer/auth_email.ex | 2 +- apps/fz_http/lib/fz_http_web/oidc/helpers.ex | 9 - .../fz_http_web/{channels => }/presence.ex | 0 apps/fz_http/lib/fz_http_web/router.ex | 22 +- apps/fz_http/lib/fz_http_web/sandbox.ex | 41 + .../{channels => sockets}/user_socket.ex | 36 +- .../fz_http/lib/fz_http_web/user_from_auth.ex | 18 +- .../lib/fz_http_web/views/error_view.ex | 4 + apps/fz_http/mix.exs | 88 +- ...104000803_add_users_sign_in_token_hash.exs | 10 + ...104181853_change_users_email_to_citext.exs | 11 + .../connectivity_check_service_test.exs | 1 - apps/fz_http/test/fz_http/devices_test.exs | 2 +- .../test/fz_http/oidc/refresher_test.exs | 41 +- apps/fz_http/test/fz_http/release_test.exs | 7 +- apps/fz_http/test/fz_http/telemetry_test.exs | 13 +- apps/fz_http/test/fz_http/users_test.exs | 754 +++++++++++------- .../fz_http_web/acceptance/admin_test.exs | 673 ++++++++++++++++ .../acceptance/authentication_test.exs | 414 ++++++++++ .../acceptance/unprivileged_user_test.exs | 166 ++++ .../channels/notification_channel_test.exs | 22 +- .../controllers/auth_controller_test.exs | 126 +-- .../controllers/json/user_controller_test.exs | 38 +- .../controllers/user_controller_test.exs | 2 +- .../live/setting_live/account_test.exs | 7 +- .../live/setting_live/security_test.exs | 9 +- .../unprivileged/account_test.exs | 9 +- .../fz_http_web/live/user_live/index_test.exs | 6 +- .../fz_http_web/live/user_live/show_test.exs | 2 +- .../mailer_test/auth_email_test.exs | 28 - .../test/fz_http_web/user_from_auth_test.exs | 28 +- apps/fz_http/test/support/acceptance_case.ex | 297 +++++++ .../test/support/acceptance_case/auth.ex | 91 +++ .../test/support/acceptance_case/vault.ex | 113 +++ apps/fz_http/test/support/data_case.ex | 3 +- .../fixtures/configurations_fixtures.ex | 122 ++- .../test/support/fixtures/devices_fixtures.ex | 13 + .../test/support/fixtures/users_fixtures.ex | 43 +- .../test/support/mocks/openid_connect.ex | 9 - apps/fz_http/test/support/test_helpers.ex | 3 +- apps/fz_http/test/test_helper.exs | 7 +- apps/fz_vpn/.formatter.exs | 9 + apps/fz_vpn/lib/fz_vpn/application.ex | 8 +- apps/fz_vpn/mix.exs | 2 - apps/fz_wall/.formatter.exs | 9 + apps/fz_wall/lib/fz_wall/application.ex | 2 +- apps/fz_wall/mix.exs | 4 +- config/config.exs | 3 +- config/runtime.exs | 2 +- config/test.exs | 15 +- docker-compose.yml | 3 +- mix.exs | 17 +- mix.lock | 16 +- 109 files changed, 3503 insertions(+), 1329 deletions(-) create mode 100644 apps/fz_common/.formatter.exs create mode 100644 apps/fz_http/.formatter.exs delete mode 100644 apps/fz_http/lib/fz_http/oidc/start_proxy.ex create mode 100644 apps/fz_http/lib/fz_http/oidc/supervisor.ex delete mode 100644 apps/fz_http/lib/fz_http/users/password_helpers.ex create mode 100644 apps/fz_http/lib/fz_http/users/user/changeset.ex create mode 100644 apps/fz_http/lib/fz_http/users/user/query.ex rename apps/fz_http/lib/fz_http/{validators/common.ex => validator.ex} (51%) delete mode 100644 apps/fz_http/lib/fz_http/validators/openid_connect.ex delete mode 100644 apps/fz_http/lib/fz_http/validators/saml.ex create mode 100644 apps/fz_http/lib/fz_http_web/live/hooks/allow_ecto_sandbox.ex delete mode 100644 apps/fz_http/lib/fz_http_web/oidc/helpers.ex rename apps/fz_http/lib/fz_http_web/{channels => }/presence.ex (100%) create mode 100644 apps/fz_http/lib/fz_http_web/sandbox.ex rename apps/fz_http/lib/fz_http_web/{channels => sockets}/user_socket.ex (91%) create mode 100644 apps/fz_http/priv/repo/migrations/20230104000803_add_users_sign_in_token_hash.exs create mode 100644 apps/fz_http/priv/repo/migrations/20230104181853_change_users_email_to_citext.exs create mode 100644 apps/fz_http/test/fz_http_web/acceptance/admin_test.exs create mode 100644 apps/fz_http/test/fz_http_web/acceptance/authentication_test.exs create mode 100644 apps/fz_http/test/fz_http_web/acceptance/unprivileged_user_test.exs delete mode 100644 apps/fz_http/test/fz_http_web/mailer_test/auth_email_test.exs create mode 100644 apps/fz_http/test/support/acceptance_case.ex create mode 100644 apps/fz_http/test/support/acceptance_case/auth.ex create mode 100644 apps/fz_http/test/support/acceptance_case/vault.ex delete mode 100644 apps/fz_http/test/support/mocks/openid_connect.ex create mode 100644 apps/fz_vpn/.formatter.exs create mode 100644 apps/fz_wall/.formatter.exs diff --git a/.credo.exs b/.credo.exs index b91764922..f23eb309c 100644 --- a/.credo.exs +++ b/.credo.exs @@ -83,7 +83,7 @@ # Priority values are: `low, normal, high, higher` # {Credo.Check.Design.AliasUsage, - [priority: :low, if_nested_deeper_than: 2, if_called_more_often_than: 2]}, + [priority: :low, if_nested_deeper_than: 5, if_called_more_often_than: 6]}, # You can also customize the exit_status of each check. # If you don't want TODO comments to cause `mix credo` to fail, just # set this value to 0 (zero). @@ -112,7 +112,6 @@ {Credo.Check.Readability.TrailingWhiteSpace, []}, {Credo.Check.Readability.UnnecessaryAliasExpansion, []}, {Credo.Check.Readability.VariableNames, []}, - {Credo.Check.Readability.WithSingleClause, []}, # ## Refactoring Opportunities diff --git a/.formatter.exs b/.formatter.exs index 2ded1cba3..e72b167e6 100644 --- a/.formatter.exs +++ b/.formatter.exs @@ -1,17 +1,7 @@ -# Used by "mix format" [ - import_deps: [ - :ecto, - :phoenix - ], + subdirectories: ["apps/*"], inputs: [ - "*.{heex,ex,exs}", - "{config,priv}/**/*.{heex,ex,exs}", - "apps/{fz_vpn,fz_wall}/**/*.{heex,ex,exs}", - "apps/fz_http/*.exs", - "apps/fz_http/{lib,test,priv}/**/*.{heex,ex,exs}" - ], - plugins: [ - Phoenix.LiveView.HTMLFormatter + "*.{ex,exs}", + "{config,priv}/**/*.{ex,exs}" ] ] diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4e877d8a5..b4fcb50fb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -54,11 +54,147 @@ jobs: key: ${{ runner.os }}-${{ env.cache-name }}-${{ hashFiles('**/mix.lock') }} - name: Install Dependencies run: mix deps.get --only $MIX_ENV + - name: Compile Dependencies + run: mix deps.compile --skip-umbrella-children + - name: Compile Application + run: mix compile - name: Setup Database run: | mix ecto.create mix ecto.migrate - name: Run Tests and Upload Coverage Report + env: + E2E_MAX_WAIT_SECONDS: 20 run: | # XXX: This can fail when coveralls is down mix coveralls.github --umbrella + - name: Test Report + uses: dorny/test-reporter@v1 + if: success() || failure() + with: + name: Elixir Unit Test Report + path: _build/test/lib/*/test-junit-report.xml + reporter: java-junit + acceptance-test: + runs-on: ubuntu-latest + env: + MIX_ENV: test + POSTGRES_HOST: localhost + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + MIX_TEST_PARTITIONS: 4 + strategy: + fail-fast: false + matrix: + MIX_TEST_PARTITION: [1, 2, 3, 4] + services: + postgres: + image: postgres:15 + ports: + - 5432:5432 + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + vault: + image: vault:1.12.2 + env: + VAULT_ADDR: 'http://127.0.0.1:8200' + VAULT_DEV_ROOT_TOKEN_ID: 'firezone' + ports: + - 8200:8200/tcp + options: + --cap-add=IPC_LOCK + steps: + - uses: nanasess/setup-chromedriver@v1 + with: + chromedriver-version: '108.0.5359.71' + - run: | + export DISPLAY=:99 + chromedriver --url-base=/wd/hub & + sudo Xvfb -ac :99 -screen 0 1280x1024x24 > /dev/null 2>&1 & + - name: Install package dependencies + run: | + sudo apt-get install -q -y \ + net-tools \ + wireguard + - uses: actions/checkout@v3 + - uses: erlef/setup-beam@v1 + with: + otp-version: '25' + elixir-version: '1.14' + - uses: actions/cache@v3 + name: Elixir Deps Cache + env: + cache-name: cache-elixir-deps + with: + path: deps + key: ${{ runner.os }}-${{ env.cache-name }}-${{ hashFiles('**/mix.lock') }} + restore-keys: | + ${{ runner.os }}-${{ env.cache-name }}- + - uses: actions/cache@v3 + name: Elixir Build Cache + env: + cache-name: cache-elixir-build + with: + path: _build + key: ${{ runner.os }}-${{ env.cache-name }}-${{ hashFiles('**/mix.lock') }} + - uses: actions/cache@v3 + name: Yarn Deps Cache + env: + cache-name: cache-yarn-build + with: + path: apps/fz_http/assets/node_modules + key: ${{ runner.os }}-${{ env.cache-name }}-${{ hashFiles('**/yarn.lock') }} + - uses: actions/cache@v3 + name: Assets Cache + env: + cache-name: cache-assets-build + with: + path: apps/fz_http/priv/static/dist + key: ${{ runner.os }}-${{ env.cache-name }}-${{ hashFiles('**/yarn.lock') }} + - name: Install Dependencies + run: mix deps.get --only $MIX_ENV + - name: Compile Dependencies + run: mix deps.compile --skip-umbrella-children + - name: Compile Application + run: mix compile + - name: Install Node Dependencies + run: | + cd apps/fz_http/assets + yarn install --frozen-lockfile + - name: Build Assets + run: | + cd apps/fz_http/assets + yarn deploy + - name: Setup Database + run: | + mix ecto.create + mix ecto.migrate + - name: Run Tests and Upload Coverage Report + env: + MIX_TEST_PARTITION: ${{ matrix.MIX_TEST_PARTITION }} + E2E_MAX_WAIT_SECONDS: 20 + run: | + mix test --only acceptance:true \ + --partitions=${{ env.MIX_TEST_PARTITIONS }} \ + --no-compile \ + --no-archives-check \ + --no-deps-check \ + || mix test --failed + - name: Save Screenshots + if: always() + uses: actions/upload-artifact@v3 + with: + name: screenshots + path: apps/fz_http/screenshots + - name: Test Report + uses: dorny/test-reporter@v1 + if: success() || failure() + with: + name: Elixir Acceptance Test Report + path: _build/test/lib/*/test-junit-report.xml + reporter: java-junit diff --git a/apps/fz_common/.formatter.exs b/apps/fz_common/.formatter.exs new file mode 100644 index 000000000..7f9277b11 --- /dev/null +++ b/apps/fz_common/.formatter.exs @@ -0,0 +1,9 @@ +[ + locals_without_parens: [], + import_deps: [], + inputs: [ + "*.{ex,exs}", + "{lib,test,priv}/**/*.{ex,exs}" + ], + plugins: [] +] diff --git a/apps/fz_common/lib/fz_crypto.ex b/apps/fz_common/lib/fz_crypto.ex index db9cc362b..c657fc8c7 100644 --- a/apps/fz_common/lib/fz_crypto.ex +++ b/apps/fz_common/lib/fz_crypto.ex @@ -1,8 +1,4 @@ defmodule FzCommon.FzCrypto do - @moduledoc """ - Utilities for working with crypto functions - """ - @wg_psk_length 32 def psk do @@ -20,6 +16,7 @@ defmodule FzCommon.FzCrypto do defp rand_base64(length, :url) do :crypto.strong_rand_bytes(length) + # XXX: we want to add `padding: false` to shorten URLs |> Base.url_encode64() end @@ -27,4 +24,10 @@ defmodule FzCommon.FzCrypto do :crypto.strong_rand_bytes(length) |> Base.encode64() end + + def hash(value), do: Argon2.hash_pwd_salt(value) + + def equal?(token, hash) when is_nil(token) or is_nil(hash), do: Argon2.no_user_verify() + def equal?(token, hash) when token == "" or hash == "", do: Argon2.no_user_verify() + def equal?(token, hash), do: Argon2.verify_pass(token, hash) end diff --git a/apps/fz_common/mix.exs b/apps/fz_common/mix.exs index 7eb9a65b9..3b96d9522 100644 --- a/apps/fz_common/mix.exs +++ b/apps/fz_common/mix.exs @@ -32,12 +32,9 @@ defmodule FzCommon.MixProject do defp deps do [ {:file_size, "~> 3.0.1"}, + {:cidr, github: "firezone/cidr-elixir"}, {:posthog, "~> 0.1"}, - {:jason, "~> 1.2"}, - {:cidr, github: "firezone/cidr-elixir"} - # {:dep_from_hexpm, "~> 0.3.0"}, - # {:dep_from_git, git: "https://github.com/elixir-lang/my_dep.git", tag: "0.1.0"}, - # {:sibling_app_in_umbrella, in_umbrella: true} + {:argon2_elixir, "~> 2.0"} ] end end diff --git a/apps/fz_http/.formatter.exs b/apps/fz_http/.formatter.exs new file mode 100644 index 000000000..ae02b79cd --- /dev/null +++ b/apps/fz_http/.formatter.exs @@ -0,0 +1,18 @@ +[ + locals_without_parens: [ + assert_authenticated: 2, + assert_unauthenticated: 1 + ], + import_deps: [ + :ecto, + :phoenix, + :phoenix_live_view + ], + inputs: [ + "*.{heex,ex,exs}", + "{lib,test,priv}/**/*.{heex,ex,exs}" + ], + plugins: [ + Phoenix.LiveView.HTMLFormatter + ] +] diff --git a/apps/fz_http/assets/js/live_view.js b/apps/fz_http/assets/js/live_view.js index 1ed31568c..fb3c8f2a1 100644 --- a/apps/fz_http/assets/js/live_view.js +++ b/apps/fz_http/assets/js/live_view.js @@ -20,8 +20,7 @@ const channelToken = document .getAttribute("content") const notificationChannel = userSocket.channel("notification:session", { - token: channelToken, - user_agent: window.navigator.userAgent + token: channelToken }) // LiveView setup @@ -71,3 +70,4 @@ notificationChannel.join() // >> liveSocket.enableLatencySim(1000) window.liveSocket = liveSocket +window.userSocket = userSocket diff --git a/apps/fz_http/lib/fz_http.ex b/apps/fz_http/lib/fz_http.ex index 913700aa9..481a2438e 100644 --- a/apps/fz_http/lib/fz_http.ex +++ b/apps/fz_http/lib/fz_http.ex @@ -12,6 +12,19 @@ defmodule FzHttp do end end + def changeset do + quote do + import Ecto.Changeset + import FzHttp.Validator + end + end + + def query do + quote do + import Ecto.Query + end + end + @doc """ When used, dispatch to the appropriate schema/context/changeset/query/etc. """ diff --git a/apps/fz_http/lib/fz_http/application.ex b/apps/fz_http/lib/fz_http/application.ex index 45dac53b8..a7a1af5da 100644 --- a/apps/fz_http/lib/fz_http/application.ex +++ b/apps/fz_http/lib/fz_http/application.ex @@ -11,7 +11,7 @@ defmodule FzHttp.Application do # See https://hexdocs.pm/elixir/Supervisor.html # for other strategies and supported options Telemetry.fz_http_started() - opts = [strategy: :one_for_one, name: FzHttp.Supervisor] + opts = [strategy: :one_for_one, name: __MODULE__.Supervisor] Supervisor.start_link(children(), opts) end @@ -31,17 +31,16 @@ defmodule FzHttp.Application do {Postgrex.Notifications, [name: FzHttp.Repo.Notifications] ++ FzHttp.Repo.config()}, FzHttp.Repo.Notifier, FzHttp.Vault, - FzHttpWeb.Endpoint, {Phoenix.PubSub, name: FzHttp.PubSub}, {FzHttp.Notifications, name: FzHttp.Notifications}, FzHttpWeb.Presence, FzHttp.ConnectivityCheckService, FzHttp.TelemetryPingService, FzHttp.VpnSessionScheduler, - FzHttp.OIDC.StartProxy, FzHttp.SAML.StartProxy, {DynamicSupervisor, name: FzHttp.RefresherSupervisor, strategy: :one_for_one}, - FzHttp.OIDC.RefreshManager + FzHttp.OIDC.RefreshManager, + FzHttpWeb.Endpoint ] end @@ -50,12 +49,11 @@ defmodule FzHttp.Application do FzHttp.Server, FzHttp.Repo, FzHttp.Vault, - FzHttpWeb.Endpoint, - {FzHttp.OIDC.StartProxy, :test}, {FzHttp.SAML.StartProxy, :test}, {Phoenix.PubSub, name: FzHttp.PubSub}, {FzHttp.Notifications, name: FzHttp.Notifications}, - FzHttpWeb.Presence + FzHttpWeb.Presence, + FzHttpWeb.Endpoint ] end diff --git a/apps/fz_http/lib/fz_http/configurations.ex b/apps/fz_http/lib/fz_http/configurations.ex index d4896ac80..d5e847828 100644 --- a/apps/fz_http/lib/fz_http/configurations.ex +++ b/apps/fz_http/lib/fz_http/configurations.ex @@ -11,21 +11,44 @@ defmodule FzHttp.Configurations do Map.get(get_configuration!(), key) end + def fetch_oidc_provider_config(provider_id) do + get!(:openid_connect_providers) + |> Enum.find(&(&1.id == provider_id)) + |> case do + nil -> + {:error, :not_found} + + provider -> + external_url = FzHttp.Config.fetch_env!(:fz_http, :external_url) + + {:ok, + %{ + discovery_document_uri: provider.discovery_document_uri, + client_id: provider.client_id, + client_secret: provider.client_secret, + redirect_uri: + provider.redirect_uri || "#{external_url}/auth/oidc/#{provider.id}/callback/", + response_type: provider.response_type, + scope: provider.scope + }} + end + end + def put!(key, val) do - get_configuration!() - |> Configuration.changeset(%{key => val}) - |> Repo.update!() + configuration = + get_configuration!() + |> Configuration.changeset(%{key => val}) + |> Repo.update!() + + FzHttp.SAML.StartProxy.restart() + + configuration end def get_configuration! do Repo.one!(Configuration) end - def get_provider_by_id(field, provider_id) do - FzHttp.Configurations.get!(field) - |> Enum.find(&(&1.id == provider_id)) - end - def auto_create_users?(field, provider_id) do FzHttp.Configurations.get!(field) |> Enum.find(&(&1.id == provider_id)) @@ -47,7 +70,6 @@ defmodule FzHttp.Configurations do case Repo.update(Configuration.changeset(config, attrs)) do {:ok, configuration} -> FzHttp.SAML.StartProxy.restart() - FzHttp.OIDC.StartProxy.restart() {:ok, configuration} diff --git a/apps/fz_http/lib/fz_http/configurations/configuration.ex b/apps/fz_http/lib/fz_http/configurations/configuration.ex index 0236c0a2e..be521ca50 100644 --- a/apps/fz_http/lib/fz_http/configurations/configuration.ex +++ b/apps/fz_http/lib/fz_http/configurations/configuration.ex @@ -7,7 +7,7 @@ defmodule FzHttp.Configurations.Configuration do alias FzHttp.{ Configurations.Logo, - Validators.Common + Validator } @min_mtu 576 @@ -83,12 +83,12 @@ defmodule FzHttp.Configurations.Configuration do |> cast_embed(:saml_identity_providers, with: {FzHttp.Configurations.Configuration.SAMLIdentityProvider, :changeset, []} ) - |> Common.trim_change(:default_client_dns) - |> Common.trim_change(:default_client_allowed_ips) - |> Common.trim_change(:default_client_endpoint) - |> Common.validate_no_duplicates(:default_client_dns) - |> Common.validate_list_of_ips_or_cidrs(:default_client_allowed_ips) - |> Common.validate_no_duplicates(:default_client_allowed_ips) + |> Validator.trim_change(:default_client_dns) + |> Validator.trim_change(:default_client_allowed_ips) + |> Validator.trim_change(:default_client_endpoint) + |> Validator.validate_no_duplicates(:default_client_dns) + |> Validator.validate_list_of_ips_or_cidrs(:default_client_allowed_ips) + |> Validator.validate_no_duplicates(:default_client_allowed_ips) |> validate_number(:default_client_mtu, greater_than_or_equal_to: @min_mtu, less_than_or_equal_to: @max_mtu diff --git a/apps/fz_http/lib/fz_http/configurations/configuration/openid_connect_provider.ex b/apps/fz_http/lib/fz_http/configurations/configuration/openid_connect_provider.ex index 8597b7888..c503e70c7 100644 --- a/apps/fz_http/lib/fz_http/configurations/configuration/openid_connect_provider.ex +++ b/apps/fz_http/lib/fz_http/configurations/configuration/openid_connect_provider.ex @@ -4,7 +4,7 @@ defmodule FzHttp.Configurations.Configuration.OpenIDConnectProvider do """ use FzHttp, :schema import Ecto.Changeset - alias FzHttp.Validators + alias FzHttp.Validator @reserved_config_ids [ "identity", @@ -23,7 +23,7 @@ defmodule FzHttp.Configurations.Configuration.OpenIDConnectProvider do field :client_secret, :string field :discovery_document_uri, :string field :redirect_uri, :string - field :auto_create_users, :boolean, default: true + field :auto_create_users, :boolean end def changeset(struct \\ %__MODULE__{}, data) do @@ -54,9 +54,22 @@ defmodule FzHttp.Configurations.Configuration.OpenIDConnectProvider do ]) # Don't allow users to enter reserved config ids |> validate_exclusion(:id, @reserved_config_ids) - |> Validators.OpenIDConnect.validate_discovery_document_uri() - |> Validators.Common.validate_uri([ + |> validate_discovery_document_uri() + |> Validator.validate_uri([ :redirect_uri ]) end + + def validate_discovery_document_uri(changeset) do + changeset + |> validate_change(:discovery_document_uri, fn :discovery_document_uri, value -> + case OpenIDConnect.Document.fetch_document(value) do + {:ok, _update_result} -> + [] + + {:error, reason} -> + [discovery_document_uri: "is invalid. Reason: #{inspect(reason)}"] + end + end) + end end diff --git a/apps/fz_http/lib/fz_http/configurations/configuration/saml_identity_provider.ex b/apps/fz_http/lib/fz_http/configurations/configuration/saml_identity_provider.ex index 9317b8930..5fab12e57 100644 --- a/apps/fz_http/lib/fz_http/configurations/configuration/saml_identity_provider.ex +++ b/apps/fz_http/lib/fz_http/configurations/configuration/saml_identity_provider.ex @@ -4,7 +4,6 @@ defmodule FzHttp.Configurations.Configuration.SAMLIdentityProvider do """ use FzHttp, :schema import Ecto.Changeset - alias FzHttp.Validators @primary_key false embedded_schema do @@ -16,7 +15,7 @@ defmodule FzHttp.Configurations.Configuration.SAMLIdentityProvider do field :sign_metadata, :boolean, default: true field :signed_assertion_in_resp, :boolean, default: true field :signed_envelopes_in_resp, :boolean, default: true - field :auto_create_users, :boolean, default: true + field :auto_create_users, :boolean end def changeset(struct \\ %__MODULE__{}, data) do @@ -38,6 +37,19 @@ defmodule FzHttp.Configurations.Configuration.SAMLIdentityProvider do :metadata, :auto_create_users ]) - |> Validators.SAML.validate_metadata() + |> validate_metadata() + end + + def validate_metadata(changeset) do + changeset + |> validate_change(:metadata, fn :metadata, value -> + try do + Samly.IdpData.from_xml(value, %Samly.IdpData{}) + [] + catch + :exit, e -> + [metadata: "is invalid. Details: #{inspect(e)}."] + end + end) end end diff --git a/apps/fz_http/lib/fz_http/devices.ex b/apps/fz_http/lib/fz_http/devices.ex index 868278845..62ff3f6df 100644 --- a/apps/fz_http/lib/fz_http/devices.ex +++ b/apps/fz_http/lib/fz_http/devices.ex @@ -120,14 +120,12 @@ defmodule FzHttp.Devices do end def to_peer_list do - vpn_duration = Configurations.vpn_duration() - Repo.all( from d in Device, preload: :user ) |> Enum.filter(fn device -> - !device.user.disabled_at && !Users.vpn_session_expired?(device.user, vpn_duration) + !device.user.disabled_at && !Users.vpn_session_expired?(device.user) end) |> Enum.map(fn device -> %{ diff --git a/apps/fz_http/lib/fz_http/devices/device.ex b/apps/fz_http/lib/fz_http/devices/device.ex index 1ae8d9193..dc1671954 100644 --- a/apps/fz_http/lib/fz_http/devices/device.ex +++ b/apps/fz_http/lib/fz_http/devices/device.ex @@ -4,7 +4,7 @@ defmodule FzHttp.Devices.Device do """ use FzHttp, :schema import Ecto.Changeset - alias FzHttp.Validators.Common + alias FzHttp.Validator alias FzHttp.Devices require Logger @@ -72,8 +72,8 @@ defmodule FzHttp.Devices.Device do def create_changeset(attrs) do %__MODULE__{} |> cast(attrs, @fields) - |> Common.put_default_value(:name, &FzHttp.Devices.new_name/0) - |> Common.put_default_value(:preshared_key, &FzCommon.FzCrypto.psk/0) + |> Validator.put_default_value(:name, &FzHttp.Devices.new_name/0) + |> Validator.put_default_value(:preshared_key, &FzCommon.FzCrypto.psk/0) |> changeset() |> validate_max_devices() |> validate_required(@required_fields) @@ -88,13 +88,13 @@ defmodule FzHttp.Devices.Device do defp changeset(changeset) do changeset - |> Common.trim_change(:allowed_ips) - |> Common.trim_change(:dns) - |> Common.trim_change(:endpoint) - |> Common.trim_change(:name) - |> Common.trim_change(:description) - |> Common.validate_base64(:public_key) - |> Common.validate_base64(:preshared_key) + |> Validator.trim_change(:allowed_ips) + |> Validator.trim_change(:dns) + |> Validator.trim_change(:endpoint) + |> Validator.trim_change(:name) + |> Validator.trim_change(:description) + |> Validator.validate_base64(:public_key) + |> Validator.validate_base64(:preshared_key) |> validate_length(:public_key, is: @key_length) |> validate_length(:preshared_key, is: @key_length) |> validate_length(:description, max: @description_max_length) @@ -108,8 +108,8 @@ defmodule FzHttp.Devices.Device do persistent_keepalive mtu ]a) - |> Common.validate_list_of_ips_or_cidrs(:allowed_ips) - |> Common.validate_no_duplicates(:dns) + |> Validator.validate_list_of_ips_or_cidrs(:allowed_ips) + |> Validator.validate_no_duplicates(:dns) |> validate_number(:persistent_keepalive, greater_than_or_equal_to: 0, less_than_or_equal_to: 120 @@ -194,7 +194,7 @@ defmodule FzHttp.Devices.Device do end defp validate_omitted_if_default(changeset, fields) when is_list(fields) do - Common.validate_omitted( + Validator.validate_omitted( changeset, filter_default_fields(changeset, fields, use_default: true) ) diff --git a/apps/fz_http/lib/fz_http/devices/device/query.ex b/apps/fz_http/lib/fz_http/devices/device/query.ex index 31088887c..4bd5d2e83 100644 --- a/apps/fz_http/lib/fz_http/devices/device/query.ex +++ b/apps/fz_http/lib/fz_http/devices/device/query.ex @@ -1,5 +1,5 @@ defmodule FzHttp.Devices.Device.Query do - import Ecto.Query + use FzHttp, :query @doc """ Returns IP address at given integer offset relative to start of CIDR range. @@ -40,7 +40,7 @@ defmodule FzHttp.Devices.Device.Query do end def all do - from(device in FzHttp.Devices.Device, as: :device) + from(device in FzHttp.Devices.Device, as: :devices) end @doc """ @@ -103,13 +103,19 @@ defmodule FzHttp.Devices.Device.Query do end defp select_not_used_ips(queryable, network_cidr, reserved_ips) do + host_as_string = network_cidr.address |> :inet.ntoa() |> List.to_string() + queryable |> where( [q: q], offset_to_ip(q.ip, ^network_cidr) not in subquery(used_ips_subquery(network_cidr)) ) |> where([q: q], offset_to_ip(q.ip, ^network_cidr) not in ^reserved_ips) - |> where([q: q], acquire_advisory_lock(q.ip) == true) + |> where( + [q: q], + acquire_advisory_lock(fragment("hashtext(?) + ?", ^host_as_string, q.ip)) == + true + ) |> select([q: q], offset_to_ip(q.ip, ^network_cidr)) end @@ -117,10 +123,10 @@ defmodule FzHttp.Devices.Device.Query do defp used_ips_subquery(queryable, %Postgrex.INET{address: address}) when tuple_size(address) == 4 do - select(queryable, [device: device], device.ipv4) + select(queryable, [devices: devices], devices.ipv4) end defp used_ips_subquery(queryable, %Postgrex.INET{address: _address}) do - select(queryable, [device: device], device.ipv6) + select(queryable, [devices: devices], devices.ipv6) end end diff --git a/apps/fz_http/lib/fz_http/events.ex b/apps/fz_http/lib/fz_http/events.ex index 3cf31902e..9f80c2d55 100644 --- a/apps/fz_http/lib/fz_http/events.ex +++ b/apps/fz_http/lib/fz_http/events.ex @@ -23,7 +23,7 @@ defmodule FzHttp.Events do information. """, timestamp: DateTime.utc_now(), - user: Users.get_user!(device.user_id).email + user: Users.fetch_user_by_id!(device.user_id).email }) end end @@ -54,7 +54,7 @@ defmodule FzHttp.Events do information. """, timestamp: DateTime.utc_now(), - user: Users.get_user!(device.user_id).email + user: Users.fetch_user_by_id!(device.user_id).email }) end end diff --git a/apps/fz_http/lib/fz_http/oidc/refresher.ex b/apps/fz_http/lib/fz_http/oidc/refresher.ex index 9180cc31f..97ed3ac9c 100644 --- a/apps/fz_http/lib/fz_http/oidc/refresher.ex +++ b/apps/fz_http/lib/fz_http/oidc/refresher.ex @@ -3,11 +3,8 @@ defmodule FzHttp.OIDC.Refresher do Worker module for refreshing OIDC connections """ use GenServer, restart: :temporary - import Ecto.{Changeset, Query} - import FzHttpWeb.OIDC.Helpers - - alias FzHttp.{OIDC, OIDC.Connection, Repo, Users} + alias FzHttp.{Configurations, OIDC, OIDC.Connection, Repo, Users} require Logger def start_link(init_opts) do @@ -36,22 +33,16 @@ defmodule FzHttp.OIDC.Refresher do defp do_refresh(user_id, %{provider: provider_id, refresh_token: refresh_token} = conn) do Logger.info("Refreshing user\##{user_id} @ #{provider_id}...") - result = - openid_connect().fetch_tokens( - provider_id, - %{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"} + with {:ok, config} <- Configurations.fetch_oidc_provider_config(provider_id), + {:ok, tokens} <- + OpenIDConnect.fetch_tokens(config, %{ + grant_type: "refresh_token", + refresh_token: refresh_token + }) do + tokens + else + {:error, reason} -> %{error: inspect(reason)} end OIDC.update_connection(conn, %{ @@ -60,7 +51,7 @@ defmodule FzHttp.OIDC.Refresher do }) with %{error: _} <- refresh_response do - user = Users.get_user!(user_id) + user = Users.fetch_user_by_id!(user_id) Logger.info("Disabling user #{user.email} due to OIDC token refresh failure...") diff --git a/apps/fz_http/lib/fz_http/oidc/start_proxy.ex b/apps/fz_http/lib/fz_http/oidc/start_proxy.ex deleted file mode 100644 index df6e7a8b9..000000000 --- a/apps/fz_http/lib/fz_http/oidc/start_proxy.ex +++ /dev/null @@ -1,55 +0,0 @@ -defmodule FzHttp.OIDC.StartProxy do - @moduledoc """ - This proxy simply gets the relevant config at an appropriate timing - """ - - require Logger - - def child_spec(arg) do - %{id: __MODULE__, start: {__MODULE__, :start_link, [arg]}} - end - - def start_link(:test) do - :ignore - end - - def start_link(_) do - FzHttp.Configurations.get!(:openid_connect_providers) - |> parse() - |> OpenIDConnect.Worker.start_link() - end - - # XXX: Remove when configurations support test fixtures - if Mix.env() == :test do - def restart, do: :ignore - else - def restart do - :ok = Supervisor.terminate_child(FzHttp.Supervisor, __MODULE__) - Supervisor.restart_child(FzHttp.Supervisor, __MODULE__) - end - end - - # Convert the configuration record to something openid_connect expects, - # atom-keyed configs eg. [provider: [client_id: "CLIENT_ID" ...]] - defp parse(nil), do: [] - - defp parse(auth_oidc_config) when is_list(auth_oidc_config) do - external_url = FzHttp.Config.fetch_env!(:fz_http, :external_url) - - Enum.map(auth_oidc_config, fn provider -> - { - provider.id, - [ - discovery_document_uri: provider.discovery_document_uri, - client_id: provider.client_id, - client_secret: provider.client_secret, - redirect_uri: - provider.redirect_uri || "#{external_url}/auth/oidc/#{provider.id}/callback/", - response_type: provider.response_type, - scope: provider.scope, - label: provider.label - ] - } - end) - end -end diff --git a/apps/fz_http/lib/fz_http/oidc/supervisor.ex b/apps/fz_http/lib/fz_http/oidc/supervisor.ex new file mode 100644 index 000000000..e69de29bb diff --git a/apps/fz_http/lib/fz_http/release.ex b/apps/fz_http/lib/fz_http/release.ex index 34cdbfb85..d82f8e144 100644 --- a/apps/fz_http/lib/fz_http/release.ex +++ b/apps/fz_http/lib/fz_http/release.ex @@ -31,11 +31,11 @@ defmodule FzHttp.Release do change_password(email(), default_password()) reset_role(email(), :admin) else - Users.create_admin_user( + Users.create_admin_user(%{ email: email(), password: default_password(), password_confirmation: default_password() - ) + }) end # Notify the user @@ -57,14 +57,13 @@ defmodule FzHttp.Release do "password_confirmation" => password } - {:ok, _user} = - Users.get_user!(email: email) - |> Users.admin_update_user(params) + {:ok, user} = Users.fetch_user_by_email(email) + {:ok, _user} = Users.admin_update_user(user, params) end def reset_role(email, role) do - Users.get_user!(email: email) - |> Users.update_user_role(role) + {:ok, user} = Users.fetch_user_by_email(email) + Users.update_user_role(user, role) end def repos do @@ -80,7 +79,10 @@ defmodule FzHttp.Release do end defp default_admin_user do - Users.get_by_email(email()) + case Users.fetch_user_by_email(email()) do + {:ok, user} -> user + {:error, :not_found} -> nil + end end defp mint_jwt(%User{} = user) do diff --git a/apps/fz_http/lib/fz_http/repo.ex b/apps/fz_http/lib/fz_http/repo.ex index 0e8355ed4..52c5d082b 100644 --- a/apps/fz_http/lib/fz_http/repo.ex +++ b/apps/fz_http/lib/fz_http/repo.ex @@ -3,5 +3,23 @@ defmodule FzHttp.Repo do otp_app: :fz_http, adapter: Ecto.Adapters.Postgres - require Logger + @doc """ + Similar to `Ecto.Repo.one/2`, fetches a single result from the query. + + Returns `{:ok, schema}` or `{:error, :not_found}` if no result was found. + Raises if there is more than one row matching the query. + """ + @spec fetch(queryable :: Ecto.Queryable.t(), opts :: Keyword.t()) :: + {:ok, Ecto.Schema.t()} | {:error, :not_found} + def fetch(queryable, opts \\ []) do + case __MODULE__.one(queryable, opts) do + nil -> {:error, :not_found} + schema -> {:ok, schema} + end + end + + @doc """ + Alias of `Ecto.Repo.one!/2` added for naming convenience. + """ + def fetch!(queryable, opts \\ []), do: __MODULE__.one!(queryable, opts) end diff --git a/apps/fz_http/lib/fz_http/telemetry.ex b/apps/fz_http/lib/fz_http/telemetry.ex index ba685ce45..73c21ae54 100644 --- a/apps/fz_http/lib/fz_http/telemetry.ex +++ b/apps/fz_http/lib/fz_http/telemetry.ex @@ -97,7 +97,7 @@ defmodule FzHttp.Telemetry do common_fields() ++ [ devices_active_within_24h: Devices.count_active_within(@active_device_window), - admin_count: Users.count(role: :admin), + admin_count: Users.count_by_role(:admin), user_count: Users.count(), in_docker: in_docker?(), device_count: Devices.count(), diff --git a/apps/fz_http/lib/fz_http/users.ex b/apps/fz_http/lib/fz_http/users.ex index 9641e648f..9268fbd76 100644 --- a/apps/fz_http/lib/fz_http/users.ex +++ b/apps/fz_http/lib/fz_http/users.ex @@ -1,140 +1,127 @@ defmodule FzHttp.Users do - @moduledoc """ - The Users context. - """ - - import Ecto.Changeset - import Ecto.Query, warn: false - - alias FzHttp.Devices.Device - alias FzHttp.Repo + alias FzHttp.{Repo, Validator, Configurations} alias FzHttp.Telemetry alias FzHttp.Users.User - alias FzHttpWeb.Mailer - - require Logger - - # one hour - @sign_in_token_validity_secs 3600 + require Ecto.Query def count do - Repo.one(from u in User, select: count(u.id)) + User.Query.all() + |> Repo.aggregate(:count) end - def count(role: role) do - Repo.one(from u in User, select: count(u.id), where: u.role == ^role) + def count_by_role(role) do + User.Query.by_role(role) + |> Repo.aggregate(:count) end - def consume_sign_in_token(token) when is_binary(token) do - case find_and_clear_token(token) do - {:ok, {:ok, user}} -> {:ok, user} - {:ok, {:error, msg}} -> {:error, msg} + def fetch_user_by_id(id) do + if Validator.valid_uuid?(id) do + User.Query.by_id(id) + |> Repo.fetch() + else + {:error, :not_found} end end - def exists?(user_id) when is_nil(user_id) do - false + def fetch_user_by_id!(id) do + User.Query.by_id(id) + |> Repo.fetch!() end - def exists?(user_id) do - Repo.exists?(from u in User, where: u.id == ^user_id) + def fetch_user_by_email(email) do + User.Query.by_email(email) + |> Repo.fetch() end - def list_admins do - Repo.all(from User, where: [role: :admin]) - end - - def get_user!(email: email) do - Repo.get_by!(User, email: email) - end - - def get_user!(id), do: Repo.get!(User, id) - - def get_user(id), do: Repo.get(User, id) - - def get_by_email(email) do - Repo.get_by(User, email: email) - end - - def get_by_email!(email) do - Repo.get_by!(User, email: email) - end - - def create_admin_user(attrs) do - create_user_with_role(attrs, :admin) - end - - def create_unprivileged_user(attrs) do - create_user_with_role(attrs, :unprivileged) - end - - def create_user_with_role(attrs, role) do - attrs - |> Enum.into(%{}) - |> create_user(role: role) - end - - def create_user(attrs, overwrites \\ []) do - changeset = - User - |> struct(sign_in_keys()) - |> User.create_changeset(attrs) - - result = - overwrites - |> Enum.reduce(changeset, fn {k, v}, cs -> put_change(cs, k, v) end) - |> Repo.insert() - - case result do - {:ok, _user} -> - Telemetry.add_user() - - _ -> - nil + def fetch_user_by_id_or_email(id_or_email) do + if Validator.valid_uuid?(id_or_email) do + fetch_user_by_id(id_or_email) + else + fetch_user_by_email(id_or_email) end - - result end - def sign_in_keys do - %{ - sign_in_token: FzCommon.FzCrypto.rand_string(), - sign_in_token_created_at: DateTime.utc_now() - } + def list_users(opts \\ []) do + {hydrate, _opts} = Keyword.pop(opts, :hydrate, []) + + User.Query.all() + |> hydrate_fields(hydrate) + |> Repo.all() end - def admin_update_user(%User{} = user, attrs) do + defp hydrate_fields(queryable, []), do: queryable + + defp hydrate_fields(queryable, [:device_count | rest]) do + queryable + |> User.Query.hydrate_device_count() + |> hydrate_fields(rest) + end + + def request_sign_in_token(%User{} = user) do user - |> User.update_email(attrs) - |> User.update_role(attrs) - |> User.update_password(attrs) + |> User.Changeset.generate_sign_in_token() |> Repo.update() end - def admin_update_self(%User{} = user, attrs) do + def consume_sign_in_token(%User{sign_in_token_hash: nil}, _token) do + {:error, :no_token} + end + + def consume_sign_in_token(%User{} = user, token) when is_binary(token) do + if FzCommon.FzCrypto.equal?(token, user.sign_in_token_hash) do + User.Query.by_id(user.id) + |> User.Query.where_sign_in_token_is_not_expired() + |> Ecto.Query.update(set: [sign_in_token_hash: nil, sign_in_token_created_at: nil]) + |> Ecto.Query.select([users: users], users) + |> Repo.update_all([]) + |> case do + {1, [user]} -> {:ok, user} + {0, []} -> {:error, :token_expired} + end + else + {:error, :invalid_token} + end + end + + def create_admin_user(attrs) do + create_user(attrs, :admin) + end + + def create_unprivileged_user(attrs) do + create_user(attrs, :unprivileged) + end + + def create_user(attrs, role \\ :unprivileged) do + User.Changeset.create_changeset(role, attrs) + |> insert_user() + end + + defp insert_user(%Ecto.Changeset{} = changeset) do + with {:ok, user} <- Repo.insert(changeset) do + Telemetry.add_user() + {:ok, user} + end + end + + # XXX: This should go down to single function update_user(self, attrs, subject) + # where subject will know role of an updater and if he is updating himself. + def admin_update_user(%User{} = user, attrs) do user - |> User.update_email(attrs) - |> User.update_password(attrs) - |> User.require_current_password(attrs) + |> User.Changeset.update_user_role(attrs) + |> User.Changeset.update_user_email(attrs) + |> User.Changeset.update_user_password(attrs) |> Repo.update() end def unprivileged_update_self(%User{} = user, attrs) do user - |> User.require_password_change(attrs) - |> User.update_password(attrs) + |> User.Changeset.update_user_password(attrs) |> Repo.update() end def update_user_role(%User{} = user, role) do user - |> User.update_role(%{role: role}) - |> Repo.update() - end - - def update_user_sign_in_token(%User{} = user, attrs) do - user - |> User.update_sign_in_token(attrs) + |> User.Changeset.update_user_role(%{role: role}) |> Repo.update() end @@ -143,20 +130,14 @@ defmodule FzHttp.Users do Repo.delete(user) end - def change_user(%User{} = user \\ struct(User)) do - change(user) - end - - def new_user do - change_user(%User{}) - end - - def list_users do - Repo.all(User) + # XXX: This should return real changeset not just a dummy one listing all the fields + def change_user(%User{} = user \\ %User{}) do + Ecto.Changeset.change(user) end def as_settings do - Repo.all(from u in User, select: %{id: u.id}) + User.Query.select_id_map() + |> Repo.all() |> Enum.map(&setting_projection/1) |> MapSet.new() end @@ -165,120 +146,35 @@ defmodule FzHttp.Users do user.id end - @doc """ - Fetches all users and groups into an Enumerable that can be used for an HTML form input. - """ - def as_options_for_select do - Repo.all(from u in User, select: {u.email, u.id}) - end - - def list_users(:with_device_counts) do - query = - from( - user in User, - left_join: device in Device, - on: device.user_id == user.id, - group_by: user.id, - select_merge: %{device_count: count(device.id)} - ) - - Repo.all(query) - end - - def update_last_signed_in(user, %{provider: provider} = _auth) do + def update_last_signed_in(user, %{provider: provider}) do method = case provider do :identity -> "email" - m -> to_string(m) + other -> to_string(other) end user - |> User.update_last_signed_in(%{ + |> User.Changeset.update_last_signed_in(%{ last_signed_in_at: DateTime.utc_now(), last_signed_in_method: method }) |> Repo.update() 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!() + def vpn_session_expires_at(user) do + DateTime.add(user.last_signed_in_at, Configurations.vpn_duration()) end - @doc """ - Returns DateTime that VPN sessions expire based on last_signed_in_at - and the security.require_auth_for_vpn_frequency setting. - """ - def vpn_session_expires_at(user, duration) do - DateTime.add(user.last_signed_in_at, duration) - end - - def vpn_session_expired?(user, duration) do - max = FzHttp.Configurations.Configuration.max_vpn_session_duration() - - case duration do - 0 -> + def vpn_session_expired?(user) do + cond do + is_nil(user.last_signed_in_at) -> false - ^max -> - is_nil(user.last_signed_in_at) + not Configurations.vpn_sessions_expire?() -> + false - _num -> - is_nil(user.last_signed_in_at) || - DateTime.diff(vpn_session_expires_at(user, duration), DateTime.utc_now()) <= 0 - end - end - - def reset_sign_in_token(email) do - with %User{} = user <- Repo.get_by(User, email: email), - {:ok, user} <- update_user_sign_in_token(user, sign_in_keys()) do - Mailer.AuthEmail.magic_link(user) |> Mailer.deliver!() - :ok - else - nil -> - Logger.info("Attempt to reset password of non-existing email: #{email}") - :ok - - {:error, _changeset} -> - # failed to update user, something wrong internally - Logger.error("Could not update user #{email} for magic link.") - :error - end - end - - defp find_by_token(token) do - validity_secs = -1 * @sign_in_token_validity_secs - now = DateTime.utc_now() - - Repo.one( - from(u in User, - where: - u.sign_in_token == ^token and - u.sign_in_token_created_at > datetime_add(^now, ^validity_secs, "second") - ) - ) - end - - defp find_and_clear_token(token) do - Repo.transaction(fn -> - case find_by_token(token) do - nil -> {:error, "Token invalid."} - user -> clear_token(user) - end - end) - end - - defp clear_token(user) do - result = update_user_sign_in_token(user, %{sign_in_token: nil, sign_in_token_created_at: nil}) - - case result do - {:ok, user} -> {:ok, user} - _ -> {:error, "Unexpected error attempting to clear sign in token."} + true -> + DateTime.diff(vpn_session_expires_at(user), DateTime.utc_now()) <= 0 end end end diff --git a/apps/fz_http/lib/fz_http/users/password_helpers.ex b/apps/fz_http/lib/fz_http/users/password_helpers.ex deleted file mode 100644 index 4d1362fd2..000000000 --- a/apps/fz_http/lib/fz_http/users/password_helpers.ex +++ /dev/null @@ -1,43 +0,0 @@ -defmodule FzHttp.Users.PasswordHelpers do - @moduledoc """ - Helpers for validating changesets with passwords - """ - - import Ecto.Changeset - - def validate_password_equality(%Ecto.Changeset{valid?: true} = changeset) do - password = changeset.changes[:password] - password_confirmation = changeset.changes[:password_confirmation] - - if password != password_confirmation do - add_error(changeset, :password, "does not match password confirmation.") - else - changeset - end - end - - def validate_password_equality(changeset), do: changeset - - def put_password_hash(%Ecto.Changeset{changes: %{password: password}} = changeset) - when password in ["", nil] do - changeset - end - - def put_password_hash( - %Ecto.Changeset{ - valid?: true, - changes: %{password: password} - } = changeset - ) do - changeset - |> put_change(:password_hash, Argon2.hash_pwd_salt(password)) - |> delete_change(:password) - |> delete_change(:password_confirmation) - end - - def put_password_hash(changeset) do - changeset - |> delete_change(:password) - |> delete_change(:password_confirmation) - end -end diff --git a/apps/fz_http/lib/fz_http/users/user.ex b/apps/fz_http/lib/fz_http/users/user.ex index b64b76bb4..0eeb05490 100644 --- a/apps/fz_http/lib/fz_http/users/user.ex +++ b/apps/fz_http/lib/fz_http/users/user.ex @@ -1,123 +1,30 @@ defmodule FzHttp.Users.User do - @moduledoc """ - Represents a User. - """ use FzHttp, :schema - import Ecto.Changeset - import FzHttp.Users.PasswordHelpers - - alias FzHttp.{ - ApiTokens.ApiToken, - Devices.Device, - OIDC.Connection, - Validators.Common - } - - @min_password_length 12 - @max_password_length 64 schema "users" do - field :role, Ecto.Enum, values: [:unprivileged, :admin], default: :unprivileged + field :role, Ecto.Enum, values: [:unprivileged, :admin] field :email, :string + field :password_hash, :string + field :last_signed_in_at, :utc_datetime_usec field :last_signed_in_method, :string - field :password_hash, :string - field :sign_in_token, :string + + field :sign_in_token, :string, virtual: true, redact: true + field :sign_in_token_hash, :string field :sign_in_token_created_at, :utc_datetime_usec - field :disabled_at, :utc_datetime_usec - # VIRTUAL FIELDS + # Virtual fields + field :password, :string, virtual: true, redact: true + field :password_confirmation, :string, virtual: true, redact: true + + # Virtual fields that can be hydrated field :device_count, :integer, virtual: true - field :password, :string, virtual: true - field :password_confirmation, :string, virtual: true - field :current_password, :string, virtual: true - has_many :devices, Device - has_many :oidc_connections, Connection - has_many :api_tokens, ApiToken + has_many :devices, FzHttp.Devices.Device + has_many :oidc_connections, FzHttp.OIDC.Connection + has_many :api_tokens, FzHttp.ApiTokens.ApiToken + field :disabled_at, :utc_datetime_usec timestamps() end - - def create_changeset(user, attrs \\ %{}) do - user - |> cast(attrs, [ - :email, - :password_hash, - :password, - :password_confirmation - ]) - |> update_change(:email, &String.trim/1) - |> validate_required([:email]) - |> validate_password_equality() - |> validate_length(:password, min: @min_password_length, max: @max_password_length) - |> validate_format(:email, ~r/@/) - |> unique_constraint(:email) - |> put_password_hash() - end - - def require_current_password(user, attrs) do - user - |> cast(attrs, [:current_password]) - |> validate_required([:current_password]) - |> verify_current_password() - end - - def update_password(user, attrs) do - user - |> cast(attrs, [:password, :password_confirmation]) - |> then(fn - %{changes: %{password: _}} = changeset -> - validate_length(changeset, :password, min: @min_password_length, max: @max_password_length) - - changeset -> - changeset - end) - |> validate_password_equality() - |> put_password_hash() - |> validate_required([:password_hash]) - end - - def require_password_change(user, attrs) do - user - |> cast(attrs, [:password, :password_confirmation]) - |> validate_required([:password, :password_confirmation]) - end - - def update_email(user, attrs) do - user - |> cast(attrs, [:email]) - |> Common.trim_change(:email) - |> validate_required([:email]) - |> validate_format(:email, ~r/@/) - end - - def update_role(user, attrs) do - user - |> cast(attrs, [:role]) - |> validate_required([:role]) - end - - def update_sign_in_token(user, attrs) do - cast(user, attrs, [:sign_in_token, :sign_in_token_created_at]) - end - - def update_last_signed_in(user, attrs) do - cast(user, attrs, [:last_signed_in_method, :last_signed_in_at]) - end - - defp verify_current_password( - %Ecto.Changeset{ - data: %{password_hash: password_hash}, - changes: %{current_password: current_password} - } = changeset - ) do - if Argon2.verify_pass(current_password, password_hash) do - delete_change(changeset, :current_password) - else - add_error(changeset, :current_password, "invalid password") - end - end - - defp verify_current_password(changeset), do: changeset end diff --git a/apps/fz_http/lib/fz_http/users/user/changeset.ex b/apps/fz_http/lib/fz_http/users/user/changeset.ex new file mode 100644 index 000000000..1da37e9f1 --- /dev/null +++ b/apps/fz_http/lib/fz_http/users/user/changeset.ex @@ -0,0 +1,68 @@ +defmodule FzHttp.Users.User.Changeset do + use FzHttp, :changeset + alias FzHttp.Users + + @min_password_length 12 + @max_password_length 64 + + def create_changeset(role, attrs) when is_atom(role) do + %Users.User{} + |> cast(attrs, ~w[ + email + password + password_confirmation + ]a) + |> put_change(:role, role) + |> change_email_changeset() + |> validate_if_changed(:password, &change_password_changeset/1) + end + + def update_user_password(user, attrs) do + user + |> cast(attrs, [:password]) + |> validate_if_changed(:password, &change_password_changeset/1) + end + + def update_user_email(user, attrs) do + user + |> cast(attrs, [:email]) + |> validate_if_changed(:email, &change_email_changeset/1) + end + + def update_user_role(user, attrs) do + user + |> cast(attrs, [:role]) + |> validate_required([:role]) + end + + defp change_email_changeset(%Ecto.Changeset{} = changeset) do + changeset + |> trim_change(:email) + |> validate_required([:email, :role]) + |> validate_email(:email) + |> unique_constraint(:email) + end + + defp change_password_changeset(%Ecto.Changeset{} = changeset) do + changeset + |> validate_required([:password]) + |> validate_confirmation(:password, required: true) + |> validate_length(:password, min: @min_password_length, max: @max_password_length) + |> put_hash(:password, to: :password_hash) + |> redact_field(:password) + |> redact_field(:password_confirmation) + |> validate_required([:password_hash]) + end + + def generate_sign_in_token(%Users.User{} = user) do + user + |> change() + |> put_change(:sign_in_token, FzCommon.FzCrypto.rand_string()) + |> put_hash(:sign_in_token, to: :sign_in_token_hash) + |> put_change(:sign_in_token_created_at, DateTime.utc_now()) + end + + def update_last_signed_in(user, attrs) do + cast(user, attrs, [:last_signed_in_method, :last_signed_in_at]) + end +end diff --git a/apps/fz_http/lib/fz_http/users/user/query.ex b/apps/fz_http/lib/fz_http/users/user/query.ex new file mode 100644 index 000000000..42318e4dc --- /dev/null +++ b/apps/fz_http/lib/fz_http/users/user/query.ex @@ -0,0 +1,45 @@ +defmodule FzHttp.Users.User.Query do + use FzHttp, :query + + def all do + from(users in FzHttp.Users.User, as: :users) + end + + def by_id(queryable \\ all(), id) do + where(queryable, [users: users], users.id == ^id) + end + + def by_email(queryable \\ all(), email) do + where(queryable, [users: users], users.email == ^email) + end + + def by_role(queryable \\ all(), role) do + where(queryable, [users: users], users.role == ^role) + end + + def where_sign_in_token_is_not_expired(queryable \\ all()) do + queryable + |> where( + [users: users], + datetime_add(users.sign_in_token_created_at, 1, "hour") >= fragment("NOW()") + ) + end + + def select_id_map(queryable \\ all()) do + queryable + |> select([users: users], %{id: users.id}) + end + + def hydrate_device_count(queryable \\ all()) do + queryable + |> with_assoc(:devices) + |> group_by([users: users], users.id) + |> select_merge([users: users, devices: devices], %{device_count: count(devices.id)}) + end + + def with_assoc(queryable \\ all(), assoc) do + with_named_binding(queryable, assoc, fn query, binding -> + join(query, :left, [users: users], a in assoc(users, ^binding), as: ^binding) + end) + end +end diff --git a/apps/fz_http/lib/fz_http/validators/common.ex b/apps/fz_http/lib/fz_http/validator.ex similarity index 51% rename from apps/fz_http/lib/fz_http/validators/common.ex rename to apps/fz_http/lib/fz_http/validator.ex index 8a9a618bd..19841400e 100644 --- a/apps/fz_http/lib/fz_http/validators/common.ex +++ b/apps/fz_http/lib/fz_http/validator.ex @@ -1,15 +1,13 @@ -defmodule FzHttp.Validators.Common do - @moduledoc """ - Shared validators to use between schemas. +defmodule FzHttp.Validator do + @doc """ + A set of changeset helpers and schema extensions to simplify our changesets and make validation more reliable. """ - import Ecto.Changeset + alias FzCommon.FzNet - import FzCommon.FzNet, - only: [ - valid_ip?: 1, - valid_cidr?: 1 - ] + def validate_email(changeset, field) do + validate_format(changeset, field, ~r/@/, message: "is invalid email address") + end def validate_uri(changeset, fields) when is_list(fields) do Enum.reduce(fields, changeset, fn field, accumulated_changeset -> @@ -46,7 +44,7 @@ defmodule FzHttp.Validators.Common do validate_change(changeset, field, fn _current_field, value -> value |> split_comma_list() - |> Enum.find(&(not valid_ip?(&1))) + |> Enum.find(&(not FzNet.valid_ip?(&1))) |> error_if( &(!is_nil(&1)), &{field, "is invalid: #{&1} is not a valid IPv4 / IPv6 address"} @@ -58,7 +56,7 @@ defmodule FzHttp.Validators.Common do validate_change(changeset, field, fn _current_field, value -> value |> split_comma_list() - |> Enum.find(&(not (valid_ip?(&1) or valid_cidr?(&1)))) + |> Enum.find(&(not (FzNet.valid_ip?(&1) or FzNet.valid_cidr?(&1)))) |> error_if( &(!is_nil(&1)), &{field, "is invalid: #{&1} is not a valid IPv4 / IPv6 address or CIDR range"} @@ -105,6 +103,73 @@ defmodule FzHttp.Validators.Common do end end + @doc """ + Takes value from `value_field` and puts it's hash to `hash_field`. + """ + def put_hash(%Ecto.Changeset{} = changeset, value_field, to: hash_field) do + with {:ok, value} when is_binary(value) and value != "" <- + fetch_change(changeset, value_field) do + put_change(changeset, hash_field, FzCommon.FzCrypto.hash(value)) + else + _ -> changeset + end + end + + @doc """ + Validates that value in a given `value_field` equals to hash stored in `hash_field`. + """ + def validate_hash(changeset, value_field, hash_field: hash_field) do + with {:data, hash} <- fetch_field(changeset, hash_field) do + validate_change(changeset, value_field, fn value_field, token -> + if FzCommon.FzCrypto.equal?(token, hash) do + [] + else + [{value_field, {"is invalid", [validation: :hash]}}] + end + end) + else + {:changes, _hash} -> + add_error(changeset, value_field, "can not be verified", validation: :hash) + + :error -> + add_error(changeset, value_field, "is already verified", validation: :hash) + end + end + + def validate_if_true(%Ecto.Changeset{} = changeset, field, callback) + when is_function(callback, 1) do + case fetch_field(changeset, field) do + {_data_or_changes, true} -> + callback.(changeset) + + _else -> + changeset + end + end + + def validate_if_changed(%Ecto.Changeset{} = changeset, field, callback) + when is_function(callback, 1) do + with {:ok, _value} <- fetch_change(changeset, field) do + callback.(changeset) + else + _ -> changeset + end + end + + @doc """ + Removes change for a given field and original value from it from `changeset.params`. + + Even though `changeset.params` considered to be a private field it leaks values even + after they are removed from a changeset if you `inspect(struct, structs: false)` or + just access it directly. + """ + def redact_field(%Ecto.Changeset{} = changeset, field) do + changeset = delete_change(changeset, field) + %{changeset | params: Map.drop(changeset.params, field_variations(field))} + end + + defp field_variations(field) when is_atom(field), do: [field, Atom.to_string(field)] + @doc """ Puts the change if field is not changed or it's value is set to `nil`. """ @@ -126,4 +191,13 @@ defmodule FzHttp.Validators.Common do def trim_change(changeset, field) do update_change(changeset, field, &if(!is_nil(&1), do: String.trim(&1))) end + + @doc """ + Returns `true` when binary representation of Ecto UUID is valid, otherwise - `false`. + """ + def valid_uuid?(binary) when is_binary(binary), + do: match?(<<_::64, ?-, _::32, ?-, _::32, ?-, _::32, ?-, _::96>>, binary) + + def valid_uuid?(_binary), + do: false end diff --git a/apps/fz_http/lib/fz_http/validators/openid_connect.ex b/apps/fz_http/lib/fz_http/validators/openid_connect.ex deleted file mode 100644 index eb3f58839..000000000 --- a/apps/fz_http/lib/fz_http/validators/openid_connect.ex +++ /dev/null @@ -1,21 +0,0 @@ -defmodule FzHttp.Validators.OpenIDConnect do - @moduledoc """ - Validators various fields related to OpenID Connect - before they're saved and passed to the underlying - openid_connect library where they could become an issue. - """ - import Ecto.Changeset - - def validate_discovery_document_uri(changeset) do - changeset - |> validate_change(:discovery_document_uri, fn :discovery_document_uri, value -> - case OpenIDConnect.update_documents(discovery_document_uri: value) do - {:ok, _update_result} -> - [] - - {:error, :update_documents, reason} -> - [discovery_document_uri: "is invalid. Reason: #{inspect(reason)}"] - end - end) - end -end diff --git a/apps/fz_http/lib/fz_http/validators/saml.ex b/apps/fz_http/lib/fz_http/validators/saml.ex deleted file mode 100644 index f886ecb23..000000000 --- a/apps/fz_http/lib/fz_http/validators/saml.ex +++ /dev/null @@ -1,21 +0,0 @@ -defmodule FzHttp.Validators.SAML do - @moduledoc """ - Validators for SAML configs. - """ - - alias Samly.IdpData - import Ecto.Changeset - - def validate_metadata(changeset) do - changeset - |> validate_change(:metadata, fn :metadata, value -> - try do - IdpData.from_xml(value, %IdpData{}) - [] - catch - :exit, e -> - [metadata: "is invalid. Details: #{inspect(e)}."] - end - end) - end -end diff --git a/apps/fz_http/lib/fz_http_web/auth/html/authentication.ex b/apps/fz_http/lib/fz_http_web/auth/html/authentication.ex index 7111b5f8e..6cae29cf5 100644 --- a/apps/fz_http/lib/fz_http_web/auth/html/authentication.ex +++ b/apps/fz_http/lib/fz_http_web/auth/html/authentication.ex @@ -4,15 +4,11 @@ defmodule FzHttpWeb.Auth.HTML.Authentication do """ use Guardian, otp_app: :fz_http use FzHttpWeb, :controller - + alias FzHttp.Configurations alias FzHttp.Telemetry alias FzHttp.Users alias FzHttp.Users.User - import FzHttpWeb.OIDC.Helpers - - require Logger - @guardian_token_name "guardian_default_token" @impl Guardian @@ -22,9 +18,9 @@ defmodule FzHttpWeb.Auth.HTML.Authentication do @impl Guardian def resource_from_claims(%{"sub" => id}) do - case Users.get_user(id) do - nil -> {:error, :resource_not_found} - user -> {:ok, user} + case Users.fetch_user_by_id(id) do + {:ok, user} -> {:ok, user} + {:error, :not_found} -> {:error, :resource_not_found} end end @@ -76,12 +72,10 @@ defmodule FzHttpWeb.Auth.HTML.Authentication do def sign_out(conn) do with provider_id when not is_nil(provider_id) <- Plug.Conn.get_session(conn, "login_method"), - provider when not is_nil(provider) <- - FzHttp.Configurations.get_provider_by_id(:openid_connect_providers, provider_id), token when not is_nil(token) <- Plug.Conn.get_session(conn, "id_token"), - end_session_uri when not is_nil(end_session_uri) <- - openid_connect().end_session_uri(provider_id, %{ - client_id: provider.client_id, + {:ok, config} <- Configurations.fetch_oidc_provider_config(provider_id), + {:ok, end_session_uri} <- + OpenIDConnect.end_session_uri(config, %{ id_token_hint: token, post_logout_redirect_uri: url(~p"/") }) do diff --git a/apps/fz_http/lib/fz_http_web/auth/json/authentication.ex b/apps/fz_http/lib/fz_http_web/auth/json/authentication.ex index 2983ddd97..4da5a61e4 100644 --- a/apps/fz_http/lib/fz_http_web/auth/json/authentication.ex +++ b/apps/fz_http/lib/fz_http_web/auth/json/authentication.ex @@ -19,7 +19,7 @@ defmodule FzHttpWeb.Auth.JSON.Authentication do @impl Guardian def resource_from_claims(%{"api" => api_token_id}) do with %ApiTokens.ApiToken{} = api_token <- ApiTokens.get_unexpired_api_token(api_token_id), - %Users.User{} = user <- Users.get_user(api_token.user_id) do + {:ok, %Users.User{} = user} <- Users.fetch_user_by_id(api_token.user_id) do {:ok, user} else _ -> diff --git a/apps/fz_http/lib/fz_http_web/channels/notification_channel.ex b/apps/fz_http/lib/fz_http_web/channels/notification_channel.ex index b44affeda..88a46e9f6 100644 --- a/apps/fz_http/lib/fz_http_web/channels/notification_channel.ex +++ b/apps/fz_http/lib/fz_http_web/channels/notification_channel.ex @@ -6,25 +6,16 @@ defmodule FzHttpWeb.NotificationChannel do alias FzHttp.Users alias FzHttpWeb.Presence - @token_verify_opts [max_age: 86_400] - @impl Phoenix.Channel - def join("notification:session", %{"user_agent" => user_agent, "token" => token}, socket) do - case Phoenix.Token.verify(socket, "channel auth", token, @token_verify_opts) do - {:ok, user_id} -> - socket = - socket - |> assign(:current_user, Users.get_user!(user_id)) - |> assign(:user_agent, user_agent) + def join("notification:session", _attrs, socket) do + socket = FzHttpWeb.Sandbox.allow_channel_sql_sandbox(socket) - send(self(), :after_join) - - {:ok, - socket - |> assign(:current_user, Users.get_user!(user_id))} - - {:error, _} -> - {:error, %{reason: "unauthorized"}} + with {:ok, user} <- Users.fetch_user_by_id(socket.assigns.current_user_id) do + socket = assign(socket, :current_user, user) + send(self(), :after_join) + {:ok, socket} + else + _ -> {:error, %{reason: "unauthorized"}} end end diff --git a/apps/fz_http/lib/fz_http_web/controller_helpers.ex b/apps/fz_http/lib/fz_http_web/controller_helpers.ex index 4583ab967..348dd9778 100644 --- a/apps/fz_http/lib/fz_http_web/controller_helpers.ex +++ b/apps/fz_http/lib/fz_http_web/controller_helpers.ex @@ -6,6 +6,10 @@ defmodule FzHttpWeb.ControllerHelpers do alias FzHttp.Users.User + def root_path_for_user(nil) do + ~p"/" + end + def root_path_for_user(%User{role: :admin}) do ~p"/users" end diff --git a/apps/fz_http/lib/fz_http_web/controllers/auth_controller.ex b/apps/fz_http/lib/fz_http_web/controllers/auth_controller.ex index 8ba003a67..7902ebb6a 100644 --- a/apps/fz_http/lib/fz_http_web/controllers/auth_controller.ex +++ b/apps/fz_http/lib/fz_http_web/controllers/auth_controller.ex @@ -3,17 +3,15 @@ defmodule FzHttpWeb.AuthController do Implements the CRUD for a Session """ use FzHttpWeb, :controller - require Logger - - @local_auth_providers [:identity, :magic_link] - alias FzHttp.Users + alias FzHttp.Configurations alias FzHttpWeb.Auth.HTML.Authentication alias FzHttpWeb.OAuth.PKCE alias FzHttpWeb.OIDC.State alias FzHttpWeb.UserFromAuth + require Logger - import FzHttpWeb.OIDC.Helpers + @local_auth_providers [:identity, :magic_link] # Uncomment when Helpers.callback_url/1 is fixed # alias Ueberauth.Strategy.Helpers @@ -40,6 +38,14 @@ defmodule FzHttpWeb.AuthController do {:ok, user} -> maybe_sign_in(conn, user, auth) + {:error, reason} when reason in [:not_found, :invalid_credentials] -> + conn + |> put_flash( + :error, + "Error signing in: user credentials are invalid or user does not exist" + ) + |> request(%{}) + {:error, reason} -> conn |> put_flash(:error, "Error signing in: #{reason}") @@ -62,8 +68,9 @@ defmodule FzHttpWeb.AuthController do token_params = Map.merge(params, PKCE.token_params(conn)) with :ok <- State.verify_state(conn, state), - {:ok, tokens} <- openid_connect().fetch_tokens(provider_id, token_params), - {:ok, claims} <- openid_connect().verify(provider_id, tokens["id_token"]) do + {:ok, config} <- Configurations.fetch_oidc_provider_config(provider_id), + {:ok, tokens} <- OpenIDConnect.fetch_tokens(config, token_params), + {:ok, claims} <- OpenIDConnect.verify(config, tokens["id_token"]) do case UserFromAuth.find_or_create(provider_id, claims) do {:ok, user} -> # only first-time connect will include refresh token @@ -117,25 +124,28 @@ defmodule FzHttpWeb.AuthController do end def magic_link(conn, %{"email" => email}) do - case Users.reset_sign_in_token(email) do - :ok -> - conn - |> put_flash(:info, "Please check your inbox for the magic link.") - |> redirect(to: ~p"/") + with {:ok, user} <- Users.fetch_user_by_email(email), + {:ok, user} <- Users.request_sign_in_token(user) do + FzHttpWeb.Mailer.AuthEmail.magic_link(user) + |> FzHttpWeb.Mailer.deliver!() - :error -> + conn + |> put_flash(:info, "Please check your inbox for the magic link.") + |> redirect(to: ~p"/") + else + {:error, :not_found} -> conn |> put_flash(:warning, "Failed to send magic link email.") |> redirect(to: ~p"/auth/reset_password") end end - def magic_sign_in(conn, %{"token" => token}) do - case Users.consume_sign_in_token(token) do - {:ok, user} -> - maybe_sign_in(conn, user, %{provider: :magic_link}) - - {:error, _} -> + def magic_sign_in(conn, %{"user_id" => user_id, "token" => token}) do + with {:ok, user} <- Users.fetch_user_by_id(user_id), + {:ok, _user} <- Users.consume_sign_in_token(user, token) do + maybe_sign_in(conn, user, %{provider: :magic_link}) + else + {:error, _reason} -> conn |> put_flash(:error, "The magic link is not valid or has expired.") |> redirect(to: ~p"/") @@ -152,12 +162,23 @@ defmodule FzHttpWeb.AuthController do code_challenge: PKCE.code_challenge(verifier) } - uri = openid_connect().authorization_uri(provider_id, params) + with {:ok, config} <- Configurations.fetch_oidc_provider_config(provider_id), + {:ok, uri} <- OpenIDConnect.authorization_uri(config, params) do + conn + |> PKCE.put_cookie(verifier) + |> State.put_cookie(params.state) + |> redirect(external: uri) + else + {:error, :not_found} -> + {:error, :not_found} - conn - |> PKCE.put_cookie(verifier) - |> State.put_cookie(params.state) - |> redirect(external: uri) + {:error, reason} -> + Logger.error("Can not redirect user to OIDC auth uri", reason: inspect(reason)) + + conn + |> put_flash(:error, "Error while processing OpenID request.") + |> redirect(to: ~p"/") + end end defp maybe_sign_in(conn, user, %{provider: provider_key} = auth) diff --git a/apps/fz_http/lib/fz_http_web/controllers/json/user_controller.ex b/apps/fz_http/lib/fz_http_web/controllers/json/user_controller.ex index 674a3a877..bc96f93f4 100644 --- a/apps/fz_http/lib/fz_http_web/controllers/json/user_controller.ex +++ b/apps/fz_http/lib/fz_http_web/controllers/json/user_controller.ex @@ -14,7 +14,6 @@ defmodule FzHttpWeb.JSON.UserController do """ use FzHttpWeb, :controller alias FzHttp.Users - alias FzHttp.Users.User action_fallback(FzHttpWeb.JSON.FallbackController) @@ -50,7 +49,7 @@ defmodule FzHttpWeb.JSON.UserController do """ @doc api_doc: [action: "Create a User"] def create(conn, %{"user" => %{"role" => "admin"} = user_params}) do - with {:ok, %User{} = user} <- Users.create_admin_user(user_params) do + with {:ok, %Users.User{} = user} <- Users.create_admin_user(user_params) do conn |> put_status(:created) |> put_resp_header("location", ~p"/v0/users/#{user}") @@ -59,7 +58,7 @@ defmodule FzHttpWeb.JSON.UserController do end def create(conn, %{"user" => user_params}) do - with {:ok, %User{} = user} <- Users.create_unprivileged_user(user_params) do + with {:ok, %Users.User{} = user} <- Users.create_unprivileged_user(user_params) do conn |> put_status(:created) |> put_resp_header("location", ~p"/v0/users/#{user}") @@ -69,8 +68,9 @@ defmodule FzHttpWeb.JSON.UserController do @doc api_doc: [summary: "Get User by ID or Email"] def show(conn, %{"id" => id_or_email}) do - user = get_user_by_id_or_email(id_or_email) - render(conn, "show.json", user: user) + with {:ok, %Users.User{} = user} <- Users.fetch_user_by_id_or_email(id_or_email) do + render(conn, "show.json", user: user) + end end @doc """ @@ -78,27 +78,19 @@ defmodule FzHttpWeb.JSON.UserController do """ @doc api_doc: [action: "Update a User"] def update(conn, %{"id" => id_or_email, "user" => user_params}) do - user = get_user_by_id_or_email(id_or_email) - - with {:ok, %User{} = user} <- Users.admin_update_user(user, user_params) do + with {:ok, %Users.User{} = user} <- Users.fetch_user_by_id_or_email(id_or_email), + {:ok, %Users.User{} = user} <- Users.admin_update_user(user, user_params) do render(conn, "show.json", user: user) end end @doc api_doc: [summary: "Delete a User"] def delete(conn, %{"id" => id_or_email}) do - user = get_user_by_id_or_email(id_or_email) - - with {:ok, %User{}} <- Users.delete_user(user) do - send_resp(conn, :no_content, "") - end - end - - defp get_user_by_id_or_email(id_or_email) do - if String.contains?(id_or_email, "@") do - Users.get_by_email!(id_or_email) - else - Users.get_user!(id_or_email) + with {:ok, %Users.User{} = user} <- Users.fetch_user_by_id_or_email(id_or_email), + {:ok, %Users.User{}} <- Users.delete_user(user) do + conn + |> put_resp_content_type("application/json") + |> send_resp(:no_content, "") end end end diff --git a/apps/fz_http/lib/fz_http_web/controllers/user_controller.ex b/apps/fz_http/lib/fz_http_web/controllers/user_controller.ex index 30c20283a..0efe348c5 100644 --- a/apps/fz_http/lib/fz_http_web/controllers/user_controller.ex +++ b/apps/fz_http/lib/fz_http_web/controllers/user_controller.ex @@ -13,7 +13,7 @@ defmodule FzHttpWeb.UserController do user = Authentication.get_current_user(conn) with %{role: :admin} <- user do - unless length(Users.list_admins()) > 1 do + unless Users.count_by_role(:admin) > 1 do raise "Cannot delete one last admin" end end diff --git a/apps/fz_http/lib/fz_http_web/endpoint.ex b/apps/fz_http/lib/fz_http_web/endpoint.ex index 38e7d02ed..4ac85878e 100644 --- a/apps/fz_http/lib/fz_http_web/endpoint.ex +++ b/apps/fz_http/lib/fz_http_web/endpoint.ex @@ -12,7 +12,7 @@ defmodule FzHttpWeb.Endpoint do socket "/socket", FzHttpWeb.UserSocket, websocket: [ - connect_info: [:peer_data, :x_headers, :uri], + connect_info: [:user_agent, :peer_data, :x_headers, :uri], # XXX: channel token should prevent CSWH but double check check_origin: false ], @@ -21,6 +21,7 @@ defmodule FzHttpWeb.Endpoint do socket "/live", Phoenix.LiveView.Socket, websocket: [ connect_info: [ + :user_agent, :peer_data, :x_headers, :uri, diff --git a/apps/fz_http/lib/fz_http_web/live/device_live/admin/show_live.ex b/apps/fz_http/lib/fz_http_web/live/device_live/admin/show_live.ex index b35dcbebe..133208e4e 100644 --- a/apps/fz_http/lib/fz_http_web/live/device_live/admin/show_live.ex +++ b/apps/fz_http/lib/fz_http_web/live/device_live/admin/show_live.ex @@ -46,7 +46,7 @@ defmodule FzHttpWeb.DeviceLive.Admin.Show do defp assigns(device) do [ device: device, - user: Users.get_user!(device.user_id), + user: Users.fetch_user_by_id!(device.user_id), page_title: device.name, allowed_ips: Devices.allowed_ips(device), dns: Devices.dns(device), 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 7ad6ffb3e..ca01cadf1 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 @@ -53,7 +53,7 @@ defmodule FzHttpWeb.DeviceLive.Unprivileged.Show do defp assigns(device) do [ device: device, - user: Users.get_user!(device.user_id), + user: Users.fetch_user_by_id!(device.user_id), page_title: device.name, allowed_ips: Devices.allowed_ips(device), port: FzHttp.Config.fetch_env!(:fz_vpn, :wireguard_port), diff --git a/apps/fz_http/lib/fz_http_web/live/hooks/allow_ecto_sandbox.ex b/apps/fz_http/lib/fz_http_web/live/hooks/allow_ecto_sandbox.ex new file mode 100644 index 000000000..6272d0a1d --- /dev/null +++ b/apps/fz_http/lib/fz_http_web/live/hooks/allow_ecto_sandbox.ex @@ -0,0 +1,6 @@ +defmodule FzHttpWeb.Hooks.AllowEctoSandbox do + def on_mount(:default, _params, _session, socket) do + socket = FzHttpWeb.Sandbox.allow_live_ecto_sandbox(socket) + {:cont, socket} + end +end diff --git a/apps/fz_http/lib/fz_http_web/live/mfa_live/register_steps_component.ex b/apps/fz_http/lib/fz_http_web/live/mfa_live/register_steps_component.ex index 1951a32a0..3783b7173 100644 --- a/apps/fz_http/lib/fz_http_web/live/mfa_live/register_steps_component.ex +++ b/apps/fz_http/lib/fz_http_web/live/mfa_live/register_steps_component.ex @@ -19,7 +19,8 @@ defmodule FzHttpWeb.MFA.RegisterStepsComponent do