diff --git a/db/migrate/20250923172757_add_organization_id_to_admin_roles.rb b/db/migrate/20250923172757_add_organization_id_to_admin_roles.rb new file mode 100644 index 0000000000000000000000000000000000000000..b51d45738a3ec6f7e9ad2f1ca60c68bd9164bbb7 --- /dev/null +++ b/db/migrate/20250923172757_add_organization_id_to_admin_roles.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddOrganizationIdToAdminRoles < Gitlab::Database::Migration[2.3] + milestone '18.5' + + def change + add_column :admin_roles, :organization_id, :bigint + end +end diff --git a/db/migrate/20250923210446_add_unique_index_to_admin_roles.rb b/db/migrate/20250923210446_add_unique_index_to_admin_roles.rb new file mode 100644 index 0000000000000000000000000000000000000000..5513f0a2e5238aa1e85d0d7b41b5c5105aa7282b --- /dev/null +++ b/db/migrate/20250923210446_add_unique_index_to_admin_roles.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddUniqueIndexToAdminRoles < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.5' + + OLD_INDEX_NAME = 'index_admin_roles_on_name' + NEW_INDEX_NAME = 'index_admin_roles_on_organization_id_and_name' + + def up + add_concurrent_index :admin_roles, [:organization_id, :name], unique: true, name: NEW_INDEX_NAME + remove_concurrent_index_by_name :admin_roles, OLD_INDEX_NAME, if_exists: true + end + + def down + add_concurrent_index :admin_roles, :name, name: OLD_INDEX_NAME, unique: true + remove_concurrent_index_by_name :admin_roles, NEW_INDEX_NAME, if_exists: true + end +end diff --git a/db/post_migrate/20250923194656_add_organization_id_fk_to_admin_roles.rb b/db/post_migrate/20250923194656_add_organization_id_fk_to_admin_roles.rb new file mode 100644 index 0000000000000000000000000000000000000000..948b3efebe6e8388261bff8de00fae85e3333a6e --- /dev/null +++ b/db/post_migrate/20250923194656_add_organization_id_fk_to_admin_roles.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddOrganizationIdFkToAdminRoles < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.5' + + def up + add_concurrent_foreign_key :admin_roles, :organizations, column: :organization_id, validate: true + end + + def down + with_lock_retries do + remove_foreign_key_if_exists :admin_roles, column: :organization_id + end + end +end diff --git a/db/schema_migrations/20250923172757 b/db/schema_migrations/20250923172757 new file mode 100644 index 0000000000000000000000000000000000000000..62931d3af53c868d994631779e93d231260de5f3 --- /dev/null +++ b/db/schema_migrations/20250923172757 @@ -0,0 +1 @@ +dadd632e97145f5fa7f138356b3824dfdb31b057efa9e658483a9c4cd1391b30 \ No newline at end of file diff --git a/db/schema_migrations/20250923194656 b/db/schema_migrations/20250923194656 new file mode 100644 index 0000000000000000000000000000000000000000..d01382efe4e7bcbabafcb4ac641127e1bdf2c541 --- /dev/null +++ b/db/schema_migrations/20250923194656 @@ -0,0 +1 @@ +78bce6e51dbebfb36a80aa1b40ab3d77370ed14b4a8115f4bebff7adf8f50992 \ No newline at end of file diff --git a/db/schema_migrations/20250923210446 b/db/schema_migrations/20250923210446 new file mode 100644 index 0000000000000000000000000000000000000000..0baddc8c846b1553a59be073515f78b71bb822cb --- /dev/null +++ b/db/schema_migrations/20250923210446 @@ -0,0 +1 @@ +44ada38b89cbae34f79adf7a8315d9acd60489adacd6ebc6a57f826431785a79 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index c772285ef9dfb80ed4c826951c9b00a9ae68f5be..1bad81473e96433471a2f2f8f0028b1e9eb1f09c 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9754,6 +9754,7 @@ CREATE TABLE admin_roles ( permissions jsonb DEFAULT '{}'::jsonb NOT NULL, created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, + organization_id bigint, CONSTRAINT check_89a2f4f799 CHECK ((char_length(name) <= 255)), CONSTRAINT check_a8c6d1de58 CHECK ((char_length(description) <= 255)) ); @@ -38188,7 +38189,7 @@ CREATE UNIQUE INDEX index_add_on_purchases_on_add_on_id_and_namespace_id_null ON CREATE INDEX index_add_on_purchases_on_organization_id ON subscription_add_on_purchases USING btree (organization_id); -CREATE UNIQUE INDEX index_admin_roles_on_name ON admin_roles USING btree (name); +CREATE UNIQUE INDEX index_admin_roles_on_organization_id_and_name ON admin_roles USING btree (organization_id, name); CREATE INDEX index_agent_activity_events_on_agent_id_and_recorded_at_and_id ON agent_activity_events USING btree (agent_id, recorded_at, id); @@ -48220,6 +48221,9 @@ ALTER TABLE ONLY snippet_statistics ALTER TABLE ONLY granular_scopes ADD CONSTRAINT fk_73a513f489 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; +ALTER TABLE ONLY admin_roles + ADD CONSTRAINT fk_74591b3a95 FOREIGN KEY (organization_id) REFERENCES organizations(id) ON DELETE CASCADE; + ALTER TABLE ONLY index_statuses ADD CONSTRAINT fk_74b2492545 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; diff --git a/ee/app/models/authz/admin_role.rb b/ee/app/models/authz/admin_role.rb index a9e9b998b3295007bc21202d0d154f688255f05a..5c78374495d038fb5947ff2d379a6733d3590db5 100644 --- a/ee/app/models/authz/admin_role.rb +++ b/ee/app/models/authz/admin_role.rb @@ -2,10 +2,11 @@ module Authz class AdminRole < Authz::BaseRole + belongs_to :organization, class_name: 'Organizations::Organization' has_many :user_admin_roles, class_name: 'Authz::UserAdminRole' has_many :users, through: :user_admin_roles - validates :name, presence: true, uniqueness: true + validates :name, presence: true, uniqueness: { scope: :organization_id } validates :permissions, json_schema: { filename: 'admin_role_permissions' } alias_method :user_member_roles, :user_admin_roles diff --git a/ee/spec/factories/authz/admin_roles.rb b/ee/spec/factories/authz/admin_roles.rb index 89280a2cdb3d519045c2374872dcca7ce09d8bf5..f3888e12a8bee788c304bd1af53d934c5908a1fd 100644 --- a/ee/spec/factories/authz/admin_roles.rb +++ b/ee/spec/factories/authz/admin_roles.rb @@ -4,6 +4,7 @@ factory :admin_role, class: 'Authz::AdminRole' do name { FFaker::Lorem.unique.word } description { FFaker::Lorem.sentence } + organization { association(:common_organization) } transient do without_any_permissions { false } diff --git a/ee/spec/models/authz/admin_role_spec.rb b/ee/spec/models/authz/admin_role_spec.rb index 4b3e7dbd299a5f2db9d15b18b1f90dbb48dfd6ed..9b16dbcbe1e9d9b24d927951191f7cbe00a87f2b 100644 --- a/ee/spec/models/authz/admin_role_spec.rb +++ b/ee/spec/models/authz/admin_role_spec.rb @@ -16,7 +16,7 @@ describe 'validation' do it { is_expected.to validate_presence_of(:name) } - it { is_expected.to validate_uniqueness_of(:name) } + it { is_expected.to validate_uniqueness_of(:name).scoped_to(:organization_id) } context 'for json schema' do let(:permissions) { { read_admin_users: true } } diff --git a/ee/spec/services/authz/admin_roles/create_service_spec.rb b/ee/spec/services/authz/admin_roles/create_service_spec.rb index 8914eb6f311b7746e82cee1324de01d7eddb536a..60c6fdbdfec65e63e871eb59a73611bbe09a6685 100644 --- a/ee/spec/services/authz/admin_roles/create_service_spec.rb +++ b/ee/spec/services/authz/admin_roles/create_service_spec.rb @@ -4,6 +4,7 @@ RSpec.describe Authz::AdminRoles::CreateService, feature_category: :permissions do let_it_be(:user) { create(:admin) } + let_it_be(:organization) { create(:organization) } # used in tracking custom role action shard examples let(:namespace) { nil } @@ -13,7 +14,8 @@ let(:params) do { - name: role_name + name: role_name, + organization_id: organization.id }.merge(abilities) end @@ -33,6 +35,8 @@ end context 'when admin_mode is enabled', :enable_admin_mode do + let(:expected_organization) { organization } + context 'when creating an admin custom role' do it_behaves_like 'custom role creation' do let(:fail_condition!) do diff --git a/ee/spec/support/shared_examples/authz/custom_role_examples.rb b/ee/spec/support/shared_examples/authz/custom_role_examples.rb index 1313fbc75bc7c18e2e374e7ccbe6ed12c98cf3a5..b762aa76a813c51ca7437d501ad797dc20b3ef63 100644 --- a/ee/spec/support/shared_examples/authz/custom_role_examples.rb +++ b/ee/spec/support/shared_examples/authz/custom_role_examples.rb @@ -77,9 +77,7 @@ role = role_class.last expect(role.name).to eq(role_name) - # TODO: remove the condition as part of https://gitlab.com/gitlab-org/gitlab/-/issues/553437 - # as the shared_examples are used by admin_roles but they do not have organization yet - expect(role.organization).to eq(expected_organization) if role.respond_to?(:organization) + expect(role.organization).to eq(expected_organization) expect(role.permissions.select { |_k, v| v }.symbolize_keys).to eq(abilities) end diff --git a/spec/lib/gitlab/database/sharding_key_spec.rb b/spec/lib/gitlab/database/sharding_key_spec.rb index bb6eda7f55334f6248beb22a5b1b92be3ee86642..2fe1806080bb6830b230c5be5de720cb45101427 100644 --- a/spec/lib/gitlab/database/sharding_key_spec.rb +++ b/spec/lib/gitlab/database/sharding_key_spec.rb @@ -273,6 +273,7 @@ 'todos' => 'https://gitlab.com/gitlab-org/gitlab/-/issues/562437', # All the tables below related to uploads are part of the same work to # add sharding key to the table + "admin_roles" => "https://gitlab.com/gitlab-org/gitlab/-/issues/553437", "uploads" => "https://gitlab.com/gitlab-org/gitlab/-/issues/398199", "abuse_report_uploads" => "https://gitlab.com/gitlab-org/gitlab/-/issues/398199", "achievement_uploads" => "https://gitlab.com/gitlab-org/gitlab/-/issues/398199",