Merge branch...

Merge branch '5014-automatically-deprovision-users-when-removed-from-a-configured-identity-provider' into 'master'

Resolve "Deprovision and update users from a configured identity provider"

Closes #9154 and #5014

See merge request gitlab-org/gitlab-ee!9388

ee/app/finders/group_saml_identity_finder.rb

@@ -3,6 +3,14 @@
 class GroupSamlIdentityFinder
   attr_reader :user
 
+  # rubocop: disable CodeReuse/ActiveRecord
+  def self.find_by_group_and_uid(group:, uid:)
+    return unless group.saml_provider
+
+    group.saml_provider.identities.find_by(extern_uid: uid)
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
+
   def initialize(user:)
     @user = user
   end

ee/app/models/scim_oauth_access_token.rb

@@ -10,6 +10,13 @@ class ScimOauthAccessToken < ApplicationRecord
   validates :group, presence: true
   before_save :ensure_token
 
+  def self.token_matches_for_group?(token, group)
+    # Necessary to call `TokenAuthenticatableStrategies::Encrypted.find_token_authenticatable`
+    token = find_by_token(token)
+
+    token && (token.group_id == group.id)
+  end
+
   def as_entity_json
     ScimOauthAccessTokenEntity.new(self).as_json
   end

ee/app/services/group_saml/identity/destroy_service.rb

@@ -11,13 +11,19 @@ module GroupSaml
         @identity = identity
       end
 
-      def execute
-        identity.destroy!
-        remove_group_access
+      def execute(transactional: false)
+        with_transaction(transactional) do
+          identity.destroy!
+          remove_group_access
+        end
       end
 
       private
 
+      def with_transaction(transactional, &block)
+        transactional ? ::Identity.transaction { yield } : yield
+      end
+
       def remove_group_access
         return unless group_membership
         return if group.last_owner?(user)

ee/changelogs/unreleased/5014-automatically-deprovision-users-when-removed-from-a-configured-identity-provider.yml

+---
+title: Automatically deprovision and update users from a configured identity via SCIM
+merge_request: 9388
+author:
+type: added

ee/lib/api/scim.rb

