diff --git a/app/controllers/join_codes_controller.rb b/app/controllers/join_codes_controller.rb index 971b3c6f8..eb328df18 100644 --- a/app/controllers/join_codes_controller.rb +++ b/app/controllers/join_codes_controller.rb @@ -18,7 +18,7 @@ class JoinCodesController < ApplicationController if identity == Current.identity && user.setup? redirect_to landing_url(script_name: @join_code.account.slug) elsif identity == Current.identity - redirect_to new_users_join_url(script_name: @join_code.account.slug) + redirect_to new_users_verification_url(script_name: @join_code.account.slug) else logout_and_send_new_magic_link(identity) redirect_to session_magic_link_url(script_name: nil) @@ -44,6 +44,6 @@ class JoinCodesController < ApplicationController magic_link = identity.send_magic_link serve_development_magic_link(magic_link) - session[:return_to_after_authenticating] = new_users_join_url(script_name: @join_code.account.slug) + session[:return_to_after_authenticating] = new_users_verification_url(script_name: @join_code.account.slug) end end diff --git a/app/controllers/users/verifications_controller.rb b/app/controllers/users/verifications_controller.rb new file mode 100644 index 000000000..d99492576 --- /dev/null +++ b/app/controllers/users/verifications_controller.rb @@ -0,0 +1,11 @@ +class Users::VerificationsController < ApplicationController + layout "public" + + def new + end + + def create + Current.user.verify + redirect_to new_users_join_path + end +end diff --git a/app/javascript/controllers/magic_link_controller.js b/app/javascript/controllers/magic_link_controller.js index 4d6c45ab5..3e2e4618c 100644 --- a/app/javascript/controllers/magic_link_controller.js +++ b/app/javascript/controllers/magic_link_controller.js @@ -4,12 +4,18 @@ import { onNextEventLoopTick } from "helpers/timing_helpers" export default class extends Controller { static targets = [ "input" ] + submitOnEnter(event) { + event.preventDefault() + this.submit() + } + + submitOnPaste() { + onNextEventLoopTick(() => this.submit()) + } + submit() { - onNextEventLoopTick(() => { - if (!this.inputTarget.disabled) { - this.element.submit() - this.inputTarget.disabled = true - } - }) + if (this.inputTarget.disabled) return + this.element.submit() + this.inputTarget.disabled = true } } diff --git a/app/models/account.rb b/app/models/account.rb index 50f3bf3ee..61a636adb 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -21,7 +21,7 @@ class Account < ApplicationRecord def create_with_owner(account:, owner:) create!(**account).tap do |account| account.users.create!(role: :system, name: "System") - account.users.create!(**owner.reverse_merge(role: "owner")) + account.users.create!(**owner.reverse_merge(role: "owner", verified_at: Time.current)) end end end diff --git a/app/models/user.rb b/app/models/user.rb index 8c0ac42a5..20ee1c40c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -28,6 +28,14 @@ class User < ApplicationRecord name != identity.email_address end + def verified? + verified_at.present? + end + + def verify + update!(verified_at: Time.current) unless verified? + end + private def close_remote_connections ActionCable.server.remote_connections.where(current_user: self).disconnect(reconnect: false) diff --git a/app/models/user/settings.rb b/app/models/user/settings.rb index 1651e9a64..598ede9c5 100644 --- a/app/models/user/settings.rb +++ b/app/models/user/settings.rb @@ -21,7 +21,7 @@ class User::Settings < ApplicationRecord end def bundling_emails? - !bundle_email_never? && !user.system? && user.active? + !bundle_email_never? && !user.system? && user.active? && user.verified? end def timezone diff --git a/app/views/sessions/magic_links/show.html.erb b/app/views/sessions/magic_links/show.html.erb index 931156ff6..434bb93c3 100644 --- a/app/views/sessions/magic_links/show.html.erb +++ b/app/views/sessions/magic_links/show.html.erb @@ -10,7 +10,7 @@ <%= form.text_field :code, required: true, class: "input center txt-align-enter txt-large", autofocus: true, autocorrect: "off", autocapitalize: "off", spellcheck: "false", "data-1p-ignore": true, autocomplete: "one-time-code", maxlength: "6", placeholder: "••••••", value: params[:code], - data: { magic_link_target: "input", action: "keydown.enter->magic-link#submit paste->magic-link#submit" } %> + data: { magic_link_target: "input", action: "keydown.enter->magic-link#submitOnEnter paste->magic-link#submitOnPaste" } %> <% end %>

