diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 32de4a0145c5d493910a866eeff4c5be27a947cf..607f3435394bd54e58c8ac915c4716ae3c07d8c6 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 527759de0bbef9204cf89b53aad24aa929b77a94..94c0e98c91a7a594b44dfd2b27cfd007f359bb6b 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 a9ef33bf3b9bc8c766462c8734146af7c7526ca0..882fef7a342639c2f72a8ab578ebde4b30e37f94 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 4b6339c21cd0e0c89fd2e88ce5bdc3ca3a345d7c..ebc354489646d2ce2dd0b9fad243834f8ef41a8f 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 c9791703413071d2da7a4fa85ab1243f6e4da993..af502c083d7f37c5448adff5af38bad32362234e 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 085fbfd08da2f01cec3b111dc552730421439d2a..6b65b1c0bff798c6160d90ce0dd3d30efd478e7d 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 c170e58b4ce3688ef6d4f752fb98ccc2c9d960f8..db144f63f9223bff381595cfd0c7a2fd1d4cf0c0 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 44eb2fefb3fdbe8b612da5d4c20e33f06dedfe8d..dbc09a3c9b207fed7661e9f74841a49f18feecb1 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 c067199b52cc3a5b15d200e69ac180a49006d6f0..dba72f889864b333572621186ef35cc9c8b51557 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 e32a889c906233ea45600c41813473c759c9dfaa..1c19751cf0df98af049023e22db596f833573989 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 54bd5cf4072f3d2b7f275d681ad2b85815a8f2c0..65e9c8ec6043006761c72ef2da0554e6bbd811fb 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 7cbef6b00b17b79aa891914a47607dd2811e8212..9e25e6db15f45bd9e8860fbda0e7b84b480a4a16 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 0000000000000000000000000000000000000000..82d8267dc5b1637052c6c67e3433333bdc753895 --- /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 1b0a0053cf4f80980a5cc32793830b482dd59efa..0000000000000000000000000000000000000000 --- 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 0000000000000000000000000000000000000000..a7b634596d235346f8563973e86a57212b2ead97 --- /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 0000000000000000000000000000000000000000..d10e9176a710ab09a535a7e59bb97e50986a426c --- /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 443d54d8c7b6be7d048727be5b954879d0bc8a38..0080ea7f415759fe69fc2c4826e3b7e4888eae67 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 c42df25542a2b7be13bffbafcdaa8094b5789880..913a3699fe48caadc6442458b00ee676e69d69db 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 e9c95a142366a2e168f75f56c29290131a1c388e..62cc2543fc49f6f26c7cf7af0d62bcc5764b4f55 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 a34a63f4543b1919bee643f89b18c730d9c0bae8..50fd6a353545c2a56e1e2f410081cee1bc49630d 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 3b8b0ce30bf76405db9cf50692ff15051f9a8f50..20c16f56d2de495137b02a7700ce8f627b907a8e 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 3c6feee5adb4c300398a58260c9587d7da0f613c..6e89aa72befa967d8c585ea54627efb7129f3ab1 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 8641271f2dfa23ff5c9d5821df2f41a707b2d176..8822a30d4a15d0d26913a9b129e6b2659b37b8cb 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 3dd01b96e391402640b4acbcc6bcc7142866a4f1..664b05ea0108a6a3e30ef4c261af104033722c96 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 64a72b4cb7f77db334b4f17b03a4b35d5f91c1c1..95d0c525cedf227a27fd3dcaf35487d2213c7058 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 59c855a1b73489f7f8f6f869a1c9f541f639eefa..2f32cc7ad91e90825f4a50f8bc2b814c119701d9 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 36204e1bee0bcfe6232d3a885402ac3db8834e72..1b33743a0e93a35482089b3283a06201cdb22b3f 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 6366867a0facf630d811338494257b94d74578fb..c4e64705d6e331ab57c99ceec64b594e802dd3a5 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 283087ede9577d13f21191b278bc934f140c6153..f0add286da8400bed71cb258c80365a432a0f3e2 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 "" @@ -25900,6 +25903,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 2b562e2dd64efafa5e574aa28f649786ccc439e6..6258dd304386b16427107cff0272bf3f0889a1de 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 c6ad1a00484728b7027a6a84216fc5baf5ecea80..106b1d53fd2bd3af78b86cd5bdb4e9a4c7a69b55 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 6b06e2241891dca97cc4aa55a08bc09c66a821fb..474e3a3b009dae708ad3e217f2d2c63206ab04c9 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 d2bcd6d71dbcbf9519540e339895462b6aa7ad4f..633de20c82dc476c3890d53d51193d767c16c1d0 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 249621f5835ed6e3cb16de91b0111ff2802fbc5a..f47db1342e3d87ac2503c78279a7112e4be53c07 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 eadcb9cd008dc0668dc4f9bdd496d156d553fb03..559edb8bf532b27e1c953cc7df2e4fe2d5404285 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 c50092d7f0edc012a125b561ea2f9ea0b00c1dad..e0d2eff8a219f044131746243ac5afd21a822173 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 c7470f31ad840cb3c660d9b4a5ff205d387b7525..3ccf5ded9f57dd27cafef4395923ffc8c5348ff8 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 c4795a814ba0662f2355343651f0a010fdcbc847..2efff3402c5d9f06d94fe0fcdc54498ee700aced 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 ac6ad31120eb79bd3b9dd8876bbeccc1e5aa8e8b..6e8909ca129bca7ad0b73da06d60a0ef1c821e62 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 39029322e2537c156222e48ce747aee28d78b33f..e70b34d65578fa0a41768fef9d38e8824811e846 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 3ba08455d019db153e3b8c07700b6ca4b80e0cf0..9d5d5f28eab3b5e1cb98a455af1a04d386a8c47a 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 e8322b118756dad03609800b2dac89cea3a09369..3ab1a9cd2f4dcdf65a24b9974238e76165d5582d 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 f2877bed9cf48baf101c4422b137f14ece00529e..dc80e30216ac3ac1e3fc36f715eb9108fe346349 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 2d38c91499a80130ce50f11b925cd685ab169c95..2e8364b2987c9972e3cee1e35c78c8670efd30cc 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 86999c4adaa484ea420c2d528164e80bf53bc8b6..d9d021ba758512d52628bdc2cc30ca63288226ad 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 3b84c812010c56bc87c03e9db245aa0d683b183d..48f5bd114a1126e3581738e03cb9496d8a02b043 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 e154e691d5fd7a2ffca9aef49bd36709b0d01d27..8be26784a3d4bb6ca302614d6072295163ecf7d6 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 5ffc9d778d1907b9c10bfb36c0d5de1503d28c8b..d12b960d4fcbe32b48b6e1038fcb31e847a9a576 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