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
53 changes: 47 additions & 6 deletions backend/app/controllers/api/v1/groups_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class GroupsController < BaseController
before_action :set_group, only: [ :show, :update, :destroy ]

def index
@groups = policy_scope(Group).includes(:parent_group, :users).order(:name)
@groups = policy_scope(Group).includes(:parent_group, :group_memberships, :users).order(:name)
end

def show
Expand Down Expand Up @@ -37,15 +37,19 @@ def set_group
def persist_group(group, status = :ok)
attributes = group_params
user_ids = normalize_user_ids(attributes.delete(:user_ids))
admin_user_ids = normalize_user_ids(attributes.delete(:admin_user_ids))
user_ids = include_current_user(user_ids) if group.new_record?
admin_user_ids = include_current_user(admin_user_ids) if group.new_record?

group.assign_attributes(attributes)
authorize group
return render_invalid_user_ids unless valid_user_ids?(user_ids)
return render_invalid_user_ids(:user_ids) unless valid_user_ids?(user_ids)
return render_invalid_user_ids(:admin_user_ids) unless valid_user_ids?(admin_user_ids)

Group.transaction do
group.save!
group.user_ids = user_ids if user_ids
sync_group_memberships(group, user_ids, admin_user_ids)
ensure_group_has_admin!(group)
end

@group = group.reload
Expand All @@ -55,7 +59,7 @@ def persist_group(group, status = :ok)
end

def group_params
params.require(:group).permit(:name, :description, :parent_group_id, user_ids: [])
params.require(:group).permit(:name, :description, :parent_group_id, user_ids: [], admin_user_ids: [])
end

def normalize_user_ids(raw_user_ids)
Expand Down Expand Up @@ -85,10 +89,47 @@ def include_current_user(user_ids)
(user_ids + [ current_user.id ]).uniq
end

def render_invalid_user_ids
def sync_group_memberships(group, user_ids, admin_user_ids)
return if user_ids.nil? && admin_user_ids.nil?

memberships = group.group_memberships.index_by(&:user_id)
final_user_ids = user_ids || memberships.keys
final_admin_user_ids = admin_user_ids || memberships.values.select(&:admin?).map(&:user_id)
final_user_ids = (final_user_ids + final_admin_user_ids).uniq

if final_admin_user_ids.empty?
group.errors.add(:admin_user_ids, :blank)
raise ActiveRecord::RecordInvalid, group
end

final_admin_user_ids.each do |user_id|
membership = memberships[user_id] || group.group_memberships.build(user_id:)
membership.admin = true
membership.save!
end

(final_user_ids - final_admin_user_ids).each do |user_id|
membership = memberships[user_id] || group.group_memberships.build(user_id:)
membership.admin = false
membership.save!
end

(memberships.keys - final_user_ids).each do |user_id|
memberships[user_id].destroy!
end
end

def ensure_group_has_admin!(group)
return if group.group_memberships.where(admin: true).exists?

group.errors.add(:admin_user_ids, :blank)
raise ActiveRecord::RecordInvalid, group
end

def render_invalid_user_ids(attribute)
render "api/v1/errors/show",
formats: :json,
locals: { errors: { user_ids: [ I18n.t("api.errors.include_unknown_users") ] } },
locals: { errors: { attribute => [ I18n.t("api.errors.include_unknown_users") ] } },
status: :unprocessable_entity
end
end
Expand Down
5 changes: 4 additions & 1 deletion backend/app/controllers/api/v1/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ def set_user
end

def user_params
params.require(:user).permit(:email, :display_name, :title, :bio)
permitted_fields = [ :email, :display_name, :title, :bio ]
permitted_fields << :system_admin if current_user.system_admin?

params.require(:user).permit(*permitted_fields)
end

def page_param
Expand Down
2 changes: 2 additions & 0 deletions backend/app/models/group.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
class Group < ApplicationRecord
include SearchableResource

attr_accessor :admin_user_ids

search_index_attributes :name, :description

belongs_to :parent_group,
Expand Down
7 changes: 7 additions & 0 deletions backend/app/models/group_hierarchy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ def self.accessible_group_ids_for(user)
.select(:descendant_group_id)
end

def self.adminable_group_ids_for(user)
joins(ancestor_group: :group_memberships)
.where(group_memberships: { user_id: user.id, admin: true })
.distinct
.select(:descendant_group_id)
end

def self.rebuild!
transaction do
delete_all
Expand Down
26 changes: 26 additions & 0 deletions backend/app/models/group_membership.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,30 @@ class GroupMembership < ApplicationRecord
belongs_to :user

validates :user_id, uniqueness: { scope: :group_id }
validate :group_must_keep_admin, if: :removing_admin?

before_destroy :ensure_group_keeps_admin

private

def removing_admin?
persisted? && will_save_change_to_admin? && admin == false
end

def group_must_keep_admin
return if group_has_another_admin?

errors.add(:admin, "must have at least one group admin")
end

def ensure_group_keeps_admin
return if destroyed_by_association.present? || group_has_another_admin?

errors.add(:admin, "must have at least one group admin")
throw :abort
end

def group_has_another_admin?
group.group_memberships.where(admin: true).where.not(id:).exists?
end
end
16 changes: 16 additions & 0 deletions backend/app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,29 @@ class User < ApplicationRecord
has_many :oauth_identities, dependent: :destroy

def accessible_group_ids
return Group.ids if system_admin?

@accessible_group_ids ||= GroupHierarchy.accessible_group_ids_for(self).pluck(:descendant_group_id)
end

def can_access_group?(group_id)
return group_id.present? if system_admin?

group_id.present? && accessible_group_ids.include?(group_id)
end
Comment thread
dsh0416 marked this conversation as resolved.

def adminable_group_ids
return Group.ids if system_admin?

@adminable_group_ids ||= GroupHierarchy.adminable_group_ids_for(self).pluck(:descendant_group_id)
end
Comment thread
Copilot marked this conversation as resolved.

def can_admin_group?(group_id)
return group_id.present? if system_admin?

group_id.present? && adminable_group_ids.include?(group_id)
end

def self.from_omniauth(auth)
identity = OauthIdentity.find_by(provider: auth.provider, uid: auth.uid)
return identity.user if identity
Expand Down
2 changes: 1 addition & 1 deletion backend/app/policies/application_setting_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ def show?
end

def update?
user.present?
user&.system_admin?
end
end
15 changes: 11 additions & 4 deletions backend/app/policies/group_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,21 @@ def show?
def create?
return false if user.blank?

record.parent_group_id.blank? || user.can_access_group?(record.parent_group_id)
record.parent_group_id.blank? || user.can_admin_group?(record.parent_group_id)
end

def update?
group_member? && allowed_parent_group?
group_admin? && allowed_parent_group?
end

def destroy?
group_member?
group_admin?
end

class Scope < ApplicationPolicy::Scope
def resolve
return scope.none if user.blank?
return scope.all if user.system_admin?

scope.where(id: GroupHierarchy.accessible_group_ids_for(user))
end
Expand All @@ -35,7 +36,13 @@ def group_member?
user.present? && record.id.present? && user.can_access_group?(record.id)
end

def group_admin?
user.present? && record.id.present? && user.can_admin_group?(record.id)
end

def allowed_parent_group?
record.parent_group_id.blank? || user.can_access_group?(record.parent_group_id)
return true unless record.will_save_change_to_parent_group_id?

record.parent_group_id.blank? || user.can_admin_group?(record.parent_group_id)
end
Comment thread
dsh0416 marked this conversation as resolved.
end
18 changes: 15 additions & 3 deletions backend/app/policies/project_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ def show?
end

def create?
user.present? && record.user == user && group_member?
user.present? && record.user == user && group_admin?
end

def update?
group_member?
group_admin? && target_group_admin?
end

def destroy?
group_member?
group_admin?
end

def board?
Expand All @@ -26,6 +26,7 @@ def board?
class Scope < ApplicationPolicy::Scope
def resolve
return scope.none if user.blank?
return scope.all if user.system_admin?

scope.where(group_id: GroupHierarchy.accessible_group_ids_for(user))
end
Expand All @@ -38,4 +39,15 @@ def group_member?

user.can_access_group?(record.group_id)
end

def group_admin?
return false if user.blank?

group_id = record.group_id_in_database || record.group_id
group_id.present? && user.can_admin_group?(group_id)
end

def target_group_admin?
record.group_id.present? && user.can_admin_group?(record.group_id)
end
end
4 changes: 4 additions & 0 deletions backend/app/views/api/v1/groups/_group.json.jbuilder
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
user_ids = group.user_ids
admin_user_ids = group.group_memberships.filter_map { |membership| membership.user_id if membership.admin? }
parent_group_visible = group.parent_group_id.present? && viewer.can_access_group?(group.parent_group_id)

