From 3399e45130ce87cc265dc2c3da43f7d1424a857d Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Thu, 9 Oct 2025 17:23:31 -0400 Subject: [PATCH] Introduce an "Identity" model to ease login - New untenanted Identity and Membership models - New `identity_token` cookie with path "/" holds state across tenants We're not sure whether the untenanted database will be sqlite or MySQL, and so I've been careful to minimize - database reads, placing them behind etags and caching - database writes, only writing when a new Session is created (login) Note that we track two things in the identity_token cookie: a signed id, and the updated_at for the underlying Identity object. This allows us to effectively cache on the Identity without having to hit the database, by using an Identity::Mock object that is compatible with etag and cache methods. The new integration test shows the desired user-facing behavior, which is to make it easy to login without a tenanted URL and to jump between tenants. - the untenanted "login_help" page shows all linked memberships - the jump menu shows all linked memberships (except the current) Also introduced a utility script to populate existing employee Identities, grouping accounts by email address. --- app/assets/stylesheets/icons.css | 1 + app/controllers/concerns/authentication.rb | 14 ++++ app/helpers/filters_helper.rb | 5 +- app/models/current.rb | 2 +- app/models/identity.rb | 7 ++ app/models/membership.rb | 13 +++ app/models/untenanted_record.rb | 4 + app/models/user.rb | 2 +- app/models/user/identifiable.rb | 28 +++++++ app/views/filters/menu/_button.html.erb | 2 +- app/views/filters/menu/_places.html.erb | 5 ++ app/views/sessions/login_menu.html.erb | 35 ++++---- app/views/sessions/new.html.erb | 4 - config/database.yml | 22 +++-- .../20250924190529_create_identities.rb | 7 ++ .../20250924190729_create_memberships.rb | 15 ++++ db/untenanted_schema.rb | 33 ++++++++ .../20250924-populate-identities.rb | 29 +++++++ .../controller_authentication_test.rb | 2 +- test/controllers/sessions_controller_test.rb | 6 +- test/integration/identity_membership_test.rb | 37 +++++++++ test/models/identity_test.rb | 7 ++ test/models/membership_test.rb | 7 ++ test/models/user/identifiable_test.rb | 81 +++++++++++++++++++ 24 files changed, 336 insertions(+), 32 deletions(-) create mode 100644 app/models/identity.rb create mode 100644 app/models/membership.rb create mode 100644 app/models/untenanted_record.rb create mode 100644 app/models/user/identifiable.rb create mode 100644 db/untenanted_migrate/20250924190529_create_identities.rb create mode 100644 db/untenanted_migrate/20250924190729_create_memberships.rb create mode 100644 db/untenanted_schema.rb create mode 100755 script/migrations/20250924-populate-identities.rb create mode 100644 test/integration/identity_membership_test.rb create mode 100644 test/models/identity_test.rb create mode 100644 test/models/membership_test.rb create mode 100644 test/models/user/identifiable_test.rb diff --git a/app/assets/stylesheets/icons.css b/app/assets/stylesheets/icons.css index 02d418624..c3eac00fd 100644 --- a/app/assets/stylesheets/icons.css +++ b/app/assets/stylesheets/icons.css @@ -57,6 +57,7 @@ .icon--lifebuoy { --svg: url("lifebuoy.svg "); } .icon--lock { --svg: url("lock.svg "); } .icon--logout { --svg: url("logout.svg "); } + .icon--logo-color { --svg: url("logo-color.svg "); mask-size: contain; } .icon--menu { --svg: url("menu.svg "); } .icon--menu-dots-horizontal { --svg: url("menu-dots-horizontal.svg "); } .icon--menu-dots-vertical { --svg: url("menu-dots-vertical.svg "); } diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb index 2ede3c108..43643e6cc 100644 --- a/app/controllers/concerns/authentication.rb +++ b/app/controllers/concerns/authentication.rb @@ -38,6 +38,7 @@ module Authentication def require_tenant unless ApplicationRecord.current_tenant.present? + set_current_identity_token render "sessions/login_menu" end end @@ -77,14 +78,27 @@ module Authentication end def start_new_session_for(user) + link_identity(user) user.sessions.create!(user_agent: request.user_agent, ip_address: request.remote_ip).tap do |session| set_current_session session end end + def link_identity(user) + token_value = cookies.signed[:identity_token] + token_identity = Identity.find_signed(token_value["id"]) if token_value.present? + identity = user.set_identity(token_identity) + cookies.signed.permanent[:identity_token] = { value: { "id" => identity.signed_id, "updated_at" => identity.updated_at }, httponly: true, same_site: :lax } + end + + def set_current_identity_token + Current.identity_token = Identity::Mock.new(**cookies.signed[:identity_token]) + end + def set_current_session(session) logger.struct " Authorized User##{session.user.id}", authentication: { user: { id: session.user.id } } Current.session = session + set_current_identity_token cookies.signed.permanent[:session_token] = { value: session.signed_id, httponly: true, same_site: :lax, path: Account.sole.slug } end diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb index 7c75954ae..c93eb9735 100644 --- a/app/helpers/filters_helper.rb +++ b/app/helpers/filters_helper.rb @@ -15,10 +15,11 @@ module FiltersHelper user_filtering.selected_collection_titles.collect { tag.strong it }.to_sentence.html_safe end - def filter_place_menu_item(path, label, icon) + def filter_place_menu_item(path, label, icon, new_window: false) + link_to_params = new_window ? { target: "_blank" } : {} tag.li class: "popup__item", id: "filter-place-#{label.parameterize}", data: { filter_target: "item", navigable_list_target: "item" } do concat icon_tag(icon, class: "popup__icon") - concat(link_to(path, class: "popup__btn btn") do + concat(link_to(path, link_to_params.merge(class: "popup__btn btn")) do concat tag.span(label, class: "overflow-ellipsis") end) end diff --git a/app/models/current.rb b/app/models/current.rb index f67ef117d..074d22055 100644 --- a/app/models/current.rb +++ b/app/models/current.rb @@ -1,6 +1,6 @@ class Current < ActiveSupport::CurrentAttributes attribute :session - attribute :http_method, :request_id, :user_agent, :ip_address, :referrer + attribute :http_method, :request_id, :user_agent, :ip_address, :referrer, :identity_token delegate :user, to: :session, allow_nil: true end diff --git a/app/models/identity.rb b/app/models/identity.rb new file mode 100644 index 000000000..bbf798dd1 --- /dev/null +++ b/app/models/identity.rb @@ -0,0 +1,7 @@ +class Identity < UntenantedRecord + # This is used to instantiate an Identity-like object from the `identity_token` without hitting + # the untenanted database. It is intended to be used with caching/etagging methods. + Mock = Struct.new(:id, :updated_at) + + has_many :memberships, dependent: :destroy +end diff --git a/app/models/membership.rb b/app/models/membership.rb new file mode 100644 index 000000000..a8ac585ab --- /dev/null +++ b/app/models/membership.rb @@ -0,0 +1,13 @@ +class Membership < UntenantedRecord + belongs_to :identity, touch: true + + # I want this to be `belongs_to :user`, but ActiveRecord::Tenanted doesn't yet support + # associations from untenanted to untenanted models. + # + # See https://github.com/basecamp/activerecord-tenanted/issues/201 + # + # In the meantime, when creating a Membership, specify both `user_id` and `user_tenant` attributes. + def user + User.with_tenant(user_tenant) { User.find_by(id: user_id) } + end +end diff --git a/app/models/untenanted_record.rb b/app/models/untenanted_record.rb new file mode 100644 index 000000000..94da45603 --- /dev/null +++ b/app/models/untenanted_record.rb @@ -0,0 +1,4 @@ +class UntenantedRecord < ActiveRecord::Base + self.abstract_class = true + connects_to database: { writing: :untenanted } +end diff --git a/app/models/user.rb b/app/models/user.rb index beb7250ac..4757d4e37 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,6 +1,6 @@ class User < ApplicationRecord include Accessor, AiQuota, Assignee, Attachable, Configurable, Conversational, Highlights, - Mentionable, Named, Notifiable, Role, Searcher, Staff, Transferable, Watcher + Identifiable, Mentionable, Named, Notifiable, Role, Searcher, Staff, Transferable, Watcher include Timelined # Depends on Accessor has_one_attached :avatar diff --git a/app/models/user/identifiable.rb b/app/models/user/identifiable.rb new file mode 100644 index 000000000..0b3f0abbc --- /dev/null +++ b/app/models/user/identifiable.rb @@ -0,0 +1,28 @@ +module User::Identifiable + extend ActiveSupport::Concern + + included do + has_one :membership, ->(user) { where(user_tenant: user.tenant) } + has_one :identity, through: :membership + end + + def set_identity(token_identity) + if token_identity.present? + if identity.nil? + token_identity.memberships.create!(user_id: id, user_tenant: tenant, email_address: email_address, account_name: Account.sole.name) + elsif identity != token_identity + Identity.transaction do + identity.memberships.update_all(identity_id: token_identity.id) + identity.destroy + end + end + token_identity + elsif identity.present? + identity + else + Identity.create!.tap do |identity| + identity.memberships.create!(user_id: id, user_tenant: tenant, email_address: email_address, account_name: Account.sole.name) + end + end + end +end diff --git a/app/views/filters/menu/_button.html.erb b/app/views/filters/menu/_button.html.erb index 30434b95b..66b748641 100644 --- a/app/views/filters/menu/_button.html.erb +++ b/app/views/filters/menu/_button.html.erb @@ -3,5 +3,5 @@ action: "click->dialog#open:stop keydown.j@document->hotkey#click keydown.meta+j@document->hotkey#click keydown.ctrl+j@document->hotkey#click", controller: "hotkey" } do %> <%= inline_svg "fizzy-logo" %> - Fizzy + <%= Account.sole.name %> <% end %> diff --git a/app/views/filters/menu/_places.html.erb b/app/views/filters/menu/_places.html.erb index c0e8de7e6..c7afd625e 100644 --- a/app/views/filters/menu/_places.html.erb +++ b/app/views/filters/menu/_places.html.erb @@ -3,4 +3,9 @@ <%= filter_place_menu_item user_path(Current.user), "My Profile", "person" %> <%= filter_place_menu_item notifications_path, "Notifications", "bell" %> <%= filter_place_menu_item notifications_settings_path, "Notification Settings", "settings" %> + <% cache [ Current.user, Current.identity_token ] do %> + <% Current.user.identity.memberships.where.not(user_tenant: Current.user.tenant).each do |membership| %> + <%= filter_place_menu_item root_url(script_name: "/#{membership.user_tenant}"), "#{membership.account_name}", "logo-color", new_window: true %> + <% end %> + <% end %> <% end %> diff --git a/app/views/sessions/login_menu.html.erb b/app/views/sessions/login_menu.html.erb index e8c78fc61..cd2f7e99b 100644 --- a/app/views/sessions/login_menu.html.erb +++ b/app/views/sessions/login_menu.html.erb @@ -1,15 +1,22 @@ -<% cache ApplicationRecord.tenants do %> -

We're migrating Fizzy authentication

-

- While we migrate Fizzy away from Launchpad and 37id, please login at one of the following URLs: +<% @page_title = "Choose Account" %> -

-

-

- Mike should have sent you your temporary password by now. If you haven't received it, please ping him! -

-<% end %> +
+

Fizzy

+ + <% cache [ Current.user, Current.identity_token ] do %> + <% identity = Identity.find_signed(Current.identity_token.id) %> + <% memberships = identity&.memberships %> + <% if memberships.present? %> +

Your Fizzy Accounts

+ + <% else %> +

You don't have any existing Fizzy accounts.

+ <% end %> + <% end %> +
diff --git a/app/views/sessions/new.html.erb b/app/views/sessions/new.html.erb index d806a904f..8d714d9de 100644 --- a/app/views/sessions/new.html.erb +++ b/app/views/sessions/new.html.erb @@ -24,7 +24,3 @@ <% end %> - -<% content_for :footer do %> -
Fizzy™ version 0
-<% end %> diff --git a/config/database.yml b/config/database.yml index ce21ef3cd..30dd0f4be 100644 --- a/config/database.yml +++ b/config/database.yml @@ -16,6 +16,10 @@ development: <<: *default database: storage/tenants/<%= Rails.env %>/%{tenant}/db/main.sqlite3 tenanted: true + untenanted: + <<: *default + database: storage/untenanted/development.sqlite3 + migrations_paths: db/untenanted_migrate cable: <<: *default database: storage/development_cable.sqlite3 @@ -33,9 +37,14 @@ development: # re-generated from your development database when you run "rake". # Do not set this db to the same as development or production. test: - <<: *default - database: storage/tenants/<%= Rails.env %>/%{tenant}/db/main.sqlite3 - tenanted: true + primary: + <<: *default + database: storage/tenants/<%= Rails.env %>/%{tenant}/db/main.sqlite3 + tenanted: true + untenanted: + <<: *default + database: storage/untenanted/test.sqlite3 + migrations_paths: db/untenanted_migrate production: &production primary: @@ -44,11 +53,10 @@ production: &production tenanted: true extensions: - <%= SqliteVec.loadable_path %> - primary_original: + untenanted: <<: *default - database: storage/production.sqlite3 - replica: true - readonly: true + database: storage/untenanted/production.sqlite3 + migrations_paths: db/untenanted_migrate cable: <<: *default database: storage/production_cable.sqlite3 diff --git a/db/untenanted_migrate/20250924190529_create_identities.rb b/db/untenanted_migrate/20250924190529_create_identities.rb new file mode 100644 index 000000000..d879926a8 --- /dev/null +++ b/db/untenanted_migrate/20250924190529_create_identities.rb @@ -0,0 +1,7 @@ +class CreateIdentities < ActiveRecord::Migration[8.1] + def change + create_table :identities do |t| + t.timestamps + end + end +end diff --git a/db/untenanted_migrate/20250924190729_create_memberships.rb b/db/untenanted_migrate/20250924190729_create_memberships.rb new file mode 100644 index 000000000..261fbe524 --- /dev/null +++ b/db/untenanted_migrate/20250924190729_create_memberships.rb @@ -0,0 +1,15 @@ +class CreateMemberships < ActiveRecord::Migration[8.1] + def change + create_table :memberships do |t| + t.string :email_address, null: false + t.references :identity, null: false, foreign_key: true + t.references :user, null: false, foreign_key: false, index: false + t.string :user_tenant, null: false + t.string :account_name, null: false + + t.timestamps + end + add_index :memberships, :email_address + add_index :memberships, [ :user_tenant, :user_id ] + end +end diff --git a/db/untenanted_schema.rb b/db/untenanted_schema.rb new file mode 100644 index 000000000..d7f090873 --- /dev/null +++ b/db/untenanted_schema.rb @@ -0,0 +1,33 @@ +# This file is auto-generated from the current state of the database. Instead +# of editing this file, please use the migrations feature of Active Record to +# incrementally modify your database, and then regenerate this schema definition. +# +# This file is the source Rails uses to define your schema when running `bin/rails +# db:schema:load`. When creating a new database, `bin/rails db:schema:load` tends to +# be faster and is potentially less error prone than running all of your +# migrations from scratch. Old migrations may fail to apply correctly if those +# migrations use external dependencies or application code. +# +# It's strongly recommended that you check this file into your version control system. + +ActiveRecord::Schema[8.1].define(version: 2025_09_24_190729) do + create_table "identities", force: :cascade do |t| + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + create_table "memberships", force: :cascade do |t| + t.string "account_name", null: false + t.datetime "created_at", null: false + t.string "email_address", null: false + t.integer "identity_id", null: false + t.datetime "updated_at", null: false + t.integer "user_id", null: false + t.string "user_tenant", null: false + t.index ["email_address"], name: "index_memberships_on_email_address" + t.index ["identity_id"], name: "index_memberships_on_identity_id" + t.index ["user_tenant", "user_id"], name: "index_memberships_on_user_tenant_and_user_id" + end + + add_foreign_key "memberships", "identities" +end diff --git a/script/migrations/20250924-populate-identities.rb b/script/migrations/20250924-populate-identities.rb new file mode 100755 index 000000000..8fab8d3f6 --- /dev/null +++ b/script/migrations/20250924-populate-identities.rb @@ -0,0 +1,29 @@ +#!/usr/bin/env ruby + +require_relative "../../config/environment" + +ApplicationRecord.with_each_tenant do |tenant| + puts "# #{tenant}" + User.find_each do |user| + next if user.system? || !user.active? + + if user.membership.present? + puts "Found identity #{user.identity.id} for user #{user.id} (#{user.email_address})" + else + memberships = Membership.where(email_address: user.email_address) + if memberships.empty? + # Create a new Identity + Identity.transaction do + identity = Identity.create! + user.membership = identity.memberships.create!(user_id: user.id, user_tenant: user.tenant, email_address: user.email_address, account_name: Account.sole.name) + puts "Created identity #{identity.id} for user #{user.id} (#{user.email_address})" + end + else + # Merge this User's Membership into the existing Identity + identity = memberships.first.identity + user.membership = identity.memberships.create!(user_id: user.id, user_tenant: user.tenant, email_address: user.email_address, account_name: Account.sole.name) + puts "Merged membership for user #{user.id} (#{user.email_address}) into identity #{identity.id}" + end + end + end +end diff --git a/test/controllers/controller_authentication_test.rb b/test/controllers/controller_authentication_test.rb index 72e0df995..212f7819f 100644 --- a/test/controllers/controller_authentication_test.rb +++ b/test/controllers/controller_authentication_test.rb @@ -7,7 +7,7 @@ class ControllerAuthenticationTest < ActionDispatch::IntegrationTest get cards_path assert_response :success - assert_dom "h2", text: "We're migrating Fizzy authentication" + assert_dom "p", text: "You don't have any existing Fizzy accounts." end test "access with an account slug but no session redirects to new session" do diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 0c85ad83c..09f1f13f0 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -17,7 +17,11 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest end test "create with valid credentials" do - post session_path, params: { email_address: "david@37signals.com", password: "secret123456" } + assert_difference -> { Identity.count }, 1 do + assert_difference -> { Membership.count }, 1 do + post session_path, params: { email_address: "david@37signals.com", password: "secret123456" } + end + end assert_redirected_to root_path assert cookies[:session_token].present? diff --git a/test/integration/identity_membership_test.rb b/test/integration/identity_membership_test.rb new file mode 100644 index 000000000..21b8bd7dd --- /dev/null +++ b/test/integration/identity_membership_test.rb @@ -0,0 +1,37 @@ +require "test_helper" + +class IdentityMembershipTest < ActionDispatch::IntegrationTest + setup do + @tenants = [ ActiveRecord::FixtureSet.identify("identity-tenant-1"), + ActiveRecord::FixtureSet.identify("identity-tenant-2") ] + @tenant_paths = @tenants.map { "/#{_1}" } + @tenant_urls = @tenant_paths.map { root_url(script_name: _1) } + @user_params = { email_address: "user@example.com", password: "password1234" } + @users = @tenants.map do |tenant| + ApplicationRecord.create_tenant(tenant) do + Account.create! name: "Account for #{tenant}" + User.create! @user_params.merge(name: "Harold Hancox") + end + end + end + + test "multiple signins on the same browser" do + post session_path(script_name: @tenant_paths[0], params: @user_params) + assert_redirected_to root_path(script_name: @tenant_paths[0]) + + post session_path(script_name: @tenant_paths[1], params: @user_params) + assert_redirected_to root_path(script_name: @tenant_paths[1]) + + # Render links for other Fizzies in the jump menu + get my_menu_path(script_name: @tenant_paths[0]) + assert_select "#my_menu ul li a[href='#{@tenant_urls[1]}']", "Account for #{@tenants[1]}" + + get my_menu_path(script_name: @tenant_paths[1]) + assert_select "#my_menu ul li a[href='#{@tenant_urls[0]}']", "Account for #{@tenants[0]}" + + # Render links for all the identity's Fizzies + get root_path(script_name: nil) + assert_select "ul li a[href='#{@tenant_urls[0]}']", "Account for #{@tenants[0]}" + assert_select "ul li a[href='#{@tenant_urls[1]}']", "Account for #{@tenants[1]}" + end +end diff --git a/test/models/identity_test.rb b/test/models/identity_test.rb new file mode 100644 index 000000000..da7146fbd --- /dev/null +++ b/test/models/identity_test.rb @@ -0,0 +1,7 @@ +require "test_helper" + +class IdentityTest < ActiveSupport::TestCase + # test "the truth" do + # assert true + # end +end diff --git a/test/models/membership_test.rb b/test/models/membership_test.rb new file mode 100644 index 000000000..8506331ed --- /dev/null +++ b/test/models/membership_test.rb @@ -0,0 +1,7 @@ +require "test_helper" + +class MembershipTest < ActiveSupport::TestCase + # test "the truth" do + # assert true + # end +end diff --git a/test/models/user/identifiable_test.rb b/test/models/user/identifiable_test.rb new file mode 100644 index 000000000..fb0529a06 --- /dev/null +++ b/test/models/user/identifiable_test.rb @@ -0,0 +1,81 @@ +require "test_helper" + +class User::IdentifiableTest < ActiveSupport::TestCase + test "set_identity when token is an identity and user has a matching identity" do + user = users(:david) + token_identity = Identity.create! + token_identity.memberships.create!(user_id: user.id, user_tenant: user.tenant, email_address: user.email_address, account_name: "asdf") + assert_equal(token_identity, user.identity) + + result = user.set_identity(token_identity) + user.reload + + assert_equal(token_identity, result) + assert_equal(token_identity, user.identity) + end + + test "set_identity when token is an identity and user has no identity" do + user = users(:david) + token_identity = Identity.create! + assert_nil(user.membership) + assert_nil(user.identity) + + result = user.set_identity(token_identity) + user.reload + + assert_equal(token_identity, result) + assert_equal(token_identity, user.identity) + assert_equal(user.email_address, user.membership.email_address) + assert_equal(Account.sole.name, user.membership.account_name) + end + + test "set_identity when token is an identity and user has a different identity" do + user = users(:david) + user2 = users(:jz) + token_identity = Identity.create! + user_identity = Identity.create! + user_identity.memberships.create!(user_id: user.id, user_tenant: user.tenant, email_address: user.email_address, account_name: "asdf") + user_identity.memberships.create!(user_id: user2.id, user_tenant: user2.tenant, email_address: user2.email_address, account_name: "qwer") + assert_equal(user_identity, user.identity) + assert_equal(user_identity, user2.identity) + + result = user.set_identity(token_identity) + user.reload + user2.reload + + assert_equal(token_identity, result) + assert_equal(token_identity, user.identity) + assert_equal(token_identity, user2.identity) + + assert_raises(ActiveRecord::RecordNotFound) do + user_identity.reload + end + end + + test "set_identity when token is nil and user has an identity" do + user = users(:david) + user_identity = Identity.create! + user_identity.memberships.create!(user_id: user.id, user_tenant: user.tenant, email_address: user.email_address, account_name: "asdf") + assert_equal(user_identity, user.identity) + + result = user.set_identity(nil) + user.reload + + assert_equal(user_identity, result) + assert_equal(user_identity, user.identity) + end + + test "set_identity when token is nil and user has no identity" do + user = users(:david) + assert_nil(user.identity) + + result = user.set_identity(nil) + user.reload + + assert_not_nil(result) + assert_equal(result, user.identity) + assert_equal(1, result.memberships.count) + assert_equal(user.email_address, user.membership.email_address) + assert_equal(Account.sole.name, user.membership.account_name) + end +end