Prohibit duplicate memberships

app/controllers/join_codes_controller.rb

@@ -10,8 +10,10 @@ class JoinCodesController < ApplicationController
 
   def create
     Identity.transaction do
-      identity = Identity.find_or_create_by(email_address: params.expect(:email_address))
-      identity.memberships.create!(tenant: tenant, join_code: code)
+      identity = Identity.find_or_create_by!(email_address: params.expect(:email_address))
+      identity.memberships.find_or_create_by!(tenant: tenant) do |membership|
+        membership.join_code = code
+      end
       identity.send_magic_link
     end
 

app/controllers/memberships/email_addresses/confirmations_controller.rb

@@ -11,7 +11,7 @@ class Memberships::EmailAddresses::ConfirmationsController < ApplicationControll
   def create
     membership = Membership.change_email_address_using_token(token)
 
-    terminate_session
+    terminate_session if Current.session
     start_new_session_for membership.reload.identity
 
     redirect_to edit_user_url(script_name: "/#{@membership.tenant}", id: @membership.user)

app/controllers/memberships/email_addresses_controller.rb

@@ -8,7 +8,14 @@ class Memberships::EmailAddressesController < ApplicationController
   end
 
   def create
-    @membership.send_email_address_change_confirmation(new_email_address)
+    identity = Identity.find_by_email_address(new_email_address)
+
+    if identity&.memberships&.exists?(tenant: @membership.tenant)
+      flash[:alert] = "You already have a user in this account with that email address"
+      redirect_to new_email_address_path
+    else
+      @membership.send_email_address_change_confirmation(new_email_address)
+    end
   end
 
   private

config/routes.rb

@@ -151,7 +151,7 @@ Rails.application.routes.draw do
   end
 
   scope module: :memberships, path: "memberships/:membership_id" do
-    resource :unlink, controller: :unlink, as: :unlink_membership
+    resource :unlink, only: %i[ show create ], controller: :unlink, as: :unlink_membership
 
     resources :email_addresses, param: :token do
       resource :confirmation, module: :email_addresses

db/untenanted_migrate/20251105082803_add_unique_constraint_on_identity_id_and_tenant_to_memberships.rb

@@ -0,0 +1,15 @@
+class AddUniqueConstraintOnIdentityIdAndTenantToMemberships < ActiveRecord::Migration[8.2]
+  def change
+    # Remove duplicates, keeping the youngest membership for each identity_id + tenant combination
+    execute <<-SQL
+      DELETE FROM memberships
+      WHERE id NOT IN (
+        SELECT MAX(id)
+        FROM memberships
+        GROUP BY identity_id, tenant
+      )
+    SQL
+
+    add_index :memberships, %i[ tenant identity_id ], unique: true
+  end
+end

db/untenanted_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_03_125952) do
+ActiveRecord::Schema[8.2].define(version: 2025_11_05_082803) do
   create_table "identities", force: :cascade do |t|
     t.datetime "created_at", null: false
     t.string "email_address", null: false
@@ -36,6 +36,7 @@ ActiveRecord::Schema[8.2].define(version: 2025_11_03_125952) do
     t.string "tenant", null: false
     t.datetime "updated_at", null: false
     t.index ["identity_id"], name: "index_memberships_on_identity_id"
+    t.index ["tenant", "identity_id"], name: "index_memberships_on_tenant_and_identity_id", unique: true
     t.index ["tenant"], name: "index_memberships_on_user_tenant_and_user_id"
   end
 

test/controllers/join_codes_controller_test.rb

@@ -51,7 +51,7 @@ class JoinCodesControllerTest < ActionDispatch::IntegrationTest
 
     untenanted do
       assert_no_difference -> { Identity.count } do
-        assert_difference -> { Membership.count }, 1 do
+        assert_no_difference -> { Membership.count } do
           post join_path(tenant: @tenant, code: @join_code.code), params: { email_address: identity.email_address }
         end
       end

test/controllers/memberships/email_addresses/confirmations_controller_test.rb

@@ -0,0 +1,42 @@
+require "test_helper"
+
+class Memberships::EmailAddresses::ConfirmationsControllerTest < ActionDispatch::IntegrationTest
+  test "show" do
+    untenanted do
+      membership = memberships(:kevin_in_37signals)
+
+      get email_address_confirmation_path(
+        membership_id: membership.id,
+        email_address_token: "dummy_token"
+      )
+
+      assert_response :success
+    end
+  end
+
+  test "create" do
+    untenanted do
+      membership = memberships(:kevin_in_37signals)
+      old_identity = membership.identity
+      new_email = "updated@example.com"
+
+      # Generate a real token
+      token = membership.send(:generate_email_address_change_token, to: new_email)
+
+      assert_difference -> { Identity.count }, 1 do
+        post email_address_confirmation_path(
+          membership_id: membership.id,
+          email_address_token: token
+        ),
+        params: { email_address_token: token }
+      end
+
+      membership.reload
+      assert_equal new_email, membership.identity.email_address
+      assert_not_equal old_identity.id, membership.identity_id
+
+      assert cookies[:session_token].present?, "Should have started new session"
+      assert_redirected_to edit_user_url(script_name: "/#{membership.tenant}", id: membership.user)
+    end
+  end
+end

test/controllers/memberships/email_addresses_controller_test.rb

@@ -0,0 +1,43 @@
+require "test_helper"
+
+class Memberships::EmailAddressesControllerTest < ActionDispatch::IntegrationTest
+  test "new" do
+    untenanted do
+      sign_in_as :kevin
+
+      membership = memberships(:kevin_in_37signals)
+
+      get new_email_address_path(membership_id: membership.id)
+      assert_response :success
+    end
+  end
+
+  test "create" do
+    untenanted do
+      sign_in_as :kevin
+
+      membership = memberships(:kevin_in_37signals)
+
+      assert_enqueued_emails 1 do
+        post email_addresses_path(membership_id: membership.id),
+             params: { email_address: "newemail@example.com" }
+      end
+
+      assert_response :success
+    end
+  end
+
+  test "create with an email for someone already in the account" do
+    untenanted do
+      sign_in_as :kevin
+
+      membership = memberships(:kevin_in_37signals)
+
+      post email_addresses_path(membership_id: membership.id),
+           params: { email_address: identities(:david).email_address }
+
+      assert_redirected_to new_email_address_path
+      assert_equal "You already have a user in this account with that email address", flash[:alert]
+    end
+  end
+end

test/controllers/memberships/unlink_controller_test.rb

@@ -0,0 +1,30 @@
+require "test_helper"
+
+class Memberships::UnlinkControllerTest < ActionDispatch::IntegrationTest
+  test "show" do
+    untenanted do
+      sign_in_as :kevin
+
+      membership = memberships(:kevin_in_37signals)
+      signed_id = membership.signed_id(purpose: :unlinking)
+
+      get unlink_membership_path(membership_id: signed_id)
+      assert_response :success
+    end
+  end
+
+  test "create" do
+    untenanted do
+      sign_in_as :kevin
+
+      membership = memberships(:kevin_in_37signals)
+      signed_id = membership.signed_id(purpose: :unlinking)
+
+      assert_difference -> { Membership.count }, -1 do
+        post unlink_membership_path(membership_id: signed_id)
+      end
+
+      assert_redirected_to session_menu_path
+    end
+  end
+end

test/models/membership/email_address_changeable_test.rb

@@ -27,6 +27,9 @@ class Membership::EmailAddressChangeableTest < ActiveSupport::TestCase
     assert_not old_identity.reload.memberships.exists?(id: @membership.id)
     assert_equal @new_email, @membership.reload.identity.email_address
 
+    # Make sure that a prior membership doesn't exist
+    identities(:david).memberships.where(tenant: @tenant).delete_all
+
     assert_no_difference -> { Identity.count } do
       @membership.change_email_address(identities(:david).email_address)
     end