Merge branch 'dm-external-authorization-403' into 'master'

Treat external authorization service response status 403 as failure

Closes #6235

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

doc/user/admin_area/settings/external_authorization.md

@@ -80,7 +80,7 @@ through LDAP.
 
 When the external authorization service responds with a status code 200, the
 user is granted access. When the external service responds with a status code
-401, the user is denied access. In any case, the request is cached for 6 hours.
+401 or 403, the user is denied access. In any case, the request is cached for 6 hours.
 
 When denying access, a `reason` can be optionally specified in the JSON body:
 
@@ -90,7 +90,7 @@ When denying access, a `reason` can be optionally specified in the JSON body:
 }
 ```
 
-Any other status code than 401 or 200 will also deny access to the user, but the
+Any other status code than 200, 401 or 403 will also deny access to the user, but the
 response will not be cached.
 
 If the service times out (after 500ms), a message "External Policy Server did

ee/changelogs/unreleased/dm-external-authorization-403.yml

+---
+title: Treat external authorization service response status 403 as failure
+merge_request:
+author:
+type: fixed

ee/lib/ee/gitlab/external_authorization/response.rb

@@ -9,7 +9,7 @@ module EE
         end
 
         def valid?
-          @excon_response && [200, 401].include?(@excon_response.status)
+          @excon_response && [200, 401, 403].include?(@excon_response.status)
         end
 
         def successful?

ee/spec/lib/ee/gitlab/external_authorization/response_spec.rb

@@ -5,9 +5,9 @@ describe EE::Gitlab::ExternalAuthorization::Response do
   subject(:response) { described_class.new(excon_response) }
 
   describe '#valid?' do
-    it 'is valid for 200 & 401 responses' do
-      [200, 401].each do |status|
-        expect(excon_response).to receive(:status).and_return(status)
+    it 'is valid for 200, 401, and 403 responses' do
+      [200, 401, 403].each do |status|
+        allow(excon_response).to receive(:status).and_return(status)
 
         expect(response).to be_valid
       end
@@ -40,5 +40,13 @@ describe EE::Gitlab::ExternalAuthorization::Response do
 
       expect(response).to be_successful
     end
+
+    it 'is `false` if the status is 401 or 403' do
+      [401, 403].each do |status|
+        allow(excon_response).to receive(:status).and_return(status)
+
+        expect(response).not_to be_successful
+      end
+    end
   end
 end