Commit 4cc86e3c authored by Gabriel Mazetto's avatar Gabriel Mazetto

Merge branch 'dblessing-remove-scim-identities-feature-flag' into 'master'

Remove `scim_identities` feature flag

See merge request gitlab-org/gitlab!43458
parents 966edcc7 ab9cb62c
...@@ -21,22 +21,12 @@ class ScimFinder ...@@ -21,22 +21,12 @@ class ScimFinder
private private
def scim_identities_enabled?
strong_memoize(:scim_identities_enabled) do
::EE::Gitlab::Scim::Feature.scim_identities_enabled?(group)
end
end
def null_identity def null_identity
return ScimIdentity.none if scim_identities_enabled? ScimIdentity.none
Identity.none
end end
def all_identities def all_identities
return group.scim_identities if scim_identities_enabled? group.scim_identities
saml_provider.identities
end end
def unfiltered?(params) def unfiltered?(params)
...@@ -63,9 +53,7 @@ class ScimFinder ...@@ -63,9 +53,7 @@ class ScimFinder
end end
def by_extern_uid(extern_uid) def by_extern_uid(extern_uid)
return group.scim_identities.with_extern_uid(extern_uid) if scim_identities_enabled? group.scim_identities.with_extern_uid(extern_uid)
Identity.where_group_saml_uid(saml_provider, extern_uid)
end end
def eq_filter_on_username?(parser) def eq_filter_on_username?(parser)
...@@ -79,9 +67,7 @@ class ScimFinder ...@@ -79,9 +67,7 @@ class ScimFinder
user ||= User.find_by_any_email(username) || User.find_by_username(email_local_part(username)) user ||= User.find_by_any_email(username) || User.find_by_username(email_local_part(username))
end end
return group.scim_identities.for_user(user) if scim_identities_enabled? group.scim_identities.for_user(user)
saml_provider.identities.for_user(user)
end end
def email?(email) def email?(email)
......
---
name: scim_identities
introduced_by_url:
rollout_issue_url:
group:
type: development
default_enabled: true
...@@ -97,23 +97,12 @@ module API ...@@ -97,23 +97,12 @@ module API
def find_user_identity(group, extern_uid) def find_user_identity(group, extern_uid)
return unless group.saml_provider return unless group.saml_provider
return group.scim_identities.with_extern_uid(extern_uid).first if scim_identities_enabled?
GroupSamlIdentityFinder.find_by_group_and_uid(group: group, uid: extern_uid) group.scim_identities.with_extern_uid(extern_uid).first
end
def scim_identities_enabled?
strong_memoize(:scim_identities_enabled) do
::EE::Gitlab::Scim::Feature.scim_identities_enabled?(@group)
end
end end
def deprovision(identity) def deprovision(identity)
if scim_identities_enabled? ::EE::Gitlab::Scim::DeprovisionService.new(identity).execute
::EE::Gitlab::Scim::DeprovisionService.new(identity).execute
else
GroupSaml::Identity::DestroyService.new(identity).execute(transactional: true)
end
true true
rescue => e rescue => e
......
# frozen_string_literal: true
module EE
module Gitlab
module Scim
class Feature
def self.scim_identities_enabled?(group)
::Feature.enabled?(:scim_identities, group, default_enabled: true)
end
end
end
end
end
...@@ -50,24 +50,8 @@ module EE ...@@ -50,24 +50,8 @@ module EE
error_response(objects: [user, identity, member]) error_response(objects: [user, identity, member])
end end
def scim_identities_enabled?
strong_memoize(:scim_identities_enabled) do
::EE::Gitlab::Scim::Feature.scim_identities_enabled?(@group)
end
end
def identity_provider
strong_memoize(:identity_provider) do
next ::Users::BuildService::GROUP_SCIM_PROVIDER if scim_identities_enabled?
::Users::BuildService::GROUP_SAML_PROVIDER
end
end
def identity def identity
strong_memoize(:identity) do strong_memoize(:identity) do
next saml_identity unless scim_identities_enabled?
identity = @group.scim_identities.with_extern_uid(@parsed_hash[:extern_uid]).first identity = @group.scim_identities.with_extern_uid(@parsed_hash[:extern_uid]).first
next identity if identity next identity if identity
...@@ -75,14 +59,8 @@ module EE ...@@ -75,14 +59,8 @@ module EE
end end
end end
def saml_identity
::Identity.with_extern_uid(identity_provider, @parsed_hash[:extern_uid]).first
end
def user def user
strong_memoize(:user) do strong_memoize(:user) do
next build_user unless scim_identities_enabled?
user = ::User.find_by_any_email(@parsed_hash[:email]) user = ::User.find_by_any_email(@parsed_hash[:email])
next user if user next user if user
...@@ -127,7 +105,7 @@ module EE ...@@ -127,7 +105,7 @@ module EE
hash[:skip_confirmation] = SKIP_EMAIL_CONFIRMATION hash[:skip_confirmation] = SKIP_EMAIL_CONFIRMATION
hash[:saml_provider_id] = @group.saml_provider.id hash[:saml_provider_id] = @group.saml_provider.id
hash[:group_id] = @group.id hash[:group_id] = @group.id
hash[:provider] = identity_provider hash[:provider] = ::Users::BuildService::GROUP_SCIM_PROVIDER
hash[:username] = valid_username hash[:username] = valid_username
hash[:password] = hash[:password_confirmation] = random_password hash[:password] = hash[:password_confirmation] = random_password
hash[:password_automatically_set] = PASSWORD_AUTOMATICALLY_SET hash[:password_automatically_set] = PASSWORD_AUTOMATICALLY_SET
...@@ -161,7 +139,7 @@ module EE ...@@ -161,7 +139,7 @@ module EE
end end
def create_identity_only? def create_identity_only?
scim_identities_enabled? && existing_user? && existing_member?(user) existing_user? && existing_member?(user)
end end
def existing_identity_and_member? def existing_identity_and_member?
......
...@@ -10,15 +10,7 @@ RSpec.describe ScimFinder do ...@@ -10,15 +10,7 @@ RSpec.describe ScimFinder do
describe '#search' do describe '#search' do
context 'without a SAML provider' do context 'without a SAML provider' do
it 'returns an empty identity relation when scim_identities is disabled' do it 'returns an empty scim identity relation' do
stub_feature_flags(scim_identities: false)
expect(finder.search(unused_params)).to eq Identity.none
end
it 'returns an empty scim identity relation when scim_identities is enabled' do
stub_feature_flags(scim_identities: true)
expect(finder.search(unused_params)).to eq ScimIdentity.none expect(finder.search(unused_params)).to eq ScimIdentity.none
end end
end end
...@@ -28,15 +20,7 @@ RSpec.describe ScimFinder do ...@@ -28,15 +20,7 @@ RSpec.describe ScimFinder do
create(:saml_provider, group: group, enabled: false) create(:saml_provider, group: group, enabled: false)
end end
it 'returns an empty identity relation when scim_identities is disabled' do it 'returns an empty scim identity relation' do
stub_feature_flags(scim_identities: false)
expect(finder.search(unused_params)).to eq Identity.none
end
it 'returns an empty scim identity relation when scim_identities is enabled' do
stub_feature_flags(scim_identities: true)
expect(finder.search(unused_params)).to eq ScimIdentity.none expect(finder.search(unused_params)).to eq ScimIdentity.none
end end
end end
...@@ -45,67 +29,41 @@ RSpec.describe ScimFinder do ...@@ -45,67 +29,41 @@ RSpec.describe ScimFinder do
let_it_be(:saml_provider) { create(:saml_provider, group: group) } let_it_be(:saml_provider) { create(:saml_provider, group: group) }
context 'with an eq filter' do context 'with an eq filter' do
shared_examples 'valid lookups' do let_it_be(:user) { create(:user, username: 'foo', email: 'bar@example.com') }
it 'allows identity lookup by id/externalId' do let_it_be(:id) { create(:scim_identity, group: group, user: user) }
expect(finder.search(filter: "id eq #{id.extern_uid}")).to be_a ActiveRecord::Relation
expect(finder.search(filter: "id eq #{id.extern_uid}").first).to eq id
expect(finder.search(filter: "externalId eq #{id.extern_uid}").first).to eq id
end
it 'allows lookup by userName' do
expect(finder.search(filter: "userName eq \"#{id.user.username}\"").first).to eq id
end
context 'allows lookup by userName' do it 'allows identity lookup by id/externalId' do
it 'finds user by an email address' do expect(finder.search(filter: "id eq #{id.extern_uid}")).to be_a ActiveRecord::Relation
expect(finder.search(filter: "userName eq #{id.user.email}").first).to eq id expect(finder.search(filter: "id eq #{id.extern_uid}").first).to eq id
end expect(finder.search(filter: "externalId eq #{id.extern_uid}").first).to eq id
end
it 'finds user by using local part of email address as username' do
email = "#{id.user.username}@example.com"
expect(finder.search(filter: "userName eq #{email}").first).to eq id
end
it 'finds user by username' do
expect(finder.search(filter: "userName eq \"#{id.user.username}\"").first).to eq id
end
it 'finds user by extern_uid' do it 'allows lookup by userName' do
expect(finder.search(filter: "userName eq \"#{id.extern_uid}\"").first).to eq id expect(finder.search(filter: "userName eq \"#{id.user.username}\"").first).to eq id
end
end
end end
context 'when scim_identities is disabled' do context 'allows lookup by userName' do
before do it 'finds user by an email address' do
stub_feature_flags(scim_identities: false) expect(finder.search(filter: "userName eq #{id.user.email}").first).to eq id
end end
let_it_be(:id) { create(:group_saml_identity, saml_provider: saml_provider) }
it_behaves_like 'valid lookups' it 'finds user by using local part of email address as username' do
end email = "#{id.user.username}@example.com"
expect(finder.search(filter: "userName eq #{email}").first).to eq id
end
context 'when scim_identities is enabled' do it 'finds user by username' do
before do expect(finder.search(filter: "userName eq \"#{id.user.username}\"").first).to eq id
stub_feature_flags(scim_identities: true)
end end
let_it_be(:user) { create(:user, username: 'foo', email: 'bar@example.com') }
let_it_be(:id) { create(:scim_identity, group: group, user: user) }
it_behaves_like 'valid lookups' it 'finds user by extern_uid' do
expect(finder.search(filter: "userName eq \"#{id.extern_uid}\"").first).to eq id
end
end end
end end
context 'with no filter' do context 'with no filter' do
it 'returns all related identities when scim_identities is disabled' do it 'returns all related scim_identities' do
stub_feature_flags(scim_identities: false)
create_list(:group_saml_identity, 2, saml_provider: saml_provider)
expect(finder.search({}).count).to eq 2
end
it 'returns all related identities when scim_identities is enabled' do
stub_feature_flags(scim_identities: true)
create_list(:scim_identity, 4, group: group) create_list(:scim_identity, 4, group: group)
expect(finder.search({}).count).to eq 4 expect(finder.search({}).count).to eq 4
......
...@@ -27,89 +27,7 @@ RSpec.describe ::EE::Gitlab::Scim::ProvisioningService do ...@@ -27,89 +27,7 @@ RSpec.describe ::EE::Gitlab::Scim::ProvisioningService do
end end
end end
shared_examples 'scim provisioning' do shared_examples 'existing user' do
context 'valid params' do
let_it_be(:service_params) do
{
email: 'work@example.com',
name: 'Test Name',
extern_uid: 'test_uid',
username: 'username'
}
end
def user
User.find_by(email: service_params[:email])
end
it_behaves_like 'success response'
it 'creates the user' do
expect { service.execute }.to change { User.count }.by(1)
end
it 'creates the group member' do
expect { service.execute }.to change { GroupMember.count }.by(1)
end
it 'creates the correct user attributes' do
service.execute
expect(user).to be_a(User)
end
context 'access level of created group member' do
let!(:saml_provider) do
create(:saml_provider, group: group, default_membership_role: Gitlab::Access::DEVELOPER)
end
it 'sets the access level of the member as specified in saml_provider' do
service.execute
access_level = group.group_member(user).access_level
expect(access_level).to eq(Gitlab::Access::DEVELOPER)
end
end
it 'user record requires confirmation' do
service.execute
expect(user).to be_present
expect(user).not_to be_confirmed
end
context 'when the current minimum password length is different from the default minimum password length' do
before do
stub_application_setting minimum_password_length: 21
end
it 'creates the user' do
expect { service.execute }.to change { User.count }.by(1)
end
end
end
context 'invalid params' do
let_it_be(:service_params) do
{
email: 'work@example.com',
name: 'Test Name',
extern_uid: 'test_uid'
}
end
it 'fails with error' do
expect(service.execute.status).to eq(:error)
end
it 'fails with missing params' do
expect(service.execute.message).to eq("Missing params: [:username]")
end
end
end
shared_examples 'existing user when scim identities are enabled' do
it 'does not create a new user' do it 'does not create a new user' do
expect { service.execute }.not_to change { User.count } expect { service.execute }.not_to change { User.count }
end end
...@@ -125,13 +43,7 @@ RSpec.describe ::EE::Gitlab::Scim::ProvisioningService do ...@@ -125,13 +43,7 @@ RSpec.describe ::EE::Gitlab::Scim::ProvisioningService do
end end
end end
context 'when scim_identities is disabled' do context 'valid params' do
before do
stub_feature_flags(scim_identities: false)
end
it_behaves_like 'scim provisioning'
let_it_be(:service_params) do let_it_be(:service_params) do
{ {
email: 'work@example.com', email: 'work@example.com',
...@@ -141,93 +53,132 @@ RSpec.describe ::EE::Gitlab::Scim::ProvisioningService do ...@@ -141,93 +53,132 @@ RSpec.describe ::EE::Gitlab::Scim::ProvisioningService do
} }
end end
it 'creates the SAML identity' do def user
expect { service.execute }.to change { Identity.count }.by(1) User.find_by(email: service_params[:email])
end end
it 'does not create the SCIM identity' do it_behaves_like 'success response'
expect { service.execute }.not_to change { ScimIdentity.count }
it 'creates the user' do
expect { service.execute }.to change { User.count }.by(1)
end end
context 'existing user' do it 'creates the group member' do
before do expect { service.execute }.to change { GroupMember.count }.by(1)
create(:user, email: 'work@example.com') end
end
it 'does not create a new user' do it 'creates the correct user attributes' do
expect { service.execute }.not_to change { User.count } service.execute
expect(user).to be_a(User)
end
context 'access level of created group member' do
let!(:saml_provider) do
create(:saml_provider, group: group, default_membership_role: Gitlab::Access::DEVELOPER)
end end
it 'fails with conflict' do it 'sets the access level of the member as specified in saml_provider' do
expect(service.execute.status).to eq(:conflict) service.execute
access_level = group.group_member(user).access_level
expect(access_level).to eq(Gitlab::Access::DEVELOPER)
end end
end end
end
context 'when scim_identities is enabled' do it 'user record requires confirmation' do
before do service.execute
stub_feature_flags(scim_identities: true)
expect(user).to be_present
expect(user).not_to be_confirmed
end end
it_behaves_like 'scim provisioning' context 'when the current minimum password length is different from the default minimum password length' do
before do
stub_application_setting minimum_password_length: 21
end
it 'creates the user' do
expect { service.execute }.to change { User.count }.by(1)
end
end
end
context 'invalid params' do
let_it_be(:service_params) do let_it_be(:service_params) do
{ {
email: 'work@example.com', email: 'work@example.com',
name: 'Test Name', name: 'Test Name',
extern_uid: 'test_uid', extern_uid: 'test_uid'
username: 'username'
} }
end end
it 'creates the SCIM identity' do it 'fails with error' do
expect { service.execute }.to change { ScimIdentity.count }.by(1) expect(service.execute.status).to eq(:error)
end end
it 'creates the SAML identity' do it 'fails with missing params' do
expect { service.execute }.to change { Identity.count }.by(1) expect(service.execute.message).to eq("Missing params: [:username]")
end end
end
context 'existing user' do let_it_be(:service_params) do
before do {
create(:email, user: user, email: 'work@example.com') email: 'work@example.com',
end name: 'Test Name',
let(:user) { create(:user) } extern_uid: 'test_uid',
username: 'username'
context 'when user is not an existing group member' do }
it_behaves_like 'existing user when scim identities are enabled' end
it 'creates the group member' do it 'creates the SCIM identity' do
expect { service.execute }.to change { GroupMember.count }.by(1) expect { service.execute }.to change { ScimIdentity.count }.by(1)
end end
context 'with enforced SSO' do it 'creates the SAML identity' do
let(:enforced_sso) { true } expect { service.execute }.to change { Identity.count }.by(1)
end
it 'does not create the group member' do context 'for an existing user' do
expect { service.execute }.not_to change { GroupMember.count } before do
end create(:email, user: user, email: 'work@example.com')
end
let(:user) { create(:user) }
it 'does not create the SAML identity' do context 'when user is not a group member' do
expect { service.execute }.not_to change { Identity.count } it_behaves_like 'existing user'
end
it 'does not create the SCIM identity' do it 'creates the group member' do
expect { service.execute }.not_to change { ScimIdentity.count } expect { service.execute }.to change { GroupMember.count }.by(1)
end
end
end end
context 'when user is an existing group member' do context 'with enforced SSO' do
before do let(:enforced_sso) { true }
group.add_guest(user)
end
it_behaves_like 'existing user when scim identities are enabled'
it 'does not create the group member' do it 'does not create the group member' do
expect { service.execute }.not_to change { GroupMember.count } expect { service.execute }.not_to change { GroupMember.count }
end end
it 'does not create the SAML identity' do
expect { service.execute }.not_to change { Identity.count }
end
it 'does not create the SCIM identity' do
expect { service.execute }.not_to change { ScimIdentity.count }
end
end
end
context 'when user is an existing group member' do
before do
group.add_guest(user)
end
it_behaves_like 'existing user'
it 'does not create the group member' do
expect { service.execute }.not_to change { GroupMember.count }
end end
end end
end end
......
...@@ -5,14 +5,15 @@ require 'spec_helper' ...@@ -5,14 +5,15 @@ require 'spec_helper'
RSpec.describe API::Scim do RSpec.describe API::Scim do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:scim_token) { create(:scim_oauth_access_token, group: group) } let(:scim_token) { create(:scim_oauth_access_token, group: group) }
let(:group) { identity.group }
let_it_be(:password) { 'secret_pass' } let_it_be(:password) { 'secret_pass' }
let_it_be(:access_token) { 'secret_token' } let_it_be(:access_token) { 'secret_token' }
before do before do
stub_licensed_features(group_allowed_email_domains: true, group_saml: true) stub_licensed_features(group_allowed_email_domains: true, group_saml: true)
group.add_owner(user) group.add_owner(user)
create(:saml_provider, group: group, default_membership_role: Gitlab::Access::DEVELOPER)
end end
def scim_api(url, token: true) def scim_api(url, token: true)
...@@ -62,6 +63,7 @@ RSpec.describe API::Scim do ...@@ -62,6 +63,7 @@ RSpec.describe API::Scim do
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response['Resources']).not_to be_empty expect(json_response['Resources']).not_to be_empty
expect(json_response['totalResults']).to eq(ScimIdentity.count)
end end
it 'responds with an error for unsupported filters' do it 'responds with an error for unsupported filters' do
...@@ -202,6 +204,29 @@ RSpec.describe API::Scim do ...@@ -202,6 +204,29 @@ RSpec.describe API::Scim do
it 'created the member with access level set in saml_provider' do it 'created the member with access level set in saml_provider' do
expect(member.access_level).to eq(::Gitlab::Access::DEVELOPER) expect(member.access_level).to eq(::Gitlab::Access::DEVELOPER)
end end
it 'created the identity' do
expect(group.scim_identities.with_extern_uid('test_uid').first).not_to be_nil
end
end
context 'existing user' do
before do
old_user = create(:user, email: 'work@example.com')
create(:scim_identity, user: old_user, group: group, extern_uid: 'test_uid')
group.add_guest(old_user)
post scim_api("scim/v2/groups/#{group.full_path}/Users?params=#{post_params}")
end
it 'responds with 201' do
expect(response).to have_gitlab_http_status(:created)
end
it 'has the user external ID' do
expect(json_response['id']).to eq('test_uid')
end
end end
it_behaves_like 'storing arguments in the application context' do it_behaves_like 'storing arguments in the application context' do
...@@ -266,6 +291,10 @@ RSpec.describe API::Scim do ...@@ -266,6 +291,10 @@ RSpec.describe API::Scim do
end end
describe 'PATCH api/scim/v2/groups/:group/Users/:id' do describe 'PATCH api/scim/v2/groups/:group/Users/:id' do
def call_patch_api(params)
patch scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}")
end
it_behaves_like 'SCIM token authenticated' it_behaves_like 'SCIM token authenticated'
it 'responds with 404 for a non existent group' do it 'responds with 404 for a non existent group' do
...@@ -281,7 +310,7 @@ RSpec.describe API::Scim do ...@@ -281,7 +310,7 @@ RSpec.describe API::Scim do
params = { Operations: [{ 'op': 'Replace', 'path': 'id', 'value': 'new_uid' }] }.to_query params = { Operations: [{ 'op': 'Replace', 'path': 'id', 'value': 'new_uid' }] }.to_query
patch scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}") call_patch_api(params)
expect(response).to have_gitlab_http_status(:not_found) expect(response).to have_gitlab_http_status(:not_found)
end end
...@@ -292,12 +321,39 @@ RSpec.describe API::Scim do ...@@ -292,12 +321,39 @@ RSpec.describe API::Scim do
expect(response).to have_gitlab_http_status(:not_found) expect(response).to have_gitlab_http_status(:not_found)
end end
it 'deactivates the scim_identity' do
params = { Operations: [{ 'op': 'Replace', 'path': 'active', 'value': 'False' }] }.to_query
call_patch_api(params)
expect(identity.reload.active).to be false
end
context 'Reprovision user' do
let_it_be(:params) { { Operations: [{ 'op': 'Replace', 'path': 'active', 'value': 'true' }] }.to_query }
it 'activates the scim_identity' do
identity.update(active: false)
call_patch_api(params)
expect(identity.reload.active).to be true
end
it 'does not call reprovision service when identity is already active' do
expect(::EE::Gitlab::Scim::ReprovisionService).not_to receive(:new)
expect(::Users::UpdateService).to receive(:new).and_call_original
call_patch_api(params)
end
end
context 'existing user' do context 'existing user' do
context 'extern UID' do context 'extern UID' do
before do before do
params = { Operations: [{ 'op': 'Replace', 'path': 'id', 'value': 'new_uid' }] }.to_query params = { Operations: [{ 'op': 'Replace', 'path': 'id', 'value': 'new_uid' }] }.to_query
patch scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}") call_patch_api(params)
end end
it 'responds with 204' do it 'responds with 204' do
...@@ -313,7 +369,7 @@ RSpec.describe API::Scim do ...@@ -313,7 +369,7 @@ RSpec.describe API::Scim do
before do before do
params = { Operations: [{ 'op': 'Replace', 'path': 'name.formatted', 'value': 'new_name' }] }.to_query params = { Operations: [{ 'op': 'Replace', 'path': 'name.formatted', 'value': 'new_name' }] }.to_query
patch scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}") call_patch_api(params)
end end
it 'responds with 204' do it 'responds with 204' do
...@@ -334,7 +390,7 @@ RSpec.describe API::Scim do ...@@ -334,7 +390,7 @@ RSpec.describe API::Scim do
before do before do
params = { Operations: [{ 'op': 'Replace', 'path': 'emails[type eq "work"].value', 'value': 'new@mail.com' }] }.to_query params = { Operations: [{ 'op': 'Replace', 'path': 'emails[type eq "work"].value', 'value': 'new@mail.com' }] }.to_query
patch scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}") call_patch_api(params)
end end
it 'updates the email' do it 'updates the email' do
...@@ -352,7 +408,7 @@ RSpec.describe API::Scim do ...@@ -352,7 +408,7 @@ RSpec.describe API::Scim do
params = { Operations: [{ 'op': 'Replace', 'path': 'emails[type eq "work"].value', 'value': 'new@mail.com' }] }.to_query params = { Operations: [{ 'op': 'Replace', 'path': 'emails[type eq "work"].value', 'value': 'new@mail.com' }] }.to_query
patch scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}") call_patch_api(params)
end end
it 'does not update a duplicated email' do it 'does not update a duplicated email' do
...@@ -380,355 +436,17 @@ RSpec.describe API::Scim do ...@@ -380,355 +436,17 @@ RSpec.describe API::Scim do
it 'responds with an empty response' do it 'responds with an empty response' do
expect(response.body).to eq('') expect(response.body).to eq('')
end end
end
it 'responds with 404 if there is no user' do
delete scim_api("scim/v2/groups/#{group.full_path}/Users/123")
expect(response).to have_gitlab_http_status(:not_found)
end
it 'responds with 404 for a non existent group' do
delete scim_api("scim/v2/groups/#{non_existing_record_id}/Users/#{identity.extern_uid}")
expect(response).to have_gitlab_http_status(:not_found)
end
it 'responds with 404 for a group with no SAML SSO configuration' do
group.saml_provider.destroy!
delete scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}")
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
shared_examples 'SCIM API endpoints with scim_identities disabled' do
describe 'GET api/scim/v2/groups/:group/Users' do
it 'responds with paginated users when there is no filter' do
get scim_api("scim/v2/groups/#{group.full_path}/Users")
expect(json_response['totalResults']).to eq(Identity.count)
end
it 'responds with 404 for a non existent group' do
get scim_api("scim/v2/groups/#{non_existing_record_id}/Users")
expect(response).to have_gitlab_http_status(:not_found)
end
it 'responds with 404 for a group with no SAML SSO configuration' do
group.saml_provider.destroy!
get scim_api("scim/v2/groups/#{group.full_path}/Users")
expect(response).to have_gitlab_http_status(:not_found)
end
end
describe 'POST api/scim/v2/groups/:group/Users' do
let_it_be(:post_params) do
{
externalId: 'test_uid',
active: nil,
userName: 'username',
emails: [{ primary: true, type: 'work', value: 'work@example.com' }],
name: { formatted: 'Test Name', familyName: 'Name', givenName: 'Test' },
access_token: access_token,
password: password
}.to_query
end
it 'responds with 404 for a non existent group' do
post scim_api("scim/v2/groups/#{non_existing_record_id}/Users?params=#{post_params}")
expect(response).to have_gitlab_http_status(:not_found)
end
it 'responds with 404 for a group with no SAML SSO configuration' do
group.saml_provider.destroy!
post scim_api("scim/v2/groups/#{group.full_path}/Users?params=#{post_params}")
expect(response).to have_gitlab_http_status(:not_found)
end
context 'without an existing user' do
let(:new_user) { User.find_by_email('work@example.com') }
let(:member) { GroupMember.find_by(user: new_user, group: group) }
before do
post scim_api("scim/v2/groups/#{group.full_path}/Users?params=#{post_params}")
end
it 'created the identity' do
expect(Identity.find_by_extern_uid(:group_saml, 'test_uid')).not_to be_nil
end
it 'has the right saml provider' do
identity = Identity.find_by_extern_uid(:group_saml, 'test_uid')
expect(identity.saml_provider_id).to eq(group.saml_provider.id)
end
end
context 'existing user with group saml identity' do
before do
old_user = create(:user, email: 'work@example.com')
create(:group_saml_identity, user: old_user, extern_uid: 'test_uid')
group.add_guest(old_user)
post scim_api("scim/v2/groups/#{group.full_path}/Users?params=#{post_params}")
end
it 'responds with 201' do
expect(response).to have_gitlab_http_status(:created)
end
it 'has the user external ID' do
expect(json_response['id']).to eq('test_uid')
end
end
context 'existing user without a group saml identity' do
before do
create(:user, email: 'work@example.com')
post scim_api("scim/v2/groups/#{group.full_path}/Users?params=#{post_params}")
end
it 'responds with 409' do
expect(response).to have_gitlab_http_status(:conflict)
end
it_behaves_like 'filtered params in errors'
end
end
describe 'PATCH api/scim/v2/groups/:group/Users/:id' do
context 'Remove user' do
shared_examples 'remove user' do
it 'responds with 204' do
expect(response).to have_gitlab_http_status(:no_content)
end
it 'removes the identity link' do
expect { identity.reload }.to raise_error(ActiveRecord::RecordNotFound)
end
end
context 'when path key is present' do
before do
params = { Operations: [{ 'op': 'Replace', 'path': 'active', 'value': 'False' }] }.to_query
patch scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}")
end
it_behaves_like 'remove user'
end
context 'when the path key is not present' do
before do
params = { Operations: [{ 'op': 'replace', 'value': { 'active': false } }] }.to_query
patch scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}")
end
it_behaves_like 'remove user'
end
it 'responds with 404 for a non existent group' do
params = { Operations: [{ 'op': 'Replace', 'path': 'id', 'value': 'new_uid' }] }.to_query
patch scim_api("scim/v2/groups/#{non_existing_record_id}/Users/#{identity.extern_uid}?#{params}")
expect(response).to have_gitlab_http_status(:not_found)
end
it 'responds with 404 for a group with no SAML SSO configuration' do
group.saml_provider.destroy!
params = { Operations: [{ 'op': 'Replace', 'path': 'id', 'value': 'new_uid' }] }.to_query
patch scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}")
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
describe 'DELETE /scim/v2/groups/:group/Users/:id' do
context 'existing user' do
it 'removes the identity link' do
delete scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}")
expect { identity.reload }.to raise_error(ActiveRecord::RecordNotFound)
end
end
it 'responds with 404 for a non existent group' do
delete scim_api("scim/v2/groups/#{non_existing_record_id}/Users/#{identity.extern_uid}")
expect(response).to have_gitlab_http_status(:not_found)
end
it 'responds with 404 for a group with no SAML SSO configuration' do
group.saml_provider.destroy!
delete scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}")
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
shared_examples 'SCIM API endpoints with scim_identities enabled' do
describe 'GET api/scim/v2/groups/:group/Users' do
it 'responds with paginated users when there is no filter' do
get scim_api("scim/v2/groups/#{group.full_path}/Users")
expect(json_response['totalResults']).to eq(ScimIdentity.count)
end
it 'responds with 404 for a non existent group' do
get scim_api("scim/v2/groups/#{non_existing_record_id}/Users")
expect(response).to have_gitlab_http_status(:not_found)
end
it 'responds with 404 for a group with no SAML SSO configuration' do
group.saml_provider.destroy!
get scim_api("scim/v2/groups/#{group.full_path}/Users")
expect(response).to have_gitlab_http_status(:not_found)
end
end
describe 'POST api/scim/v2/groups/:group/Users' do
let_it_be(:post_params) do
{
externalId: 'test_uid',
active: nil,
userName: 'username',
emails: [{ primary: true, type: 'work', value: 'work@example.com' }],
name: { formatted: 'Test Name', familyName: 'Name', givenName: 'Test' }
}.to_query
end
it 'responds with 404 for a non existent group' do
post scim_api("scim/v2/groups/#{non_existing_record_id}/Users?params=#{post_params}")
expect(response).to have_gitlab_http_status(:not_found)
end
it 'responds with 404 for a group with no SAML SSO configuration' do
group.saml_provider.destroy!
post scim_api("scim/v2/groups/#{group.full_path}/Users?params=#{post_params}")
expect(response).to have_gitlab_http_status(:not_found)
end
context 'without an existing user' do
let(:new_user) { User.find_by_email('work@example.com') }
let(:member) { GroupMember.find_by(user: new_user, group: group) }
before do
post scim_api("scim/v2/groups/#{group.full_path}/Users?params=#{post_params}")
end
it 'created the identity' do
expect(group.scim_identities.with_extern_uid('test_uid').first).not_to be_nil
end
end
context 'existing user' do
before do
old_user = create(:user, email: 'work@example.com')
create(:scim_identity, user: old_user, group: group, extern_uid: 'test_uid')
group.add_guest(old_user)
post scim_api("scim/v2/groups/#{group.full_path}/Users?params=#{post_params}")
end
it 'responds with 201' do
expect(response).to have_gitlab_http_status(:created)
end
it 'has the user external ID' do
expect(json_response['id']).to eq('test_uid')
end
end
end
describe 'PATCH api/scim/v2/groups/:group/Users/:id' do
def call_patch_api
patch scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}")
end
context 'Remove user' do
it 'deactivates the scim_identity' do
params = { Operations: [{ 'op': 'Replace', 'path': 'active', 'value': 'False' }] }.to_query
patch scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}")
it 'deactivates the identity' do
expect(identity.reload.active).to be false expect(identity.reload.active).to be false
end end
end end
context 'Reprovision user' do it 'responds with 404 if there is no user' do
def call_patch_api delete scim_api("scim/v2/groups/#{group.full_path}/Users/123")
patch scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}")
end
let_it_be(:params) { { Operations: [{ 'op': 'Replace', 'path': 'active', 'value': 'true' }] }.to_query }
it 'activates the scim_identity' do
identity.update(active: false)
call_patch_api
expect(identity.reload.active).to be true
end
it 'does not call reprovision service when identity is already active' do
expect(::EE::Gitlab::Scim::ReprovisionService).not_to receive(:new)
expect(::Users::UpdateService).to receive(:new).and_call_original
call_patch_api
end
end
it 'responds with 404 for a non existent group' do
params = { Operations: [{ 'op': 'Replace', 'path': 'id', 'value': 'new_uid' }] }.to_query
patch scim_api("scim/v2/groups/#{non_existing_record_id}/Users/#{identity.extern_uid}?#{params}")
expect(response).to have_gitlab_http_status(:not_found)
end
it 'responds with 404 for a group with no SAML SSO configuration' do
group.saml_provider.destroy!
params = { Operations: [{ 'op': 'Replace', 'path': 'id', 'value': 'new_uid' }] }.to_query
patch scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}")
expect(response).to have_gitlab_http_status(:not_found) expect(response).to have_gitlab_http_status(:not_found)
end end
end
describe 'DELETE /scim/v2/groups/:group/Users/:id' do
context 'existing user' do
it 'deactivates the identity' do
delete scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}")
expect(identity.reload.active).to be false
end
end
it 'responds with 404 for a non existent group' do it 'responds with 404 for a non existent group' do
delete scim_api("scim/v2/groups/#{non_existing_record_id}/Users/#{identity.extern_uid}") delete scim_api("scim/v2/groups/#{non_existing_record_id}/Users/#{identity.extern_uid}")
...@@ -746,52 +464,15 @@ RSpec.describe API::Scim do ...@@ -746,52 +464,15 @@ RSpec.describe API::Scim do
end end
end end
context 'when scim_identities is disabled' do context 'user with an alphanumeric extern_uid' do
before do let(:identity) { create(:scim_identity, user: user, extern_uid: generate(:username)) }
stub_feature_flags(scim_identities: false)
end
let(:saml_provider) { create(:saml_provider, default_membership_role: Gitlab::Access::DEVELOPER) }
let(:group) { identity.saml_provider.group }
context 'user with an alphanumeric extern_uid' do
let(:identity) do
create(:group_saml_identity, user: user, extern_uid: generate(:username), saml_provider: saml_provider)
end
it_behaves_like 'SCIM API endpoints'
it_behaves_like 'SCIM API endpoints with scim_identities disabled'
end
context 'user with an email extern_uid' do
let(:identity) do
create(:group_saml_identity, user: user, extern_uid: user.email, saml_provider: saml_provider)
end
it_behaves_like 'SCIM API endpoints' it_behaves_like 'SCIM API endpoints'
it_behaves_like 'SCIM API endpoints with scim_identities disabled'
end
end end
context 'when scim_identities is enabled' do context 'user with an email extern_uid' do
before do let(:identity) { create(:scim_identity, user: user, extern_uid: user.email) }
stub_feature_flags(scim_identities: true)
create(:saml_provider, group: group, default_membership_role: Gitlab::Access::DEVELOPER)
end
let(:group) { identity.group }
context 'user with an alphanumeric extern_uid' do it_behaves_like 'SCIM API endpoints'
let(:identity) { create(:scim_identity, user: user, extern_uid: generate(:username)) }
it_behaves_like 'SCIM API endpoints'
it_behaves_like 'SCIM API endpoints with scim_identities enabled'
end
context 'user with an email extern_uid' do
let(:identity) { create(:scim_identity, user: user, extern_uid: user.email) }
it_behaves_like 'SCIM API endpoints'
it_behaves_like 'SCIM API endpoints with scim_identities enabled'
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