From dff9c269051bf10ee4f6189454e92048440d37ed Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Sat, 31 Jan 2026 14:54:44 -0600 Subject: [PATCH] fix TOTP enrollment with recovery codes --- .gitignore | 2 + lib/towerops/accounts.ex | 260 +++++++- lib/towerops/accounts/user_recovery_code.ex | 76 +++ lib/towerops/accounts/user_totp_device.ex | 44 ++ lib/towerops/profiles/profile_watcher.ex | 4 +- .../controllers/user_session_controller.ex | 12 +- .../user_session_html/totp.html.heex | 7 +- .../live/account_live/totp_enrollment.ex | 459 ++++++++----- lib/towerops_web/live/user_settings_live.ex | 608 ++++++++++++++++++ ...0260131195010_create_user_totp_devices.exs | 20 + ...60131195046_create_user_recovery_codes.exs | 20 + priv/static/changelog.txt | 1 + test/towerops/accounts_test.exs | 291 +++++++++ .../user_session_controller_test.exs | 118 ++++ .../account_live/totp_enrollment_test.exs | 71 +- 15 files changed, 1824 insertions(+), 169 deletions(-) create mode 100644 lib/towerops/accounts/user_recovery_code.ex create mode 100644 lib/towerops/accounts/user_totp_device.ex create mode 100644 priv/repo/migrations/20260131195010_create_user_totp_devices.exs create mode 100644 priv/repo/migrations/20260131195046_create_user_recovery_codes.exs diff --git a/.gitignore b/.gitignore index 7156d9d2..4ae3db95 100644 --- a/.gitignore +++ b/.gitignore @@ -69,3 +69,5 @@ profiles.json /priv/*.so /priv/*.dylib /c_src/*.o +/.expert/ + diff --git a/lib/towerops/accounts.ex b/lib/towerops/accounts.ex index 7b56dbc6..d9c9510d 100644 --- a/lib/towerops/accounts.ex +++ b/lib/towerops/accounts.ex @@ -12,7 +12,9 @@ defmodule Towerops.Accounts do alias Towerops.Accounts.UserAgentParser alias Towerops.Accounts.UserConsent alias Towerops.Accounts.UserNotifier + alias Towerops.Accounts.UserRecoveryCode alias Towerops.Accounts.UserToken + alias Towerops.Accounts.UserTotpDevice alias Towerops.GeoIP alias Towerops.Repo @@ -298,16 +300,43 @@ defmodule Towerops.Accounts do @doc """ Checks if a user has TOTP enabled. + + Checks both the new multi-device system (user_totp_devices table) + and the legacy totp_secret field for backward compatibility. """ - def totp_enabled?(%User{totp_secret: secret}) when is_binary(secret), do: true + def totp_enabled?(%User{id: user_id, totp_secret: secret}) do + # Check new multi-device system first + device_count = count_user_totp_devices(user_id) + + cond do + device_count > 0 -> true + is_binary(secret) -> true + true -> false + end + end + def totp_enabled?(_user), do: false @doc """ Verifies a TOTP code for a user who has TOTP enabled. + Now checks ALL user devices (multi-device support), not just single secret. + Falls back to legacy single secret for backward compatibility. Returns {:ok, user} if valid, {:error, :invalid_code} otherwise. """ - def verify_user_totp(%User{totp_secret: secret} = user, code) when is_binary(secret) and is_binary(code) do + def verify_user_totp(%User{} = user, code) when is_binary(code) do + # First try new multi-device approach + case verify_user_totp_any_device(user, code) do + {:ok, _device} -> + {:ok, user} + + {:error, :invalid_code} -> + # Fallback to legacy single secret (backward compatibility during migration) + verify_legacy_totp(user, code) + end + end + + defp verify_legacy_totp(%User{totp_secret: secret} = user, code) when is_binary(secret) do if verify_totp(secret, code) do {:ok, user} else @@ -315,10 +344,235 @@ defmodule Towerops.Accounts do end end - def verify_user_totp(%User{}, _code) do + defp verify_legacy_totp(%User{}, _code) do {:error, :totp_not_enabled} end + ## TOTP Device Management + + @doc """ + Lists all TOTP devices for a user, ordered by most recently used. + """ + def list_user_totp_devices(user_id) do + UserTotpDevice + |> where([d], d.user_id == ^user_id) + |> order_by([d], desc_nulls_last: d.last_used_at, desc: d.inserted_at) + |> Repo.all() + end + + @doc """ + Counts TOTP devices for a user. + """ + def count_user_totp_devices(user_id) do + UserTotpDevice + |> where([d], d.user_id == ^user_id) + |> Repo.aggregate(:count, :id) + end + + @doc """ + Creates a new TOTP device for a user. + + Returns {:ok, device, secret} where secret is the plain-text TOTP secret + that should be displayed once to the user as a QR code. + + ## Examples + + iex> create_totp_device(user_id, "iPhone 15 Pro") + {:ok, %UserTotpDevice{}, "base32secret"} + """ + def create_totp_device(user_id, device_name) do + secret = generate_totp_secret() + create_totp_device(user_id, device_name, secret) + end + + @doc """ + Creates a TOTP device with a pre-existing secret. + Used during initial enrollment when the secret has already been generated and shown to the user. + """ + def create_totp_device(user_id, device_name, secret) when is_binary(secret) do + changeset = + UserTotpDevice.changeset(%UserTotpDevice{}, %{ + user_id: user_id, + name: device_name, + totp_secret: secret, + created_at: DateTime.utc_now(:second) + }) + + case Repo.insert(changeset) do + {:ok, device} -> {:ok, device, secret} + {:error, changeset} -> {:error, changeset} + end + end + + @doc """ + Verifies a TOTP code against ANY of the user's devices. + + Returns {:ok, device} if valid, updating last_used_at. + Returns {:error, :invalid_code} if no device matches. + """ + def verify_user_totp_any_device(%User{id: user_id}, code) when is_binary(code) do + devices = list_user_totp_devices(user_id) + + Enum.find_value(devices, {:error, :invalid_code}, fn device -> + if verify_totp(device.totp_secret, code) do + # Update last_used_at and return updated device + updated_device = + device + |> UserTotpDevice.touch_changeset() + |> Repo.update!() + + {:ok, updated_device} + end + end) + end + + @doc """ + Deletes a TOTP device. + + Enforces "at least one device" rule - returns error if last device. + """ + def delete_totp_device(device_id, user_id) do + device = Repo.get(UserTotpDevice, device_id) + + cond do + is_nil(device) -> + {:error, :not_found} + + device.user_id != user_id -> + {:error, :unauthorized} + + count_user_totp_devices(user_id) == 1 -> + {:error, :last_device} + + true -> + Repo.delete(device) + end + end + + @doc """ + Renames a TOTP device. + """ + def rename_totp_device(device_id, user_id, new_name) do + device = Repo.get(UserTotpDevice, device_id) + + cond do + is_nil(device) -> + {:error, :not_found} + + device.user_id != user_id -> + {:error, :unauthorized} + + true -> + device + |> UserTotpDevice.changeset(%{name: new_name}) + |> Repo.update() + end + end + + ## Recovery Codes + + @doc """ + Generates 12 recovery codes for a user. + + Returns {:ok, codes} where codes is a list of plain-text codes. + Stores hashed versions in database. + """ + def generate_recovery_codes(user_id) do + # Delete any existing unused recovery codes + UserRecoveryCode + |> where([rc], rc.user_id == ^user_id and is_nil(rc.used_at)) + |> Repo.delete_all() + + now = DateTime.utc_now(:second) + codes = for _ <- 1..12, do: UserRecoveryCode.generate_code() + + # Insert hashed codes + records = + Enum.map(codes, fn code -> + %{ + id: Ecto.UUID.generate(), + user_id: user_id, + code_hash: UserRecoveryCode.hash_code(code), + created_at: now, + inserted_at: now + } + end) + + case Repo.insert_all(UserRecoveryCode, records) do + {12, _} -> {:ok, codes} + _ -> {:error, :generation_failed} + end + end + + @doc """ + Verifies a recovery code for a user. + + Returns {:ok, code_record} if valid and unused. + Marks code as used on successful verification. + """ + def verify_recovery_code(user_id, code) when is_binary(code) do + code_hash = UserRecoveryCode.hash_code(code) + + recovery_code = + UserRecoveryCode + |> where([rc], rc.user_id == ^user_id) + |> where([rc], rc.code_hash == ^code_hash) + |> where([rc], is_nil(rc.used_at)) + |> Repo.one() + + case recovery_code do + nil -> + {:error, :invalid_code} + + code_record -> + code_record + |> UserRecoveryCode.use_changeset() + |> Repo.update() + end + end + + @doc """ + Counts unused recovery codes for a user. + """ + def count_unused_recovery_codes(user_id) do + UserRecoveryCode + |> where([rc], rc.user_id == ^user_id) + |> where([rc], is_nil(rc.used_at)) + |> Repo.aggregate(:count, :id) + end + + @doc """ + Lists all recovery codes for a user (for display in UI). + + Returns list with status (used/unused) and creation date. + """ + def list_user_recovery_codes(user_id) do + UserRecoveryCode + |> where([rc], rc.user_id == ^user_id) + |> order_by([rc], desc: rc.inserted_at) + |> Repo.all() + end + + @doc """ + Verifies a TOTP code OR recovery code during login. + + Tries TOTP first, falls back to recovery code. + Returns {:ok, user, :totp} or {:ok, user, :recovery_code} or {:error, :invalid_code}. + """ + def verify_user_mfa(%User{} = user, code) when is_binary(code) do + case verify_user_totp(user, code) do + {:ok, user} -> + {:ok, user, :totp} + + {:error, :invalid_code} -> + # Try recovery code as fallback + case verify_recovery_code(user.id, code) do + {:ok, _code_record} -> {:ok, user, :recovery_code} + {:error, _} -> {:error, :invalid_code} + end + end + end + ## Settings @doc """ diff --git a/lib/towerops/accounts/user_recovery_code.ex b/lib/towerops/accounts/user_recovery_code.ex new file mode 100644 index 00000000..b756ff15 --- /dev/null +++ b/lib/towerops/accounts/user_recovery_code.ex @@ -0,0 +1,76 @@ +defmodule Towerops.Accounts.UserRecoveryCode do + @moduledoc """ + Schema for user recovery codes (backup codes for account recovery). + + Recovery codes are single-use backup codes that can be used to access + an account if all TOTP devices are lost. Codes are stored as SHA-256 hashes. + """ + use Ecto.Schema + + import Ecto.Changeset + + alias Towerops.Accounts.User + + @primary_key {:id, :binary_id, autogenerate: true} + @foreign_key_type :binary_id + schema "user_recovery_codes" do + field :code_hash, :string + field :used_at, :utc_datetime + field :created_at, :utc_datetime + + belongs_to :user, User + + timestamps(type: :utc_datetime, updated_at: false) + end + + @doc """ + Generates a cryptographically secure 8-character recovery code. + + Format: XXXX-XXXX (uppercase letters and numbers, no ambiguous chars). + Uses base32 alphabet without 0, O, I, 1 to avoid confusion. + """ + def generate_code do + # Use base32 alphabet without ambiguous characters (0, O, I, 1) + alphabet = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789" + + for_result = + for _ <- 1..8 do + Enum.random(String.graphemes(alphabet)) + end + + code = Enum.join(for_result) + + # Format: XXXX-XXXX + String.slice(code, 0..3) <> "-" <> String.slice(code, 4..7) + end + + @doc """ + Hashes a recovery code using SHA-256. + + ## Examples + + iex> hash_code("ABCD-EFGH") + "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08" + """ + def hash_code(code) when is_binary(code) do + :sha256 |> :crypto.hash(code) |> Base.encode16(case: :lower) + end + + @doc """ + Changeset for creating a recovery code. + """ + def changeset(recovery_code, attrs) do + recovery_code + |> cast(attrs, [:code_hash, :user_id, :created_at]) + |> validate_required([:code_hash, :user_id, :created_at]) + |> unique_constraint(:code_hash) + |> foreign_key_constraint(:user_id) + end + + @doc """ + Changeset for marking a recovery code as used. + """ + def use_changeset(recovery_code) do + change(recovery_code, used_at: DateTime.utc_now(:second)) + end +end diff --git a/lib/towerops/accounts/user_totp_device.ex b/lib/towerops/accounts/user_totp_device.ex new file mode 100644 index 00000000..f2cd0634 --- /dev/null +++ b/lib/towerops/accounts/user_totp_device.ex @@ -0,0 +1,44 @@ +defmodule Towerops.Accounts.UserTotpDevice do + @moduledoc """ + Schema for user TOTP (Time-based One-Time Password) devices. + + Allows users to register multiple authenticator apps (phone, tablet, etc.) + for two-factor authentication. + """ + use Ecto.Schema + + import Ecto.Changeset + + alias Towerops.Accounts.User + + @primary_key {:id, :binary_id, autogenerate: true} + @foreign_key_type :binary_id + schema "user_totp_devices" do + field :name, :string + field :totp_secret, :binary, redact: true + field :last_used_at, :utc_datetime + field :created_at, :utc_datetime + + belongs_to :user, User + + timestamps(type: :utc_datetime) + end + + @doc """ + Changeset for creating or updating a TOTP device. + """ + def changeset(device, attrs) do + device + |> cast(attrs, [:name, :totp_secret, :user_id, :created_at]) + |> validate_required([:name, :totp_secret, :user_id]) + |> validate_length(:name, min: 1, max: 100) + |> foreign_key_constraint(:user_id) + end + + @doc """ + Changeset for updating last_used_at timestamp. + """ + def touch_changeset(device) do + change(device, last_used_at: DateTime.utc_now(:second)) + end +end diff --git a/lib/towerops/profiles/profile_watcher.ex b/lib/towerops/profiles/profile_watcher.ex index 3e1ad1ad..485400ec 100644 --- a/lib/towerops/profiles/profile_watcher.ex +++ b/lib/towerops/profiles/profile_watcher.ex @@ -49,8 +49,8 @@ defmodule Towerops.Profiles.ProfileWatcher do Logger.info("ProfileWatcher: Detected change in #{Path.relative_to_cwd(path)}, reloading profiles...") case YamlProfiles.reload() do - {:ok, count} -> - Logger.info("ProfileWatcher: Successfully reloaded #{count} profiles") + {:ok, profiles} when is_list(profiles) -> + Logger.info("ProfileWatcher: Successfully reloaded #{length(profiles)} profiles") {:error, reason} -> Logger.error("ProfileWatcher: Failed to reload profiles: #{inspect(reason)}") diff --git a/lib/towerops_web/controllers/user_session_controller.ex b/lib/towerops_web/controllers/user_session_controller.ex index 1e029565..a44e2d0f 100644 --- a/lib/towerops_web/controllers/user_session_controller.ex +++ b/lib/towerops_web/controllers/user_session_controller.ex @@ -142,10 +142,18 @@ defmodule ToweropsWeb.UserSessionController do defp handle_totp_verification(conn, user_id, code) do user = Accounts.get_user!(user_id) - case Accounts.verify_user_totp(user, code) do - {:ok, user} -> + case Accounts.verify_user_mfa(user, code) do + {:ok, user, :totp} -> complete_totp_login(conn, user) + {:ok, user, :recovery_code} -> + conn + |> put_flash( + :warning, + "You used a recovery code. Consider regenerating codes from your account settings." + ) + |> complete_totp_login(user) + {:error, _reason} -> render_totp_error(conn, code) end diff --git a/lib/towerops_web/controllers/user_session_html/totp.html.heex b/lib/towerops_web/controllers/user_session_html/totp.html.heex index 65672f5b..f8573962 100644 --- a/lib/towerops_web/controllers/user_session_html/totp.html.heex +++ b/lib/towerops_web/controllers/user_session_html/totp.html.heex @@ -18,11 +18,14 @@ placeholder="000000" autocomplete="one-time-code" inputmode="numeric" - pattern="[0-9]{6}" - maxlength="6" required phx-mounted={JS.focus()} /> +
+

+ Enter the 6-digit code from your authenticator app, or use a recovery code (XXXX-XXXX format). +

+
<.button class="w-full" variant="primary"> Verify diff --git a/lib/towerops_web/live/account_live/totp_enrollment.ex b/lib/towerops_web/live/account_live/totp_enrollment.ex index b2d18adf..805d83ae 100644 --- a/lib/towerops_web/live/account_live/totp_enrollment.ex +++ b/lib/towerops_web/live/account_live/totp_enrollment.ex @@ -10,26 +10,64 @@ defmodule ToweropsWeb.AccountLive.TotpEnrollment do @impl true def mount(_params, _session, socket) do + require Logger + user = socket.assigns.current_scope.user # If user already has TOTP enabled, redirect them if Accounts.totp_enabled?(user) do {:ok, redirect(socket, to: ~p"/devices")} else - # Generate secret once and store in process state - # If user refreshes, they'll get a NEW secret - they must scan the LATEST QR code - secret = Accounts.generate_totp_secret() - qr_code = Accounts.generate_totp_qr_code(user, secret) + # Only generate secret when socket is connected + # On initial render (disconnected), use temporary placeholder + if connected?(socket) do + # Generate secret once per LiveView process + # IMPORTANT: If you refresh this page, you'll get a NEW secret + # You must scan the CURRENT QR code that's displayed + secret = Accounts.generate_totp_secret() + uri = Accounts.generate_totp_uri(user, secret) + qr_code = Accounts.generate_totp_qr_code(user, secret) - socket = - socket - |> assign(:page_title, "Set Up Two-Factor Authentication") - |> assign(:secret, secret) - |> assign(:qr_code, qr_code) - |> assign(:code, "") - |> assign(:error, nil) + # Base32 encode secret for storage in socket assigns + # This prevents corruption during LiveView serialization + secret_base32 = Base.encode32(secret, padding: false) - {:ok, socket} + Logger.info("TOTP enrollment started", + user_email: user.email, + secret_length: byte_size(secret), + secret_base32: secret_base32, + secret_hex: Base.encode16(secret), + uri: uri + ) + + socket = + socket + |> assign(:page_title, "Set Up Two-Factor Authentication") + |> assign(:secret_base32, secret_base32) + |> assign(:qr_code, qr_code) + |> assign(:code, "") + |> assign(:error, nil) + |> assign(:enrollment_complete, false) + |> assign(:recovery_codes, []) + |> assign(:show_recovery_codes, false) + + {:ok, socket} + else + # Initial disconnected render - use placeholder + # Real secret will be generated when socket connects + socket = + socket + |> assign(:page_title, "Set Up Two-Factor Authentication") + |> assign(:secret_base32, nil) + |> assign(:qr_code, nil) + |> assign(:code, "") + |> assign(:error, nil) + |> assign(:enrollment_complete, false) + |> assign(:recovery_codes, []) + |> assign(:show_recovery_codes, false) + + {:ok, socket} + end end end @@ -38,12 +76,17 @@ defmodule ToweropsWeb.AccountLive.TotpEnrollment do require Logger user = socket.assigns.current_scope.user - secret = socket.assigns.secret + secret_base32 = socket.assigns.secret_base32 + + # Decode base32 secret back to binary + secret = Base.decode32!(secret_base32, padding: false, case: :upper) Logger.info("TOTP enrollment verification attempt", user_email: user.email, provided_code: code, - secret_length: byte_size(secret) + secret_length: byte_size(secret), + secret_base32_from_assigns: secret_base32, + secret_binary_hex: Base.encode16(secret) ) if Accounts.verify_totp(secret, code) do @@ -55,15 +98,30 @@ defmodule ToweropsWeb.AccountLive.TotpEnrollment do end end - defp handle_valid_code(socket, user, secret) do - case Accounts.enable_totp(user, secret) do - {:ok, updated_user} -> - redirect_path = get_post_enrollment_path(updated_user) + @impl true + def handle_event("confirm_recovery_codes_saved", _params, socket) do + user = socket.assigns.current_scope.user + redirect_path = get_post_enrollment_path(user) + {:noreply, + socket + |> put_flash(:info, "Two-factor authentication enabled successfully!") + |> redirect(to: redirect_path)} + end + + defp handle_valid_code(socket, user, secret) do + # Create first TOTP device (new multi-device system) + case Accounts.create_totp_device(user.id, "Primary Device", secret) do + {:ok, _device, _secret} -> + # Generate recovery codes for the user + {:ok, codes} = Accounts.generate_recovery_codes(user.id) + + # Show recovery codes to user - they can only be displayed once {:noreply, socket - |> put_flash(:info, "Two-factor authentication enabled successfully!") - |> redirect(to: redirect_path)} + |> assign(:enrollment_complete, true) + |> assign(:recovery_codes, codes) + |> assign(:show_recovery_codes, true)} {:error, _changeset} -> {:noreply, assign(socket, :error, "Failed to enable two-factor authentication. Please try again.")} @@ -83,146 +141,247 @@ defmodule ToweropsWeb.AccountLive.TotpEnrollment do
-
- <.icon - name="hero-shield-check" - class="w-16 h-16 mx-auto mb-4 text-blue-600 dark:text-blue-400" - /> -

- Set Up Two-Factor Authentication -

-

- To keep your account secure, two-factor authentication is required for all users. -

-
- -
- -
-

- - 1 - - Install an Authenticator App -

-

- Download an authenticator app on your phone if you haven't already: + <%= if @show_recovery_codes do %> + +

+ <.icon + name="hero-shield-check" + class="w-16 h-16 mx-auto mb-4 text-green-600 dark:text-green-400" + /> +

+ Save Your Recovery Codes +

+

+ Two-factor authentication has been enabled successfully!

-
    -
  • - <.icon - name="hero-check-circle" - class="w-5 h-5 mr-2 text-green-600 dark:text-green-400" - /> Google Authenticator (iOS, Android) -
  • -
  • - <.icon - name="hero-check-circle" - class="w-5 h-5 mr-2 text-green-600 dark:text-green-400" - /> Authy (iOS, Android, Desktop) -
  • -
  • - <.icon - name="hero-check-circle" - class="w-5 h-5 mr-2 text-green-600 dark:text-green-400" - /> 1Password, Bitwarden (with TOTP support) -
  • -
- - -
-

- - 2 - - Scan the QR Code -

-

- Open your authenticator app and scan this QR code: -

-
-
- TOTP QR Code + +
+ +
+
+ <.icon + name="hero-exclamation-triangle" + class="h-5 w-5 text-yellow-400 mt-0.5" + /> +
+

+ Important: Save these recovery codes now +

+
+
    +
  • These codes will only be shown once and cannot be retrieved later
  • +
  • Store them in a secure location like a password manager
  • +
  • Each code can only be used once
  • +
  • Use these codes if you lose access to your authenticator app
  • +
+
+
-
- - -
-

- - 3 - - Enter the Code from Your App -

- <.form - for={%{}} - as={:form} - phx-submit="verify_code" - class="ml-11 flex flex-col items-center" - > -
-
- - -
- - <%= if @error do %> -
-
- <.icon name="hero-exclamation-circle" class="h-5 w-5 text-red-400" /> -
-

- {@error} -

-
-
+ +
+

+ Your Recovery Codes +

+
+ <%= for code <- @recovery_codes do %> +
+ + {code} +
<% end %> - -
- <.button type="submit" variant="primary" class="w-full"> - <.icon name="hero-shield-check" class="w-5 h-5 mr-2" /> Verify and Enable - -
- -
-
-
-
- <.icon - name="hero-information-circle" - class="h-5 w-5 text-blue-600 dark:text-blue-400 mt-0.5" - /> - + + +
+
-
+ <% else %> + +
+ <.icon + name="hero-shield-check" + class="w-16 h-16 mx-auto mb-4 text-blue-600 dark:text-blue-400" + /> +

+ Set Up Two-Factor Authentication +

+

+ To keep your account secure, two-factor authentication is required for all users. +

+
+ +
+ +
+

+ + 1 + + Install an Authenticator App +

+

+ Download an authenticator app on your phone if you haven't already: +

+
    +
  • + <.icon + name="hero-check-circle" + class="w-5 h-5 mr-2 text-green-600 dark:text-green-400" + /> Google Authenticator (iOS, Android) +
  • +
  • + <.icon + name="hero-check-circle" + class="w-5 h-5 mr-2 text-green-600 dark:text-green-400" + /> Authy (iOS, Android, Desktop) +
  • +
  • + <.icon + name="hero-check-circle" + class="w-5 h-5 mr-2 text-green-600 dark:text-green-400" + /> 1Password, Bitwarden (with TOTP support) +
  • +
+
+ + +
+

+ + 2 + + Scan the QR Code +

+

+ Open your authenticator app and scan this QR code: +

+ <%= if @qr_code do %> +
+
+ TOTP QR Code +
+
+ <% else %> +
+
+

Loading QR code...

+
+
+ <% end %> +
+ + +
+

+ + 3 + + Enter the Code from Your App +

+ <.form + for={%{}} + as={:form} + phx-submit="verify_code" + class="ml-11 flex flex-col items-center" + > +
+
+ + +
+ + <%= if @error do %> +
+
+ <.icon name="hero-exclamation-circle" class="h-5 w-5 text-red-400" /> +
+

+ {@error} +

+
+
+
+ <% end %> + +
+ <.button type="submit" variant="primary" class="w-full"> + <.icon name="hero-shield-check" class="w-5 h-5 mr-2" /> Verify and Enable + +
+
+ +
+
+ +
+
+ <.icon + name="hero-information-circle" + class="h-5 w-5 text-blue-600 dark:text-blue-400 mt-0.5" + /> +
+

+ Keep your device safe: + You'll need to enter a code from your authenticator app each time you log in. + Make sure to keep your phone secure and backed up. +

+
+
+
+ <% end %>
diff --git a/lib/towerops_web/live/user_settings_live.ex b/lib/towerops_web/live/user_settings_live.ex index 73ca1f6f..2eb8b022 100644 --- a/lib/towerops_web/live/user_settings_live.ex +++ b/lib/towerops_web/live/user_settings_live.ex @@ -7,6 +7,7 @@ defmodule ToweropsWeb.UserSettingsLive do use ToweropsWeb, :live_view alias Towerops.Accounts + alias Towerops.Accounts.UserTotpDevice alias Towerops.Admin.AuditLogger alias Towerops.MobileSessions @@ -28,11 +29,20 @@ defmodule ToweropsWeb.UserSettingsLive do |> assign_browser_sessions() |> assign_login_history() |> assign_security_alerts() + |> assign_totp_devices() + |> assign_recovery_codes_count() |> assign(:show_add_token_modal, false) |> assign(:show_token_modal, false) |> assign(:created_token, nil) |> assign(:show_revoke_all_modal, false) |> assign(:login_history_page, 1) + |> assign(:show_add_device_modal, false) + |> assign(:show_device_qr_modal, false) + |> assign(:show_recovery_codes_modal, false) + |> assign(:new_device_id, nil) + |> assign(:new_device_secret, nil) + |> assign(:new_device_qr_code, nil) + |> assign(:generated_recovery_codes, nil) {:ok, socket} end @@ -286,6 +296,140 @@ defmodule ToweropsWeb.UserSettingsLive do {:noreply, socket} end + # Security tab - TOTP Device Management + + @impl true + def handle_event("show_add_device_modal", _params, socket) do + {:noreply, assign(socket, :show_add_device_modal, true)} + end + + @impl true + def handle_event("cancel_add_device", _params, socket) do + {:noreply, assign(socket, :show_add_device_modal, false)} + end + + @impl true + def handle_event("create_device", %{"name" => name}, socket) do + user = socket.assigns.current_scope.user + + case Accounts.create_totp_device(user.id, name) do + {:ok, device, secret} -> + qr_code = Accounts.generate_totp_qr_code(user, secret) + + socket = + socket + |> assign(:show_add_device_modal, false) + |> assign(:show_device_qr_modal, true) + |> assign(:new_device_id, device.id) + |> assign(:new_device_secret, secret) + |> assign(:new_device_qr_code, qr_code) + + {:noreply, socket} + + {:error, _changeset} -> + {:noreply, put_flash(socket, :error, "Failed to create device.")} + end + end + + @impl true + def handle_event("verify_new_device", %{"code" => code}, socket) do + secret = socket.assigns.new_device_secret + device_id = socket.assigns.new_device_id + + if Accounts.verify_totp(secret, code) do + # Device already created, just mark as verified by touching it + device = Towerops.Repo.get!(UserTotpDevice, device_id) + + device + |> UserTotpDevice.touch_changeset() + |> Towerops.Repo.update!() + + socket = + socket + |> put_flash(:info, "Device added successfully!") + |> assign(:show_device_qr_modal, false) + |> assign(:new_device_id, nil) + |> assign(:new_device_secret, nil) + |> assign(:new_device_qr_code, nil) + |> assign_totp_devices() + + {:noreply, socket} + else + {:noreply, put_flash(socket, :error, "Invalid code. Please try again.")} + end + end + + @impl true + def handle_event("close_device_qr_modal", _params, socket) do + # Delete the unverified device if user cancels + if device_id = socket.assigns.new_device_id do + user = socket.assigns.current_scope.user + Accounts.delete_totp_device(device_id, user.id) + end + + socket = + socket + |> assign(:show_device_qr_modal, false) + |> assign(:new_device_id, nil) + |> assign(:new_device_secret, nil) + |> assign(:new_device_qr_code, nil) + |> assign_totp_devices() + + {:noreply, socket} + end + + @impl true + def handle_event("delete_device", %{"device-id" => device_id}, socket) do + user = socket.assigns.current_scope.user + + case Accounts.delete_totp_device(device_id, user.id) do + {:ok, _} -> + socket = + socket + |> put_flash(:info, "Device removed successfully.") + |> assign_totp_devices() + + {:noreply, socket} + + {:error, :last_device} -> + {:noreply, put_flash(socket, :error, "Cannot remove last device. You must have at least one.")} + + {:error, _} -> + {:noreply, put_flash(socket, :error, "Failed to remove device.")} + end + end + + # Security tab - Recovery Codes + + @impl true + def handle_event("regenerate_recovery_codes", _params, socket) do + user = socket.assigns.current_scope.user + + case Accounts.generate_recovery_codes(user.id) do + {:ok, codes} -> + socket = + socket + |> assign(:show_recovery_codes_modal, true) + |> assign(:generated_recovery_codes, codes) + |> assign_recovery_codes_count() + + {:noreply, socket} + + {:error, _} -> + {:noreply, put_flash(socket, :error, "Failed to generate recovery codes.")} + end + end + + @impl true + def handle_event("close_recovery_codes_modal", _params, socket) do + socket = + socket + |> assign(:show_recovery_codes_modal, false) + |> assign(:generated_recovery_codes, nil) + + {:noreply, socket} + end + defp get_current_token_id(%{current_token: token}) when is_binary(token) do Accounts.get_user_token_id_by_value(token) end @@ -356,6 +500,18 @@ defmodule ToweropsWeb.UserSettingsLive do |> assign(:show_security_alert, failed_count >= 3) end + defp assign_totp_devices(socket) do + user = socket.assigns.current_scope.user + devices = Accounts.list_user_totp_devices(user.id) + assign(socket, :totp_devices, devices) + end + + defp assign_recovery_codes_count(socket) do + user = socket.assigns.current_scope.user + count = Accounts.count_unused_recovery_codes(user.id) + assign(socket, :unused_recovery_codes_count, count) + end + # Sessions tab helper functions defp current_session?(session, current_token_id) do @@ -517,6 +673,20 @@ defmodule ToweropsWeb.UserSettingsLive do Notifications +
  • + + Security + +
  • <.link navigate={~p"/users/my-data"} @@ -1193,6 +1363,170 @@ defmodule ToweropsWeb.UserSettingsLive do
  • <% end %> + + + <%= if @active_tab == "security" do %> + +
    +
    +

    + Authenticator Apps +

    +

    + Manage TOTP devices for two-factor authentication. You must have at least one device configured. +

    +
    + +
    + <%= if Enum.empty?(@totp_devices) do %> +
    + <.icon name="hero-device-phone-mobile" class="mx-auto h-12 w-12 text-gray-400" /> +

    + No authenticator apps +

    +

    + Add your first authenticator app to enable two-factor authentication. +

    +
    + +
    +
    + <% else %> +
      + <%= for device <- @totp_devices do %> +
    • +
      +
      +

      + {device.name} +

      +
      +
      +

      + Added {Calendar.strftime(device.inserted_at, "%B %d, %Y")} +

      + <%= if device.last_used_at do %> + + + +

      + Last used {format_relative_time(device.last_used_at)} +

      + <% end %> +
      +
      + +
    • + <% end %> +
    + +
    + +
    + <% end %> +
    +
    + + +
    +
    +

    + Recovery Codes +

    +

    + Single-use backup codes for account access if you lose your authenticator app. +

    +
    + +
    +
    +
    +
    +

    + Available Recovery Codes +

    +

    + <%= if @unused_recovery_codes_count == 0 do %> + + No recovery codes available + + - Generate new codes immediately + <% else %> + You have + + {@unused_recovery_codes_count} + + unused recovery code{if @unused_recovery_codes_count != 1, do: "s"} + <% end %> +

    +
    +
    + +
    +
    +
    + + <%= if @unused_recovery_codes_count == 0 do %> +
    +
    +
    + <.icon name="hero-exclamation-triangle" class="h-5 w-5 text-amber-400" /> +
    +
    +

    + No Recovery Codes +

    +
    +

    + You have no recovery codes available. If you lose access to your authenticator app, + you won't be able to log in. Generate new codes now. +

    +
    +
    +
    +
    + <% end %> +
    +
    + <% end %>
    @@ -1437,6 +1771,280 @@ defmodule ToweropsWeb.UserSettingsLive do
    <% end %> + + <%= if @show_add_device_modal do %> + + <% end %> + + + <%= if @show_device_qr_modal && @new_device_qr_code do %> + + <% end %> + + + <%= if @show_recovery_codes_modal && @generated_recovery_codes do %> + + <% end %> +
    + assert Regex.match?(~r/^[A-Z2-9]{4}-[A-Z2-9]{4}$/, code) + # No ambiguous chars + refute String.contains?(code, ["O", "I", "0", "1"]) + end) + end + + test "generate_recovery_codes/1 deletes existing unused codes before generating", %{user: user} do + {:ok, old_codes} = Accounts.generate_recovery_codes(user.id) + old_code = List.first(old_codes) + + {:ok, new_codes} = Accounts.generate_recovery_codes(user.id) + + # Old codes should be invalid + assert {:error, :invalid_code} = Accounts.verify_recovery_code(user.id, old_code) + + # New codes should work + new_code = List.first(new_codes) + assert {:ok, _} = Accounts.verify_recovery_code(user.id, new_code) + end + + test "generate_recovery_codes/1 preserves used codes when regenerating", %{user: user} do + {:ok, codes} = Accounts.generate_recovery_codes(user.id) + used_code = List.first(codes) + + # Use one code + {:ok, _} = Accounts.verify_recovery_code(user.id, used_code) + + # Generate new codes + {:ok, _new_codes} = Accounts.generate_recovery_codes(user.id) + + # Used code should still be marked as used (not deleted) + assert 12 = Accounts.count_unused_recovery_codes(user.id) + end + + test "verify_recovery_code/2 verifies valid unused code", %{user: user} do + {:ok, codes} = Accounts.generate_recovery_codes(user.id) + code = List.first(codes) + + assert {:ok, record} = Accounts.verify_recovery_code(user.id, code) + assert is_nil(record.used_at) == false + end + + test "verify_recovery_code/2 marks code as used after verification", %{user: user} do + {:ok, codes} = Accounts.generate_recovery_codes(user.id) + code = List.first(codes) + + {:ok, _} = Accounts.verify_recovery_code(user.id, code) + + # Second attempt should fail + assert {:error, :invalid_code} = Accounts.verify_recovery_code(user.id, code) + end + + test "verify_recovery_code/2 rejects invalid code format", %{user: user} do + assert {:error, :invalid_code} = Accounts.verify_recovery_code(user.id, "INVALID") + assert {:error, :invalid_code} = Accounts.verify_recovery_code(user.id, "1234-5678") + end + + test "count_unused_recovery_codes/1 returns correct count", %{user: user} do + assert 0 = Accounts.count_unused_recovery_codes(user.id) + + {:ok, codes} = Accounts.generate_recovery_codes(user.id) + assert 12 = Accounts.count_unused_recovery_codes(user.id) + + # Use one code + code = List.first(codes) + {:ok, _} = Accounts.verify_recovery_code(user.id, code) + assert 11 = Accounts.count_unused_recovery_codes(user.id) + end + + test "list_user_recovery_codes/1 returns all codes with status", %{user: user} do + {:ok, codes} = Accounts.generate_recovery_codes(user.id) + + all_codes = Accounts.list_user_recovery_codes(user.id) + assert length(all_codes) == 12 + + # Use one code + code = List.first(codes) + {:ok, _} = Accounts.verify_recovery_code(user.id, code) + + all_codes = Accounts.list_user_recovery_codes(user.id) + used_codes = Enum.filter(all_codes, & &1.used_at) + unused_codes = Enum.filter(all_codes, &is_nil(&1.used_at)) + + assert length(used_codes) == 1 + assert length(unused_codes) == 11 + end + end + + describe "verify_user_mfa/2" do + setup do + user = user_fixture() + {:ok, user: user} + end + + test "accepts TOTP code from any device", %{user: user} do + {:ok, _device, secret} = Accounts.create_totp_device(user.id, "Device") + code = NimbleTOTP.verification_code(secret) + + assert {:ok, ^user, :totp} = Accounts.verify_user_mfa(user, code) + end + + test "accepts recovery code as fallback", %{user: user} do + {:ok, _device, _secret} = Accounts.create_totp_device(user.id, "Device") + {:ok, codes} = Accounts.generate_recovery_codes(user.id) + recovery_code = List.first(codes) + + assert {:ok, ^user, :recovery_code} = Accounts.verify_user_mfa(user, recovery_code) + end + + test "prefers TOTP over recovery code when both would be valid", %{user: user} do + {:ok, _device, secret} = Accounts.create_totp_device(user.id, "Device") + totp_code = NimbleTOTP.verification_code(secret) + + # Should return :totp, not :recovery_code + assert {:ok, ^user, :totp} = Accounts.verify_user_mfa(user, totp_code) + end + + test "rejects invalid code that matches neither TOTP nor recovery", %{user: user} do + {:ok, _device, _secret} = Accounts.create_totp_device(user.id, "Device") + {:ok, _codes} = Accounts.generate_recovery_codes(user.id) + + assert {:error, :invalid_code} = Accounts.verify_user_mfa(user, "000000") + end + + test "marks recovery code as used when verified via MFA", %{user: user} do + {:ok, _device, _secret} = Accounts.create_totp_device(user.id, "Device") + {:ok, codes} = Accounts.generate_recovery_codes(user.id) + recovery_code = List.first(codes) + + assert {:ok, ^user, :recovery_code} = Accounts.verify_user_mfa(user, recovery_code) + + # Code should be marked as used + assert 11 = Accounts.count_unused_recovery_codes(user.id) + end + end end diff --git a/test/towerops_web/controllers/user_session_controller_test.exs b/test/towerops_web/controllers/user_session_controller_test.exs index c2f04913..b9a5b16f 100644 --- a/test/towerops_web/controllers/user_session_controller_test.exs +++ b/test/towerops_web/controllers/user_session_controller_test.exs @@ -216,6 +216,124 @@ defmodule ToweropsWeb.UserSessionControllerTest do end end + describe "POST /users/log-in/totp - recovery codes" do + test "logs in with valid recovery code", %{conn: conn, user_with_totp: user} do + # Generate recovery codes + {:ok, codes} = Accounts.generate_recovery_codes(user.id) + recovery_code = List.first(codes) + + conn = + conn + |> init_test_session(pending_totp_user_id: user.id) + |> post(~p"/users/log-in/totp", %{"user" => %{"totp_code" => recovery_code}}) + + # Should create session + assert get_session(conn, :user_token) + # Should clear pending state + refute get_session(conn, :pending_totp_user_id) + # Should show warning about recovery code usage + assert Phoenix.Flash.get(conn.assigns.flash, :warning) =~ + "You used a recovery code" + + assert Phoenix.Flash.get(conn.assigns.flash, :warning) =~ + "Consider regenerating codes" + + assert redirected_to(conn) == ~p"/orgs" + end + + test "marks recovery code as used after login", %{conn: conn, user_with_totp: user} do + {:ok, codes} = Accounts.generate_recovery_codes(user.id) + recovery_code = List.first(codes) + + # Verify code is unused + assert Accounts.count_unused_recovery_codes(user.id) == 12 + + conn = + conn + |> init_test_session(pending_totp_user_id: user.id) + |> post(~p"/users/log-in/totp", %{"user" => %{"totp_code" => recovery_code}}) + + assert get_session(conn, :user_token) + + # Verify code was marked as used + assert Accounts.count_unused_recovery_codes(user.id) == 11 + end + + test "rejects already-used recovery code", %{conn: conn, user_with_totp: user} do + {:ok, codes} = Accounts.generate_recovery_codes(user.id) + recovery_code = List.first(codes) + + # Use the code once + conn = + conn + |> init_test_session(pending_totp_user_id: user.id) + |> post(~p"/users/log-in/totp", %{"user" => %{"totp_code" => recovery_code}}) + + assert get_session(conn, :user_token) + + # Try to use it again + conn = + build_conn() + |> init_test_session(pending_totp_user_id: user.id) + |> post(~p"/users/log-in/totp", %{"user" => %{"totp_code" => recovery_code}}) + + response = html_response(conn, 200) + assert response =~ "Invalid authentication code" + refute get_session(conn, :user_token) + end + + test "rejects invalid recovery code format", %{conn: conn, user_with_totp: user} do + conn = + conn + |> init_test_session(pending_totp_user_id: user.id) + |> post(~p"/users/log-in/totp", %{"user" => %{"totp_code" => "INVALID-CODE"}}) + + response = html_response(conn, 200) + assert response =~ "Invalid authentication code" + refute get_session(conn, :user_token) + end + + test "accepts both TOTP and recovery codes", %{conn: conn, user_with_totp: user} do + {:ok, codes} = Accounts.generate_recovery_codes(user.id) + recovery_code = List.first(codes) + + # First login with TOTP code + totp_code = NimbleTOTP.verification_code(user.totp_secret) + + conn = + conn + |> init_test_session(pending_totp_user_id: user.id) + |> post(~p"/users/log-in/totp", %{"user" => %{"totp_code" => totp_code}}) + + assert get_session(conn, :user_token) + # No warning for TOTP + refute Phoenix.Flash.get(conn.assigns.flash, :warning) + + # Second login with recovery code + conn = + build_conn() + |> init_test_session(pending_totp_user_id: user.id) + |> post(~p"/users/log-in/totp", %{"user" => %{"totp_code" => recovery_code}}) + + assert get_session(conn, :user_token) + # Warning shown for recovery code + assert Phoenix.Flash.get(conn.assigns.flash, :warning) =~ "recovery code" + end + + test "respects remember_me with recovery code", %{conn: conn, user_with_totp: user} do + {:ok, codes} = Accounts.generate_recovery_codes(user.id) + recovery_code = List.first(codes) + + conn = + conn + |> init_test_session(pending_totp_user_id: user.id, pending_totp_remember_me: true) + |> post(~p"/users/log-in/totp", %{"user" => %{"totp_code" => recovery_code}}) + + assert conn.resp_cookies["_towerops_web_user_remember_me"] + assert get_session(conn, :user_token) + end + end + describe "POST /users/log-in - magic link" do test "sends magic link email when user exists", %{conn: conn, user: user} do conn = diff --git a/test/towerops_web/live/account_live/totp_enrollment_test.exs b/test/towerops_web/live/account_live/totp_enrollment_test.exs index c4bfce1f..0b982196 100644 --- a/test/towerops_web/live/account_live/totp_enrollment_test.exs +++ b/test/towerops_web/live/account_live/totp_enrollment_test.exs @@ -13,6 +13,24 @@ defmodule ToweropsWeb.AccountLive.TotpEnrollmentTest do %{conn: log_in_user(build_conn(), user), user: user} end + test "generates valid 20-byte secret", %{conn: conn} do + {:ok, view, _html} = live(conn, ~p"/account/totp-enrollment") + + # Get the base32 secret from the view's assigns + secret_base32 = :sys.get_state(view.pid).socket.assigns.secret_base32 + + # Decode to binary + secret = Base.decode32!(secret_base32, padding: false, case: :upper) + + # Verify secret is 20 bytes (NimbleTOTP default) + assert byte_size(secret) == 20 + + # Verify we can generate valid codes from it + code = NimbleTOTP.verification_code(secret) + assert String.length(code) == 6 + assert NimbleTOTP.valid?(secret, code) + end + test "renders enrollment page with QR code", %{conn: conn} do {:ok, view, html} = live(conn, ~p"/account/totp-enrollment") @@ -31,8 +49,9 @@ defmodule ToweropsWeb.AccountLive.TotpEnrollmentTest do {:ok, view, _html} = live(conn, ~p"/account/totp-enrollment") - # Get the secret from the view's assigns (via testing API) - secret = :sys.get_state(view.pid).socket.assigns.secret + # Get the base32 secret from the view's assigns and decode it + secret_base32 = :sys.get_state(view.pid).socket.assigns.secret_base32 + secret = Base.decode32!(secret_base32, padding: false, case: :upper) # Generate a valid code code = NimbleTOTP.verification_code(secret) @@ -42,20 +61,41 @@ defmodule ToweropsWeb.AccountLive.TotpEnrollmentTest do |> form("form", %{code: code}) |> render_submit() - # Should redirect to devices page (user has organization) - assert_redirect(view, ~p"/devices") + # Render the view to see the recovery codes page + html = render(view) - # Verify TOTP was enabled in database + # Should show recovery codes page + assert html =~ "Save Your Recovery Codes" + assert html =~ "These codes will only be shown once" + assert html =~ "Your Recovery Codes" + + # Verify TOTP device was created + devices = Accounts.list_user_totp_devices(user.id) + assert length(devices) == 1 + assert hd(devices).name == "Primary Device" + + # Verify recovery codes were generated + assert Accounts.count_unused_recovery_codes(user.id) == 12 + + # Verify TOTP is enabled via new system updated_user = Accounts.get_user!(user.id) assert Accounts.totp_enabled?(updated_user) - assert updated_user.totp_secret == secret + + # Click the confirmation button + view + |> element("button", "I've Saved My Recovery Codes") + |> render_click() + + # Should redirect to devices page (user has organization) + assert_redirect(view, ~p"/devices") end test "redirects to orgs page when user has no organizations", %{conn: conn, user: user} do {:ok, view, _html} = live(conn, ~p"/account/totp-enrollment") - # Get the secret from the view's assigns - secret = :sys.get_state(view.pid).socket.assigns.secret + # Get the base32 secret from the view's assigns and decode it + secret_base32 = :sys.get_state(view.pid).socket.assigns.secret_base32 + secret = Base.decode32!(secret_base32, padding: false, case: :upper) # Generate a valid code code = NimbleTOTP.verification_code(secret) @@ -65,12 +105,23 @@ defmodule ToweropsWeb.AccountLive.TotpEnrollmentTest do |> form("form", %{code: code}) |> render_submit() - # Should redirect to orgs page (user has no organizations) - assert_redirect(view, ~p"/orgs") + # Render the view to see the recovery codes page + html = render(view) + + # Should show recovery codes page + assert html =~ "Save Your Recovery Codes" # Verify TOTP was enabled in database updated_user = Accounts.get_user!(user.id) assert Accounts.totp_enabled?(updated_user) + + # Click the confirmation button + view + |> element("button", "I've Saved My Recovery Codes") + |> render_click() + + # Should redirect to orgs page (user has no organizations) + assert_redirect(view, ~p"/orgs") end test "rejects invalid TOTP code", %{conn: conn} do