+# frozen_string_literal: true
+
+module API
+  class Scim < Grape::API
+    prefix 'api/scim'
+    version 'v2'
+    content_type :json, 'application/scim+json'
+
+    namespace 'groups/:group' do
+      params do
+        requires :group, type: String
+      end
+
+      helpers do
+        def logger
+          API.logger
+        end
+
+        def destroy_identity(identity)
+          GroupSaml::Identity::DestroyService.new(identity).execute(transactional: true)
+
+          true
+        rescue => e
+          logger.error(identity: identity, error: e.class.name, message: e.message, source: "#{__FILE__}:#{__LINE__}")
+
+          false
+        end
+
+        def scim_not_found!(message:)
+          error!({ with: EE::Gitlab::Scim::NotFound }.merge(detail: message), 404)
+        end
+
+        def scim_error!(message:)
+          error!({ with: EE::Gitlab::Scim::Error }.merge(detail: message), 409)
+        end
+
+        def find_and_authenticate_group!(group_path)
+          group = find_group(group_path)
+
+          scim_not_found!(message: "Group #{group_path} not found") unless group
+
+          token = Doorkeeper::OAuth::Token.from_request(current_request, *Doorkeeper.configuration.access_token_methods)
+          unauthorized! unless token
+
+          scim_token = ScimOauthAccessToken.token_matches_for_group?(token, group)
+          unauthorized! unless scim_token
+
+          group
+        end
+
+        # rubocop: disable CodeReuse/ActiveRecord
+        def update_scim_user(identity)
+          parser = EE::Gitlab::Scim::ParamsParser.new(params)
+          parsed_hash = parser.to_hash
+
+          if parser.deprovision_user?
+            destroy_identity(identity)
+          elsif parsed_hash[:extern_uid]
+            identity.update(parsed_hash.slice(:extern_uid))
+          else
+            scim_error!(message: 'Email has already been taken') if email_taken?(parsed_hash[:email], identity)
+
+            result = ::Users::UpdateService.new(identity.user,
+                                                parsed_hash.except(:extern_uid)
+                                                  .merge(user: identity.user)).execute
+
+            result[:status] == :success
+          end
+        end
+        # rubocop: enable CodeReuse/ActiveRecord
+
+        # rubocop: disable CodeReuse/ActiveRecord
+        def email_taken?(email, identity)
+          return unless email
+
+          User.by_any_email(email.downcase).where.not(id: identity.user.id).exists?
+        end
+        # rubocop: enable CodeReuse/ActiveRecord
+      end
+
+      resource :Users do
+        before do
+          check_group_scim_enabled(find_group(params[:group]))
+          check_group_saml_configured
+        end
+
+        desc 'Get SAML users' do
+          detail 'This feature was introduced in GitLab 11.10.'
+        end
+        get do
+          group = find_and_authenticate_group!(params[:group])
+
+          scim_error!(message: 'Missing filter params') unless params[:filter]
+
+          parsed_hash = EE::Gitlab::Scim::ParamsParser.new(params).to_hash
+          identity = GroupSamlIdentityFinder.find_by_group_and_uid(group: group, uid: parsed_hash[:extern_uid])
+
+          status 200
+
+          present identity || {}, with: ::EE::Gitlab::Scim::Users
+        end
+
+        desc 'Get a SAML user' do
+          detail 'This feature was introduced in GitLab 11.10.'
+        end
+        get ':id' do
+          group = find_and_authenticate_group!(params[:group])
+
+          identity = GroupSamlIdentityFinder.find_by_group_and_uid(group: group, uid: params[:id])
+
+          scim_not_found!(message: "Resource #{params[:id]} not found") unless identity
+
+          status 200
+
+          present identity, with: ::EE::Gitlab::Scim::User
+        end
+        desc 'Updates a SAML user' do
+          detail 'This feature was introduced in GitLab 11.10.'
+        end
+        patch ':id' do
+          scim_error!(message: 'Missing ID') unless params[:id]
+
+          group = find_and_authenticate_group!(params[:group])
+          identity = GroupSamlIdentityFinder.find_by_group_and_uid(group: group, uid: params[:id])
+
+          scim_not_found!(message: "Resource #{params[:id]} not found") unless identity
+
+          updated = update_scim_user(identity)
+
+          if updated
+            status 204
+
+            {}
+          else
+            scim_error!(message: "Error updating #{identity.user.name} with #{params.inspect}")
+          end
+        end
+
+        desc 'Removes a SAML user' do
+          detail 'This feature was introduced in GitLab 11.10.'
+        end
+        delete ":id" do
+          scim_error!(message: 'Missing ID') unless params[:id]
+
+          group = find_and_authenticate_group!(params[:group])
+          identity = GroupSamlIdentityFinder.find_by_group_and_uid(group: group, uid: params[:id])
+
+          scim_not_found!(message: "Resource #{params[:id]} not found") unless identity
+
+          destroyed = destroy_identity(identity)
+
+          scim_not_found!(message: "Resource #{params[:id]} not found") unless destroyed
+
+          status 204
+
+          {}
+        end
+      end
+    end
+  end
+end

ee/lib/ee/api/endpoints.rb

@@ -26,6 +26,7 @@ module EE
         mount ::API::NpmPackages
         mount ::API::Packages
         mount ::API::PackageFiles
+        mount ::API::Scim
         mount ::API::ManagedLicenses
         mount ::API::ProjectApprovals
 

ee/lib/ee/api/helpers.rb

@@ -130,6 +130,18 @@ module EE
       def geo_token
         ::Gitlab::Geo.current_node.system_hook.token
       end
+
+      def authorize_manage_saml!(group)
+        unauthorized! unless can?(current_user, :admin_group_saml, group)
+      end
+
+      def check_group_scim_enabled(group)
+        forbidden!('Group SCIM not enabled.') unless ::Feature.enabled?(:group_scim, group)
+      end
+
+      def check_group_saml_configured
+        forbidden!('Group SAML not enabled.') unless ::Gitlab::Auth::GroupSaml::Config.enabled?
+      end
     end
   end
 end

ee/lib/ee/gitlab/auth/user_auth_finders.rb

@@ -5,6 +5,7 @@ module EE
     module Auth
       module UserAuthFinders
         extend ActiveSupport::Concern
+        extend ::Gitlab::Utils::Override
 
         JOB_TOKEN_HEADER = "HTTP_JOB_TOKEN".freeze
         JOB_TOKEN_PARAM = :job_token
@@ -22,6 +23,17 @@ module EE
 
           job.user
         end
