diff options
| author | erdgeist <erdgeist@erdgeist.org> | 2026-07-24 13:11:51 +0200 |
|---|---|---|
| committer | erdgeist <erdgeist@erdgeist.org> | 2026-07-24 13:11:51 +0200 |
| commit | 02e1aefa24cdd0339995d14431713822f4bf4718 (patch) | |
| tree | a622727150214f8e9dab165ae48803180c38403d | |
| parent | 38920de3910705dac42af2370da7b3ba504577a9 (diff) | |
Add TOTP enrollment and verification to User, witnessed in the action log
| -rw-r--r-- | Gemfile | 8 | ||||
| -rw-r--r-- | Gemfile.lock | 12 | ||||
| -rw-r--r-- | app/models/node_action.rb | 7 | ||||
| -rw-r--r-- | app/models/user.rb | 71 | ||||
| -rw-r--r-- | config/initializers/otp.rb | 2 | ||||
| -rw-r--r-- | db/migrate/20260724110241_add_otp_to_users.rb | 8 | ||||
| -rw-r--r-- | test/models/user_otp_test.rb | 83 |
7 files changed, 190 insertions, 1 deletions
| @@ -38,7 +38,7 @@ gem 'rails_icons' | |||
| 38 | gem 'globalize', '~> 7.0' # translated model attributes (Page title/abstract/body) | 38 | gem 'globalize', '~> 7.0' # translated model attributes (Page title/abstract/body) |
| 39 | gem 'acts_as_list' # page revision ordering | 39 | gem 'acts_as_list' # page revision ordering |
| 40 | gem 'will_paginate', '~> 3.0' | 40 | gem 'will_paginate', '~> 3.0' |
| 41 | gem 'bcrypt', '~> 3.1' | 41 | |
| 42 | 42 | ||
| 43 | # Pinned to git until a release widens the activerecord < 8.1 ceiling. | 43 | # Pinned to git until a release widens the activerecord < 8.1 ceiling. |
| 44 | # Both gems work correctly on Rails 8.1; the gemspec constraint is overly conservative. | 44 | # Both gems work correctly on Rails 8.1; the gemspec constraint is overly conservative. |
| @@ -47,6 +47,12 @@ gem 'acts-as-taggable-on', | |||
| 47 | git: 'https://github.com/mbleigh/acts-as-taggable-on.git', | 47 | git: 'https://github.com/mbleigh/acts-as-taggable-on.git', |
| 48 | branch: 'master' | 48 | branch: 'master' |
| 49 | 49 | ||
| 50 | # ── Auth and sessions ───────────────────────────────────────────────────────── | ||
| 51 | |||
| 52 | gem "rotp", ">= 6.3.0" | ||
| 53 | gem "rqrcode", "~> 3.1.0" | ||
| 54 | gem 'bcrypt', '~> 3.1' | ||
| 55 | |||
| 50 | # ── XML / parsing / diffing ─────────────────────────────────────────────────── | 56 | # ── XML / parsing / diffing ─────────────────────────────────────────────────── |
| 51 | 57 | ||
| 52 | gem 'libxml-ruby', '~> 5.0', require: 'xml' # body link rewriting in Page model | 58 | gem 'libxml-ruby', '~> 5.0', require: 'xml' # body link rewriting in Page model |
diff --git a/Gemfile.lock b/Gemfile.lock index 8e4d5afa..a1046d0d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock | |||
| @@ -98,6 +98,7 @@ GEM | |||
| 98 | bcrypt (3.1.22) | 98 | bcrypt (3.1.22) |
| 99 | bigdecimal (4.1.2) | 99 | bigdecimal (4.1.2) |
| 100 | builder (3.3.0) | 100 | builder (3.3.0) |
| 101 | chunky_png (1.4.0) | ||
| 101 | coffee-rails (4.2.2) | 102 | coffee-rails (4.2.2) |
| 102 | coffee-script (>= 2.2.0) | 103 | coffee-script (>= 2.2.0) |
| 103 | railties (>= 4.0.0) | 104 | railties (>= 4.0.0) |
| @@ -275,6 +276,11 @@ GEM | |||
| 275 | io-console (~> 0.5) | 276 | io-console (~> 0.5) |
| 276 | request_store (1.7.0) | 277 | request_store (1.7.0) |
| 277 | rack (>= 1.4) | 278 | rack (>= 1.4) |
| 279 | rotp (6.3.0) | ||
| 280 | rqrcode (3.1.1) | ||
| 281 | chunky_png (~> 1.0) | ||
| 282 | rqrcode_core (~> 2.0) | ||
| 283 | rqrcode_core (2.1.0) | ||
| 278 | sass-rails (6.0.0) | 284 | sass-rails (6.0.0) |
| 279 | sassc-rails (~> 2.1, >= 2.1.1) | 285 | sassc-rails (~> 2.1, >= 2.1.1) |
| 280 | sassc (2.4.0) | 286 | sassc (2.4.0) |
| @@ -353,6 +359,8 @@ DEPENDENCIES | |||
| 353 | rails-controller-testing | 359 | rails-controller-testing |
| 354 | rails-i18n (~> 8.1) | 360 | rails-i18n (~> 8.1) |
| 355 | rails_icons | 361 | rails_icons |
| 362 | rotp (>= 6.3.0) | ||
| 363 | rqrcode (~> 3.1.0) | ||
| 356 | sass-rails (~> 6.0) | 364 | sass-rails (~> 6.0) |
| 357 | sprockets-rails | 365 | sprockets-rails |
| 358 | test-unit (~> 3.5) | 366 | test-unit (~> 3.5) |
| @@ -382,6 +390,7 @@ CHECKSUMS | |||
| 382 | builder (3.3.0) sha256=497918d2f9dca528fdca4b88d84e4ef4387256d984b8154e9d5d3fe5a9c8835f | 390 | builder (3.3.0) sha256=497918d2f9dca528fdca4b88d84e4ef4387256d984b8154e9d5d3fe5a9c8835f |
| 383 | bundler (4.0.15) sha256=a4ceb882fe94a0e0ac63cd0813932bbfd631a14e5ac0b7975189b19a4d28d9e7 | 391 | bundler (4.0.15) sha256=a4ceb882fe94a0e0ac63cd0813932bbfd631a14e5ac0b7975189b19a4d28d9e7 |
| 384 | chaos_calendar (0.1.3) | 392 | chaos_calendar (0.1.3) |
| 393 | chunky_png (1.4.0) sha256=89d5b31b55c0cf4da3cf89a2b4ebc3178d8abe8cbaf116a1dba95668502fdcfe | ||
| 385 | coffee-rails (4.2.2) sha256=080a5c49e9568019018a8699cf80f7ea8e7b841744d832a98ac6b4c8e40be84a | 394 | coffee-rails (4.2.2) sha256=080a5c49e9568019018a8699cf80f7ea8e7b841744d832a98ac6b4c8e40be84a |
| 386 | coffee-script (2.4.1) sha256=82fe281e11b93c8117b98c5ea8063e71741870f1c4fbb27177d7d6333dd38765 | 395 | coffee-script (2.4.1) sha256=82fe281e11b93c8117b98c5ea8063e71741870f1c4fbb27177d7d6333dd38765 |
| 387 | coffee-script-source (1.12.2) sha256=e12b16fd8927fbbf8b87cb2e9a85a6cf457c6881cc7ff8b1af15b31f70da07a4 | 396 | coffee-script-source (1.12.2) sha256=e12b16fd8927fbbf8b87cb2e9a85a6cf457c6881cc7ff8b1af15b31f70da07a4 |
| @@ -468,6 +477,9 @@ CHECKSUMS | |||
| 468 | rdoc (8.0.0) sha256=03bf8c08a9639658855a0cfd77c0abca8325c227693f7f33f82957811348c469 | 477 | rdoc (8.0.0) sha256=03bf8c08a9639658855a0cfd77c0abca8325c227693f7f33f82957811348c469 |
| 469 | reline (0.6.3) sha256=1198b04973565b36ec0f11542ab3f5cfeeec34823f4e54cebde90968092b1835 | 478 | reline (0.6.3) sha256=1198b04973565b36ec0f11542ab3f5cfeeec34823f4e54cebde90968092b1835 |
| 470 | request_store (1.7.0) sha256=e1b75d5346a315f452242a68c937ef8e48b215b9453a77a6c0acdca2934c88cb | 479 | request_store (1.7.0) sha256=e1b75d5346a315f452242a68c937ef8e48b215b9453a77a6c0acdca2934c88cb |
| 480 | rotp (6.3.0) sha256=75d40087e65ed0d8022c33055a6306c1c400d1c12261932533b5d6cbcd868854 | ||
| 481 | rqrcode (3.1.1) sha256=4c2e1e36dab80720062388cfc827986ab622dc652437214a5fb6382985b6f00f | ||
| 482 | rqrcode_core (2.1.0) sha256=f303b85df89c1b8fc5ee8dc19808c9dc4330e6329b660d99d4a8cbb36ca13051 | ||
| 471 | sass-rails (6.0.0) sha256=e0b6448ea1c7929fd5929fc7a8eb2d78045e44cc82fc0765a249d3fa1c5810d3 | 483 | sass-rails (6.0.0) sha256=e0b6448ea1c7929fd5929fc7a8eb2d78045e44cc82fc0765a249d3fa1c5810d3 |
| 472 | sassc (2.4.0) sha256=4c60a2b0a3b36685c83b80d5789401c2f678c1652e3288315a1551d811d9f83e | 484 | sassc (2.4.0) sha256=4c60a2b0a3b36685c83b80d5789401c2f678c1652e3288315a1551d811d9f83e |
| 473 | sassc-rails (2.1.2) sha256=5f4fdf3881fc9bdc8e856ffbd9850d70a2878866feae8114aa45996179952db5 | 485 | sassc-rails (2.1.2) sha256=5f4fdf3881fc9bdc8e856ffbd9850d70a2878866feae8114aa45996179952db5 |
diff --git a/app/models/node_action.rb b/app/models/node_action.rb index afa2195c..a9bb0e45 100644 --- a/app/models/node_action.rb +++ b/app/models/node_action.rb | |||
| @@ -98,6 +98,13 @@ class NodeAction < ApplicationRecord | |||
| 98 | # "detached_from" -- array of unique_names, only when any | 98 | # "detached_from" -- array of unique_names, only when any |
| 99 | # "headline_removed_from" -- array of unique_names, only when any | 99 | # "headline_removed_from" -- array of unique_names, only when any |
| 100 | # | 100 | # |
| 101 | # "otp_enroll" / "otp_disable" / "otp_reset" (second-factor lifecycle; | ||
| 102 | # node column nil; participants: the affected User -- the table's first | ||
| 103 | # User-typed subject. otp_disable is self-service; otp_reset is an | ||
| 104 | # administrator clearing someone else's factor, where actor and | ||
| 105 | # participant differ): | ||
| 106 | # "target_login" -- flat string, the affected account's login | ||
| 107 | # | ||
| 101 | # Reserved: "demote" (via "trash" | "depublish") for an explicit | 108 | # Reserved: "demote" (via "trash" | "depublish") for an explicit |
| 102 | # depublish workflow, if ever built. | 109 | # depublish workflow, if ever built. |
| 103 | # | 110 | # |
diff --git a/app/models/user.rb b/app/models/user.rb index 5e47ae7d..4d712f6c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb | |||
| @@ -105,6 +105,77 @@ class User < ApplicationRecord | |||
| 105 | def is_admin? | 105 | def is_admin? |
| 106 | !!admin | 106 | !!admin |
| 107 | end | 107 | end |
| 108 | |||
| 109 | # otp_secret present == enrolled. otp_pending_secret holds the secret | ||
| 110 | # between QR display and first-code confirmation. otp_consumed_timestep | ||
| 111 | # makes every accepted code single-use (replay guard within the drift | ||
| 112 | # window). | ||
| 113 | |||
| 114 | def otp_enrolled? | ||
| 115 | otp_secret.present? | ||
| 116 | end | ||
| 117 | |||
| 118 | # Starts (or restarts) enrollment. Returns the provisioning URI the QR | ||
| 119 | # encodes; otp_pending_secret itself doubles as the manual-entry string. | ||
| 120 | def begin_otp_enrollment! | ||
| 121 | update!(:otp_pending_secret => ROTP::Base32.random) | ||
| 122 | pending_otp_provisioning_uri | ||
| 123 | end | ||
| 124 | |||
| 125 | def pending_otp_provisioning_uri | ||
| 126 | return nil if otp_pending_secret.blank? | ||
| 127 | ROTP::TOTP.new(otp_pending_secret, :issuer => OTP_ISSUER) | ||
| 128 | .provisioning_uri(login) | ||
| 129 | end | ||
| 130 | |||
| 131 | # Confirms enrollment with the first generated code. Promotion and | ||
| 132 | # witnessing are one transaction; the consumed timestep is recorded so | ||
| 133 | # the confirmation code cannot be replayed at login. | ||
| 134 | def confirm_otp_enrollment!(code, actor: self) | ||
| 135 | return false if otp_pending_secret.blank? | ||
| 136 | timestep = ROTP::TOTP.new(otp_pending_secret) | ||
| 137 | .verify(code.to_s.strip, | ||
| 138 | :drift_behind => OTP_DRIFT, | ||
| 139 | :drift_ahead => OTP_DRIFT) | ||
| 140 | return false unless timestep | ||
| 141 | |||
| 142 | transaction do | ||
| 143 | update!(:otp_secret => otp_pending_secret, | ||
| 144 | :otp_pending_secret => nil, | ||
| 145 | :otp_consumed_timestep => timestep) | ||
| 146 | NodeAction.record!(:participants => [self], :user => actor, | ||
| 147 | :action => "otp_enroll", :target_login => login) | ||
| 148 | end | ||
| 149 | true | ||
| 150 | end | ||
| 151 | |||
| 152 | # Login-time verification. Each code is accepted at most once. | ||
| 153 | def verify_otp!(code) | ||
| 154 | return false unless otp_enrolled? | ||
| 155 | timestep = ROTP::TOTP.new(otp_secret) | ||
| 156 | .verify(code.to_s.strip, | ||
| 157 | :drift_behind => OTP_DRIFT, | ||
| 158 | :drift_ahead => OTP_DRIFT, | ||
| 159 | :after => otp_consumed_timestep) | ||
| 160 | return false unless timestep | ||
| 161 | |||
| 162 | update!(:otp_consumed_timestep => timestep) | ||
| 163 | true | ||
| 164 | end | ||
| 165 | |||
| 166 | # Self-service disable and administrative reset share one witnessed | ||
| 167 | # teardown; the verb records which of the two it was. The controller | ||
| 168 | # is responsible for the self-service guards (password + current code). | ||
| 169 | def disable_otp!(actor:) | ||
| 170 | verb = (actor == self) ? "otp_disable" : "otp_reset" | ||
| 171 | transaction do | ||
| 172 | update!(:otp_secret => nil, :otp_pending_secret => nil, | ||
| 173 | :otp_consumed_timestep => nil) | ||
| 174 | NodeAction.record!(:participants => [self], :user => actor, | ||
| 175 | :action => verb, :target_login => login) | ||
| 176 | end | ||
| 177 | true | ||
| 178 | end | ||
| 108 | 179 | ||
| 109 | private | 180 | private |
| 110 | 181 | ||
diff --git a/config/initializers/otp.rb b/config/initializers/otp.rb new file mode 100644 index 00000000..0d727277 --- /dev/null +++ b/config/initializers/otp.rb | |||
| @@ -0,0 +1,2 @@ | |||
| 1 | OTP_ISSUER = "CCC CMS" | ||
| 2 | OTP_DRIFT = 30 # seconds of clock skew accepted behind and ahead | ||
diff --git a/db/migrate/20260724110241_add_otp_to_users.rb b/db/migrate/20260724110241_add_otp_to_users.rb new file mode 100644 index 00000000..b43542bf --- /dev/null +++ b/db/migrate/20260724110241_add_otp_to_users.rb | |||
| @@ -0,0 +1,8 @@ | |||
| 1 | class AddOtpToUsers < ActiveRecord::Migration[8.1] | ||
| 2 | def change | ||
| 3 | add_column :users, :otp_secret, :string | ||
| 4 | add_column :users, :otp_pending_secret, :string | ||
| 5 | add_column :users, :otp_consumed_timestep, :integer | ||
| 6 | add_column :users, :otp_required, :boolean, :null => false, :default => false | ||
| 7 | end | ||
| 8 | end | ||
diff --git a/test/models/user_otp_test.rb b/test/models/user_otp_test.rb new file mode 100644 index 00000000..81f25575 --- /dev/null +++ b/test/models/user_otp_test.rb | |||
| @@ -0,0 +1,83 @@ | |||
| 1 | require "test_helper" | ||
| 2 | |||
| 3 | class UserOtpTest < ActiveSupport::TestCase | ||
| 4 | fixtures :users | ||
| 5 | |||
| 6 | def setup | ||
| 7 | @user = users(:quentin) | ||
| 8 | end | ||
| 9 | |||
| 10 | test "begin_otp_enrollment! stores a pending secret and yields a provisioning URI" do | ||
| 11 | uri = @user.begin_otp_enrollment! | ||
| 12 | assert @user.otp_pending_secret.present? | ||
| 13 | assert_not @user.otp_enrolled? | ||
| 14 | assert_match %r{\Aotpauth://totp/}, uri | ||
| 15 | assert_includes uri, "issuer=" | ||
| 16 | end | ||
| 17 | |||
| 18 | test "confirm_otp_enrollment! promotes the pending secret and witnesses it" do | ||
| 19 | @user.begin_otp_enrollment! | ||
| 20 | code = ROTP::TOTP.new(@user.otp_pending_secret).now | ||
| 21 | |||
| 22 | assert @user.confirm_otp_enrollment!(code) | ||
| 23 | assert @user.otp_enrolled? | ||
| 24 | assert_nil @user.otp_pending_secret | ||
| 25 | |||
| 26 | action = NodeAction.where(:action => "otp_enroll").last | ||
| 27 | assert_equal @user, action.user | ||
| 28 | assert_equal [["User", @user.id]], | ||
| 29 | action.action_participants.map { |p| [p.subject_type, p.subject_id] } | ||
| 30 | end | ||
| 31 | |||
| 32 | test "confirm_otp_enrollment! rejects a wrong code and stays unenrolled" do | ||
| 33 | @user.begin_otp_enrollment! | ||
| 34 | assert_not @user.confirm_otp_enrollment!("000000") | ||
| 35 | assert_not @user.otp_enrolled? | ||
| 36 | assert @user.otp_pending_secret.present? | ||
| 37 | end | ||
| 38 | |||
| 39 | test "verify_otp! accepts a current code exactly once" do | ||
| 40 | @user.update!(:otp_secret => ROTP::Base32.random) | ||
| 41 | code = ROTP::TOTP.new(@user.otp_secret).now | ||
| 42 | |||
| 43 | assert @user.verify_otp!(code) | ||
| 44 | assert_not @user.verify_otp!(code), "replayed code must be rejected" | ||
| 45 | end | ||
| 46 | |||
| 47 | test "the confirmation code cannot be replayed at login" do | ||
| 48 | @user.begin_otp_enrollment! | ||
| 49 | code = ROTP::TOTP.new(@user.otp_pending_secret).now | ||
| 50 | @user.confirm_otp_enrollment!(code) | ||
| 51 | assert_not @user.verify_otp!(code) | ||
| 52 | end | ||
| 53 | |||
| 54 | test "verify_otp! rejects wrong codes and unenrolled users" do | ||
| 55 | assert_not @user.verify_otp!("123456") | ||
| 56 | enroll!(@user) | ||
| 57 | assert_not @user.verify_otp!("000000") | ||
| 58 | end | ||
| 59 | |||
| 60 | test "disable_otp! by the user themselves is witnessed as otp_disable" do | ||
| 61 | enroll!(@user) | ||
| 62 | assert @user.disable_otp!(:actor => @user) | ||
| 63 | assert_not @user.otp_enrolled? | ||
| 64 | assert_equal "otp_disable", NodeAction.last.action | ||
| 65 | end | ||
| 66 | |||
| 67 | test "an admin clearing another user's factor is witnessed as otp_reset" do | ||
| 68 | enroll!(@user) | ||
| 69 | admin = users(:aaron) | ||
| 70 | assert @user.disable_otp!(:actor => admin) | ||
| 71 | |||
| 72 | action = NodeAction.where(:action => "otp_reset").last | ||
| 73 | assert_equal admin, action.user | ||
| 74 | assert_equal @user.login, action.metadata["target_login"] | ||
| 75 | end | ||
| 76 | |||
| 77 | private | ||
| 78 | |||
| 79 | def enroll!(user) | ||
| 80 | user.begin_otp_enrollment! | ||
| 81 | user.confirm_otp_enrollment!(ROTP::TOTP.new(user.otp_pending_secret).now) | ||
| 82 | end | ||
| 83 | end | ||
