Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ Fizzy uses **URL path-based multi-tenancy**:

**Passwordless magic link authentication**:
- Global `Identity` (email-based) can have `Users` in multiple Accounts
- Users belong to an Account and have roles: admin, member, system
- Users belong to an Account and have roles: owner, admin, member, system
- Sessions managed via signed cookies
- Board-level access control via `Access` records

Expand All @@ -84,7 +84,7 @@ Fizzy uses **URL path-based multi-tenancy**:

**User** → Account membership
- Belongs to Account and Identity
- Has role (admin/member/system)
- Has role (owner/admin/member/system)
- Board access via explicit `Access` records

**Board** → Primary organizational unit
Expand Down
8 changes: 8 additions & 0 deletions app/helpers/users_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module UsersHelper
def role_display_name(user)
case user.role
when "admin" then "Administrator"
else user.role.titleize
end
end
end
4 changes: 2 additions & 2 deletions app/models/account.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ class Account < ApplicationRecord
validates :name, presence: true

class << self
def create_with_admin_user(account:, owner:)
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: "admin"))
account.users.create!(**owner.reverse_merge(role: "owner"))
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/account/seedeable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ module Account::Seedeable
extend ActiveSupport::Concern

def setup_customer_template
Account::Seeder.new(self, users.active.where(role: :admin).first).seed
Account::Seeder.new(self, users.admin.first).seed
end
end
4 changes: 2 additions & 2 deletions app/models/signup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def handle_account_creation_error(error)
end

def create_account
@account = Account.create_with_admin_user(
@account = Account.create_with_owner(
account: {
external_account_id: @tenant,
name: generate_account_name
Expand All @@ -64,7 +64,7 @@ def create_account
identity: identity
}
)
@user = @account.users.find_by!(role: :admin)
@user = @account.users.find_by!(role: :owner)
@account.setup_customer_template
end

Expand Down
16 changes: 11 additions & 5 deletions app/models/user/role.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,24 @@ module User::Role
extend ActiveSupport::Concern

included do
enum :role, %i[ admin member system ].index_by(&:itself), scopes: false
enum :role, %i[ owner admin member system ].index_by(&:itself), scopes: false

scope :member, -> { where(role: :member) }
scope :active, -> { where(active: true, role: %i[ admin member ]) }
scope :owner, -> { where(active: true, role: :owner) }
scope :admin, -> { where(active: true, role: %i[ owner admin ]) }
scope :member, -> { where(active: true, role: :member) }
scope :active, -> { where(active: true, role: %i[ owner admin member ]) }

def admin?
super || owner?
end
end

def can_change?(other)
admin? || other == self
(admin? && !other.owner?) || other == self
end

def can_administer?(other)
admin? && other != self
admin? && !other.owner? && other != self
end

def can_administer_board?(board)
Expand Down
12 changes: 7 additions & 5 deletions app/views/account/settings/_user.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@
<hr class="separator--horizontal flex-item-grow" style="--border-color: var(--color-ink-medium); --border-style: dashed" aria-hidden="true">

<%= form_with model: user, url: user_role_path(user), data: { controller: "form" }, method: :patch do | form | %>
<label class="btn btn--circle" for="<%= dom_id(user, :role) %>" arial-label="Role: <%= user.admin? ? "Administrator" : "Member" %>">
<%= icon_tag "crown" %>
<span class="for-screen-reader">Role: <%= user.admin? ? "Administrator" : "Member" %></span>
<%= form.check_box :role, { data: { action: "form#submit" }, disabled: !Current.user.can_administer?(user), hidden: true, id: dom_id(user, :role) }, "admin", "member" %>
</label>
<span title="<%= role_display_name(user) %>">
<label class="btn btn--circle" for="<%= dom_id(user, :role) %>" aria-label="Role: <%= role_display_name(user) %>">
<%= icon_tag "crown" %>
<span class="for-screen-reader">Role: <%= role_display_name(user) %></span>
<%= form.check_box :role, { data: { action: "form#submit" }, disabled: !Current.user.can_administer?(user), checked: user.admin?, hidden: true, id: dom_id(user, :role) }, "admin", "member" %>
</label>
</span>
<% end %>

<%# FIXME: Move this Current.user check to a stimulus controller that just checks for admin? or the like we so we can cache user list %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/admin/stats/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@

<ul class="margin-block-half">
<% @top_accounts.each do |account| %>
<% admin_user = account.users.find { |u| u.role == "admin" } %>
<% admin_user = account.users.owner.first %>
<li class="flex align-start gap-half margin-block-start-half">
<div
class="flex-item-grow min-width overflow-ellipsis"
Expand Down
14 changes: 14 additions & 0 deletions db/migrate/20251129175717_promote_first_admin_to_owner.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
class PromoteFirstAdminToOwner < ActiveRecord::Migration[8.2]
def up
Account.find_each do |account|
next if account.users.exists?(role: :owner)

first_admin = account.users.where(role: :admin).order(:created_at).first
first_admin&.update!(role: :owner)
end
end

def down
User.where(role: :owner).update_all(role: :admin)
end
end
2 changes: 1 addition & 1 deletion db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion db/schema_sqlite.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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_29_110120) do
ActiveRecord::Schema[8.2].define(version: 2025_11_29_175717) do
create_table "accesses", id: :uuid, force: :cascade do |t|
t.datetime "accessed_at"
t.uuid "account_id", null: false
Expand Down
2 changes: 1 addition & 1 deletion db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def create_tenant(signal_account_name)
identity = Identity.find_or_create_by!(email_address: email_address)