The code you receive will work for <%= distance_of_time_in_words(MagicLink::EXPIRATION_TIME) %>.

diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index eec6444a0..17f418e84 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -18,20 +18,25 @@

<%= @user.name %>

- <% if @user.active? %> - <%= mail_to @user.identity.email_address %> - <% else %> + <% if !@user.active? %> <%= @user.name %> is no longer on this account + <% elsif !@user.verified? %> + Unverified +
A sign-in code has been sent to this email address, but the user has not yet logged in to confirm their identity.
+ <% else %> + <%= mail_to @user.identity.email_address %> <% end %>
-
- <%= link_to "Which cards are assigned to #{me_or_you}?", - cards_path(assignee_ids: [ @user.id ], sorted_by: "newest"), class: "btn", data: { turbo_frame: "_top" } %> - <%= link_to "Which cards were added by #{me_or_you}?", - cards_path(creator_ids: [ @user.id ], sorted_by: "newest"), class: "btn", data: { turbo_frame: "_top" } %> -
+ <% if @user.verified? %> +
+ <%= link_to "Which cards are assigned to #{me_or_you}?", + cards_path(assignee_ids: [ @user.id ], sorted_by: "newest"), class: "btn", data: { turbo_frame: "_top" } %> + <%= link_to "Which cards were added by #{me_or_you}?", + cards_path(creator_ids: [ @user.id ], sorted_by: "newest"), class: "btn", data: { turbo_frame: "_top" } %> +
+ <% end %> @@ -48,4 +53,6 @@ <% end %> -<%= turbo_frame_tag "user_events", src: user_events_path(@user) %> +<% if @user.verified? %> + <%= turbo_frame_tag "user_events", src: user_events_path(@user) %> +<% end %> diff --git a/app/views/users/verifications/new.html.erb b/app/views/users/verifications/new.html.erb new file mode 100644 index 000000000..fe830cd28 --- /dev/null +++ b/app/views/users/verifications/new.html.erb @@ -0,0 +1 @@ +<%= auto_submit_form_with url: users_verifications_path, method: :post %> diff --git a/config/routes.rb b/config/routes.rb index 9fa8f81bf..e67e4ef7c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -138,6 +138,7 @@ Rails.application.routes.draw do namespace :users do resources :joins + resources :verifications, only: %i[ new create ] end resource :session do diff --git a/db/migrate/20251205010536_add_verified_at_to_users.rb b/db/migrate/20251205010536_add_verified_at_to_users.rb new file mode 100644 index 000000000..b853cb952 --- /dev/null +++ b/db/migrate/20251205010536_add_verified_at_to_users.rb @@ -0,0 +1,5 @@ +class AddVerifiedAtToUsers < ActiveRecord::Migration[8.2] + def change + add_column :users, :verified_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index b84d3c160..42458db17 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.2].define(version: 2025_12_01_100607) do +ActiveRecord::Schema[8.2].define(version: 2025_12_05_010536) do create_table "accesses", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "accessed_at" t.uuid "account_id", null: false @@ -726,6 +726,7 @@ ActiveRecord::Schema[8.2].define(version: 2025_12_01_100607) do t.string "name", null: false t.string "role", default: "member", null: false t.datetime "updated_at", null: false + t.datetime "verified_at" t.index ["account_id", "identity_id"], name: "index_users_on_account_id_and_identity_id", unique: true t.index ["account_id", "role"], name: "index_users_on_account_id_and_role" t.index ["identity_id"], name: "index_users_on_identity_id" diff --git a/db/schema_sqlite.rb b/db/schema_sqlite.rb index 6f6d6cf53..1dd2b3e00 100644 --- a/db/schema_sqlite.rb +++ b/db/schema_sqlite.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.2].define(version: 2025_12_01_100607) do +ActiveRecord::Schema[8.2].define(version: 2025_12_05_010536) do create_table "accesses", id: :uuid, force: :cascade do |t| t.datetime "accessed_at" t.uuid "account_id", null: false @@ -499,6 +499,7 @@ ActiveRecord::Schema[8.2].define(version: 2025_12_01_100607) do t.string "name", limit: 255, null: false t.string "role", limit: 255, default: "member", null: false t.datetime "updated_at", null: false + t.datetime "verified_at" t.index ["account_id", "identity_id"], name: "index_users_on_account_id_and_identity_id", unique: true t.index ["account_id", "role"], name: "index_users_on_account_id_and_role" t.index ["identity_id"], name: "index_users_on_identity_id" diff --git a/db/seeds.rb b/db/seeds.rb index bb8db7db8..7cba1b5f9 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -36,7 +36,7 @@ else if user = identity.users.find_by(account: Current.account) user else - User.create!(name: full_name, identity: identity, account: Current.account) + User.create!(name: full_name, identity: identity, account: Current.account, verified_at: Time.current) end end diff --git a/script/migrations/20251205-backfill-verified-at.rb b/script/migrations/20251205-backfill-verified-at.rb new file mode 100755 index 000000000..7cd4f0030 --- /dev/null +++ b/script/migrations/20251205-backfill-verified-at.rb @@ -0,0 +1,112 @@ +#!/usr/bin/env ruby + +require_relative "../../config/environment" + +BACKFILL_TIMESTAMP = Time.parse("2025-12-02 12:00:00 UTC") + +def collect_verified_user_ids + verified_ids = Set.new + + # Owners (they created the account) + verified_ids.merge(User.where(role: :owner).pluck(:id)) + puts "After owners: #{verified_ids.size} users" + + # Card creators + verified_ids.merge(Card.distinct.pluck(:creator_id).compact) + puts "After card creators: #{verified_ids.size} users" + + # Comment creators + verified_ids.merge(Comment.distinct.pluck(:creator_id).compact) + puts "After comment creators: #{verified_ids.size} users" + + # Board creators + verified_ids.merge(Board.distinct.pluck(:creator_id).compact) + puts "After board creators: #{verified_ids.size} users" + + # Event creators + verified_ids.merge(Event.distinct.pluck(:creator_id).compact) + puts "After event creators: #{verified_ids.size} users" + + # Assigners (not assignees - they could be assigned without logging in) + verified_ids.merge(Assignment.distinct.pluck(:assigner_id).compact) + puts "After assigners: #{verified_ids.size} users" + + # Manual closers (user_id is nil for automatic closures) + verified_ids.merge(Closure.where.not(user_id: nil).distinct.pluck(:user_id).compact) + puts "After closers: #{verified_ids.size} users" + + # Manual postponers (user_id is nil for automatic entropy postponements) + verified_ids.merge(Card::NotNow.where.not(user_id: nil).distinct.pluck(:user_id).compact) + puts "After postponers: #{verified_ids.size} users" + + # Reactors + verified_ids.merge(Reaction.distinct.pluck(:reacter_id).compact) + puts "After reactors: #{verified_ids.size} users" + + # Filter creators + verified_ids.merge(Filter.distinct.pluck(:creator_id).compact) + puts "After filter creators: #{verified_ids.size} users" + + # Pinners + verified_ids.merge(Pin.distinct.pluck(:user_id).compact) + puts "After pinners: #{verified_ids.size} users" + + # Board accessors (accessed_at is touched when viewing boards) + verified_ids.merge(Access.where.not(accessed_at: nil).distinct.pluck(:user_id).compact) + puts "After board accessors: #{verified_ids.size} users" + + # Export requesters + verified_ids.merge(Account::Export.distinct.pluck(:user_id).compact) + puts "After export requesters: #{verified_ids.size} users" + + # Push subscribers + verified_ids.merge(Push::Subscription.distinct.pluck(:user_id).compact) + puts "After push subscribers: #{verified_ids.size} users" + + # Users who completed setup (name != email) + verified_ids.merge( + User.joins(:identity) + .where.not("users.name = identities.email_address") + .pluck(:id) + ) + puts "After setup completers: #{verified_ids.size} users" + + # Users whose identity has at least one session + verified_ids.merge( + User.where(identity_id: Session.distinct.select(:identity_id)).pluck(:id) + ) + puts "After identity sessions: #{verified_ids.size} users" + + verified_ids +end + +puts "Collecting verified user IDs..." +verified_user_ids = collect_verified_user_ids + +puts "\nFiltering to unverified users only..." +users_to_update = User.where(id: verified_user_ids.to_a) + .where(verified_at: nil) + .where(active: true) + .where.not(identity_id: nil) + .where.not(role: :system) + +update_count = users_to_update.count +puts "Found #{update_count} users to backfill" + +# Report remaining unverified users (before update) +remaining_before = User.where(verified_at: nil, active: true) + .where.not(identity_id: nil) + .where.not(role: :system) + .count +remaining_after = remaining_before - update_count +puts "\nCurrently unverified active users: #{remaining_before}" +puts "After backfill, remaining unverified: #{remaining_after}" +puts "These users will need to verify on next login." + +if update_count > 0 + puts "\nBackfilling verified_at..." + updated = users_to_update.update_all(verified_at: BACKFILL_TIMESTAMP) + puts "Updated #{updated} users" +end + +puts "\nDone!" diff --git a/test/controllers/join_codes_controller_test.rb b/test/controllers/join_codes_controller_test.rb index fe8c820d9..0a4bf5793 100644 --- a/test/controllers/join_codes_controller_test.rb +++ b/test/controllers/join_codes_controller_test.rb @@ -36,7 +36,7 @@ class JoinCodesControllerTest < ActionDispatch::IntegrationTest end assert_redirected_to session_magic_link_url(script_name: nil) - assert_equal new_users_join_url(script_name: @account.slug), session[:return_to_after_authenticating] + assert_equal new_users_verification_url(script_name: @account.slug), session[:return_to_after_authenticating] end test "create for existing identity" do @@ -55,7 +55,7 @@ class JoinCodesControllerTest < ActionDispatch::IntegrationTest assert_redirected_to landing_url(script_name: @account.slug) end - test "create for signed-in identity without a user in the account redirects to user setup" do + test "create for signed-in identity without a user in the account redirects to verification" do identity = identities(:mike) sign_in_as :mike @@ -67,6 +67,6 @@ class JoinCodesControllerTest < ActionDispatch::IntegrationTest end end - assert_redirected_to new_users_join_url(script_name: @account.slug) + assert_redirected_to new_users_verification_url(script_name: @account.slug) end end diff --git a/test/controllers/users/verifications_controller_test.rb b/test/controllers/users/verifications_controller_test.rb new file mode 100644 index 000000000..25ecdba1c --- /dev/null +++ b/test/controllers/users/verifications_controller_test.rb @@ -0,0 +1,24 @@ +require "test_helper" + +class Users::VerificationsControllerTest < ActionDispatch::IntegrationTest + test "new renders the auto-submit form" do + sign_in_as :david + + get new_users_verification_path + + assert_response :ok + end + + test "create verifies the user and redirects to join" do + sign_in_as :david + + user = users(:david) + user.update_column(:verified_at, nil) + assert_not user.verified? + + post users_verifications_path + + assert_redirected_to new_users_join_path + assert user.reload.verified? + end +end diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 4c7dad414..cf4d9e6d0 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -4,6 +4,7 @@ david: role: member identity: david account: 37s_uuid + verified_at: <%= Time.current.to_fs(:db) %> jz: id: <%= ActiveRecord::FixtureSet.identify("jz", :uuid) %> @@ -11,6 +12,7 @@ jz: role: member identity: jz account: 37s_uuid + verified_at: <%= Time.current.to_fs(:db) %> jason: id: <%= ActiveRecord::FixtureSet.identify("jason", :uuid) %> @@ -18,6 +20,7 @@ jason: role: owner identity: jason account: 37s_uuid + verified_at: <%= Time.current.to_fs(:db) %> kevin: id: <%= ActiveRecord::FixtureSet.identify("kevin", :uuid) %> @@ -25,6 +28,7 @@ kevin: role: admin identity: kevin account: 37s_uuid + verified_at: <%= Time.current.to_fs(:db) %> system: id: <%= ActiveRecord::FixtureSet.identify("system", :uuid) %> @@ -38,6 +42,7 @@ mike: role: admin identity: mike account: initech_uuid + verified_at: <%= Time.current.to_fs(:db) %> system_initech: id: <%= ActiveRecord::FixtureSet.identify("system_initech", :uuid) %> diff --git a/test/models/account_test.rb b/test/models/account_test.rb index 490d6e32e..75da41fb9 100644 --- a/test/models/account_test.rb +++ b/test/models/account_test.rb @@ -44,6 +44,8 @@ class AccountTest < ActiveSupport::TestCase assert owner.admin?, "owner should also be considered an admin" assert_predicate account.system_user, :present? + + assert owner.verified?, "owner should be verified on account creation" end end diff --git a/test/models/user/settings_test.rb b/test/models/user/settings_test.rb index b72de92c5..69145c419 100644 --- a/test/models/user/settings_test.rb +++ b/test/models/user/settings_test.rb @@ -49,5 +49,9 @@ class User::SettingsTest < ActiveSupport::TestCase @user.update!(role: :member, active: false) assert_not @user.settings.bundling_emails?, "Inactive users should not receive bundled emails" + + @user.update!(active: true) + @user.update_column(:verified_at, nil) + assert_not @user.settings.bundling_emails?, "Unverified users should not receive bundled emails" end end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 36b0e7986..645e3a386 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -48,4 +48,36 @@ class UserTest < ActiveSupport::TestCase user.update!(name: "Kevin") assert user.setup? end + + test "verified? returns true when verified_at is present" do + user = users(:david) + user.update_column(:verified_at, Time.current) + + assert user.verified? + end + + test "verified? returns false when verified_at is nil" do + user = users(:david) + user.update_column(:verified_at, nil) + + assert_not user.verified? + end + + test "verify sets verified_at when not already verified" do + user = users(:david) + user.update_column(:verified_at, nil) + + assert_nil user.verified_at + user.verify + assert_not_nil user.reload.verified_at + end + + test "verify does not update verified_at when already verified" do + user = users(:david) + original_time = 1.day.ago + user.update_column(:verified_at, original_time) + + user.verify + assert_equal original_time.to_i, user.reload.verified_at.to_i + end end diff --git a/test/system/smoke_test.rb b/test/system/smoke_test.rb index e4bc9cae9..485fba810 100644 --- a/test/system/smoke_test.rb +++ b/test/system/smoke_test.rb @@ -1,6 +1,27 @@ require "application_system_test_case" class SmokeTest < ApplicationSystemTestCase + test "joining an account" do + account = accounts("37s") + + visit join_url(code: account.join_code.code, script_name: account.slug) + fill_in "Email address", with: "newbie@example.com" + click_on "Continue" + + assert_selector "h1", text: "Check your email" + identity = Identity.find_by!(email_address: "newbie@example.com") + code = identity.magic_links.active.first.code + fill_in "code", with: code + send_keys :enter + + assert_selector "input[id=user_name]" + assert account.users.find_by!(identity:).verified?, "User was not properly verified" + fill_in "Full name", with: "New Bee" + click_on "Continue" + + assert_selector "h1", text: "Writebook" + end + test "create a card" do sign_in_as(users(:david))