diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb index 6acbd92b6..daedbb582 100644 --- a/app/controllers/concerns/authentication.rb +++ b/app/controllers/concerns/authentication.rb @@ -23,6 +23,7 @@ module Authentication def allow_unauthenticated_access(**options) skip_before_action :require_authentication, **options before_action :resume_session, **options + allow_unauthorized_access **options end def require_untenanted_access(**options) diff --git a/app/controllers/concerns/authorization.rb b/app/controllers/concerns/authorization.rb index 0e5e9240d..6d31f844e 100644 --- a/app/controllers/concerns/authorization.rb +++ b/app/controllers/concerns/authorization.rb @@ -2,7 +2,7 @@ module Authorization extend ActiveSupport::Concern included do - before_action :ensure_can_access_account, if: -> { ApplicationRecord.current_tenant && Current.session } + before_action :ensure_can_access_account, if: -> { ApplicationRecord.current_tenant && authenticated? } end class_methods do diff --git a/app/controllers/join_codes_controller.rb b/app/controllers/join_codes_controller.rb index 972420439..4bd5293ea 100644 --- a/app/controllers/join_codes_controller.rb +++ b/app/controllers/join_codes_controller.rb @@ -2,6 +2,7 @@ class JoinCodesController < ApplicationController require_untenanted_access allow_unauthenticated_access before_action :set_join_code + before_action :ensure_join_code_is_valid def new @account_name = ApplicationRecord.with_tenant(tenant) { Account.sole.name } @@ -14,10 +15,15 @@ class JoinCodesController < ApplicationController identity.send_magic_link end + session[:return_to_after_authenticating] = root_url(script_name: "/#{tenant}") redirect_to session_magic_link_path end private + def ensure_join_code_is_valid + head :not_found unless @join_code&.active? + end + def set_join_code @join_code ||= ApplicationRecord.with_tenant(tenant) { Account::JoinCode.active.find_by(code: code) } end diff --git a/app/controllers/notifications/unsubscribes_controller.rb b/app/controllers/notifications/unsubscribes_controller.rb index ca15e78d4..f7486d91e 100644 --- a/app/controllers/notifications/unsubscribes_controller.rb +++ b/app/controllers/notifications/unsubscribes_controller.rb @@ -1,6 +1,5 @@ class Notifications::UnsubscribesController < ApplicationController allow_unauthenticated_access - allow_unauthorized_access skip_before_action :verify_authenticity_token before_action :set_user diff --git a/app/controllers/public/base_controller.rb b/app/controllers/public/base_controller.rb index 71209224f..7fa2e69fb 100644 --- a/app/controllers/public/base_controller.rb +++ b/app/controllers/public/base_controller.rb @@ -1,6 +1,5 @@ class Public::BaseController < ApplicationController allow_unauthenticated_access - allow_unauthorized_access before_action :set_collection, :set_card, :set_public_cache_expiration diff --git a/app/models/identity.rb b/app/models/identity.rb index 70bc089eb..f5daf3be1 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -6,7 +6,6 @@ class Identity < UntenantedRecord has_many :sessions, dependent: :destroy normalizes :email_address, with: ->(value) { value.strip.downcase } - validates :email_address, presence: true def send_magic_link magic_links.create!.tap do |magic_link| diff --git a/app/models/user.rb b/app/models/user.rb index 2046b64d5..fabb0224a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -3,8 +3,6 @@ class User < ApplicationRecord Mentionable, Named, Notifiable, Role, Searcher, Watcher include Timelined # Depends on Accessor - self.ignored_columns = %i[ password_digest ] - has_one_attached :avatar belongs_to :membership, optional: true @@ -18,8 +16,6 @@ class User < ApplicationRecord has_many :pins, dependent: :destroy has_many :pinned_cards, through: :pins, source: :card - normalizes :email_address, with: ->(value) { value.strip.downcase } - delegate :staff?, to: :identity, allow_nil: true def deactivate diff --git a/app/views/account/settings/_user.html.erb b/app/views/account/settings/_user.html.erb index 78395ff1c..45fc77c07 100644 --- a/app/views/account/settings/_user.html.erb +++ b/app/views/account/settings/_user.html.erb @@ -3,7 +3,7 @@ <%= avatar_preview_tag user, hidden_for_screen_reader: true %>
<%= user.name %> -
<%= user.email_address %>
+
<%= user.identity.email_address %>
<% end %> diff --git a/app/views/collections/_access_toggle.erb b/app/views/collections/_access_toggle.erb index 824cace65..829efa05e 100644 --- a/app/views/collections/_access_toggle.erb +++ b/app/views/collections/_access_toggle.erb @@ -3,7 +3,7 @@ <%= avatar_preview_tag user, hidden_for_screen_reader: true %>
<%= user.name %> -
<%= user.email_address %>
+
<%= user.identity.email_address %>
<% end %> diff --git a/app/views/users/_user.json.jbuilder b/app/views/users/_user.json.jbuilder index 6b11459f3..6a49bd1de 100644 --- a/app/views/users/_user.json.jbuilder +++ b/app/views/users/_user.json.jbuilder @@ -1,6 +1,7 @@ json.cache! user do - json.(user, :id, :name, :email_address, :role, :active) + json.(user, :id, :name, :role, :active) + json.email_address user.identity.email_address json.created_at user.created_at.utc json.url user_url(user) diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 1ae06e2c0..d5cbd8865 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -19,7 +19,7 @@