unless account = Account.find_by(external_account_id: tenant_id)
account = Account.create_with_admin_user(
account = Account.create_with_owner(
account: {
external_account_id: tenant_id,
name: signal_account_name
Expand Down
6 changes: 3 additions & 3 deletions script/import-sqlite-database.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ def generate_uuid

def setup_account
step("Setting up account", "Account set up in %{duration}") do
oldest_admin = import.users.order(id: :asc).where(role: :admin, active: true).first
oldest_admin = import.users.order(id: :asc).admin.first
raise "No admin user found in the database" unless oldest_admin

membership = untenanted.memberships.find(oldest_admin.membership_id)
Expand All @@ -133,7 +133,7 @@ def setup_account
if Account.all.exists?(external_account_id: account.external_account_id)
raise AccountExistsError, "Account already exists"
else
@account = Account.create_with_admin_user(
@account = Account.create_with_owner(
account: {
external_account_id: account.external_account_id,
name: account.name.truncate(255, omission: "")
Expand All @@ -144,7 +144,7 @@ def setup_account
}
)
@tenant = @account.external_account_id
@admin = @account.users.find_by(role: :admin)
@admin = @account.users.find_by(role: :owner)
end

old_join_code = import.account_join_codes.sole
Expand Down
4 changes: 2 additions & 2 deletions script/load-prod-db-in-dev.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@
ApplicationRecord.with_tenant(tenant) do |tenant|
Current.account.destroy!

Account.create_with_admin_user \
Account.create_with_owner \
account: { name: "Company #{identifier}" },
owner: { name: "Developer #{identifier}", email_address: "dev-#{identifier}@example.com" }

user = User.find_by(role: :admin)
user = User.find_by(role: :owner)
identity = Identity.find_or_create_by(email_address: user.email_address)
identity.link_to(user.tenant)
Board.find_each do |board|
Expand Down
24 changes: 24 additions & 0 deletions test/controllers/users/roles_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,29 @@ class Users::RolesControllerTest < ActionDispatch::IntegrationTest
assert_no_changes -> { users(:david).reload.role } do
put user_role_path(users(:david)), params: { user: { role: "system" } }
end

assert_no_changes -> { users(:david).reload.role } do
put user_role_path(users(:david)), params: { user: { role: "owner" } }
end
end

test "admin cannot demote the owner" do
assert users(:jason).owner?

assert_no_changes -> { users(:jason).reload.role } do
put user_role_path(users(:jason)), params: { user: { role: "admin" } }
end

assert_response :forbidden
end

test "admin cannot change owner role to member" do
assert users(:jason).owner?

assert_no_changes -> { users(:jason).reload.role } do
put user_role_path(users(:jason)), params: { user: { role: "member" } }
end

assert_response :forbidden
end
end
14 changes: 14 additions & 0 deletions test/controllers/users_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,20 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
assert_nil User.active.find_by(id: users(:david).id)
end

test "admin cannot deactivate the owner" do
sign_in_as :kevin

assert users(:jason).owner?
assert users(:jason).active

assert_no_difference -> { User.active.count } do
delete user_path(users(:jason))
end

assert_response :forbidden
assert users(:jason).reload.active
end

test "non-admins cannot perform actions" do
sign_in_as :jz

Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/identities.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ david:
jz:
email_address: jz@37signals.com

jason:
email_address: jason@37signals.com
staff: true

kevin:
email_address: kevin@37signals.com
staff: true
Expand Down
7 changes: 7 additions & 0 deletions test/fixtures/users.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ jz:
identity: jz
account: 37s_uuid

jason:
id: <%= ActiveRecord::FixtureSet.identify("jason", :uuid) %>
name: Jason
role: owner
identity: jason
account: 37s_uuid

kevin:
id: <%= ActiveRecord::FixtureSet.identify("kevin", :uuid) %>
name: Kevin
Expand Down
23 changes: 12 additions & 11 deletions test/models/account_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,17 @@ class AccountTest < ActiveSupport::TestCase
assert_equal "/#{account.external_account_id}", account.slug
end

test ".create_with_admin_user creates a new local account" do
test ".create_with_owner creates a new local account" do
Current.without_account do
identity = identities(:david)
account = nil

assert_changes -> { Account.count }, +1 do
assert_changes -> { User.count }, +2 do
account = Account.create_with_admin_user(
account = Account.create_with_owner(
account: {
external_account_id: ActiveRecord::FixtureSet.identify("account-create-with-admin-user-test"),
name: "Account Create With Admin"
external_account_id: ActiveRecord::FixtureSet.identify("account-create-with-owner-test"),
name: "Account Create With Owner"
},
owner: {
name: "David",
Expand All @@ -34,13 +34,14 @@ class AccountTest < ActiveSupport::TestCase

assert_not_nil account
assert account.persisted?
assert_equal ActiveRecord::FixtureSet.identify("account-create-with-admin-user-test"), account.external_account_id
assert_equal "Account Create With Admin", account.name

admin = account.users.find_by(role: "admin")
assert_equal "David", admin.name
assert_equal "david@37signals.com", admin.identity.email_address
assert_equal "admin", admin.role
assert_equal ActiveRecord::FixtureSet.identify("account-create-with-owner-test"), account.external_account_id
assert_equal "Account Create With Owner", account.name

owner = account.users.find_by(role: "owner")
assert_equal "David", owner.name
assert_equal "david@37signals.com", owner.identity.email_address
assert_equal "owner", owner.role
assert owner.admin?, "owner should also be considered an admin"

assert_predicate account.system_user, :present?
end
Expand Down
50 changes: 50 additions & 0 deletions test/models/user/role_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,56 @@ class User::RoleTest < ActiveSupport::TestCase
assert_not users(:jz).can_administer?(users(:kevin))
end

test "owner can administer admins and members" do
assert users(:jason).can_administer?(users(:kevin))
assert users(:jason).can_administer?(users(:david))
assert users(:jason).can_administer?(users(:jz))
end

test "owner cannot administer themselves" do
assert_not users(:jason).can_administer?(users(:jason))
end

test "admin cannot administer the owner" do
assert_not users(:kevin).can_administer?(users(:jason))
end

test "owner is included in active scope" do
active_users = User.active
assert_includes active_users, users(:jason)
assert_includes active_users, users(:kevin)
assert_includes active_users, users(:david)
assert_not_includes active_users, users(:system)
end

test "owner is also considered an admin" do
assert users(:jason).owner?
assert users(:jason).admin?

assert users(:kevin).admin?
assert_not users(:kevin).owner?
end

test "owner scope returns only active owners" do
owners = accounts("37s").users.owner
assert_includes owners, users(:jason)
assert_not_includes owners, users(:kevin)
assert_not_includes owners, users(:david)

users(:jason).update!(active: false)
assert_not_includes accounts("37s").users.owner, users(:jason)
end

test "admin scope returns active owners and admins" do
admins = accounts("37s").users.admin
assert_includes admins, users(:jason)
assert_includes admins, users(:kevin)
assert_not_includes admins, users(:david)

users(:kevin).update!(active: false)
assert_not_includes accounts("37s").users.admin, users(:kevin)
end

test "can administer board?" do
writebook_board = boards(:writebook)
private_board = boards(:private)
Expand Down