+
+        override :find_oauth_access_token
+        def find_oauth_access_token
+          return if scim_request?
+
+          super
+        end
+
+        def scim_request?
+          current_request.path.starts_with?("/api/scim/")
+        end
       end
     end
   end

ee/lib/ee/gitlab/scim/emails.rb

+# frozen_string_literal: true
+
+module EE
+  module Gitlab
+    module Scim
+      class Emails < Grape::Entity
+        expose :type
+        expose :value do |user, _options|
+          user.email
+        end
+        expose :primary
+
+        private
+
+        def type
+          'work'
+        end
+
+        def primary
+          true
+        end
+      end
+    end
+  end
+end

ee/lib/ee/gitlab/scim/error.rb

+# frozen_string_literal: true
+
+module EE
+  module Gitlab
+    module Scim
+      class Error < Grape::Entity
+        expose :schemas
+        expose :detail, safe: true
+        expose :status
+
+        private
+
+        DEFAULT_SCHEMA = 'urn:ietf:params:scim:api:messages:2.0:Error'
+
+        def schemas
+          [DEFAULT_SCHEMA]
+        end
+
+        def status
+          409
+        end
+      end
+    end
+  end
+end

ee/lib/ee/gitlab/scim/not_found.rb

+# frozen_string_literal: true
+
+module EE
+  module Gitlab
+    module Scim
+      class NotFound < EE::Gitlab::Scim::Error
+        def status
+          404
+        end
+      end
+    end
+  end
+end

ee/lib/ee/gitlab/scim/params_parser.rb

+# frozen_string_literal: true
+
+module EE
+  module Gitlab
+    module Scim
+      class ParamsParser
+        FILTER_OPERATORS = %w[eq].freeze
+        OPERATIONS_OPERATORS = %w[Replace Add].freeze
+
+        ATTRIBUTE_MAP = {
+          id: :extern_uid,
+          'name.formatted': :name,
+          'emails[type eq "work"].value': :email,
+          active: :active
+        }.with_indifferent_access.freeze
+
+        COERCED_VALUES = {
+          'True' => true,
+          'False' => false
+        }.freeze
+
+        def initialize(params)
+          @filter = params[:filter]
+          @operations = params[:Operations]
+        end
+
+        def deprovision_user?
+          data[:active] == false
+        end
+
+        def to_hash
+          @data ||=
+            begin
+              hash = {}
+
+              process_filter(hash)
+              process_operations(hash)
+
+              hash
+            end
+        end
+
+        alias_method :data, :to_hash
+        private :data
+
+        private
+
+        def process_filter(hash)
+          return unless @filter
+
+          attribute, operator, value = @filter.split(' ')
+
+          return unless FILTER_OPERATORS.include?(operator)
+          return unless ATTRIBUTE_MAP[attribute]
+
+          hash[ATTRIBUTE_MAP[attribute]] = coerce(value.delete('\"'))
+        end
+
+        def process_operations(hash)
+          return unless @operations
+
+          @operations.each do |operation|
+            next unless OPERATIONS_OPERATORS.include?(operation[:op])
+
+            attribute = ATTRIBUTE_MAP[operation[:path]]
+
+            hash[attribute] = coerce(operation[:value]) if attribute
+          end
+        end
+
+        def coerce(value)
+          coerced = COERCED_VALUES[value]
+
+          coerced.nil? ? value : coerced
+        end
+      end
+    end
+  end
+end

ee/lib/ee/gitlab/scim/user.rb

+# frozen_string_literal: true
+
+module EE
+  module Gitlab
+    module Scim
+      class User < Grape::Entity
+        expose :schemas
+        expose :extern_uid, as: :id
+        expose :active
+        expose 'name.formatted' do |identity, _options|
+          identity.user.name
+        end
+
+        expose :email_user, as: :emails, using: '::EE::Gitlab::Scim::Emails'
+
+        private
+
+        DEFAULT_SCHEMA = 'urn:ietf:params:scim:schemas:core:2.0:User'
+
+        def schemas
+          [DEFAULT_SCHEMA]
+        end
+
+        def active
+          # We don't block the user yet when deprovisioning
+          # So the user is always active, until the identity link is removed.
+          true
+        end
+
+        def email_type
+          'work'
+        end
+
+        def email_primary
+          true
+        end
+
+        def email_user
+          [object.user]
+        end
+      end
+    end
+  end
+end

ee/lib/ee/gitlab/scim/users.rb

+# frozen_string_literal: true
+
+module EE
+  module Gitlab
+    module Scim
+      class Users < Grape::Entity
+        expose :schemas
+        expose :total_results, as: :totalResults
+        expose :items_per_page, as: :itemsPerPage
+        expose :start_index, as: :startIndex
+
+        expose :resources, as: :Resources, using: ::EE::Gitlab::Scim::User
+
+        private
+
+        DEFAULT_SCHEMA = 'urn:ietf:params:scim:api:messages:2.0:ListResponse'
+        ITEMS_PER_PAGE = 20
+        START_INDEX = 1
+
+        def schemas
+          [DEFAULT_SCHEMA]
+        end
+
+        def total_results
+          resources.count
+        end
+
+        def items_per_page
+          ITEMS_PER_PAGE
+        end
+
+        def start_index
+          START_INDEX
+        end
+
+        # We only support a single resource at the moment
+        def resources
+          [object].select(&:present?)
+        end
+      end
+    end
+  end
+end

ee/spec/controllers/groups/scim_oauth_controller_spec.rb

@@ -77,7 +77,7 @@ describe Groups::ScimOauthController do
           end
 
           it 'shows the url' do
-            expect(json_response['scim_api_url']).not_to be_empty
+            expect(json_response['scim_api_url']).to eq("http://localhost/api/scim/v2/groups/#{group.full_path}")
           end
         end
       end

ee/spec/finders/group_saml_identity_finder_spec.rb

@@ -11,6 +11,18 @@ describe GroupSamlIdentityFinder do
 
   subject { described_class.new(user: user) }
 
+  describe ".find_by_group_and_uid" do
+    it "finds identity matching user and group" do
+      expect(described_class.find_by_group_and_uid(group: group, uid: identity.extern_uid)).to eq(identity)
+    end
+
+    it "returns nil when no saml_provider exists" do
+      group.saml_provider.destroy!
+
+      expect(described_class.find_by_group_and_uid(group: group, uid: identity.extern_uid)).to eq(nil)
+    end
+  end
+
   describe "#find_linked" do
     it "finds identity matching user and group" do
       expect(subject.find_linked(group: group)).to eq(identity)

ee/spec/lib/ee/gitlab/scim/emails_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ::EE::Gitlab::Scim::Emails do
+  let(:user) { build(:user) }
+  let(:identity) { build(:group_saml_identity, user: user) }
+
+  let(:entity) do
+    described_class.new(user)
+  end
+
+  subject { entity.as_json }
+
+  it 'contains the email' do
+    expect(subject[:value]).to eq(user.email)
+  end
+
+  it 'contains the type' do
+    expect(subject[:type]).to eq('work')
+  end
+
+  it 'contains the email' do
+    expect(subject[:primary]).to be true
+  end
+end

ee/spec/lib/ee/gitlab/scim/error_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ::EE::Gitlab::Scim::Error do
+  let(:params) { { detail: 'error' } }
+  let(:entity) do
+    described_class.new(params)
+  end
+
+  subject { entity.as_json }
+
+  it 'contains the schemas' do
+    expect(subject[:schemas]).not_to be_empty
+  end
+
+  it 'contains the detail' do
+    expect(subject[:detail]).to eq(params[:detail])
+  end
+
+  it 'contains the status' do
+    expect(subject[:status]).to eq(409)
+  end
+end

ee/spec/lib/ee/gitlab/scim/not_found_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ::EE::Gitlab::Scim::NotFound do
+  let(:entity) do
+    described_class.new({})
+  end
+
+  subject { entity.as_json }
+
+  it 'contains the schemas' do
+    expect(subject[:schemas]).not_to be_empty
+  end
+
+  it 'contains the detail' do
+    expect(subject[:detail]).to be_nil
+  end
+
+  it 'contains the status' do
+    expect(subject[:status]).to eq(404)
+  end
+end

ee/spec/lib/ee/gitlab/scim/params_parser_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe EE::Gitlab::Scim::ParamsParser do
+  describe '#to_hash' do
+    it 'returns the correct filter attributes' do
+      filter = 'id eq "6ba81b08-77da"'
+
+      expect(described_class.new(filter: filter).to_hash).to eq(extern_uid: '6ba81b08-77da')
+    end
+
+    it 'returns an empty hash for the wrong filter' do
+      filter = 'blah eq "6ba81b08-77da"'
+
+      expect(described_class.new(filter: filter).to_hash).to eq({})
+    end
+
+    it 'returns the correct operation attributes' do
+      operations = [{ "op": "Replace", "path": "active", "value": "False" }]
+
+      expect(described_class.new(Operations: operations).to_hash).to eq(active: false)
+    end
+
+    it 'returns an empty hash for the wrong operations' do
+      operations = [{ "op": "Replace", "path": "test", "value": "False" }]
+
+      expect(described_class.new(Operations: operations).to_hash).to eq({})
+    end
+  end
+
+  describe '#deprovision_user?' do
+    it 'returns true when deprovisioning' do
+      operations = [{ "op": "Replace", "path": "active", "value": "False" }]
+
+      expect(described_class.new(Operations: operations).deprovision_user?).to be true
+    end
+
+    it 'returns false when not deprovisioning' do
+      operations = [{ "op": "Replace", "path": "active", "value": "True" }]
+
+      expect(described_class.new(Operations: operations).deprovision_user?).to be false
+    end
+  end
+end

ee/spec/lib/ee/gitlab/scim/user_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ::EE::Gitlab::Scim::User do
+  let(:user) { build(:user) }
+  let(:identity) { build(:group_saml_identity, user: user) }
+
+  let(:entity) do
+    described_class.new(identity)
+  end
+
+  subject { entity.as_json }
+
+  it 'contains the schemas' do
+    expect(subject[:schemas]).to eq(["urn:ietf:params:scim:schemas:core:2.0:User"])
+  end
+
+  it 'contains the extern UID' do
+    expect(subject[:id]).to eq(identity.extern_uid)
+  end
+
+  it 'contains the active flag' do
+    expect(subject[:active]).to be true
+  end
+
+  it 'contains the name' do
+    expect(subject[:'name.formatted']).to eq(user.name)
+  end
+
+  it 'contains the email' do
+    expect(subject[:emails].first[:value]).to eq(user.email)
+  end
+end

ee/spec/lib/ee/gitlab/scim/users_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ::EE::Gitlab::Scim::Users do
+  let(:user) { build(:user) }
+  let(:identity) { build(:group_saml_identity, user: user) }
+
+  let(:entity) do
+    described_class.new(identity)
+  end
+
+  subject { entity.as_json }
+
+  it 'contains the schemas' do
+    expect(subject[:schemas]).to eq(['urn:ietf:params:scim:api:messages:2.0:ListResponse'])
+  end
+
+  it 'contains the totalResults' do
+    expect(subject[:totalResults]).to eq(1)
+  end
+
+  it 'contains the itemsPerPage' do
+    expect(subject[:itemsPerPage]).to eq(20)
+  end
+
+  it 'contains the startIndex' do
+    expect(subject[:startIndex]).to eq(1)
+  end
+
+  it 'contains the user' do
+    expect(subject[:Resources]).not_to be_empty
+  end
+
+  it 'contains the user ID' do
+    expect(subject[:Resources].first[:id]).to eq(identity.extern_uid)
+  end
+end

ee/spec/models/scim_oauth_access_token_spec.rb

@@ -11,6 +11,18 @@ describe ScimOauthAccessToken do
     it { is_expected.to validate_presence_of(:group) }
   end
 
+  describe '.token_matches_for_group?' do
+    it 'finds the token' do
+      group = create(:group)
+
+      scim_token = create(:scim_oauth_access_token, group: group)
+
+      token_value = scim_token.token
+
+      expect(described_class.token_matches_for_group?(token_value, group)).to be true
+    end
+  end
+
   describe '#token' do
     it 'generates a token on creation' do
       scim_token = described_class.create(group: create(:group))