<%= @user.name %>

<% if @user.active? %> - <%= mail_to @user.email_address %> + <%= mail_to @user.identity.email_address %> <% else %> <%= @user.name %> is no longer on this account <% end %> diff --git a/config/routes.rb b/config/routes.rb index 790815c75..89699f4dd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,6 +1,5 @@ Rails.application.routes.draw do namespace :account do - post :enter, to: "entries#create" resource :join_code resource :settings resource :entropy diff --git a/db/migrate/20251103125353_remove_email_address_and_password_digest_from_users.rb b/db/migrate/20251103125353_remove_email_address_and_password_digest_from_users.rb new file mode 100644 index 000000000..c336b5b9c --- /dev/null +++ b/db/migrate/20251103125353_remove_email_address_and_password_digest_from_users.rb @@ -0,0 +1,6 @@ +class RemoveEmailAddressAndPasswordDigestFromUsers < ActiveRecord::Migration[8.2] + def change + remove_column :users, :email_address, :string + remove_column :users, :password_digest, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 049e45a1a..4c6901d26 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_11_02_115338) do +ActiveRecord::Schema[8.2].define(version: 2025_11_03_125353) do create_table "accesses", force: :cascade do |t| t.datetime "accessed_at" t.integer "collection_id", null: false @@ -389,13 +389,10 @@ ActiveRecord::Schema[8.2].define(version: 2025_11_02_115338) do create_table "users", force: :cascade do |t| t.boolean "active", default: true, null: false t.datetime "created_at", null: false - t.string "email_address" t.integer "membership_id" t.string "name", null: false - t.string "password_digest" t.string "role", default: "member", null: false t.datetime "updated_at", null: false - t.index ["email_address"], name: "index_users_on_email_address", unique: true t.index ["membership_id"], name: "index_users_on_membership_id" t.index ["role"], name: "index_users_on_role" end diff --git a/db/schema_cache.yml b/db/schema_cache.yml index e4c5ab87c..78c8e718f 100644 --- a/db/schema_cache.yml +++ b/db/schema_cache.yml @@ -1045,16 +1045,6 @@ columns: collation: comment: - *7 - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: email_address - cast_type: *5 - sql_type_metadata: *6 - 'null': true - default: - default_function: - collation: - comment: - *8 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: @@ -1067,16 +1057,6 @@ columns: collation: comment: - *10 - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: password_digest - cast_type: *5 - sql_type_metadata: *6 - 'null': true - default: - default_function: - collation: - comment: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: role @@ -2715,22 +2695,6 @@ indexes: comment: valid: true users: - - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition - table: users - name: index_users_on_email_address - unique: true - columns: - - email_address - lengths: {} - orders: {} - opclasses: {} - where: - type: - using: - include: - nulls_not_distinct: - comment: - valid: true - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: users name: index_users_on_membership_id @@ -2879,4 +2843,4 @@ indexes: nulls_not_distinct: comment: valid: true -version: 20251102115338 +version: 20251103125353 diff --git a/db/seeds.rb b/db/seeds.rb index 2d1002463..bf40aaf0a 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -33,7 +33,7 @@ def create_tenant(signal_account_name) end def find_or_create_user(full_name, email_address) - if user = User.find_by(email_address: email_address) + if user = Identity.find_by(email_address: email_address)&.memberships&.find_by(tenant: ApplicationRecord.current_tenant)&.user user else identity = Identity.find_or_create_by!(email_address: email_address) diff --git a/db/untenanted_migrate/20251103125952_make_email_addresses_mandatory_on_identities.rb b/db/untenanted_migrate/20251103125952_make_email_addresses_mandatory_on_identities.rb new file mode 100644 index 000000000..615f0320f --- /dev/null +++ b/db/untenanted_migrate/20251103125952_make_email_addresses_mandatory_on_identities.rb @@ -0,0 +1,5 @@ +class MakeEmailAddressesMandatoryOnIdentities < ActiveRecord::Migration[8.2] + def change + change_column_null :identities, :email_address, false + end +end diff --git a/db/untenanted_schema.rb b/db/untenanted_schema.rb index 9e38879e1..b4582291a 100644 --- a/db/untenanted_schema.rb +++ b/db/untenanted_schema.rb @@ -10,10 +10,10 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.2].define(version: 2025_10_27_131911) do +ActiveRecord::Schema[8.2].define(version: 2025_11_03_125952) do create_table "identities", force: :cascade do |t| t.datetime "created_at", null: false - t.string "email_address" + t.string "email_address", null: false t.datetime "updated_at", null: false t.index ["email_address"], name: "index_identities_on_email_address", unique: true end diff --git a/test/controllers/join_codes_controller_test.rb b/test/controllers/join_codes_controller_test.rb new file mode 100644 index 000000000..de8c5daa5 --- /dev/null +++ b/test/controllers/join_codes_controller_test.rb @@ -0,0 +1,62 @@ +require "test_helper" + +class JoinCodesControllerTest < ActionDispatch::IntegrationTest + setup do + @tenant = ApplicationRecord.current_tenant + @join_code = account_join_codes(:sole) + end + + test "new" do + untenanted do + get join_path(tenant: @tenant, code: @join_code.code) + end + + assert_response :success + assert_in_body "37signals" + end + + test "new with an invalid code" do + untenanted do + get join_path(tenant: @tenant, code: "INVALID-CODE") + end + + assert_response :not_found + end + + test "new with an inactive code" do + @join_code.update!(usage_count: @join_code.usage_limit) + + untenanted do + get join_path(tenant: @tenant, code: @join_code.code) + end + + assert_response :not_found + end + + test "create" do + untenanted do + assert_difference -> { Identity.count }, 1 do + assert_difference -> { Membership.count }, 1 do + post join_path(tenant: @tenant, code: @join_code.code), params: { email_address: "new_user@example.com" } + end + end + + assert_redirected_to session_magic_link_path + assert_equal root_url(script_name: "/#{@tenant}"), session[:return_to_after_authenticating] + end + end + + test "create for existing identity" do + identity = identities(:jz) + + untenanted do + assert_no_difference -> { Identity.count } do + assert_difference -> { Membership.count }, 1 do + post join_path(tenant: @tenant, code: @join_code.code), params: { email_address: identity.email_address } + end + end + + assert_redirected_to session_magic_link_path + end + end +end diff --git a/test/fixtures/account/join_codes.yml b/test/fixtures/account/join_codes.yml index d346d7e65..daba73e66 100644 --- a/test/fixtures/account/join_codes.yml +++ b/test/fixtures/account/join_codes.yml @@ -1,5 +1,3 @@ -# Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html - sole: code: 1234-5678-9XYZ usage_count: 0 diff --git a/test/models/account_test.rb b/test/models/account_test.rb index 99711439d..785799c90 100644 --- a/test/models/account_test.rb +++ b/test/models/account_test.rb @@ -13,6 +13,8 @@ class AccountTest < ActiveSupport::TestCase end test ".create_with_admin_user creates a new local account" do + membership = memberships(:david_in_37signals) + ApplicationRecord.create_tenant("account-create-with-dependents") do account = Account.create_with_admin_user( account: { @@ -21,7 +23,7 @@ class AccountTest < ActiveSupport::TestCase }, owner: { name: "David", - email_address: "david@37signals.com" + membership: membership } ) assert_not_nil account @@ -37,7 +39,7 @@ class AccountTest < ActiveSupport::TestCase admin = User.find_by(role: "admin") assert_equal "David", admin.name - assert_equal "david@37signals.com", admin.email_address + assert_equal "david@37signals.com", admin.identity.email_address assert_equal "admin", admin.role end end diff --git a/test/models/user/accessor_test.rb b/test/models/user/accessor_test.rb index 5ca1f834f..3305c37c9 100644 --- a/test/models/user/accessor_test.rb +++ b/test/models/user/accessor_test.rb @@ -2,7 +2,7 @@ require "test_helper" class User::AccessorTest < ActiveSupport::TestCase test "new users get added to all_access collections on creation" do - user = User.create!(name: "Jorge", email_address: "testregular@example.com") + user = User.create!(name: "Jorge") assert_includes user.collections, collections(:writebook) assert_equal Collection.all_access.count, user.collections.count diff --git a/test/models/user/configurable_test.rb b/test/models/user/configurable_test.rb index dbc353936..b49b78c1b 100644 --- a/test/models/user/configurable_test.rb +++ b/test/models/user/configurable_test.rb @@ -2,7 +2,7 @@ require "test_helper" class User::ConfigurableTest < ActiveSupport::TestCase test "should create settings for new users" do - user = User.create! name: "Some new user", email_address: "some.new@user.com" + user = User.create! name: "Some new user" assert user.settings.present? end end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 23f604ffb..2f3e71dca 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -4,8 +4,7 @@ class UserTest < ActiveSupport::TestCase test "create" do user = User.create! \ role: "member", - name: "Victor Cooper", - email_address: "victor@hey.com" + name: "Victor Cooper" assert_equal [ collections(:writebook) ], user.collections assert user.settings.present? @@ -14,8 +13,7 @@ class UserTest < ActiveSupport::TestCase test "creation gives access to all_access collections" do user = User.create! \ role: "member", - name: "Victor Cooper", - email_address: "victor@hey.com" + name: "Victor Cooper" assert_equal [ collections(:writebook) ], user.collections end