bterkuile/fizzy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Rework the email change flow to tie to memberships

Browse Source
This commit is contained in:
Stanko K.R.
2025-10-29 18:35:05 +01:00
parent 8e0efd2f38
commit 452c6a8373
22 changed files with 247 additions and 164 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/controllers/identity/email_addresses/confirmations_controller.rb
-34
View File
@@ -1,34 +0,0 @@
class Identity::EmailAddresses::ConfirmationsController < ApplicationController
require_untenanted_access
before_action :set_identity
rate_limit to: 3, within: 1.hour, only: :create
def show
end
def create
@identity.change_email_address_using_token(token)
# Redirect to the tenant that initiated the change
tenant = SignedGlobalID.parse(token, for: Identity::EmailAddressChangeable::EMAIL_CHANGE_TOKEN_PURPOSE)&.params&.fetch("tenant")
if tenant
redirect_to "#{tenant}/users/#{Current.user.id}/edit"
else
redirect_to session_menu_path
end
rescue ArgumentError => e
flash[:alert] = e.message
render :show, status: :unprocessable_entity
end
private
def set_identity
@identity = Current.identity
end
def token
params.expect :email_address_token
end
end
app/controllers/identity/email_addresses_controller.rb
-36
View File
@@ -1,36 +0,0 @@
class Identity::EmailAddressesController < ApplicationController
require_untenanted_access
before_action :set_identity
rate_limit to: 3, within: 1.hour, only: :create
def new
@tenant = params[:tenant]
@user = ApplicationRecord.with_tenant(@tenant) { Current.identity.user }
end
def create
@tenant = tenant
@user = ApplicationRecord.with_tenant(@tenant) { Current.identity.user }
if Identity.exists?(email_address: new_email_address)
flash.now[:alert] = "Someone else already uses that email"
render :new, status: :unprocessable_entity
else
@identity.send_email_address_change_confirmation(new_email_address, tenant: tenant)
end
end
private
def set_identity
@identity = Current.identity
end
def new_email_address
params.expect :email_address
end
def tenant
params.expect :tenant
end
end
app/controllers/memberships/email_addresses/confirmations_controller.rb
+28
View File
@@ -0,0 +1,28 @@
class Memberships::EmailAddresses::ConfirmationsController < ApplicationController
require_untenanted_access
allow_unauthenticated_access
before_action :set_membership
rate_limit to: 5, within: 1.hour, only: :create
def show
end
def create
Membership.change_email_address_using_token(token)
terminate_session
start_new_session_for @membership.reload.identity
redirect_to edit_user_url(script_name: "/#{@membership.tenant}", id: @membership.user)
end
private
def set_membership
@membership = Membership.find(params[:membership_id])
end
def token
params.expect :email_address_token
end
end
app/controllers/memberships/email_addresses_controller.rb
+22
View File
@@ -0,0 +1,22 @@
class Memberships::EmailAddressesController < ApplicationController
require_untenanted_access
before_action :set_membership
rate_limit to: 5, within: 1.hour, only: :create
def new
end
def create
@membership.send_email_address_change_confirmation(new_email_address)
end
private
def set_membership
@membership = Current.identity.memberships.find(params[:membership_id])
end
def new_email_address
params.expect :email_address
end
end
app/mailers/application_mailer.rb
+5 -1
View File
@@ -7,6 +7,10 @@ class ApplicationMailer < ActionMailer::Base
private
def default_url_options
super.merge(script_name: Account.sole.slug)
if ApplicationRecord.current_tenant
super.merge(script_name: Account.sole.slug)
else
super
end
end
end
app/mailers/identity_mailer.rb
+2 -4
View File
@@ -1,9 +1,7 @@
class IdentityMailer < ApplicationMailer
def email_change_confirmation(identity:, email_address:, token:, tenant:)
@identity = identity
def email_change_confirmation(email_address:, token:, membership:)
@token = token
@tenant = tenant
@membership = membership
mail to: email_address, subject: "Confirm your new email address"
end
end
app/models/identity.rb
+1 -11
View File
@@ -1,5 +1,5 @@
class Identity < UntenantedRecord
include EmailAddressChangeable, Transferable
include Transferable
has_many :memberships, dependent: :destroy
has_many :magic_links, dependent: :destroy
@@ -14,16 +14,6 @@ class Identity < UntenantedRecord
end
end
def link_to(tenant, context: nil)
memberships.find_or_create_by!(tenant: tenant) do |membership|
membership.context = context
end
end
def unlink_from(tenant)
memberships.find_by(tenant: tenant)&.destroy
end
def staff?
email_address.ends_with?("@37signals.com") || email_address.ends_with?("@basecamp.com")
end
app/models/identity/email_address_changeable.rb
-58
View File
@@ -1,58 +0,0 @@
module Identity::EmailAddressChangeable
EMAIL_CHANGE_TOKEN_PURPOSE = "change_email_address"
EMAIL_CHANGE_TOKEN_EXPIRATION = 30.minutes
extend ActiveSupport::Concern
def send_email_address_change_confirmation(new_email_address, tenant:)
token = generate_email_address_change_token(to: new_email_address, tenant: tenant, expires_in: EMAIL_CHANGE_TOKEN_EXPIRATION)
IdentityMailer.email_change_confirmation(identity: self, email_address: new_email_address, token: token, tenant: tenant).deliver_later
end
def generate_email_address_change_token(from: email_address, to:, tenant:, **options)
options = options.reverse_merge(
for: EMAIL_CHANGE_TOKEN_PURPOSE,
old_email_address: from,
new_email_address: to,
tenant: tenant
)
to_sgid(**options).to_s
end
def change_email_address_using_token(token)
parsed_token = SignedGlobalID.parse(token, for: EMAIL_CHANGE_TOKEN_PURPOSE)
if parsed_token.nil?
raise ArgumentError, "The token is invalid"
elsif parsed_token.find != self
raise ArgumentError, "The token is for another identity"
elsif email_address != parsed_token.params.fetch("old_email_address")
raise ArgumentError, "The token was generated for a different email address"
else
tenant = parsed_token.params.fetch("tenant")
new_email_address = parsed_token.params.fetch("new_email_address")
change_email_address(new_email_address, tenant: tenant)
end
end
private
def change_email_address(new_email_address, tenant:)
old_email_address = email_address
# First update the identity email
update!(email_address: new_email_address)
begin
# Then update the membership to point to the new identity
Membership.change_email_address(from: old_email_address, to: new_email_address, tenant: tenant)
# Also update the user's email in the tenant database (read-only from untenanted context)
# This will be handled by the user updating their own record
rescue => e
# Rollback identity email if membership update fails
update!(email_address: old_email_address)
raise e
end
end
end
app/models/membership.rb
+2
View File
@@ -1,4 +1,6 @@
class Membership < UntenantedRecord
include EmailAddressChangeable
belongs_to :identity, touch: true
serialize :context, coder: JSON
app/models/membership/email_address_changeable.rb
+55
View File
@@ -0,0 +1,55 @@
module Membership::EmailAddressChangeable
EMAIL_CHANGE_TOKEN_PURPOSE = "change_email_address"
EMAIL_CHANGE_TOKEN_EXPIRATION = 30.minutes
extend ActiveSupport::Concern
class_methods do
def change_email_address_using_token(token)
parsed_token = SignedGlobalID.parse(token, for: EMAIL_CHANGE_TOKEN_PURPOSE)
membership = parsed_token&.find
if parsed_token.nil?
raise ArgumentError, "The token is invalid"
elsif membership.nil?
raise ArgumentError, "The membership no longer exists"
elsif membership.identity.email_address != parsed_token.params.fetch("old_email_address")
raise ArgumentError, "The token was generated for a different email address"
else
new_email_address = parsed_token.params.fetch("new_email_address")
membership.change_email_address(new_email_address)
end
end
end
def send_email_address_change_confirmation(new_email_address)
token = generate_email_address_change_token(
to: new_email_address,
expires_in: EMAIL_CHANGE_TOKEN_EXPIRATION
)
IdentityMailer.email_change_confirmation(
email_address: new_email_address,
token: token,
membership: self
).deliver_later
end
def change_email_address(new_email_address)
transaction do
new_identity = Identity.find_or_create_by!(email_address: new_email_address)
update!(identity: new_identity)
end
end
private
def generate_email_address_change_token(from: identity.email_address, to:, **options)
options = options.reverse_merge(
for: EMAIL_CHANGE_TOKEN_PURPOSE,
old_email_address: from,
new_email_address: to,
)
to_sgid(**options).to_s
end
end
app/views/mailers/identity_mailer/email_change_confirmation.html.erb
+1 -1
View File
@@ -1,7 +1,7 @@
<h1 class="title">Confirm your email address change</h1>
<p class="subtitle">Hit the button below to use this email address in Fizzy.</p>
<%= link_to "Yes use use this email address", identity_email_address_confirmation_url(@token), class: "btn" %>
<%= link_to "Yes use use this email address", email_address_confirmation_url(membership_id: @membership.id, email_address_token: @token), class: "btn" %>
<p class="margin-block-start-double">If you didn’t request this change, you can ignore this email. Your email address WILL NOT be changed unless you hit the button.</p>
app/views/mailers/identity_mailer/email_change_confirmation.text.erb
+1 -1
View File
@@ -3,6 +3,6 @@ Confirm your email address change
Hit the link below to use this email address in Fizzy:
<%= identity_email_address_confirmation_url(@token) %>
<%= email_address_confirmation_url(membership_id: @membership.id, email_address_token: @token) %>
If you didn’t request this change, you can ignore this email. Your email address WILL NOT be changed unless you hit the button.
app/views/identity/email_addresses/confirmations/show.html.erb → app/views/memberships/email_addresses/confirmations/show.html.erb
+1 -1
View File
@@ -8,7 +8,7 @@
<p class="margin-none">Just a sec while we confirm your new email address.</p>
</header>
<%= form_with url: identity_email_address_confirmation_path(params[:email_address_token]), method: :post, data: { controller: "auto-submit" } do |form| %>
<%= form_with url: email_address_confirmation_path(membership_id: @membership.id), method: :post, data: { controller: "auto-submit" } do |form| %>
<%= form.hidden_field :email_address_token, value: params[:email_address_token] %>
<button type="submit" class="btn btn--link center">
app/views/identity/email_addresses/create.html.erb → app/views/memberships/email_addresses/create.html.erb
+2 -2
View File
@@ -2,7 +2,7 @@
<% content_for :header do %>
<div class="header__title">
<%= link_to edit_user_path(@user, script_name: "/#{@tenant}"), class: "btn borderless txt-medium", data: { controller: "hotkey", action: "keydown.esc@document->hotkey#click" } do %>
<%= link_to edit_user_path(@membership.user, script_name: "/#{@tenant}"), class: "btn borderless txt-medium", data: { controller: "hotkey", action: "keydown.esc@document->hotkey#click" } do %>
<span class="overflow-ellipsis">&larr; <strong>My profile</strong></span>
<% end %>
</div>
@@ -13,5 +13,5 @@
<p class="margin-none">We just sent an email to <strong><%= params[:email_address] %></strong></p>
<p class="margin-none-block-start">Hit the link in the email to confirm this is the email address you want to use with Fizzy going forward.</p>
<%= link_to "Done", edit_user_path(@user, script_name: "/#{@tenant}"), class: "btn btn--link center" %>
<%= link_to "Done", edit_user_path(@membership.user, script_name: "/#{@tenant}"), class: "btn btn--link center" %>
</div>
app/views/identity/email_addresses/new.html.erb → app/views/memberships/email_addresses/new.html.erb
+2 -3
View File
@@ -2,7 +2,7 @@
<% content_for :header do %>
<div class="header__title">
<%= link_to edit_user_path(@user, script_name: "/#{@tenant}"), class: "btn borderless txt-medium", data: { controller: "hotkey", action: "keydown.esc@document->hotkey#click" } do %>
<%= link_to edit_user_path(@membership.user, script_name: "/#{@membership.tenant}"), class: "btn borderless txt-medium", data: { controller: "hotkey", action: "keydown.esc@document->hotkey#click" } do %>
<span class="overflow-ellipsis">&larr; <strong>My profile</strong></span>
<% end %>
</div>
@@ -16,8 +16,7 @@
<p class="margin-none">Enter your new email address. We'll send you a confirmation to verify it.</p>
</header>
<%= form_with url: identity_email_addresses_path(tenant: @tenant), method: :post, class: "flex flex-column gap", data: { turbo: false } do |form| %>
<%= form.hidden_field :tenant, value: @tenant %>
<%= form_with url: email_addresses_path(membership_id: @membership.id), method: :post, class: "flex flex-column gap", data: { turbo: false } do |form| %>
<div class="flex align-center gap">
<label class="flex align-center gap input input--actor">
<%= form.email_field :email_address, class: "input full-width", autocomplete: "email", placeholder: "New email address", autofocus: true, required: true %>
app/views/users/edit.html.erb
+2 -2
View File
@@ -32,8 +32,8 @@
</div>
<div class="flex align-center gap">
<div class="flex align-center gap input input--actor">
<%= form.email_field :email_address, class: "input full-width", autocomplete: "username", placeholder: "Email address", required: true, readonly: true %>
<%= link_to "Change email", new_identity_email_address_url(script_name: nil, tenant: @user.tenant), class: "btn btn--plain txt-link txt-small txt-nowrap" %>
<%= form.email_field :email_address, class: "input full-width", autocomplete: "username", placeholder: "Email address", required: true, readonly: true, value: @user.identity.email_address %>
<%= link_to "Change email", new_email_address_url(script_name: nil, membership_id: Current.membership.id), class: "btn btn--plain txt-link txt-small txt-nowrap" %>
</div>
</div>
<button type="submit" id="log_in" class="btn btn--reversed center">
config/routes.rb
+4 -6
View File
@@ -136,12 +136,6 @@ Rails.application.routes.draw do
end
end
resource :identity, only: [] do
resources :email_addresses, only: %i[ new create ], param: :token, module: :identity do
resource :confirmation, only: %i[ show create ], module: :email_addresses
end
end
resources :users do
scope module: :users do
resource :avatar
@@ -158,6 +152,10 @@ Rails.application.routes.draw do
scope module: :memberships, path: "memberships/:membership_id" do
resource :unlink, only: %i[ show create ], controller: :unlink, as: :unlink_membership
resources :email_addresses, only: %i[ new create ], param: :token do
resource :confirmation, only: %i[ show create ], module: :email_addresses
end
end
namespace :my do
gems/fizzy-saas/app/models/signup.rb
+3 -2
View File
@@ -45,8 +45,9 @@ class Signup
def create_identity
return false unless valid?(:identity_creation)
@identity = Identity.find_or_create_by!(email_address: email_address)
@new_user = @identity.new_record?
@identity = Identity.find_by(email_address: email_address)
@new_user = !@identity
@identity = Identity.create!(email_address: email_address) unless @identity
@identity.send_magic_link
end
gems/fizzy-saas/test/controllers/signups/memberships_controller_test.rb
+2 -2
View File
@@ -51,7 +51,7 @@ class Signups::MembershipsControllerTest < ActionDispatch::IntegrationTest
end
end
test "create with validation errors" do
test "create with invalid params" do
untenanted do
assert_no_difference -> { Membership.count } do
post saas.signup_membership_path, params: {
@@ -66,7 +66,7 @@ class Signups::MembershipsControllerTest < ActionDispatch::IntegrationTest
end
end
test "create with new_user flag generates personal account name" do
test "create with new_user flag" do
untenanted do
post saas.signup_membership_path, params: {
signup: {
test/models/identity/transferable_test.rb
+37
View File
@@ -0,0 +1,37 @@
require "test_helper"
class Identity::TransferableTest < ActiveSupport::TestCase
test "transfer_id" do
identity = identities(:david)
transfer_id = identity.transfer_id
assert_not_nil transfer_id
# Should be able to find the identity using the transfer_id
found_identity = Identity.find_by_transfer_id(transfer_id)
assert_equal identity, found_identity
end
test "find_by_transfer_id" do
identity = identities(:kevin)
transfer_id = identity.transfer_id
found = Identity.find_by_transfer_id(transfer_id)
assert_equal identity, found
end
test "find_by_transfer_id with invalid id" do
found = Identity.find_by_transfer_id("invalid_id")
assert_nil found
end
test "find_by_transfer_id with expired id" do
identity = identities(:jz)
# Generate a transfer_id with short expiry
expired_id = identity.signed_id(purpose: :transfer, expires_in: -1.second)
found = Identity.find_by_transfer_id(expired_id)
assert_nil found
end
end
test/models/identity_test.rb
+21
View File
@@ -0,0 +1,21 @@
require "test_helper"
class IdentityTest < ActiveSupport::TestCase
include ActionMailer::TestHelper
test "send_magic_link" do
identity = identities(:david)
assert_emails 1 do
magic_link = identity.send_magic_link
assert_not_nil magic_link
assert_equal identity, magic_link.identity
end
end
test "staff?" do
assert Identity.new(email_address: "test@37signals.com").staff?
assert Identity.new(email_address: "test@basecamp.com").staff?
assert_not Identity.new(email_address: "test@example.com").staff?
end
end
test/models/membership/email_address_changeable_test.rb
+56
View File
@@ -0,0 +1,56 @@
require "test_helper"
class Membership::EmailAddressChangeableTest < ActiveSupport::TestCase
include ActionMailer::TestHelper
setup do
@identity = identities(:kevin)
@tenant = ApplicationRecord.current_tenant
@membership = @identity.memberships.find_by!(tenant: @tenant)
@new_email = "newart@example.com"
end
test "send_email_address_change_confirmation" do
assert_emails 1 do
@membership.send_email_address_change_confirmation(@new_email)
end
end
test "change_email_address" do
old_identity = @identity
assert_difference -> { Identity.count }, +1 do
@membership.change_email_address(@new_email)
end
assert_equal @new_email, @membership.reload.identity.email_address
assert_not old_identity.reload.memberships.exists?(id: @membership.id)
assert_equal @new_email, @membership.reload.identity.email_address
assert_no_difference -> { Identity.count } do
@membership.change_email_address(identities(:david).email_address)
end
assert_equal identities(:david).email_address, @membership.reload.identity.email_address
end
test "change_email_address_using_token" do
token = @membership.send(:generate_email_address_change_token, to: @new_email)
Membership.change_email_address_using_token(token)
assert_equal @new_email, @membership.reload.identity.email_address
end
test "change_email_address_using_token with invalid token" do
assert_raises(ArgumentError, match: /invalid/) do
Membership.change_email_address_using_token("invalid_token")
end
token = @membership.send(:generate_email_address_change_token, to: @new_email)
@identity.update!(email_address: "different@example.com")
assert_raises(ArgumentError, match: /different email address/) do
Membership.change_email_address_using_token(token)
end
end
end