From 02f7afd6924d01c59c65896c85186a913c19a8f6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Diego=20Louz=C3=A1n?= <diego.louzan.ext@siemens.com>
Date: Thu, 4 Feb 2021 14:53:31 +0100
Subject: [PATCH] Convert admin mode feature flag to application setting
Defaults to disabled
See epic https://gitlab.com/groups/gitlab-org/-/epics/2158
---
app/controllers/application_controller.rb | 2 +-
.../concerns/enforces_admin_authentication.rb | 2 +-
.../concerns/sessionless_authentication.rb | 2 +-
.../ldap/omniauth_callbacks_controller.rb | 2 +-
.../omniauth_callbacks_controller.rb | 4 +-
app/helpers/application_settings_helper.rb | 1 +
app/helpers/nav_helper.rb | 6 +-
app/models/application_setting.rb | 3 +
.../application_setting_implementation.rb | 1 +
app/policies/base_policy.rb | 2 +-
.../application_settings/_signin.html.haml | 9 +++
app/views/layouts/nav/_dashboard.html.haml | 4 +-
...ert-admin-mode-feature-flag-to-setting.yml | 5 ++
.../development/user_mode_in_session.yml | 8 ---
...6_add_admin_mode_to_application_setting.rb | 9 +++
db/schema_migrations/20210309160106 | 1 +
db/structure.sql | 1 +
doc/api/settings.md | 7 ++-
doc/development/secure_coding_guidelines.md | 4 +-
.../settings/sign_in_restrictions.md | 56 ++++++++++++++++++-
.../admin_merge_requests_approvals_spec.rb | 1 -
ee/spec/models/geo/deleted_project_spec.rb | 4 +-
lib/api/api_guard.rb | 4 +-
lib/api/internal/base.rb | 10 +++-
lib/api/settings.rb | 1 +
lib/constraints/admin_constrainer.rb | 2 +-
.../sidekiq_middleware/admin_mode/client.rb | 3 +-
.../sidekiq_middleware/admin_mode/server.rb | 3 +-
locale/gitlab.pot | 6 ++
.../application_settings_controller_spec.rb | 9 ++-
.../enforces_admin_authentication_spec.rb | 6 +-
.../projects/issues_controller_spec.rb | 4 +-
spec/features/admin/admin_mode_spec.rb | 6 +-
spec/features/admin/admin_settings_spec.rb | 6 +-
.../ide/clientside_preview_csp_spec.rb | 10 +---
.../usage_trends/measurement_type_spec.rb | 2 +-
spec/helpers/application_helper_spec.rb | 4 +-
spec/helpers/nav_helper_spec.rb | 6 +-
.../lib/constraints/admin_constrainer_spec.rb | 6 +-
.../create_group_spec.rb | 2 +-
.../admin_mode/client_spec.rb | 4 +-
.../admin_mode/server_spec.rb | 4 +-
.../concerns/cacheable_attributes_spec.rb | 2 +-
.../clusters/cluster_presenter_spec.rb | 4 +-
spec/requests/api/internal/base_spec.rb | 6 +-
spec/requests/api/settings_spec.rb | 7 ++-
spec/requests/jwt_controller_spec.rb | 7 ++-
spec/spec_helper.rb | 5 +-
48 files changed, 180 insertions(+), 83 deletions(-)
create mode 100644 changelogs/unreleased/refactor-convert-admin-mode-feature-flag-to-setting.yml
delete mode 100644 config/feature_flags/development/user_mode_in_session.yml
create mode 100644 db/migrate/20210309160106_add_admin_mode_to_application_setting.rb
create mode 100644 db/schema_migrations/20210309160106
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 32de4a0145c5d..607f3435394bd 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -482,7 +482,7 @@ def set_page_title_header
end
def set_current_admin(&block)
- return yield unless Feature.enabled?(:user_mode_in_session)
+ return yield unless Gitlab::CurrentSettings.admin_mode
return yield unless current_user
Gitlab::Auth::CurrentUserMode.with_current_admin(current_user, &block)
diff --git a/app/controllers/concerns/enforces_admin_authentication.rb b/app/controllers/concerns/enforces_admin_authentication.rb
index 527759de0bbef..94c0e98c91a7a 100644
--- a/app/controllers/concerns/enforces_admin_authentication.rb
+++ b/app/controllers/concerns/enforces_admin_authentication.rb
@@ -15,7 +15,7 @@ module EnforcesAdminAuthentication
def authenticate_admin!
return render_404 unless current_user.admin?
- return unless Feature.enabled?(:user_mode_in_session)
+ return unless Gitlab::CurrentSettings.admin_mode
unless current_user_mode.admin_mode?
current_user_mode.request_admin_mode!
diff --git a/app/controllers/concerns/sessionless_authentication.rb b/app/controllers/concerns/sessionless_authentication.rb
index a9ef33bf3b9bc..882fef7a34263 100644
--- a/app/controllers/concerns/sessionless_authentication.rb
+++ b/app/controllers/concerns/sessionless_authentication.rb
@@ -27,7 +27,7 @@ def sessionless_sign_in(user)
end
def sessionless_bypass_admin_mode!(&block)
- return yield unless Feature.enabled?(:user_mode_in_session)
+ return yield unless Gitlab::CurrentSettings.admin_mode
Gitlab::Auth::CurrentUserMode.bypass_session!(current_user.id, &block)
end
diff --git a/app/controllers/ldap/omniauth_callbacks_controller.rb b/app/controllers/ldap/omniauth_callbacks_controller.rb
index 4b6339c21cd0e..ebc354489646d 100644
--- a/app/controllers/ldap/omniauth_callbacks_controller.rb
+++ b/app/controllers/ldap/omniauth_callbacks_controller.rb
@@ -16,7 +16,7 @@ def self.define_providers!
def ldap
return unless Gitlab::Auth::Ldap::Config.sign_in_enabled?
- if Feature.enabled?(:user_mode_in_session)
+ if Gitlab::CurrentSettings.admin_mode
return admin_mode_flow(Gitlab::Auth::Ldap::User) if current_user_mode.admin_mode_requested?
end
diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index c979170341307..af502c083d7f3 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -95,7 +95,7 @@ def log_failed_login(user, provider)
end
def after_omniauth_failure_path_for(scope)
- if Feature.enabled?(:user_mode_in_session)
+ if Gitlab::CurrentSettings.admin_mode
return new_admin_session_path if current_user_mode.admin_mode_requested?
end
@@ -112,7 +112,7 @@ def omniauth_flow(auth_module, identity_linker: nil)
log_audit_event(current_user, with: oauth['provider'])
- if Feature.enabled?(:user_mode_in_session)
+ if Gitlab::CurrentSettings.admin_mode
return admin_mode_flow(auth_module::User) if current_user_mode.admin_mode_requested?
end
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index 085fbfd08da2f..6b65b1c0bff79 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -179,6 +179,7 @@ def external_authorization_client_pass_help_text
def visible_attributes
[
:abuse_notification_email,
+ :admin_mode,
:after_sign_out_path,
:after_sign_up_text,
:akismet_api_key,
diff --git a/app/helpers/nav_helper.rb b/app/helpers/nav_helper.rb
index c170e58b4ce36..db144f63f9223 100644
--- a/app/helpers/nav_helper.rb
+++ b/app/helpers/nav_helper.rb
@@ -92,10 +92,8 @@ def get_header_links
links << :admin_impersonation
end
- if Feature.enabled?(:user_mode_in_session)
- if current_user_mode.admin_mode?
- links << :admin_mode
- end
+ if Gitlab::CurrentSettings.admin_mode && current_user_mode.admin_mode?
+ links << :admin_mode
end
links
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 44eb2fefb3fdb..dbc09a3c9b207 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -465,6 +465,9 @@ def self.kroki_formats_attributes
length: { maximum: 100, message: N_('is too long (maximum is 100 entries)') },
allow_nil: false
+ validates :admin_mode,
+ inclusion: { in: [true, false], message: _('must be a boolean value') }
+
attr_encrypted :asset_proxy_secret_key,
mode: :per_attribute_iv,
key: Settings.attr_encrypted_db_key_base_truncated,
diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb
index c067199b52cc3..dba72f889864b 100644
--- a/app/models/application_setting_implementation.rb
+++ b/app/models/application_setting_implementation.rb
@@ -35,6 +35,7 @@ module ApplicationSettingImplementation
class_methods do
def defaults
{
+ admin_mode: false,
after_sign_up_text: nil,
akismet_enabled: false,
allow_local_requests_from_system_hooks: true,
diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb
index e32a889c90623..1c19751cf0df9 100644
--- a/app/policies/base_policy.rb
+++ b/app/policies/base_policy.rb
@@ -6,7 +6,7 @@ class BasePolicy < DeclarativePolicy::Base
desc "User is an instance admin"
with_options scope: :user, score: 0
condition(:admin) do
- if Feature.enabled?(:user_mode_in_session)
+ if Gitlab::CurrentSettings.admin_mode
Gitlab::Auth::CurrentUserMode.new(@user).admin_mode?
else
@user&.admin?
diff --git a/app/views/admin/application_settings/_signin.html.haml b/app/views/admin/application_settings/_signin.html.haml
index 54bd5cf4072f3..65e9c8ec60430 100644
--- a/app/views/admin/application_settings/_signin.html.haml
+++ b/app/views/admin/application_settings/_signin.html.haml
@@ -31,6 +31,15 @@
= f.check_box :require_two_factor_authentication, class: 'form-check-input'
= f.label :require_two_factor_authentication, class: 'form-check-label' do
Require all users to set up Two-factor authentication
+ .form-group
+ = f.label :admin_mode, _('Admin Mode'), class: 'label-bold'
+ = sprite_icon('lock', css_class: 'gl-icon')
+ .form-check
+ = f.check_box :admin_mode, class: 'form-check-input'
+ = f.label :admin_mode, class: 'form-check-label' do
+ = _('Require additional authentication for administrative tasks')
+ .form-text.text-muted
+ = link_to _('Learn more.'), help_page_path('user/admin_area/settings/sign_in_restrictions', anchor: 'admin-mode')
.form-group
= f.label :unknown_sign_in, _('Email notification for unknown sign-ins'), class: 'label-bold'
.form-check
diff --git a/app/views/layouts/nav/_dashboard.html.haml b/app/views/layouts/nav/_dashboard.html.haml
index 7cbef6b00b17b..9e25e6db15f45 100644
--- a/app/views/layouts/nav/_dashboard.html.haml
+++ b/app/views/layouts/nav/_dashboard.html.haml
@@ -50,7 +50,7 @@
= nav_link(controller: 'admin/dashboard') do
= link_to admin_root_path, class: 'admin-icon qa-admin-area-link d-xl-none' do
= _('Admin Area')
- - if Feature.enabled?(:user_mode_in_session)
+ - if Gitlab::CurrentSettings.admin_mode
- if header_link?(:admin_mode)
= nav_link(controller: 'admin/sessions') do
= link_to destroy_admin_session_path, method: :post, class: 'd-lg-none lock-open-icon' do
@@ -69,7 +69,7 @@
= link_to admin_root_path, class: 'admin-icon qa-admin-area-link', title: _('Admin Area'), aria: { label: _('Admin Area') }, data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do
= sprite_icon('admin', size: 18)
- - if Feature.enabled?(:user_mode_in_session)
+ - if Gitlab::CurrentSettings.admin_mode
- if header_link?(:admin_mode)
= nav_link(controller: 'admin/sessions', html_options: { class: "d-none d-lg-block"}) do
= link_to destroy_admin_session_path, method: :post, title: _('Leave Admin Mode'), aria: { label: _('Leave Admin Mode') }, data: { toggle: 'tooltip', placement: 'bottom', container: 'body' } do
diff --git a/changelogs/unreleased/refactor-convert-admin-mode-feature-flag-to-setting.yml b/changelogs/unreleased/refactor-convert-admin-mode-feature-flag-to-setting.yml
new file mode 100644
index 0000000000000..82d8267dc5b16
--- /dev/null
+++ b/changelogs/unreleased/refactor-convert-admin-mode-feature-flag-to-setting.yml
@@ -0,0 +1,5 @@
+---
+title: Convert admin mode feature flag to system application setting
+merge_request: 53610
+author: Diego Louzán
+type: added
diff --git a/config/feature_flags/development/user_mode_in_session.yml b/config/feature_flags/development/user_mode_in_session.yml
deleted file mode 100644
index 1b0a0053cf4f8..0000000000000
--- a/config/feature_flags/development/user_mode_in_session.yml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-name: user_mode_in_session
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/16981
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/321025
-milestone: 12.4
-type: development
-group: group::access
-default_enabled: false
diff --git a/db/migrate/20210309160106_add_admin_mode_to_application_setting.rb b/db/migrate/20210309160106_add_admin_mode_to_application_setting.rb
new file mode 100644
index 0000000000000..a7b634596d235
--- /dev/null
+++ b/db/migrate/20210309160106_add_admin_mode_to_application_setting.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class AddAdminModeToApplicationSetting < ActiveRecord::Migration[6.0]
+ DOWNTIME = false
+
+ def change
+ add_column :application_settings, :admin_mode, :boolean, default: false, null: false
+ end
+end
diff --git a/db/schema_migrations/20210309160106 b/db/schema_migrations/20210309160106
new file mode 100644
index 0000000000000..d10e9176a710a
--- /dev/null
+++ b/db/schema_migrations/20210309160106
@@ -0,0 +1 @@
+968ba7808c969e29f1c3b6b635bff22f986b60e56cb001737ad8aba1825fd945
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 0c4bd0d4ff6ab..506b52b6f2301 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -9473,6 +9473,7 @@ CREATE TABLE application_settings (
kroki_formats jsonb DEFAULT '{}'::jsonb NOT NULL,
in_product_marketing_emails_enabled boolean DEFAULT true NOT NULL,
asset_proxy_whitelist text,
+ admin_mode boolean DEFAULT false NOT NULL,
CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)),
CONSTRAINT check_17d9558205 CHECK ((char_length(kroki_url) <= 1024)),
diff --git a/doc/api/settings.md b/doc/api/settings.md
index c42df25542a2b..913a3699fe48c 100644
--- a/doc/api/settings.md
+++ b/doc/api/settings.md
@@ -86,7 +86,8 @@ Example response:
"require_admin_approval_after_user_signup": false,
"personal_access_token_prefix": "GL-",
"rate_limiting_response_text": null,
- "keep_latest_artifact": true
+ "keep_latest_artifact": true,
+ "admin_mode": false
}
```
@@ -181,7 +182,8 @@ Example response:
"require_admin_approval_after_user_signup": false,
"personal_access_token_prefix": "GL-",
"rate_limiting_response_text": null,
- "keep_latest_artifact": true
+ "keep_latest_artifact": true,
+ "admin_mode": false
}
```
@@ -208,6 +210,7 @@ listed in the descriptions of the relevant settings.
| Attribute | Type | Required | Description |
|------------------------------------------|------------------|:------------------------------------:|-------------|
+| `admin_mode` | boolean | no | Require admins to enable Admin Mode by re-authenticating for administrative tasks. |
| `admin_notification_email` | string | no | Deprecated: Use `abuse_notification_email` instead. If set, [abuse reports](../user/admin_area/abuse_reports.md) are sent to this address. Abuse reports are always available in the Admin Area. |
| `abuse_notification_email` | string | no | If set, [abuse reports](../user/admin_area/abuse_reports.md) are sent to this address. Abuse reports are always available in the Admin Area. |
| `after_sign_out_path` | string | no | Where to redirect users after logout. |
diff --git a/doc/development/secure_coding_guidelines.md b/doc/development/secure_coding_guidelines.md
index e9c95a142366a..62cc2543fc49f 100644
--- a/doc/development/secure_coding_guidelines.md
+++ b/doc/development/secure_coding_guidelines.md
@@ -565,7 +565,7 @@ In some scenarios such as [this one](https://gitlab.com/gitlab-org/gitlab/-/issu
return unless user
# Sessions are enforced to be unavailable for API calls, so ignore them for admin mode
- Gitlab::Auth::CurrentUserMode.bypass_session!(user.id) if Feature.enabled?(:user_mode_in_session)
+ Gitlab::Auth::CurrentUserMode.bypass_session!(user.id) if Gitlab::CurrentSettings.admin_mode
unless api_access_allowed?(user)
forbidden!(api_access_denied_message(user))
@@ -581,7 +581,7 @@ In order to prevent this from happening, it is recommended to use the method `us
user = find_user_from_sources
return unless user
- if user.is_a?(User) && Feature.enabled?(:user_mode_in_session)
+ if user.is_a?(User) && Gitlab::CurrentSettings.admin_mode
# Sessions are enforced to be unavailable for API calls, so ignore them for admin mode
Gitlab::Auth::CurrentUserMode.bypass_session!(user.id)
end
diff --git a/doc/user/admin_area/settings/sign_in_restrictions.md b/doc/user/admin_area/settings/sign_in_restrictions.md
index a34a63f4543b1..50fd6a353545c 100644
--- a/doc/user/admin_area/settings/sign_in_restrictions.md
+++ b/doc/user/admin_area/settings/sign_in_restrictions.md
@@ -23,9 +23,63 @@ You can restrict the password authentication for web interface and Git over HTTP
- **Web interface**: When this feature is disabled, an [external authentication provider](../../../administration/auth/README.md) must be used.
- **Git over HTTP(S)**: When this feature is disabled, a [Personal Access Token](../../profile/personal_access_tokens.md) must be used to authenticate.
+## Admin Mode
+
+When this feature is enabled, instance administrators are limited as regular users. During that period,
+they do not have access to all projects, groups, or the **Admin Area** menu.
+
+To access potentially dangerous resources, an administrator can activate Admin Mode by:
+
+- Selecting the *Enable Admin Mode* button
+- Trying to access any part of the UI that requires an administrator role, specifically those which call `/admin` endpoints.
+
+The main use case allows administrators to perform their regular tasks as a regular
+user, based on their memberships, without having to set up a second account for
+security reasons.
+
+When Admin Mode status is disabled, administrative users cannot access resources unless
+they've been explicitly granted access. For example, when Admin Mode is disabled, they
+get a `404` error if they try to open a private group or project, unless
+they are members of that group or project.
+
+2FA should be enabled for administrators and is supported for the Admin Mode flow, as are
+OmniAuth providers and LDAP auth. The Admin Mode status is stored in the active user
+session and remains active until it is explicitly disabled (it will be disabled
+automatically after a timeout otherwise).
+
+### Limitations
+
+The following access methods are **not** protected by Admin Mode:
+
+- Git client access (SSH using public keys or HTTPS using Personal Access Tokens).
+- API access using a Personal Access Token.
+
+In other words, administrators who are otherwise limited by Admin Mode can still use
+Git clients, and access RESTful API endpoints as administrators, without additional
+authentication steps.
+
+We may address these limitations in the future. For more information see the following epic:
+[Admin mode for GitLab Administrators](https://gitlab.com/groups/gitlab-org/-/epics/2158).
+
+### Troubleshooting
+
+If necessary, you can disable **Admin Mode** as an administrator by using one of these two methods:
+
+- **API**:
+
+ ```shell
+ curl --request PUT --header "PRIVATE-TOKEN:$ADMIN_TOKEN" "<gitlab-url>/api/v4/application/settings?admin_mode=false"
+ ```
+
+- [**Rails console**](../../../administration/operations/rails_console.md#starting-a-rails-console-session):
+
+ ```ruby
+ ::Gitlab::CurrentSettings.update_attributes!(admin_mode: false)
+ ```
+
## Two-factor authentication
-When this feature enabled, all users must use the [two-factor authentication](../../profile/account/two_factor_authentication.md).
+When this feature is enabled, all users must use the [two-factor authentication](../../profile/account/two_factor_authentication.md).
After the two-factor authentication is configured as mandatory, users are allowed
to skip forced configuration of two-factor authentication for the configurable grace
diff --git a/ee/spec/features/admin/admin_merge_requests_approvals_spec.rb b/ee/spec/features/admin/admin_merge_requests_approvals_spec.rb
index 3b8b0ce30bf76..20c16f56d2de4 100644
--- a/ee/spec/features/admin/admin_merge_requests_approvals_spec.rb
+++ b/ee/spec/features/admin/admin_merge_requests_approvals_spec.rb
@@ -5,7 +5,6 @@
RSpec.describe 'Admin interacts with merge requests approvals settings' do
include StubENV
- let_it_be(:application_settings) { create(:application_setting) }
let_it_be(:user) { create(:admin) }
let_it_be(:project) { create(:project, creator: user) }
diff --git a/ee/spec/models/geo/deleted_project_spec.rb b/ee/spec/models/geo/deleted_project_spec.rb
index 3c6feee5adb4c..6e89aa72befa9 100644
--- a/ee/spec/models/geo/deleted_project_spec.rb
+++ b/ee/spec/models/geo/deleted_project_spec.rb
@@ -53,9 +53,7 @@
end
it 'picks storage from ApplicationSetting when value is not initialized' do
- allow_next_instance_of(ApplicationSetting) do |instance|
- allow(instance).to receive(:pick_repository_storage).and_return('bar')
- end
+ stub_application_setting(pick_repository_storage: 'bar')
subject = described_class.new(id: 1, name: 'sample', disk_path: 'root/sample', repository_storage: nil)
diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb
index 8641271f2dfa2..8822a30d4a15d 100644
--- a/lib/api/api_guard.rb
+++ b/lib/api/api_guard.rb
@@ -55,7 +55,7 @@ def find_current_user!
user = find_user_from_sources
return unless user
- if user.is_a?(User) && Feature.enabled?(:user_mode_in_session)
+ if user.is_a?(User) && Gitlab::CurrentSettings.admin_mode
# Sessions are enforced to be unavailable for API calls, so ignore them for admin mode
Gitlab::Auth::CurrentUserMode.bypass_session!(user.id)
end
@@ -236,7 +236,7 @@ class AdminModeMiddleware < ::Grape::Middleware::Base
def after
# Use a Grape middleware since the Grape `after` blocks might run
# before we are finished rendering the `Grape::Entity` classes
- Gitlab::Auth::CurrentUserMode.reset_bypass_session! if Feature.enabled?(:user_mode_in_session)
+ Gitlab::Auth::CurrentUserMode.reset_bypass_session! if Gitlab::CurrentSettings.admin_mode
# Explicit nil is needed or the api call return value will be overwritten
nil
diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb
index 3dd01b96e3914..664b05ea0108a 100644
--- a/lib/api/internal/base.rb
+++ b/lib/api/internal/base.rb
@@ -52,7 +52,7 @@ def check_allowed(params)
actor.update_last_used_at!
check_result = begin
- Gitlab::Auth::CurrentUserMode.bypass_session!(actor.user&.id) do
+ with_admin_mode_bypass!(actor.user&.id) do
access_check!(actor, params)
end
rescue Gitlab::GitAccess::ForbiddenError => e
@@ -120,6 +120,14 @@ def validate_actor_key(actor, key_id)
def two_factor_otp_check
{ success: false, message: 'Feature is not available' }
end
+
+ def with_admin_mode_bypass!(actor_id)
+ return yield unless Gitlab::CurrentSettings.admin_mode
+
+ Gitlab::Auth::CurrentUserMode.bypass_session!(actor_id) do
+ yield
+ end
+ end
end
namespace 'internal' do
diff --git a/lib/api/settings.rb b/lib/api/settings.rb
index 64a72b4cb7f77..95d0c525cedf2 100644
--- a/lib/api/settings.rb
+++ b/lib/api/settings.rb
@@ -30,6 +30,7 @@ def filter_attributes_using_license(attrs)
success Entities::ApplicationSetting
end
params do
+ optional :admin_mode, type: Boolean, desc: 'Require admin users to re-authenticate for administrative (i.e. potentially dangerous) operations'
optional :admin_notification_email, type: String, desc: 'Deprecated: Use :abuse_notification_email instead. Abuse reports will be sent to this address if it is set. Abuse reports are always available in the admin area.'
optional :abuse_notification_email, type: String, desc: 'Abuse reports will be sent to this address if it is set. Abuse reports are always available in the admin area.'
optional :after_sign_up_text, type: String, desc: 'Text shown after sign up'
diff --git a/lib/constraints/admin_constrainer.rb b/lib/constraints/admin_constrainer.rb
index 59c855a1b7348..2f32cc7ad91e9 100644
--- a/lib/constraints/admin_constrainer.rb
+++ b/lib/constraints/admin_constrainer.rb
@@ -3,7 +3,7 @@
module Constraints
class AdminConstrainer
def matches?(request)
- if Feature.enabled?(:user_mode_in_session)
+ if Gitlab::CurrentSettings.admin_mode
admin_mode_enabled?(request)
else
user_is_admin?(request)
diff --git a/lib/gitlab/sidekiq_middleware/admin_mode/client.rb b/lib/gitlab/sidekiq_middleware/admin_mode/client.rb
index 36204e1bee0bc..1b33743a0e93a 100644
--- a/lib/gitlab/sidekiq_middleware/admin_mode/client.rb
+++ b/lib/gitlab/sidekiq_middleware/admin_mode/client.rb
@@ -8,7 +8,8 @@ module AdminMode
# If enabled then it injects a job field that persists through the job execution
class Client
def call(_worker_class, job, _queue, _redis_pool)
- return yield unless ::Feature.enabled?(:user_mode_in_session)
+ # Not calling Gitlab::CurrentSettings.admin_mode on purpose on sidekiq middleware
+ # Only when admin mode application setting is enabled might the admin_mode_user_id be non-nil here
# Admin mode enabled in the original request or in a nested sidekiq job
admin_mode_user_id = find_admin_user_id
diff --git a/lib/gitlab/sidekiq_middleware/admin_mode/server.rb b/lib/gitlab/sidekiq_middleware/admin_mode/server.rb
index 6366867a0facf..c4e64705d6e33 100644
--- a/lib/gitlab/sidekiq_middleware/admin_mode/server.rb
+++ b/lib/gitlab/sidekiq_middleware/admin_mode/server.rb
@@ -5,7 +5,8 @@ module SidekiqMiddleware
module AdminMode
class Server
def call(_worker, job, _queue)
- return yield unless Feature.enabled?(:user_mode_in_session)
+ # Not calling Gitlab::CurrentSettings.admin_mode on purpose on sidekiq middleware
+ # Only when admin_mode setting is enabled can it be true here
admin_mode_user_id = job['admin_mode_user_id']
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 80335ed8ef39f..ba8843caef232 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -2096,6 +2096,9 @@ msgstr ""
msgid "Admin Area"
msgstr ""
+msgid "Admin Mode"
+msgstr ""
+
msgid "Admin Note"
msgstr ""
@@ -25873,6 +25876,9 @@ msgstr ""
msgid "Requests to these domain(s)/address(es) on the local network will be allowed when local requests from hooks and services are not allowed. IP ranges such as 1:0:0:0:0:0:0:0/124 or 127.0.0.0/28 are supported. Domain wildcards are not supported currently. Use comma, semicolon, or newline to separate multiple entries. The allowlist can hold a maximum of 1000 entries. Domains should use IDNA encoding. Ex: example.com, 192.168.1.1, 127.0.0.0/28, xn--itlab-j1a.com."
msgstr ""
+msgid "Require additional authentication for administrative tasks"
+msgstr ""
+
msgid "Require admin approval for new sign-ups"
msgstr ""
diff --git a/spec/controllers/admin/application_settings_controller_spec.rb b/spec/controllers/admin/application_settings_controller_spec.rb
index 2b562e2dd64ef..6258dd304386b 100644
--- a/spec/controllers/admin/application_settings_controller_spec.rb
+++ b/spec/controllers/admin/application_settings_controller_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe Admin::ApplicationSettingsController do
+RSpec.describe Admin::ApplicationSettingsController, :do_not_mock_admin_mode_setting do
include StubENV
include UsageDataHelpers
@@ -164,6 +164,13 @@
expect(ApplicationSetting.current.default_branch_name).to eq("example_branch_name")
end
+ it "updates admin_mode setting" do
+ put :update, params: { application_setting: { admin_mode: true } }
+
+ expect(response).to redirect_to(general_admin_application_settings_path)
+ expect(ApplicationSetting.current.admin_mode).to be(true)
+ end
+
context "personal access token prefix settings" do
let(:application_settings) { ApplicationSetting.current }
diff --git a/spec/controllers/concerns/enforces_admin_authentication_spec.rb b/spec/controllers/concerns/enforces_admin_authentication_spec.rb
index c6ad1a0048472..106b1d53fd2bd 100644
--- a/spec/controllers/concerns/enforces_admin_authentication_spec.rb
+++ b/spec/controllers/concerns/enforces_admin_authentication_spec.rb
@@ -19,7 +19,7 @@ def index
end
end
- context 'feature flag :user_mode_in_session is enabled' do
+ context 'application setting :admin_mode is enabled' do
describe 'authenticate_admin!' do
context 'as an admin' do
let(:user) { create(:admin) }
@@ -61,9 +61,9 @@ def index
end
end
- context 'feature flag :user_mode_in_session is disabled' do
+ context 'application setting :admin_mode is disabled' do
before do
- stub_feature_flags(user_mode_in_session: false)
+ stub_application_setting(admin_mode: false)
end
describe 'authenticate_admin!' do
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 6b06e2241891d..474e3a3b009da 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -1446,9 +1446,7 @@ def post_verified_issue
expect_next_instance_of(Spam::AkismetService) do |akismet_service|
expect(akismet_service).to receive_messages(submit_spam: true)
end
- expect_next_instance_of(ApplicationSetting) do |setting|
- expect(setting).to receive_messages(akismet_enabled: true)
- end
+ stub_application_setting(akismet_enabled: true)
end
def post_spam
diff --git a/spec/features/admin/admin_mode_spec.rb b/spec/features/admin/admin_mode_spec.rb
index d2bcd6d71dbcb..633de20c82dc4 100644
--- a/spec/features/admin/admin_mode_spec.rb
+++ b/spec/features/admin/admin_mode_spec.rb
@@ -14,7 +14,7 @@
stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
end
- context 'feature flag :user_mode_in_session is enabled', :request_store do
+ context 'application setting :admin_mode is enabled', :request_store do
before do
sign_in(admin)
end
@@ -157,9 +157,9 @@
end
end
- context 'feature flag :user_mode_in_session is disabled' do
+ context 'application setting :admin_mode is disabled' do
before do
- stub_feature_flags(user_mode_in_session: false)
+ stub_application_setting(admin_mode: false)
sign_in(admin)
end
diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb
index 249621f5835ed..f47db1342e3d8 100644
--- a/spec/features/admin/admin_settings_spec.rb
+++ b/spec/features/admin/admin_settings_spec.rb
@@ -9,7 +9,7 @@
let(:admin) { create(:admin) }
- context 'feature flag :user_mode_in_session is enabled', :request_store do
+ context 'application setting :admin_mode is enabled', :request_store do
before do
stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
sign_in(admin)
@@ -615,9 +615,9 @@
end
end
- context 'feature flag :user_mode_in_session is disabled' do
+ context 'application setting :admin_mode is disabled' do
before do
- stub_feature_flags(user_mode_in_session: false)
+ stub_application_setting(admin_mode: false)
stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
diff --git a/spec/features/ide/clientside_preview_csp_spec.rb b/spec/features/ide/clientside_preview_csp_spec.rb
index eadcb9cd008dc..559edb8bf532b 100644
--- a/spec/features/ide/clientside_preview_csp_spec.rb
+++ b/spec/features/ide/clientside_preview_csp_spec.rb
@@ -7,9 +7,7 @@
shared_context 'disable feature' do
before do
- allow_next_instance_of(ApplicationSetting) do |instance|
- allow(instance).to receive(:web_ide_clientside_preview_enabled?).and_return(false)
- end
+ stub_application_setting(web_ide_clientside_preview_enabled: false)
end
end
@@ -24,10 +22,8 @@
end
before do
- allow_next_instance_of(ApplicationSetting) do |instance|
- allow(instance).to receive(:web_ide_clientside_preview_enabled?).and_return(true)
- allow(instance).to receive(:web_ide_clientside_preview_bundler_url).and_return(whitelisted_url)
- end
+ stub_application_setting(web_ide_clientside_preview_enabled: true)
+ stub_application_setting(web_ide_clientside_preview_bundler_url: whitelisted_url)
sign_in(user)
end
diff --git a/spec/graphql/types/admin/analytics/usage_trends/measurement_type_spec.rb b/spec/graphql/types/admin/analytics/usage_trends/measurement_type_spec.rb
index c50092d7f0edc..e0d2eff8a219f 100644
--- a/spec/graphql/types/admin/analytics/usage_trends/measurement_type_spec.rb
+++ b/spec/graphql/types/admin/analytics/usage_trends/measurement_type_spec.rb
@@ -44,7 +44,7 @@
let(:user) { create(:user, :admin) }
before do
- stub_feature_flags(user_mode_in_session: false)
+ stub_application_setting(admin_mode: false)
end
it 'returns data' do
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index c7470f31ad840..3ccf5ded9f57d 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -316,9 +316,7 @@ def element(**arguments)
let(:user) { create(:user, static_object_token: 'hunter1') }
before do
- allow_next_instance_of(ApplicationSetting) do |instance|
- allow(instance).to receive(:static_objects_external_storage_url).and_return('https://cdn.gitlab.com')
- end
+ stub_application_setting(static_objects_external_storage_url: 'https://cdn.gitlab.com')
allow(helper).to receive(:current_user).and_return(user)
end
diff --git a/spec/helpers/nav_helper_spec.rb b/spec/helpers/nav_helper_spec.rb
index c4795a814ba06..2efff3402c5d9 100644
--- a/spec/helpers/nav_helper_spec.rb
+++ b/spec/helpers/nav_helper_spec.rb
@@ -35,7 +35,7 @@
context 'as admin' do
let(:user) { create(:user, :admin) }
- context 'feature flag :user_mode_in_session is enabled' do
+ context 'application setting :admin_mode is enabled' do
it 'does not contain the admin mode link by default' do
expect(helper.header_links).not_to include(:admin_mode)
end
@@ -52,9 +52,9 @@
end
end
- context 'feature flag :user_mode_in_session is disabled' do
+ context 'application setting :admin_mode is disabled' do
before do
- stub_feature_flags(user_mode_in_session: false)
+ stub_application_setting(admin_mode: false)
end
it 'does not contain the admin mode link' do
diff --git a/spec/lib/constraints/admin_constrainer_spec.rb b/spec/lib/constraints/admin_constrainer_spec.rb
index ac6ad31120eb7..6e8909ca129bc 100644
--- a/spec/lib/constraints/admin_constrainer_spec.rb
+++ b/spec/lib/constraints/admin_constrainer_spec.rb
@@ -16,7 +16,7 @@
end
describe '#matches' do
- context 'feature flag :user_mode_in_session is enabled' do
+ context 'application setting :admin_mode is enabled' do
context 'when user is a regular user' do
it 'forbids access' do
expect(subject.matches?(request)).to be(false)
@@ -46,9 +46,9 @@
end
end
- context 'feature flag :user_mode_in_session is disabled' do
+ context 'application setting :admin_mode is disabled' do
before do
- stub_feature_flags(user_mode_in_session: false)
+ stub_application_setting(admin_mode: false)
end
context 'when user is a regular user' do
diff --git a/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb b/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb
index 39029322e2537..e70b34d65578f 100644
--- a/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb
+++ b/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb
@@ -38,7 +38,7 @@
end
end
- context 'with application settings and admin users' do
+ context 'with application settings and admin users', :do_not_mock_admin_mode_setting do
let(:group) { result[:group] }
let(:application_setting) { Gitlab::CurrentSettings.current_application_settings }
diff --git a/spec/lib/gitlab/sidekiq_middleware/admin_mode/client_spec.rb b/spec/lib/gitlab/sidekiq_middleware/admin_mode/client_spec.rb
index 3ba08455d019d..9d5d5f28eab3b 100644
--- a/spec/lib/gitlab/sidekiq_middleware/admin_mode/client_spec.rb
+++ b/spec/lib/gitlab/sidekiq_middleware/admin_mode/client_spec.rb
@@ -74,9 +74,9 @@ def perform; end
end
end
- context 'admin mode feature disabled' do
+ context 'admin mode setting disabled' do
before do
- stub_feature_flags(user_mode_in_session: false)
+ stub_application_setting(admin_mode: false)
end
it 'yields block' do
diff --git a/spec/lib/gitlab/sidekiq_middleware/admin_mode/server_spec.rb b/spec/lib/gitlab/sidekiq_middleware/admin_mode/server_spec.rb
index e8322b118756d..3ab1a9cd2f4dc 100644
--- a/spec/lib/gitlab/sidekiq_middleware/admin_mode/server_spec.rb
+++ b/spec/lib/gitlab/sidekiq_middleware/admin_mode/server_spec.rb
@@ -52,9 +52,9 @@ def perform; end
end
end
- context 'admin mode feature disabled' do
+ context 'admin mode setting disabled' do
before do
- stub_feature_flags(user_mode_in_session: false)
+ stub_application_setting(admin_mode: false)
end
it 'yields block' do
diff --git a/spec/models/concerns/cacheable_attributes_spec.rb b/spec/models/concerns/cacheable_attributes_spec.rb
index f2877bed9cf48..dc80e30216ac3 100644
--- a/spec/models/concerns/cacheable_attributes_spec.rb
+++ b/spec/models/concerns/cacheable_attributes_spec.rb
@@ -205,7 +205,7 @@ def initialize(attrs = {}, *)
end
end
- it 'uses RequestStore in addition to process memory cache', :request_store do
+ it 'uses RequestStore in addition to process memory cache', :request_store, :do_not_mock_admin_mode_setting do
# Warm up the cache
create(:application_setting).cache!
diff --git a/spec/presenters/clusters/cluster_presenter_spec.rb b/spec/presenters/clusters/cluster_presenter_spec.rb
index 2d38c91499a80..2e8364b2987c9 100644
--- a/spec/presenters/clusters/cluster_presenter_spec.rb
+++ b/spec/presenters/clusters/cluster_presenter_spec.rb
@@ -347,7 +347,7 @@
before do
project.add_maintainer(user)
- stub_feature_flags(user_mode_in_session: false)
+ stub_application_setting(admin_mode: false)
end
context 'user can read logs' do
@@ -363,7 +363,7 @@
before do
project.add_developer(user)
- stub_feature_flags(user_mode_in_session: false)
+ stub_application_setting(admin_mode: false)
end
it 'returns nil' do
diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb
index 86999c4adaa48..d9d021ba75851 100644
--- a/spec/requests/api/internal/base_spec.rb
+++ b/spec/requests/api/internal/base_spec.rb
@@ -1115,7 +1115,7 @@
end
end
- context 'feature flag :user_mode_in_session is enabled' do
+ context 'application setting :admin_mode is enabled' do
context 'with an admin user' do
let(:user) { create(:admin) }
@@ -1147,9 +1147,9 @@
end
end
- context 'feature flag :user_mode_in_session is disabled' do
+ context 'application setting :admin_mode is disabled' do
before do
- stub_feature_flags(user_mode_in_session: false)
+ stub_application_setting(admin_mode: false)
end
context 'with an admin user' do
diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb
index 3b84c812010c5..48f5bd114a112 100644
--- a/spec/requests/api/settings_spec.rb
+++ b/spec/requests/api/settings_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe API::Settings, 'Settings' do
+RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do
let(:user) { create(:user) }
let_it_be(:admin) { create(:admin) }
@@ -44,6 +44,7 @@
expect(json_response['wiki_page_max_content_bytes']).to be_a(Integer)
expect(json_response['require_admin_approval_after_user_signup']).to eq(true)
expect(json_response['personal_access_token_prefix']).to be_nil
+ expect(json_response['admin_mode']).to be(false)
end
end
@@ -124,7 +125,8 @@
disabled_oauth_sign_in_sources: 'unknown',
import_sources: 'github,bitbucket',
wiki_page_max_content_bytes: 12345,
- personal_access_token_prefix: "GL-"
+ personal_access_token_prefix: "GL-",
+ admin_mode: true
}
expect(response).to have_gitlab_http_status(:ok)
@@ -169,6 +171,7 @@
expect(json_response['import_sources']).to match_array(%w(github bitbucket))
expect(json_response['wiki_page_max_content_bytes']).to eq(12345)
expect(json_response['personal_access_token_prefix']).to eq("GL-")
+ expect(json_response['admin_mode']).to be(true)
end
end
diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb
index e154e691d5fd7..8be26784a3d4b 100644
--- a/spec/requests/jwt_controller_spec.rb
+++ b/spec/requests/jwt_controller_spec.rb
@@ -180,10 +180,11 @@
end
context 'when internal auth is disabled' do
+ before do
+ stub_application_setting(password_authentication_enabled_for_git: false)
+ end
+
it 'rejects the authorization attempt with personal access token message' do
- allow_next_instance_of(ApplicationSetting) do |instance|
- allow(instance).to receive(:password_authentication_enabled_for_git?) { false }
- end
get '/jwt/auth', params: parameters, headers: headers
expect(response).to have_gitlab_http_status(:unauthorized)
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 5ffc9d778d190..d12b960d4fcbe 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -297,7 +297,7 @@
Sidekiq::Worker.clear_all
# Administrators have to re-authenticate in order to access administrative
- # functionality when feature flag :user_mode_in_session is active. Any spec
+ # functionality when application setting admin_mode is active. Any spec
# that requires administrative access can use the tag :enable_admin_mode
# to avoid the second auth step (provided the user is already an admin):
#
@@ -314,6 +314,9 @@
end
end
+ # Make sure specs test by default admin mode setting on, unless forced to the opposite
+ stub_application_setting(admin_mode: true) unless example.metadata[:do_not_mock_admin_mode_setting]
+
allow(Gitlab::CurrentSettings).to receive(:current_application_settings?).and_return(false)
end
--
GitLab