Commit 3bb7929b authored by Drew Blessing's avatar Drew Blessing Committed by Drew Blessing

Mark SCIM-created accounts as provisioned by group

Mark all SCIM created accounts with the group they were provisioned
by. This helps us identify accounts that were created by a group
via SCIM automation versus accounts created manually by a user.
parent b2c841a0
...@@ -31,3 +31,5 @@ class UserDetail < ApplicationRecord ...@@ -31,3 +31,5 @@ class UserDetail < ApplicationRecord
self.bio = '' if bio_changed? && bio.nil? self.bio = '' if bio_changed? && bio.nil?
end end
end end
UserDetail.prepend_if_ee('EE::UserDetail')
---
title: Mark SCIM-created accounts as provisioned by group
merge_request: 48483
author:
type: added
# frozen_string_literal: true
class AddProvisionedByGroupToUserDetails < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
INDEX_NAME = 'index_user_details_on_provisioned_by_group_id'
disable_ddl_transaction!
def up
unless column_exists?(:user_details, :provisioned_by_group_id)
with_lock_retries { add_column(:user_details, :provisioned_by_group_id, :integer, limit: 8) }
end
add_concurrent_index :user_details, :provisioned_by_group_id, name: INDEX_NAME
add_concurrent_foreign_key :user_details, :namespaces, column: :provisioned_by_group_id, on_delete: :nullify
end
def down
with_lock_retries { remove_foreign_key_without_error :user_details, column: :provisioned_by_group_id }
remove_concurrent_index_by_name :user_details, INDEX_NAME
if column_exists?(:user_details, :provisioned_by_group_id)
with_lock_retries { remove_column(:user_details, :provisioned_by_group_id) }
end
end
end
9d69938cda6db1510ed17d087cc1a582af1e5482d65e4fb457e34011e09c3469
\ No newline at end of file
...@@ -16998,6 +16998,7 @@ CREATE TABLE user_details ( ...@@ -16998,6 +16998,7 @@ CREATE TABLE user_details (
cached_markdown_version integer, cached_markdown_version integer,
webauthn_xid text, webauthn_xid text,
other_role text, other_role text,
provisioned_by_group_id bigint,
CONSTRAINT check_245664af82 CHECK ((char_length(webauthn_xid) <= 100)), CONSTRAINT check_245664af82 CHECK ((char_length(webauthn_xid) <= 100)),
CONSTRAINT check_b132136b01 CHECK ((char_length(other_role) <= 100)) CONSTRAINT check_b132136b01 CHECK ((char_length(other_role) <= 100))
); );
...@@ -22472,6 +22473,8 @@ CREATE INDEX index_user_custom_attributes_on_key_and_value ON user_custom_attrib ...@@ -22472,6 +22473,8 @@ CREATE INDEX index_user_custom_attributes_on_key_and_value ON user_custom_attrib
CREATE UNIQUE INDEX index_user_custom_attributes_on_user_id_and_key ON user_custom_attributes USING btree (user_id, key); CREATE UNIQUE INDEX index_user_custom_attributes_on_user_id_and_key ON user_custom_attributes USING btree (user_id, key);
CREATE INDEX index_user_details_on_provisioned_by_group_id ON user_details USING btree (provisioned_by_group_id);
CREATE UNIQUE INDEX index_user_details_on_user_id ON user_details USING btree (user_id); CREATE UNIQUE INDEX index_user_details_on_user_id ON user_details USING btree (user_id);
CREATE INDEX index_user_highest_roles_on_user_id_and_highest_access_level ON user_highest_roles USING btree (user_id, highest_access_level); CREATE INDEX index_user_highest_roles_on_user_id_and_highest_access_level ON user_highest_roles USING btree (user_id, highest_access_level);
...@@ -23067,6 +23070,9 @@ ALTER TABLE ONLY project_features ...@@ -23067,6 +23070,9 @@ ALTER TABLE ONLY project_features
ALTER TABLE ONLY ci_pipelines ALTER TABLE ONLY ci_pipelines
ADD CONSTRAINT fk_190998ef09 FOREIGN KEY (external_pull_request_id) REFERENCES external_pull_requests(id) ON DELETE SET NULL; ADD CONSTRAINT fk_190998ef09 FOREIGN KEY (external_pull_request_id) REFERENCES external_pull_requests(id) ON DELETE SET NULL;
ALTER TABLE ONLY user_details
ADD CONSTRAINT fk_190e4fcc88 FOREIGN KEY (provisioned_by_group_id) REFERENCES namespaces(id) ON DELETE SET NULL;
ALTER TABLE ONLY vulnerabilities ALTER TABLE ONLY vulnerabilities
ADD CONSTRAINT fk_1d37cddf91 FOREIGN KEY (epic_id) REFERENCES epics(id) ON DELETE SET NULL; ADD CONSTRAINT fk_1d37cddf91 FOREIGN KEY (epic_id) REFERENCES epics(id) ON DELETE SET NULL;
......
...@@ -40,6 +40,8 @@ module EE ...@@ -40,6 +40,8 @@ module EE
has_many :project_templates, through: :projects, foreign_key: 'custom_project_templates_group_id' has_many :project_templates, through: :projects, foreign_key: 'custom_project_templates_group_id'
has_many :managed_users, class_name: 'User', foreign_key: 'managing_group_id', inverse_of: :managing_group has_many :managed_users, class_name: 'User', foreign_key: 'managing_group_id', inverse_of: :managing_group
has_many :provisioned_user_details, class_name: 'UserDetail', foreign_key: 'provisioned_by_group_id', inverse_of: :provisioned_by_group
has_many :provisioned_users, through: :provisioned_user_details, source: :user
has_many :cycle_analytics_stages, class_name: 'Analytics::CycleAnalytics::GroupStage' has_many :cycle_analytics_stages, class_name: 'Analytics::CycleAnalytics::GroupStage'
has_many :value_streams, class_name: 'Analytics::CycleAnalytics::GroupValueStream' has_many :value_streams, class_name: 'Analytics::CycleAnalytics::GroupValueStream'
......
...@@ -33,6 +33,9 @@ module EE ...@@ -33,6 +33,9 @@ module EE
delegate :shared_runners_minutes_limit, :shared_runners_minutes_limit=, delegate :shared_runners_minutes_limit, :shared_runners_minutes_limit=,
:extra_shared_runners_minutes_limit, :extra_shared_runners_minutes_limit=, :extra_shared_runners_minutes_limit, :extra_shared_runners_minutes_limit=,
to: :namespace to: :namespace
delegate :provisioned_by_group, :provisioned_by_group=,
:provisioned_by_group_id, :provisioned_by_group_id=,
to: :user_detail, allow_nil: true
has_many :epics, foreign_key: :author_id has_many :epics, foreign_key: :author_id
has_many :requirements, foreign_key: :author_id, inverse_of: :author, class_name: 'RequirementsManagement::Requirement' has_many :requirements, foreign_key: :author_id, inverse_of: :author, class_name: 'RequirementsManagement::Requirement'
......
# frozen_string_literal: true
module EE
module UserDetail
extend ActiveSupport::Concern
prepended do
belongs_to :provisioned_by_group, class_name: 'Group', optional: true, inverse_of: :provisioned_user_details
end
end
end
...@@ -62,6 +62,8 @@ module EE ...@@ -62,6 +62,8 @@ module EE
build_scim_identity(user) build_scim_identity(user)
identity_params[:provider] = GROUP_SAML_PROVIDER identity_params[:provider] = GROUP_SAML_PROVIDER
user.provisioned_by_group_id = params[:group_id]
super super
end end
......
...@@ -24,6 +24,8 @@ RSpec.describe Group do ...@@ -24,6 +24,8 @@ RSpec.describe Group do
it { is_expected.to have_many(:saml_group_links) } it { is_expected.to have_many(:saml_group_links) }
it { is_expected.to have_many(:epics) } it { is_expected.to have_many(:epics) }
it { is_expected.to have_many(:epic_boards).inverse_of(:group) } it { is_expected.to have_many(:epic_boards).inverse_of(:group) }
it { is_expected.to have_many(:provisioned_user_details).inverse_of(:provisioned_by_group) }
it { is_expected.to have_many(:provisioned_users) }
it_behaves_like 'model with wiki' do it_behaves_like 'model with wiki' do
let(:container) { create(:group, :nested, :wiki_repo) } let(:container) { create(:group, :nested, :wiki_repo) }
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe UserDetail do
it { is_expected.to belong_to(:provisioned_by_group) }
end
...@@ -208,12 +208,16 @@ RSpec.describe API::Scim do ...@@ -208,12 +208,16 @@ RSpec.describe API::Scim do
it 'created the identity' do it 'created the identity' do
expect(group.scim_identities.with_extern_uid('test_uid').first).not_to be_nil expect(group.scim_identities.with_extern_uid('test_uid').first).not_to be_nil
end end
it 'marks the user as provisioned by the group' do
expect(new_user.provisioned_by_group).to eq(group)
end
end end
context 'existing user' do context 'existing user' do
before do let(:old_user) { create(:user, email: 'work@example.com') }
old_user = create(:user, email: 'work@example.com')
before do
create(:scim_identity, user: old_user, group: group, extern_uid: 'test_uid') create(:scim_identity, user: old_user, group: group, extern_uid: 'test_uid')
group.add_guest(old_user) group.add_guest(old_user)
...@@ -227,6 +231,10 @@ RSpec.describe API::Scim do ...@@ -227,6 +231,10 @@ RSpec.describe API::Scim do
it 'has the user external ID' do it 'has the user external ID' do
expect(json_response['id']).to eq('test_uid') expect(json_response['id']).to eq('test_uid')
end end
it 'does not mark the user as provisioned' do
expect(old_user.reload.provisioned_by_group).to be_nil
end
end end
it_behaves_like 'storing arguments in the application context' do it_behaves_like 'storing arguments in the application context' do
......
...@@ -30,19 +30,26 @@ RSpec.describe Users::BuildService do ...@@ -30,19 +30,26 @@ RSpec.describe Users::BuildService do
end end
context 'with scim identity' do context 'with scim identity' do
let_it_be(:group) { create(:group) }
let_it_be(:scim_identity_params) { { extern_uid: 'uid', provider: 'group_scim', group_id: group.id } }
before do before do
params.merge!(scim_identity_params) params.merge!(scim_identity_params)
end end
let_it_be(:scim_identity_params) { { extern_uid: 'uid', provider: 'group_scim', group_id: 1 } }
it 'passes allowed attributes to both scim and saml identity' do it 'passes allowed attributes to both scim and saml identity' do
scim_identity_params.delete(:provider) scim_params = scim_identity_params.dup
scim_params.delete(:provider)
expect(ScimIdentity).to receive(:new).with(hash_including(scim_identity_params)).and_call_original expect(ScimIdentity).to receive(:new).with(hash_including(scim_params)).and_call_original
expect(Identity).to receive(:new).with(hash_including(identity_params)).and_call_original expect(Identity).to receive(:new).with(hash_including(identity_params)).and_call_original
service.execute service.execute
end end
it 'marks the user as provisioned by group' do
expect(service.execute.provisioned_by_group_id).to eq(group.id)
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment