Automatic merge of gitlab-org/gitlab master

app/models/hooks/web_hook.rb

@@ -73,6 +73,8 @@ class WebHook < ApplicationRecord
   end
 
   def enable!
+    return if recent_failures == 0 && disabled_until.nil? && backoff_count == 0
+
     update!(recent_failures: 0, disabled_until: nil, backoff_count: 0)
   end
 

doc/administration/troubleshooting/group_saml_scim.md

@@ -42,6 +42,10 @@ SCIM mapping:
 
 ![Azure AD SCIM](img/AzureAD-scim_attribute_mapping.png)
 
+Group Sync:
+
+![Azure Group Claims](img/azure_configure_group_claim.png)
+
 ## Okta
 
 Basic SAML app configuration:

doc/user/group/saml_sso/index.md

@@ -152,6 +152,10 @@ We recommend:
 - **Unique User Identifier (Name identifier)** set to `user.objectID`.
 - **nameid-format** set to persistent.
 
+If using [Group Sync](#group-sync), customize the name of the group claim to match the required attribute.
+
+See the [troubleshooting page](../../../administration/troubleshooting/group_saml_scim.md#azure-active-directory) for an example configuration.
+
 ### Okta setup notes
 
 Please follow the Okta documentation on [setting up a SAML application in Okta](https://developer.okta.com/docs/guides/build-sso-integration/saml2/overview/) with the notes below for consideration.

spec/models/hooks/web_hook_spec.rb

@@ -268,11 +268,29 @@ RSpec.describe WebHook do
   end
 
   describe '#enable!' do
-    it 'makes a hook executable' do
+    it 'makes a hook executable if it was marked as failed' do
       hook.recent_failures = 1000
 
       expect { hook.enable! }.to change(hook, :executable?).from(false).to(true)
     end
+
+    it 'makes a hook executable if it is currently backed off' do
+      hook.disabled_until = 1.hour.from_now
+
+      expect { hook.enable! }.to change(hook, :executable?).from(false).to(true)
+    end
+
+    it 'does not update hooks unless necessary' do
+      expect(hook).not_to receive(:update!)
+
+      hook.enable!
+    end
+
+    it 'is idempotent on executable hooks' do
+      expect(hook).not_to receive(:update!)
+
+      expect { hook.enable! }.not_to change(hook, :executable?)
+    end
   end
 
   describe 'backoff!' do
@@ -298,6 +316,7 @@ RSpec.describe WebHook do
 
     it 'does not allow the failure count to exceed the maximum value' do
       hook.recent_failures = described_class::MAX_FAILURES
+      expect(hook).not_to receive(:update!)
 
       expect { hook.failed! }.not_to change(hook, :recent_failures)
     end