neiam
/gitgud
Git Gud
public · Issues · Pulls · Labels · Forks · Compare · Actions success · Badges · Packages
⭐
1
Log in to mark this repository.
Accounts: a TOTP code is good once
96a5dc6 · Gabriel Morell · 2026-10-18 19:32
Message
{commit_body(@commit)}
Files changed
modified
lib/git_gud/accounts/two_factor.ex
+25
−5
@@ -85,7 +85,8 @@ defmodule GitGud.Accounts.TwoFactor do
| 85 | 85 | totp_ciphertext: enc.ciphertext, |
| 86 | 86 | totp_iv: enc.iv, |
| 87 | 87 | totp_tag: enc.tag, |
| 88 | − totp_enabled_at: now | |
| 88 | + totp_enabled_at: now, | |
| 89 | + totp_last_step: nil | |
| 89 | 90 | ) |
| 90 | 91 | |> Repo.update!() |
| 91 | 92 |
@@ -116,7 +117,8 @@ defmodule GitGud.Accounts.TwoFactor do
| 116 | 117 | totp_ciphertext: nil, |
| 117 | 118 | totp_iv: nil, |
| 118 | 119 | totp_tag: nil, |
| 119 | − totp_enabled_at: nil | |
| 120 | + totp_enabled_at: nil, | |
| 121 | + totp_last_step: nil | |
| 120 | 122 | ) |
| 121 | 123 | |> Repo.update!() |
| 122 | 124 | end) |
@@ -139,16 +141,34 @@ defmodule GitGud.Accounts.TwoFactor do
| 139 | 141 | """ |
| 140 | 142 | def verify_code(%User{totp_ciphertext: nil}, _code), do: {:error, :not_enrolled} |
| 141 | 143 | |
| 142 | − def verify_code(%User{} = user, code) when is_binary(code) do | |
| 144 | + def verify_code(%User{} = user, code, now \\ System.os_time(:second)) when is_binary(code) do | |
| 143 | 145 | with {:ok, secret} <- decrypt_secret(user) do |
| 144 | − if NimbleTOTP.valid?(secret, code) do | |
| 145 | − :ok | |
| 146 | + if NimbleTOTP.valid?(secret, code, time: now) do | |
| 147 | + claim_step(user, step(now)) | |
| 146 | 148 | else |
| 147 | 149 | {:error, :invalid_code} |
| 148 | 150 | end |
| 149 | 151 | end |
| 150 | 152 | end |
| 151 | 153 | |
| 154 | + @period 30 | |
| 155 | + | |
| 156 | + defp step(unix), do: Integer.floor_div(unix, @period) | |
| 157 | + | |
| 158 | + # A code is good once: someone who watched it typed (or phished it) | |
| 159 | + # can't replay it inside its 30-second window. The step is claimed with | |
| 160 | + # a conditional update, so two requests racing with the same code | |
| 161 | + # can't both win, and a step at or before the last one used is refused. | |
| 162 | + defp claim_step(%User{id: uid}, step) do | |
| 163 | + {count, _} = | |
| 164 | + from(u in User, | |
| 165 | + where: u.id == ^uid and (is_nil(u.totp_last_step) or u.totp_last_step < ^step) | |
| 166 | + ) | |
| 167 | + |> Repo.update_all(set: [totp_last_step: step]) | |
| 168 | + | |
| 169 | + if count == 1, do: :ok, else: {:error, :invalid_code} | |
| 170 | + end | |
| 171 | + | |
| 152 | 172 | defp decrypt_secret(user) do |
| 153 | 173 | Secrets.decrypt(%{ |
| 154 | 174 | ciphertext: user.totp_ciphertext, |
modified
lib/git_gud/accounts/user.ex
+2
−0
@@ -31,6 +31,8 @@ defmodule GitGud.Accounts.User do
| 31 | 31 | field :totp_iv, :binary |
| 32 | 32 | field :totp_tag, :binary |
| 33 | 33 | field :totp_enabled_at, :utc_datetime |
| 34 | + # Last accepted TOTP time step; see `TwoFactor.verify_code/2`. | |
| 35 | + field :totp_last_step, :integer | |
| 34 | 36 | field :second_factor_reauth_enabled, :boolean, default: false |
| 35 | 37 | |
| 36 | 38 | timestamps(type: :utc_datetime) |
added
priv/repo/migrations/20261018193024_add_totp_last_step_to_users.exs
+12
−0
@@ -0,0 +1,12 @@
| 1 | +defmodule GitGud.Repo.Migrations.AddTotpLastStepToUsers do | |
| 2 | + use Ecto.Migration | |
| 3 | + | |
| 4 | + # The last TOTP time step (unix time div 30) a code was accepted for, | |
| 5 | + # so the same code — or an older one — can't be used again inside its | |
| 6 | + # window. See GitGud.Accounts.TwoFactor.verify_code/2. | |
| 7 | + def change do | |
| 8 | + alter table(:users) do | |
| 9 | + add :totp_last_step, :bigint | |
| 10 | + end | |
| 11 | + end | |
| 12 | +end |
modified
test/git_gud/accounts/two_factor_test.exs
+23
−0
@@ -42,6 +42,29 @@ defmodule GitGud.Accounts.TwoFactorTest do
| 42 | 42 | assert {:error, :invalid_code} = TwoFactor.verify_code(user, "000001") |
| 43 | 43 | end |
| 44 | 44 | |
| 45 | + test "a code is good once: no replay inside its window, no older step after it" do | |
| 46 | + user = user_fixture() | |
| 47 | + secret = TwoFactor.new_secret() | |
| 48 | + {:ok, _} = TwoFactor.enable(user, secret, NimbleTOTP.verification_code(secret)) | |
| 49 | + user = GitGud.Repo.get!(GitGud.Accounts.User, user.id) | |
| 50 | + | |
| 51 | + # Pin the clock to the start of a step so +/-30s are whole steps. | |
| 52 | + now = div(System.os_time(:second), 30) * 30 | |
| 53 | + code = NimbleTOTP.verification_code(secret, time: now) | |
| 54 | + | |
| 55 | + assert :ok = TwoFactor.verify_code(user, code, now) | |
| 56 | + assert {:error, :invalid_code} = TwoFactor.verify_code(user, code, now) | |
| 57 | + assert {:error, :invalid_code} = TwoFactor.verify_code(user, code, now + 29) | |
| 58 | + | |
| 59 | + # An earlier step's code, even if still otherwise valid, is spent. | |
| 60 | + earlier = NimbleTOTP.verification_code(secret, time: now - 30) | |
| 61 | + assert {:error, :invalid_code} = TwoFactor.verify_code(user, earlier, now - 30) | |
| 62 | + | |
| 63 | + # The next step's code works. | |
| 64 | + later = NimbleTOTP.verification_code(secret, time: now + 30) | |
| 65 | + assert :ok = TwoFactor.verify_code(user, later, now + 30) | |
| 66 | + end | |
| 67 | + | |
| 45 | 68 | test "verify_code returns :not_enrolled when no secret stored" do |
| 46 | 69 | user = user_fixture() |
| 47 | 70 | assert {:error, :not_enrolled} = TwoFactor.verify_code(user, "123456") |
modified
test/git_gud_web/second_factor_reauth_test.exs
+22
−0
@@ -58,6 +58,28 @@ defmodule GitGudWeb.SecondFactorReauthTest do
| 58 | 58 | assert {:ok, _lv, _html} = live(conn, ~p"/users/settings") |
| 59 | 59 | end |
| 60 | 60 | |
| 61 | + test "a TOTP code that already opened sudo can't open it again", %{ | |
| 62 | + conn: conn, | |
| 63 | + user: user, | |
| 64 | + secret: secret | |
| 65 | + } do | |
| 66 | + opt_in(user) | |
| 67 | + code = NimbleTOTP.verification_code(secret) | |
| 68 | + | |
| 69 | + first = post(conn, ~p"/users/reauth/second-factor", %{"code" => code}) | |
| 70 | + assert redirected_to(first) == ~p"/users/settings" | |
| 71 | + | |
| 72 | + # A second stale session (say, a shoulder-surfed code replayed by | |
| 73 | + # someone holding a stolen session) gets nothing for it. | |
| 74 | + replay = | |
| 75 | + build_conn() | |
| 76 | + |> log_in_user(user, token_authenticated_at: @stale_auth) | |
| 77 | + |> post(~p"/users/reauth/second-factor", %{"code" => code}) | |
| 78 | + | |
| 79 | + assert redirected_to(replay) == ~p"/users/log-in" | |
| 80 | + assert Phoenix.Flash.get(replay.assigns.flash, :error) =~ "didn't match" | |
| 81 | + end | |
| 82 | + | |
| 61 | 83 | test "a bad TOTP code does not grant sudo mode", %{conn: conn, user: user} do |
| 62 | 84 | opt_in(user) |
| 63 | 85 |
modified
test/git_gud_web/two_factor_login_test.exs
+12
−0
@@ -109,6 +109,18 @@ defmodule GitGudWeb.TwoFactorLoginTest do
| 109 | 109 | end |
| 110 | 110 | |
| 111 | 111 | describe "completion with a verified factor" do |
| 112 | + test "a TOTP code that finished one login can't finish another", | |
| 113 | + %{conn: conn, user: user, secret: secret} do | |
| 114 | + code = NimbleTOTP.verification_code(secret) | |
| 115 | + | |
| 116 | + {:ok, lv, _html} = live(password_stage(conn, user), ~p"/users/log-in/two-factor") | |
| 117 | + assert {:error, {:redirect, _}} = submit_code(lv, code) | |
| 118 | + | |
| 119 | + {:ok, lv, _html} = live(password_stage(build_conn(), user), ~p"/users/log-in/two-factor") | |
| 120 | + html = submit_code(lv, code) | |
| 121 | + assert html =~ "didn't match" | |
| 122 | + end | |
| 123 | + | |
| 112 | 124 | test "a TOTP code logs in", %{conn: conn, user: user, secret: secret} do |
| 113 | 125 | conn = password_stage(conn, user) |
| 114 | 126 | {:ok, lv, _html} = live(conn, ~p"/users/log-in/two-factor") |
Parents: e3f46f9