json.extract! group, :id, :name, :description, :parent_group_id, :created_at, :updated_at
json.parent_group_name parent_group_visible ? group.parent_group&.name : nil
json.user_ids user_ids
json.users_count user_ids.size
json.admin_user_ids admin_user_ids
json.admins_count admin_user_ids.size
json.can_admin viewer.can_admin_group?(group.id)
2 changes: 1 addition & 1 deletion backend/app/views/api/v1/users/_user.json.jbuilder
Original file line number Diff line number Diff line change
@@ -1 +1 @@
json.extract! user, :id, :email, :display_name, :title, :bio, :created_at, :updated_at
json.extract! user, :id, :email, :display_name, :title, :bio, :system_admin, :created_at, :updated_at
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddSystemAdminToUsers < ActiveRecord::Migration[8.1]
def change
add_column :users, :system_admin, :boolean, null: false, default: false
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
class AddAdminToGroupMemberships < ActiveRecord::Migration[8.1]
def up
add_column :group_memberships, :admin, :boolean, null: false, default: false
add_index :group_memberships, [ :group_id, :admin ]

execute <<~SQL.squish
UPDATE group_memberships
SET admin = TRUE
SQL

execute <<~SQL.squish
INSERT INTO group_memberships (group_id, user_id, admin, created_at, updated_at)
SELECT groups.id, system_admins.id, TRUE, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP
FROM groups
CROSS JOIN LATERAL (
SELECT users.id
FROM users
WHERE users.system_admin = TRUE
ORDER BY users.id ASC
LIMIT 1
) system_admins
WHERE NOT EXISTS (
SELECT 1
FROM group_memberships
WHERE group_memberships.group_id = groups.id
)
SQL
end

def down
remove_index :group_memberships, [ :group_id, :admin ]
remove_column :group_memberships, :admin
end
end
5 changes: 4 additions & 1 deletion backend/db/schema.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.1].define(version: 2026_07_08_032000) do
ActiveRecord::Schema[8.1].define(version: 2026_07_09_091000) do
# These are extensions that must be enabled in order to support this database
enable_extension "pg_catalog.plpgsql"
enable_extension "pg_trgm"
Expand All @@ -35,10 +35,12 @@
end

create_table "group_memberships", force: :cascade do |t|
t.boolean "admin", default: false, null: false
t.datetime "created_at", null: false
t.bigint "group_id", null: false
t.datetime "updated_at", null: false
t.bigint "user_id", null: false
t.index ["group_id", "admin"], name: "index_group_memberships_on_group_id_and_admin"
t.index ["group_id", "user_id"], name: "index_group_memberships_on_group_id_and_user_id", unique: true
t.index ["group_id"], name: "index_group_memberships_on_group_id"
t.index ["user_id"], name: "index_group_memberships_on_user_id"
Expand Down Expand Up @@ -175,6 +177,7 @@
t.datetime "remember_created_at"
t.datetime "reset_password_sent_at"
t.string "reset_password_token"
t.boolean "system_admin", default: false, null: false
t.string "title"
t.datetime "updated_at", null: false
t.index ["email"], name: "index_users_on_email", unique: true
Expand Down
32 changes: 26 additions & 6 deletions backend/db/seeds.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,29 @@
# This file should ensure the existence of records required to run the application in every environment (production,
# development, test). The code here should be idempotent so that it can be executed at any point in every environment.
# The data can then be loaded with the bin/rails db:seed command (or created alongside the database with db:setup).
#
# Example:
#
# ["Action", "Comedy", "Drama", "Horror"].each do |genre_name|
# MovieGenre.find_or_create_by!(name: genre_name)
# end

require "securerandom"

root_admin_password = nil

User.transaction do
unless User.lock.exists?(system_admin: true)
root_admin_password = SecureRandom.urlsafe_base64(24)
root_user = User.lock.find_or_initialize_by(email: "root@example.org")
root_user.display_name = "Root Admin" if root_user.display_name.blank?
root_user.assign_attributes(
password: root_admin_password,
password_confirmation: root_admin_password,
system_admin: true
)
root_user.save!
end
end

if root_admin_password
puts <<~MESSAGE
Created default system admin user:
email: root@example.org
password: #{root_admin_password}
MESSAGE
end
Loading
Loading