diff --git a/db/post_migrate/20230405132104_remove_saml_provider_and_identities_non_root_group.rb b/db/post_migrate/20230405132104_remove_saml_provider_and_identities_non_root_group.rb new file mode 100644 index 0000000000000000000000000000000000000000..55a017464c20478477e36a222f2f964826c87619 --- /dev/null +++ b/db/post_migrate/20230405132104_remove_saml_provider_and_identities_non_root_group.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +class RemoveSamlProviderAndIdentitiesNonRootGroup < Gitlab::Database::Migration[2.1] + BATCH_SIZE = 500 + + disable_ddl_transaction! + restrict_gitlab_migration gitlab_schema: :gitlab_main + + def up + each_batch_range('saml_providers', scope: ->(table) { table.all }, of: BATCH_SIZE) do |min, max| + execute <<~SQL + DELETE FROM identities + WHERE identities.saml_provider_id + IN + ( + SELECT saml_providers.id FROM saml_providers + INNER JOIN namespaces ON namespaces.id=saml_providers.group_id + AND namespaces.type='Group' AND namespaces.parent_id IS NOT NULL + AND saml_providers.id BETWEEN #{min} AND #{max} + ); + + DELETE FROM saml_providers + USING namespaces + WHERE namespaces.id=saml_providers.group_id + AND namespaces.type='Group' AND namespaces.parent_id IS NOT NULL + AND saml_providers.id BETWEEN #{min} AND #{max}; + SQL + end + end + + def down + # noop + end +end diff --git a/db/post_migrate/20230405132855_remove_scim_token_and_scim_identity_non_root_group.rb b/db/post_migrate/20230405132855_remove_scim_token_and_scim_identity_non_root_group.rb new file mode 100644 index 0000000000000000000000000000000000000000..aa149acc5be8194fc668fc3d8284550bec7e7fc9 --- /dev/null +++ b/db/post_migrate/20230405132855_remove_scim_token_and_scim_identity_non_root_group.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +class RemoveScimTokenAndScimIdentityNonRootGroup < Gitlab::Database::Migration[2.1] + BATCH_SIZE = 500 + + disable_ddl_transaction! + restrict_gitlab_migration gitlab_schema: :gitlab_main + + def up + each_batch_range('scim_oauth_access_tokens', scope: ->(table) { table.all }, of: BATCH_SIZE) do |min, max| + execute <<~SQL + DELETE FROM scim_identities + WHERE scim_identities.group_id + IN + ( + SELECT namespaces.id FROM scim_oauth_access_tokens + INNER JOIN namespaces ON namespaces.id=scim_oauth_access_tokens.group_id + WHERE namespaces.type='Group' AND namespaces.parent_id IS NOT NULL + AND scim_oauth_access_tokens.id BETWEEN #{min} AND #{max} + ); + + DELETE FROM scim_oauth_access_tokens + USING namespaces + WHERE namespaces.id=scim_oauth_access_tokens.group_id + AND namespaces.type='Group' AND namespaces.parent_id IS NOT NULL + AND scim_oauth_access_tokens.id BETWEEN #{min} AND #{max}; + SQL + end + end + + def down + # noop + end +end diff --git a/db/schema_migrations/20230405132104 b/db/schema_migrations/20230405132104 new file mode 100644 index 0000000000000000000000000000000000000000..e8d9bd7ded7580de43e5d5edf5eda3b409c52160 --- /dev/null +++ b/db/schema_migrations/20230405132104 @@ -0,0 +1 @@ +eae464c7583b909d975c379d196b7ae5301580f7195907a476ca1a146d8cb6b1 \ No newline at end of file diff --git a/db/schema_migrations/20230405132855 b/db/schema_migrations/20230405132855 new file mode 100644 index 0000000000000000000000000000000000000000..209578a8ed4f75f54b921223f0686865fd80e8c0 --- /dev/null +++ b/db/schema_migrations/20230405132855 @@ -0,0 +1 @@ +a7928284883d79b1204bb39a2a2d34b173771ce6dc484cefdb1c7ec3e9e9477a \ No newline at end of file diff --git a/spec/migrations/remove_saml_provider_and_identities_non_root_group_spec.rb b/spec/migrations/remove_saml_provider_and_identities_non_root_group_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..07873d0ce7921339c6720ca466b7adce69e48b20 --- /dev/null +++ b/spec/migrations/remove_saml_provider_and_identities_non_root_group_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe RemoveSamlProviderAndIdentitiesNonRootGroup, feature_category: :system_access do + let(:namespaces) { table(:namespaces) } + let(:saml_providers) { table(:saml_providers) } + let(:identities) { table(:identities) } + let(:root_group) do + namespaces.create!(name: 'root_group', path: 'foo', parent_id: nil, type: 'Group') + end + + let(:non_root_group) do + namespaces.create!(name: 'non_root_group', path: 'non_root', parent_id: root_group.id, type: 'Group') + end + + it 'removes saml_providers that belong to non-root group and related identities' do + provider_root_group = saml_providers.create!( + group_id: root_group.id, + sso_url: 'https://saml.example.com/adfs/ls', + certificate_fingerprint: '55:44:33:22:11:aa:bb:cc:dd:ee:ff:11:22:33:44:55:66:77:88:99', + default_membership_role: ::Gitlab::Access::GUEST, + enabled: true + ) + + identity_root_group = identities.create!( + saml_provider_id: provider_root_group.id, + extern_uid: "12345" + ) + + provider_non_root_group = saml_providers.create!( + group_id: non_root_group.id, + sso_url: 'https://saml.example.com/adfs/ls', + certificate_fingerprint: '55:44:33:22:11:aa:bb:cc:dd:ee:ff:11:22:33:44:55:66:77:88:99', + default_membership_role: ::Gitlab::Access::GUEST, + enabled: true + ) + + identity_non_root_group = identities.create!( + saml_provider_id: provider_non_root_group.id, + extern_uid: "12345" + ) + + expect { migrate! }.to change { saml_providers.count }.from(2).to(1) + + expect(identities.find_by_id(identity_non_root_group.id)).to be_nil + expect(saml_providers.find_by_id(provider_non_root_group.id)).to be_nil + + expect(identities.find_by_id(identity_root_group.id)).not_to be_nil + expect(saml_providers.find_by_id(provider_root_group.id)).not_to be_nil + end +end diff --git a/spec/migrations/remove_scim_token_and_scim_identity_non_root_group_spec.rb b/spec/migrations/remove_scim_token_and_scim_identity_non_root_group_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..31915365c91d812887c2a072bf6f39da43371f5a --- /dev/null +++ b/spec/migrations/remove_scim_token_and_scim_identity_non_root_group_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe RemoveScimTokenAndScimIdentityNonRootGroup, feature_category: :system_access do + let(:namespaces) { table(:namespaces) } + let(:scim_oauth_access_tokens) { table(:scim_oauth_access_tokens) } + let(:scim_identities) { table(:scim_identities) } + let(:users) { table(:users) } + let(:root_group) do + namespaces.create!(name: 'root_group', path: 'foo', parent_id: nil, type: 'Group') + end + + let(:non_root_group) do + namespaces.create!(name: 'non_root_group', path: 'non_root', parent_id: root_group.id, type: 'Group') + end + + let(:root_group_user) do + users.create!(name: 'Example User', email: 'user@example.com', projects_limit: 0) + end + + let(:non_root_group_user) do + users.create!(username: 'user2', email: 'user2@example.com', projects_limit: 10) + end + + it 'removes scim_oauth_access_tokens that belong to non-root group and related scim_identities' do + scim_oauth_access_token_root_group = scim_oauth_access_tokens.create!( + group_id: root_group.id, + token_encrypted: Gitlab::CryptoHelper.aes256_gcm_encrypt(SecureRandom.hex(50)) + ) + scim_oauth_access_token_non_root_group = scim_oauth_access_tokens.create!( + group_id: non_root_group.id, + token_encrypted: Gitlab::CryptoHelper.aes256_gcm_encrypt(SecureRandom.hex(50)) + ) + + scim_identity_root_group = scim_identities.create!( + group_id: root_group.id, + extern_uid: "12345", + user_id: root_group_user.id, + active: true + ) + + scim_identity_non_root_group = scim_identities.create!( + group_id: non_root_group.id, + extern_uid: "12345", + user_id: non_root_group_user.id, + active: true + ) + + expect { migrate! }.to change { scim_oauth_access_tokens.count }.from(2).to(1) + expect(scim_oauth_access_tokens.find_by_id(scim_oauth_access_token_non_root_group.id)).to be_nil + expect(scim_identities.find_by_id(scim_identity_non_root_group.id)).to be_nil + + expect(scim_oauth_access_tokens.find_by_id(scim_oauth_access_token_root_group.id)).not_to be_nil + expect(scim_identities.find_by_id(scim_identity_root_group.id)).not_to be_nil + end +end