Merge branch '9-3-stable-patch-7' into '9-3-stable'

Prepare 9.3.7

See merge request !12816

10 files changed, 88 insertions, 14 deletions

diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 2192f76499d..aee4db6ae5b 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -179,6 +179,9 @@ class ApplicationSetting < ActiveRecord::Base
     Rails.cache.fetch(CACHE_KEY) do
       ApplicationSetting.last
     end
+  rescue
+    # Fall back to an uncached value if there are any problems (e.g. redis down)
+    ApplicationSetting.last
   end
 
   def self.expire
diff --git a/changelogs/unreleased/34325-reinstate-is_admin-for-user-api.yml b/changelogs/unreleased/34325-reinstate-is_admin-for-user-api.yml
new file mode 100644
index 00000000000..3bed1fbe16e
--- /dev/null
+++ b/changelogs/unreleased/34325-reinstate-is_admin-for-user-api.yml
@@ -0,0 +1,4 @@
+---
+title: Return `is_admin` attribute in the GET /user endpoint for admins
+merge_request: 12811
+author:
diff --git a/changelogs/unreleased/34728-fix-application-setting-created-when-redis-down.yml b/changelogs/unreleased/34728-fix-application-setting-created-when-redis-down.yml
new file mode 100644
index 00000000000..4fddabebf36
--- /dev/null
+++ b/changelogs/unreleased/34728-fix-application-setting-created-when-redis-down.yml
@@ -0,0 +1,4 @@
+---
+title: Prevent bad data being added to application settings when Redis is unavailable
+merge_request: 12750
+author:
diff --git a/doc/api/users.md b/doc/api/users.md
index 43d53756e44..6a8ca8fe564 100644
--- a/doc/api/users.md
+++ b/doc/api/users.md
@@ -356,7 +356,7 @@ GET /user
 
 Parameters:
 
-- `sudo` (required) - the ID of a user
+- `sudo` (optional) - the ID of a user to make the call in their place
 
 ```
 GET /user
diff --git a/lib/api/users.rb b/lib/api/users.rb
index db1f0ff392b..861d92e5b21 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -402,7 +402,16 @@ module API
         success Entities::UserPublic
       end
       get do
-        present current_user, with: sudo? ? Entities::UserWithPrivateDetails : Entities::UserPublic
+        entity =
+          if sudo?
+            Entities::UserWithPrivateDetails
+          elsif current_user.admin?
+            Entities::UserWithAdmin
+          else
+            Entities::UserPublic
+          end
+
+        present current_user, with: entity
       end
 
       desc "Get the currently authenticated user's SSH keys" do
diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb
index 284e6ad55a5..0b2a8c9d6f6 100644
--- a/lib/gitlab/current_settings.rb
+++ b/lib/gitlab/current_settings.rb
@@ -33,12 +33,7 @@ module Gitlab
     def uncached_application_settings
       return fake_application_settings unless connect_to_db?
 
-      # This loads from the database into the cache, so handle Redis errors
-      begin
-        db_settings = ApplicationSetting.current
-      rescue ::Redis::BaseError, ::Errno::ENOENT
-        # In case Redis isn't running or the Redis UNIX socket file is not available
-      end
+      db_settings = ::ApplicationSetting.current
 
       # If there are pending migrations, it's possible there are columns that
       # need to be added to the application settings. To prevent Rake tasks
diff --git a/spec/fixtures/api/schemas/public_api/v4/user/admin.json b/spec/fixtures/api/schemas/public_api/v4/user/admin.json
new file mode 100644
index 00000000000..f733914fbf8
--- /dev/null
+++ b/spec/fixtures/api/schemas/public_api/v4/user/admin.json
@@ -0,0 +1,34 @@
+{
+  "type": "object",
+  "required": [
+    "id",
+    "username",
+    "email",
+    "name",
+    "state",
+    "avatar_url",
+    "web_url",
+    "created_at",
+    "is_admin",
+    "bio",
+    "location",
+    "skype",
+    "linkedin",
+    "twitter",
+    "website_url",
+    "organization",
+    "last_sign_in_at",
+    "confirmed_at",
+    "color_scheme_id",
+    "projects_limit",
+    "current_sign_in_at",
+    "identities",
+    "can_create_group",
+    "can_create_project",
+    "two_factor_enabled",
+    "external"
+  ],
+  "properties": {
+    "$ref": "full.json"
+  }
+}
diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb
index a566f24f6a6..d57ffcae8e1 100644
--- a/spec/lib/gitlab/current_settings_spec.rb
+++ b/spec/lib/gitlab/current_settings_spec.rb
@@ -27,10 +27,23 @@ describe Gitlab::CurrentSettings do
       end
 
       it 'falls back to DB if Redis fails' do
+        db_settings = ApplicationSetting.create!(ApplicationSetting.defaults)
+
         expect(ApplicationSetting).to receive(:cached).and_raise(::Redis::BaseError)
-        expect(ApplicationSetting).to receive(:last).and_call_original
+        expect(Rails.cache).to receive(:fetch).with(ApplicationSetting::CACHE_KEY).and_raise(Redis::BaseError)
 
-        expect(current_application_settings).to be_a(ApplicationSetting)
+        expect(current_application_settings).to eq(db_settings)
+      end
+
+      it 'creates default ApplicationSettings if none are present' do
+        expect(ApplicationSetting).to receive(:cached).and_raise(::Redis::BaseError)
+        expect(Rails.cache).to receive(:fetch).with(ApplicationSetting::CACHE_KEY).and_raise(Redis::BaseError)
+
+        settings = current_application_settings
+
+        expect(settings).to be_a(ApplicationSetting)
+        expect(settings).to be_persisted
+        expect(settings).to have_attributes(ApplicationSetting.defaults)
       end
 
       context 'with migrations pending' do
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index fa229542f70..4ee2e9df099 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -153,6 +153,18 @@ describe ApplicationSetting, models: true do
     end
   end
 
+  describe '.current' do
+    context 'redis unavailable' do
+      it 'returns an ApplicationSetting' do
+        allow(Rails.cache).to receive(:fetch).and_call_original
+        allow(ApplicationSetting).to receive(:last).and_return(:last)
+        expect(Rails.cache).to receive(:fetch).with(ApplicationSetting::CACHE_KEY).and_raise(ArgumentError)
+
+        expect(ApplicationSetting.current).to eq(:last)
+      end
+    end
+  end
+
   context 'restricted signup domains' do
     it 'sets single domain' do
       setting.domain_whitelist_raw = 'example.com'
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 46f3d78e4e5..da2541fa979 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -821,11 +821,11 @@ describe API::Users do
           expect(response).to have_http_status(403)
         end
 
-        it 'returns initial current user without private token when sudo not defined' do
+        it 'returns initial current user without private token but with is_admin when sudo not defined' do
           get api("/user?private_token=#{admin_personal_access_token}")
 
           expect(response).to have_http_status(200)
-          expect(response).to match_response_schema('public_api/v4/user/public')
+          expect(response).to match_response_schema('public_api/v4/user/admin')
           expect(json_response['id']).to eq(admin.id)
         end
       end
@@ -839,11 +839,11 @@ describe API::Users do
           expect(json_response['id']).to eq(user.id)
         end
 
-        it 'returns initial current user without private token when sudo not defined' do
+        it 'returns initial current user without private token but with is_admin when sudo not defined' do
           get api("/user?private_token=#{admin.private_token}")
 
           expect(response).to have_http_status(200)
-          expect(response).to match_response_schema('public_api/v4/user/public')
+          expect(response).to match_response_schema('public_api/v4/user/admin')
           expect(json_response['id']).to eq(admin.id)
         end
       end