ee/spec/requests/api/scim_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe API::Scim do
+  let(:user) { create(:user) }
+  let(:identity) { create(:group_saml_identity, user: user) }
+  let(:group) { identity.saml_provider.group }
+  let(:scim_token) { create(:scim_oauth_access_token, group: group) }
+
+  before do
+    stub_licensed_features(group_saml: true)
+
+    group.add_owner(user)
+  end
+
+  describe 'GET api/scim/v2/groups/:group/Users' do
+    context 'without token auth' do
+      it 'responds with 401' do
+        get scim_api("scim/v2/groups/#{group.full_path}/Users?filter=id eq \"#{identity.extern_uid}\"", token: false)
+
+        expect(response).to have_gitlab_http_status(401)
+      end
+    end
+
+    it 'responds with an error if there is no filter' do
+      get scim_api("scim/v2/groups/#{group.full_path}/Users")
+
+      expect(response).to have_gitlab_http_status(409)
+    end
+
+    context 'existing user' do
+      it 'responds with 200' do
+        get scim_api("scim/v2/groups/#{group.full_path}/Users?filter=id eq \"#{identity.extern_uid}\"")
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response['Resources']).not_to be_empty
+        expect(json_response['totalResults']).to eq(1)
+      end
+    end
+
+    context 'no user' do
+      it 'responds with 200' do
+        get scim_api("scim/v2/groups/#{group.full_path}/Users?filter=id eq \"nonexistent\"")
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response['Resources']).to be_empty
+        expect(json_response['totalResults']).to eq(0)
+      end
+    end
+  end
+
+  describe 'GET api/scim/v2/groups/:group/Users/:id' do
+    it 'responds with 404 if there is no user' do
+      get scim_api("scim/v2/groups/#{group.full_path}/Users/123")
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+
+    context 'existing user' do
+      it 'responds with 200' do
+        get scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}")
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response['id']).to eq(identity.extern_uid)
+      end
+    end
+  end
+
+  describe 'PATCH api/scim/v2/groups/:group/Users/:id' do
+    it 'responds with 404 if there is no user' do
+      patch scim_api("scim/v2/groups/#{group.full_path}/Users/123")
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+
+    context 'existing user' do
+      context 'extern UID' do
+        before do
+          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}")
+        end
+
+        it 'responds with 204' do
+          expect(response).to have_gitlab_http_status(204)
+        end
+
+        it 'updates the extern_uid' do
+          expect(identity.reload.extern_uid).to eq('new_uid')
+        end
+      end
+
+      context 'name' do
+        before do
+          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}")
+        end
+
+        it 'responds with 204' do
+          expect(response).to have_gitlab_http_status(204)
+        end
+
+        it 'updates the name' do
+          expect(user.reload.name).to eq('new_name')
+        end
+
+        it 'responds with an empty response' do
+          expect(json_response).to eq({})
+        end
+      end
+
+      context 'email' do
+        context 'non existent email' do
+          before do
+            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}")
+          end
+
+          it 'updates the email' do
+            expect(user.reload.unconfirmed_email).to eq('new@mail.com')
+          end
+
+          it 'responds with 204' do
+            expect(response).to have_gitlab_http_status(204)
+          end
+        end
+
+        context 'existent email' do
+          before do
+            create(:user, email: 'new@mail.com')
+
+            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}")
+          end
+
+          it 'does not update a duplicated email' do
+            expect(user.reload.unconfirmed_email).not_to eq('new@mail.com')
+          end
+
+          it 'responds with 209' do
+            expect(response).to have_gitlab_http_status(409)
+          end
+        end
+      end
+
+      context 'Remove user' 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 'responds with 204' do
+          expect(response).to have_gitlab_http_status(204)
+        end
+
+        it 'removes the identity link' do
+          expect { identity.reload }.to raise_error(ActiveRecord::RecordNotFound)
+        end
+      end
+    end
+  end
+
+  describe 'DELETE /scim/v2/groups/:group/Users/:id' do
+    context 'existing user' do
+      before do
+        delete scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}")
+      end
+
+      it 'responds with 204' do
+        expect(response).to have_gitlab_http_status(204)
+      end
+
+      it 'removes the identity link' do
+        expect { identity.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+
+      it 'responds with an empty response' do
+        expect(json_response).to eq({})
+      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(404)
+    end
+  end
+
+  def scim_api(url, token: true)
+    api(url, user, version: '', oauth_access_token: token ? scim_token : nil)
+  end
+end

ee/spec/services/group_saml/identity/destroy_service_spec.rb

@@ -21,6 +21,18 @@ describe GroupSaml::Identity::DestroyService do
     expect(identity).to be_destroyed
   end
 
+  it "does not use a transaction" do
+    expect(::Identity).to receive(:transaction).and_yield.once
+
+    subject.execute
+  end
+
+  it "uses a transaction when transactional is set" do
+    expect(::Identity).to receive(:transaction).and_yield.twice
+
+    subject.execute(transactional: true)
+  end
+
   it "removes access to the group" do
     expect do
       subject.execute