Fix reviewif-57131-external_auth_to_ce

4 files changed, 32 insertions, 38 deletions

diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index 11124d6f5a5..b681949ab36 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -124,16 +124,14 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
   end
 
   def visible_application_setting_attributes
-    attrs = ApplicationSettingsHelper.visible_attributes + [
+    [
+      *::ApplicationSettingsHelper.visible_attributes,
+      *::ApplicationSettingsHelper.external_authorization_service_attributes,
       :domain_blacklist_file,
       disabled_oauth_sign_in_sources: [],
       import_sources: [],
       repository_storages: [],
       restricted_visibility_levels: []
     ]
-
-    attrs += ::ApplicationSettingsHelper.external_authorization_service_attributes
-
-    attrs
   end
 end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index f952e390e6d..eb4c87e05d5 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -232,9 +232,9 @@ class Issue < ApplicationRecord
 
     return publicly_visible? unless user
 
-    return readable_by?(user) if user.full_private_access?
+    return false unless readable_by?(user)
 
-    readable_by?(user) &&
+    user.full_private_access? ||
       ::Gitlab::ExternalAuthorization.access_allowed?(
         user, project.external_authorization_classification_label)
   end
diff --git a/lib/api/settings.rb b/lib/api/settings.rb
index 483c53f8d82..b064747e5fc 100644
--- a/lib/api/settings.rb
+++ b/lib/api/settings.rb
@@ -168,9 +168,9 @@ module API
         optional :usage_ping_enabled, type: Boolean, desc: 'Every week GitLab will report license usage back to GitLab, Inc.'
       end
 
-      optional_attributes = ::ApplicationSettingsHelper.visible_attributes << :performance_bar_allowed_group_id
-
-      optional_attributes += ::ApplicationSettingsHelper.external_authorization_service_attributes
+      optional_attributes = [*::ApplicationSettingsHelper.visible_attributes,
+                             *::ApplicationSettingsHelper.external_authorization_service_attributes,
+                             :performance_bar_allowed_group_id]
 
       if Gitlab.ee?
         optional_attributes += EE::ApplicationSettingsHelper.possible_licensed_attributes
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 77faad3f066..0cd69cb4817 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -788,43 +788,39 @@ describe Issue do
     end
 
     describe '#visible_to_user?' do
-      describe '#publicly_visible?' do
-        it 'is `false` when an external authorization service is enabled' do
-          issue = build(:issue, project: build(:project, :public))
+      it 'is `false` when an external authorization service is enabled' do
+        issue = build(:issue, project: build(:project, :public))
 
-          expect(issue).not_to be_visible_to_user
-        end
+        expect(issue).not_to be_visible_to_user
       end
 
-      describe '#readable_by?' do
-        it 'checks the external service to determine if an issue is readable by a user' do
-          project = build(:project, :public,
-                          external_authorization_classification_label: 'a-label')
-          issue = build(:issue, project: project)
-          user = build(:user)
+      it 'checks the external service to determine if an issue is readable by a user' do
+        project = build(:project, :public,
+                        external_authorization_classification_label: 'a-label')
+        issue = build(:issue, project: project)
+        user = build(:user)
 
-          expect(::Gitlab::ExternalAuthorization).to receive(:access_allowed?).with(user, 'a-label') { false }
-          expect(issue.visible_to_user?(user)).to be_falsy
-        end
+        expect(::Gitlab::ExternalAuthorization).to receive(:access_allowed?).with(user, 'a-label') { false }
+        expect(issue.visible_to_user?(user)).to be_falsy
+      end
 
-        it 'does not check the external service if a user does not have access to the project' do
-          project = build(:project, :private,
-                          external_authorization_classification_label: 'a-label')
-          issue = build(:issue, project: project)
-          user = build(:user)
+      it 'does not check the external service if a user does not have access to the project' do
+        project = build(:project, :private,
+                        external_authorization_classification_label: 'a-label')
+        issue = build(:issue, project: project)
+        user = build(:user)
 
-          expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?)
-          expect(issue.visible_to_user?(user)).to be_falsy
-        end
+        expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?)
+        expect(issue.visible_to_user?(user)).to be_falsy
+      end
 
-        it 'does not check the external webservice for admins' do
-          issue = build(:issue)
-          user = build(:admin)
+      it 'does not check the external webservice for admins' do
+        issue = build(:issue)
+        user = build(:admin)
 
-          expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?)
+        expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?)
 
-          issue.visible_to_user?(user)
-        end
+        issue.visible_to_user?(user)
       end
     end
   end