diff options
54 files changed, 1354 insertions, 399 deletions
diff --git a/.gitlab/ci/qa.gitlab-ci.yml b/.gitlab/ci/qa.gitlab-ci.yml index 6e6939824ec..50b29db6e8f 100644 --- a/.gitlab/ci/qa.gitlab-ci.yml +++ b/.gitlab/ci/qa.gitlab-ci.yml @@ -3,8 +3,6 @@ image: ruby:2.6-alpine stage: qa dependencies: [] - variables: - GIT_DEPTH: "1" retry: 0 script: - source scripts/utils.sh @@ -87,9 +87,9 @@ gem 'rack-cors', '~> 1.0.0', require: 'rack/cors' # GraphQL API gem 'graphql', '~> 1.9.11' -# NOTE: graphiql-rails v1.5+ doesn't work: https://gitlab.com/gitlab-org/gitlab-ce/issues/67293 +# NOTE: graphiql-rails v1.5+ doesn't work: https://gitlab.com/gitlab-org/gitlab/issues/31771 # TODO: remove app/views/graphiql/rails/editors/show.html.erb when https://github.com/rmosolgo/graphiql-rails/pull/71 is released: -# https://gitlab.com/gitlab-org/gitlab-ce/issues/67263 +# https://gitlab.com/gitlab-org/gitlab/issues/31747 gem 'graphiql-rails', '~> 1.4.10' gem 'apollo_upload_server', '~> 2.0.0.beta3' gem 'graphql-docs', '~> 1.6.0', group: [:development, :test] diff --git a/PROCESS.md b/PROCESS.md index f0a82838f62..997612dfbad 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -328,7 +328,7 @@ Thanks for the issue report. This issue has already been fixed in newer versions Due to the size of this project and our limited resources we are only able to support the latest stable release as outlined in our [contributing guidelines](https://docs.gitlab.com/ee/development/contributing/issue_workflow.html). In order to get this bug fix and enjoy many new features please -[upgrade](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/doc/update). +[upgrade](https://gitlab.com/gitlab-org/gitlab/tree/master/doc/update). If you still experience issues at that time please open a new issue following our issue tracker guidelines found in the [contributing guidelines](https://docs.gitlab.com/ee/development/contributing/issue_workflow.html#issue-tracker-guidelines). ``` @@ -337,14 +337,14 @@ tracker guidelines found in the [contributing guidelines](https://docs.gitlab.co ``` Thanks for your interest in improving the GitLab codebase! -Please update your merge request according to the [contributing guidelines](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/contributing/merge_request_workflow.md#merge-request-guidelines). +Please update your merge request according to the [contributing guidelines](https://gitlab.com/gitlab-org/gitlab/blob/master/doc/development/contributing/merge_request_workflow.md#merge-request-guidelines). ``` ### Accepting merge requests ``` Is there an issue on the -[issue tracker](https://gitlab.com/gitlab-org/gitlab-ce/issues) that is +[issue tracker](https://gitlab.com/gitlab-org/gitlab/issues) that is similar to this? Could you please link it here? Please be aware that new functionality that is not marked [`Accepting merge requests`](https://docs.gitlab.com/ee/development/contributing/issue_workflow.html#label-for-community-contributors) diff --git a/app/assets/javascripts/jobs/store/utils.js b/app/assets/javascripts/jobs/store/utils.js index fd2af0e421b..4a7d870674b 100644 --- a/app/assets/javascripts/jobs/store/utils.js +++ b/app/assets/javascripts/jobs/store/utils.js @@ -24,6 +24,46 @@ export const parseHeaderLine = (line = {}, lineNumber) => ({ }); /** + * Finds the matching header section + * for the section_duration object and adds it to it + * + * { + * isHeader: true, + * line: { + * content: [], + * lineNumber: 0, + * section_duration: "", + * }, + * lines: [] + * } + * + * @param Array data + * @param Object durationLine + */ +export function addDurationToHeader(data, durationLine) { + data.forEach(el => { + if (el.line && el.line.section === durationLine.section) { + el.line.section_duration = durationLine.section_duration; + } + }); +} + +/** + * Check is the current section belongs to a collapsible section + * + * @param Array acc + * @param Object last + * @param Object section + * + * @returns Boolean + */ +export const isCollapsibleSection = (acc = [], last = {}, section = {}) => + acc.length > 0 && + last.isHeader === true && + !section.section_duration && + section.section === last.line.section; + +/** * Parses the job log content into a structure usable by the template * * For collaspible lines (section_header = true): @@ -32,28 +72,35 @@ export const parseHeaderLine = (line = {}, lineNumber) => ({ * - adds a isHeader property to handle template logic * - adds the section_duration * For each line: - * - adds the index as lineNumber + * - adds the index as lineNumber * - * @param {Array} lines - * @returns {Array} + * @param Array lines + * @param Number lineNumberStart + * @param Array accumulator + * @returns Array parsed log lines */ -export const logLinesParser = (lines = [], lineNumberStart) => +export const logLinesParser = (lines = [], lineNumberStart, accumulator = []) => lines.reduce((acc, line, index) => { const lineNumber = lineNumberStart ? lineNumberStart + index : index; const last = acc[acc.length - 1]; + // If the object is an header, we parse it into another structure if (line.section_header) { acc.push(parseHeaderLine(line, lineNumber)); - } else if (acc.length && last.isHeader && !line.section_duration && line.content.length) { + } else if (isCollapsibleSection(acc, last, line)) { + // if the object belongs to a nested section, we append it to the new `lines` array of the + // previously formated header last.lines.push(parseLine(line, lineNumber)); - } else if (acc.length && last.isHeader && line.section_duration) { - last.section_duration = line.section_duration; - } else if (line.content.length) { + } else if (line.section_duration) { + // if the line has section_duration, we look for the correct header to add it + addDurationToHeader(acc, line); + } else { + // otherwise it's a regular line acc.push(parseLine(line, lineNumber)); } return acc; - }, []); + }, accumulator); /** * Finds the repeated offset, removes the old one diff --git a/app/assets/stylesheets/framework/card.scss b/app/assets/stylesheets/framework/card.scss index 1e51d28bba2..9911b926cbb 100644 --- a/app/assets/stylesheets/framework/card.scss +++ b/app/assets/stylesheets/framework/card.scss @@ -1,8 +1,7 @@ .card-header { &:first-child { // intended use case: card with only a header (for example empty related issues) - &.border-0, - &.border-bottom-0 { + &:last-child { @include border-radius($card-inner-border-radius); } } diff --git a/app/controllers/admin/sessions_controller.rb b/app/controllers/admin/sessions_controller.rb new file mode 100644 index 00000000000..1f946e41995 --- /dev/null +++ b/app/controllers/admin/sessions_controller.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +class Admin::SessionsController < ApplicationController + include InternalRedirect + + before_action :user_is_admin! + + def new + # Renders a form in which the admin can enter their password + end + + def create + if current_user_mode.enable_admin_mode!(password: params[:password]) + redirect_location = stored_location_for(:redirect) || admin_root_path + redirect_to safe_redirect_path(redirect_location) + else + flash.now[:alert] = _('Invalid Login or password') + render :new + end + end + + def destroy + current_user_mode.disable_admin_mode! + + redirect_to root_path, status: :found, notice: _('Admin mode disabled') + end + + private + + def user_is_admin! + render_404 unless current_user&.admin? + end +end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 9a7859fc687..0d0384ba52f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -36,6 +36,7 @@ class ApplicationController < ActionController::Base protect_from_forgery with: :exception, prepend: true helper_method :can? + helper_method :current_user_mode helper_method :import_sources_enabled?, :github_import_enabled?, :gitea_import_enabled?, :github_import_configured?, :gitlab_import_enabled?, :gitlab_import_configured?, @@ -533,6 +534,10 @@ class ApplicationController < ActionController::Base yield end end + + def current_user_mode + @current_user_mode ||= Gitlab::Auth::CurrentUserMode.new(current_user) + end end ApplicationController.prepend_if_ee('EE::ApplicationController') diff --git a/app/controllers/concerns/enforces_admin_authentication.rb b/app/controllers/concerns/enforces_admin_authentication.rb index 3ef92730df6..e731211f423 100644 --- a/app/controllers/concerns/enforces_admin_authentication.rb +++ b/app/controllers/concerns/enforces_admin_authentication.rb @@ -14,6 +14,16 @@ module EnforcesAdminAuthentication end def authenticate_admin! - render_404 unless current_user.admin? + return render_404 unless current_user.admin? + return unless Feature.enabled?(:user_mode_in_session) + + unless current_user_mode.admin_mode? + store_location_for(:redirect, request.fullpath) if storable_location? + redirect_to(new_admin_session_path, notice: _('Re-authentication required')) + end + end + + def storable_location? + request.path != new_admin_session_path end end diff --git a/app/controllers/concerns/sessionless_authentication.rb b/app/controllers/concerns/sessionless_authentication.rb index ba06384a37a..f644923443b 100644 --- a/app/controllers/concerns/sessionless_authentication.rb +++ b/app/controllers/concerns/sessionless_authentication.rb @@ -5,6 +5,12 @@ # Controller concern to handle PAT, RSS, and static objects token authentication methods # module SessionlessAuthentication + extend ActiveSupport::Concern + + included do + before_action :enable_admin_mode!, if: :sessionless_user? + end + # This filter handles personal access tokens, atom requests with rss tokens, and static object tokens def authenticate_sessionless_user!(request_format) user = Gitlab::Auth::RequestAuthenticator.new(request).find_sessionless_user(request_format) @@ -25,4 +31,8 @@ module SessionlessAuthentication sign_in(user, store: false, message: :sessionless_sign_in) end end + + def enable_admin_mode! + current_user_mode.enable_admin_mode!(skip_password_validation: true) if Feature.enabled?(:user_mode_in_session) + end end diff --git a/app/controllers/health_controller.rb b/app/controllers/health_controller.rb index dc9a52f8da5..c97057c08cb 100644 --- a/app/controllers/health_controller.rb +++ b/app/controllers/health_controller.rb @@ -20,9 +20,7 @@ class HealthController < ActionController::Base end def liveness - results = CHECKS.map { |check| [check.name, check.liveness] } - - render_check_results(results) + render json: { status: 'ok' }, status: :ok end private diff --git a/app/helpers/nav_helper.rb b/app/helpers/nav_helper.rb index c6e9a20c5b2..2ce45cec878 100644 --- a/app/helpers/nav_helper.rb +++ b/app/helpers/nav_helper.rb @@ -86,6 +86,12 @@ module NavHelper links << :admin_impersonation end + if Feature.enabled?(:user_mode_in_session) + if current_user&.admin? && current_user_mode&.admin_mode? + links << :admin_mode + end + end + links end end diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index 78379516062..2305c55033f 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -5,7 +5,13 @@ require_dependency 'declarative_policy' class BasePolicy < DeclarativePolicy::Base desc "User is an instance admin" with_options scope: :user, score: 0 - condition(:admin) { @user&.admin? } + condition(:admin) do + if Feature.enabled?(:user_mode_in_session) + Gitlab::Auth::CurrentUserMode.new(@user).admin_mode? + else + @user&.admin? + end + end desc "User is blocked" with_options scope: :user, score: 0 diff --git a/app/services/projects/container_repository/delete_tags_service.rb b/app/services/projects/container_repository/delete_tags_service.rb index 21dc1621e5c..22656b29c58 100644 --- a/app/services/projects/container_repository/delete_tags_service.rb +++ b/app/services/projects/container_repository/delete_tags_service.rb @@ -32,7 +32,7 @@ module Projects # This is a hack as the registry doesn't support deleting individual # tags. This code effectively pushes a dummy image and assigns the tag to it. # This way when the tag is deleted only the dummy image is affected. - # See https://gitlab.com/gitlab-org/gitlab-ce/issues/21405 for a discussion + # See https://gitlab.com/gitlab-org/gitlab/issues/15737 for a discussion def smart_delete(container_repository, tag_names) # generates the blobs for the dummy image dummy_manifest = container_repository.client.generate_empty_manifest(container_repository.path) diff --git a/app/views/admin/sessions/_new_base.html.haml b/app/views/admin/sessions/_new_base.html.haml new file mode 100644 index 00000000000..55aea0296e7 --- /dev/null +++ b/app/views/admin/sessions/_new_base.html.haml @@ -0,0 +1,7 @@ += form_tag(admin_session_path, method: :post, html: { class: 'new_user gl-show-field-errors', 'aria-live': 'assertive'}) do + .form-group + = label_tag :password, _('Password'), class: 'label-bold' + = password_field_tag :password, nil, class: 'form-control', required: true, title: _('This field is required.'), data: { qa_selector: 'password_field' } + + .submit-container.move-submit-down + = submit_tag _('Enter admin mode'), class: 'btn btn-success', data: { qa_selector: 'sign_in_button' } diff --git a/app/views/admin/sessions/_signin_box.html.haml b/app/views/admin/sessions/_signin_box.html.haml new file mode 100644 index 00000000000..69baa76060e --- /dev/null +++ b/app/views/admin/sessions/_signin_box.html.haml @@ -0,0 +1,11 @@ +- if form_based_providers.any? + + - if password_authentication_enabled_for_web? + .login-box.tab-pane{ id: 'login-pane', role: 'tabpanel' } + .login-body + = render 'admin/sessions/new_base' + +- elsif password_authentication_enabled_for_web? + .login-box.tab-pane.active{ id: 'login-pane', role: 'tabpanel' } + .login-body + = render 'admin/sessions/new_base' diff --git a/app/views/admin/sessions/_tabs_normal.html.haml b/app/views/admin/sessions/_tabs_normal.html.haml new file mode 100644 index 00000000000..f5dedb5ad76 --- /dev/null +++ b/app/views/admin/sessions/_tabs_normal.html.haml @@ -0,0 +1,3 @@ +%ul.nav-links.new-session-tabs.nav-tabs.nav{ role: 'tablist' } + %li.nav-item{ role: 'presentation' } + %a.nav-link.active{ href: '#login-pane', data: { toggle: 'tab', qa_selector: 'sign_in_tab' }, role: 'tab' }= _('Enter admin mode') diff --git a/app/views/admin/sessions/new.html.haml b/app/views/admin/sessions/new.html.haml new file mode 100644 index 00000000000..ee06b4a1741 --- /dev/null +++ b/app/views/admin/sessions/new.html.haml @@ -0,0 +1,15 @@ +- @hide_breadcrumbs = true +- page_title _('Enter admin mode') + +.row.justify-content-center + .col-6.new-session-forms-container + .login-page + #signin-container + = render 'admin/sessions/tabs_normal' + .tab-content + - if password_authentication_enabled_for_web? + = render 'admin/sessions/signin_box' + - else + -# Show a message if none of the mechanisms above are enabled + .prepend-top-default.center + = _('No authentication methods configured.') diff --git a/app/views/layouts/nav/_dashboard.html.haml b/app/views/layouts/nav/_dashboard.html.haml index 4b83239dfbd..015ba13be05 100644 --- a/app/views/layouts/nav/_dashboard.html.haml +++ b/app/views/layouts/nav/_dashboard.html.haml @@ -68,6 +68,15 @@ = nav_link(controller: 'admin/dashboard') do = link_to admin_root_path, class: 'd-lg-none admin-icon qa-admin-area-link' do = _('Admin Area') + - if Feature.enabled?(:user_mode_in_session) + - if header_link?(:admin_mode) + = nav_link(controller: 'admin/sessions') do + = link_to destroy_admin_session_path, class: 'd-lg-none lock-open-icon' do + = _('Leave admin mode') + - elsif current_user.admin? + = nav_link(controller: 'admin/sessions') do + = link_to new_admin_session_path, class: 'd-lg-none lock-icon' do + = _('Enter admin mode') - if Gitlab::Sherlock.enabled? %li = link_to sherlock_transactions_path, class: 'd-lg-none admin-icon' do @@ -95,6 +104,17 @@ = nav_link(controller: 'admin/dashboard', html_options: { class: "d-none d-lg-block d-xl-block"}) do = 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 header_link?(:admin_mode) + = nav_link(controller: 'admin/sessions', html_options: { class: "d-none d-lg-block d-xl-block"}) do + = link_to destroy_admin_session_path, title: _('Leave admin mode'), aria: { label: _('Leave admin mode') }, data: { toggle: 'tooltip', placement: 'bottom', container: 'body' } do + = sprite_icon('lock-open', size: 18) + - elsif current_user.admin? + = nav_link(controller: 'admin/sessions', html_options: { class: "d-none d-lg-block d-xl-block"}) do + = link_to new_admin_session_path, title: _('Enter admin mode'), aria: { label: _('Enter admin mode') }, data: { toggle: 'tooltip', placement: 'bottom', container: 'body' } do + = sprite_icon('lock', size: 18) + - if Gitlab::Sherlock.enabled? %li = link_to sherlock_transactions_path, class: 'admin-icon d-none d-lg-block d-xl-block', title: _('Sherlock Transactions'), diff --git a/babel.config.js b/babel.config.js index 05554e8763e..79c401ed975 100644 --- a/babel.config.js +++ b/babel.config.js @@ -44,7 +44,7 @@ if (isJest) { plugins.push('@babel/plugin-transform-modules-commonjs'); /* without the following, babel-plugin-istanbul throws an error: - https://gitlab.com/gitlab-org/gitlab-ce/issues/58390 + https://gitlab.com/gitlab-org/gitlab-foss/issues/58390 */ plugins.push('babel-plugin-dynamic-import-node'); } diff --git a/changelogs/unreleased/feat-user-mode-in-session-for-admins.yml b/changelogs/unreleased/feat-user-mode-in-session-for-admins.yml new file mode 100644 index 00000000000..ea8760d5224 --- /dev/null +++ b/changelogs/unreleased/feat-user-mode-in-session-for-admins.yml @@ -0,0 +1,6 @@ +--- +title: Require admins to enter admin-mode by re-authenticating before performing + administrative operations +merge_request: 16981 +author: Roger Rüttimann & Diego Louzán +type: added diff --git a/changelogs/unreleased/stop-liveness-check-returning-incorrect-data.yml b/changelogs/unreleased/stop-liveness-check-returning-incorrect-data.yml new file mode 100644 index 00000000000..0238ac469f8 --- /dev/null +++ b/changelogs/unreleased/stop-liveness-check-returning-incorrect-data.yml @@ -0,0 +1,5 @@ +--- +title: Changes response body of liveness check to be more accurate +merge_request: 17655 +author: +type: changed diff --git a/changelogs/unreleased/winh-related-issues-border.yml b/changelogs/unreleased/winh-related-issues-border.yml index 2a8beecdefe..8b6b62b440d 100644 --- a/changelogs/unreleased/winh-related-issues-border.yml +++ b/changelogs/unreleased/winh-related-issues-border.yml @@ -1,5 +1,5 @@ --- title: Redo fix for related issues border radius -merge_request: 17172 +merge_request: 17480 author: type: fixed diff --git a/config/initializers/google_api_client.rb b/config/initializers/google_api_client.rb index 611726a20c7..71338d2d9e1 100644 --- a/config/initializers/google_api_client.rb +++ b/config/initializers/google_api_client.rb @@ -6,7 +6,7 @@ # as a workaround until this is resolved. # # This can be removed once fog-google and google-api-client can be upgraded. -# See https://gitlab.com/gitlab-org/gitlab-ce/issues/66630 for more details. +# See https://gitlab.com/gitlab-org/gitlab/issues/31280 for more details. # require 'google/apis/container_v1beta1' diff --git a/config/routes/admin.rb b/config/routes/admin.rb index a003ffca270..418d3324569 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -21,6 +21,10 @@ namespace :admin do end end + resource :session, only: [:new, :create] do + get 'destroy', action: :destroy, as: :destroy + end + resource :impersonation, only: :destroy resources :abuse_reports, only: [:index, :destroy] diff --git a/doc/administration/raketasks/uploads/migrate.md b/doc/administration/raketasks/uploads/migrate.md index 3bd8f2c27d4..517d6b01438 100644 --- a/doc/administration/raketasks/uploads/migrate.md +++ b/doc/administration/raketasks/uploads/migrate.md @@ -125,7 +125,7 @@ CAUTION: **Warning:** **Extended downtime is required** so no new files are created in object storage during the migration. A configuration setting will be added soon to allow migrating from object storage to local files with only a brief moment of downtime for configuration changes. - See issue [gitlab-org/gitlab-ce#66144](https://gitlab.com/gitlab-org/gitlab-ce/issues/66144) + See issue [gitlab-org/gitlab#30979](https://gitlab.com/gitlab-org/gitlab/issues/30979) ### All-in-one rake task diff --git a/doc/user/admin_area/monitoring/health_check.md b/doc/user/admin_area/monitoring/health_check.md index ec043bcffdb..3fbfba2f839 100644 --- a/doc/user/admin_area/monitoring/health_check.md +++ b/doc/user/admin_area/monitoring/health_check.md @@ -116,28 +116,11 @@ curl 'https://gitlab.example.com/-/liveness' Example response: -On success, the endpoint will return a valid successful HTTP status code, and a response like below. +On success, the endpoint will return a `200` HTTP status code, and a response like below. ```json { - "db_check":{ - "status":"ok" - }, - "redis_check":{ - "status":"ok" - }, - "cache_check":{ - "status":"ok" - }, - "queues_check":{ - "status":"ok" - }, - "shared_state_check":{ - "status":"ok" - }, - "gitaly_check":{ - "status":"ok" - } + "status": "ok" } ``` diff --git a/doc/user/group/subgroups/index.md b/doc/user/group/subgroups/index.md index 376d4510934..a3606fadb89 100644 --- a/doc/user/group/subgroups/index.md +++ b/doc/user/group/subgroups/index.md @@ -4,7 +4,7 @@ type: reference, howto, concepts # Subgroups ->[Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/2772) in GitLab 9.0. +>[Introduced](https://gitlab.com/gitlab-org/gitlab-foss/issues/2772) in GitLab 9.0. Subgroups, also known as nested groups or hierarchical groups, allow you to have up to 20 levels of groups. diff --git a/jest.config.js b/jest.config.js index 43d68c60137..435dca876ce 100644 --- a/jest.config.js +++ b/jest.config.js @@ -3,7 +3,7 @@ const IS_EE = require('./config/helpers/is_ee_env'); const reporters = ['default']; // To have consistent date time parsing both in local and CI environments we set -// the timezone of the Node process. https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/27738 +// the timezone of the Node process. https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/27738 process.env.TZ = 'GMT'; if (process.env.CI) { diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index a3fa7cd5cf9..02ea321df67 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -17,6 +17,8 @@ module API request.access_token end + use AdminModeMiddleware + helpers HelperMethods install_error_responders(base) @@ -52,6 +54,11 @@ module API forbidden!(api_access_denied_message(user)) end + # Set admin mode for API requests (if admin) + if Feature.enabled?(:user_mode_in_session) + Gitlab::Auth::CurrentUserMode.new(user).enable_admin_mode!(skip_password_validation: true) + end + user end @@ -141,5 +148,22 @@ module API end end end + + class AdminModeMiddleware < ::Grape::Middleware::Base + def initialize(app, **options) + super + end + + def call(env) + if Feature.enabled?(:user_mode_in_session) + session = {} + Gitlab::Session.with_session(session) do + app.call(env) + end + else + app.call(env) + end + end + end end end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 72bbba8065e..4ea7d359908 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1052,7 +1052,7 @@ module API expose :job_events # Expose serialized properties expose :properties do |service, options| - # TODO: Simplify as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084 + # TODO: Simplify as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 if service.data_fields_present? service.data_fields.as_json.slice(*service.api_field_names) else diff --git a/lib/gitlab/auth/current_user_mode.rb b/lib/gitlab/auth/current_user_mode.rb new file mode 100644 index 00000000000..df5039f50c1 --- /dev/null +++ b/lib/gitlab/auth/current_user_mode.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module Gitlab + module Auth + # Keeps track of the current session user mode + # + # In order to perform administrative tasks over some interfaces, + # an administrator must have explicitly enabled admin-mode + # e.g. on web access require re-authentication + class CurrentUserMode + SESSION_STORE_KEY = :current_user_mode + ADMIN_MODE_START_TIME_KEY = 'admin_mode' + MAX_ADMIN_MODE_TIME = 6.hours + + def initialize(user) + @user = user + end + + def admin_mode? + return false unless user + + Gitlab::SafeRequestStore.fetch(request_store_key) do + user&.admin? && any_session_with_admin_mode? + end + end + + def enable_admin_mode!(password: nil, skip_password_validation: false) + return unless user&.admin? + return unless skip_password_validation || user&.valid_password?(password) + + current_session_data[ADMIN_MODE_START_TIME_KEY] = Time.now + end + + def disable_admin_mode! + current_session_data[ADMIN_MODE_START_TIME_KEY] = nil + Gitlab::SafeRequestStore.delete(request_store_key) + end + + private + + attr_reader :user + + def request_store_key + @request_store_key ||= { res: :current_user_mode, user: user.id } + end + + def current_session_data + @current_session ||= Gitlab::NamespacedSessionStore.new(SESSION_STORE_KEY) + end + + def any_session_with_admin_mode? + return true if current_session_data.initiated? && current_session_data[ADMIN_MODE_START_TIME_KEY].to_i > MAX_ADMIN_MODE_TIME.ago.to_i + + all_sessions.any? do |session| + session[ADMIN_MODE_START_TIME_KEY].to_i > MAX_ADMIN_MODE_TIME.ago.to_i + end + end + + def all_sessions + @all_sessions ||= ActiveSession.list_sessions(user).lazy.map do |session| + Gitlab::NamespacedSessionStore.new(SESSION_STORE_KEY, session.with_indifferent_access ) + end + end + end + end +end diff --git a/lib/gitlab/file_type_detection.rb b/lib/gitlab/file_type_detection.rb index c2b9dfa562d..7137720f204 100644 --- a/lib/gitlab/file_type_detection.rb +++ b/lib/gitlab/file_type_detection.rb @@ -12,7 +12,7 @@ # We use Workhorse to detect the real extension when we serve files with # the `SendsBlob` helper methods, and ask Workhorse to set the content # type when it serves the file: -# https://gitlab.com/gitlab-org/gitlab-ce/blob/33e5955/app/helpers/workhorse_helper.rb#L48. +# https://gitlab.com/gitlab-org/gitlab/blob/33e5955/app/helpers/workhorse_helper.rb#L48. # # Because Workhorse has access to the content when it is downloaded, if # the type/extension doesn't match the real type, we adjust the diff --git a/lib/gitlab/health_checks/base_abstract_check.rb b/lib/gitlab/health_checks/base_abstract_check.rb index 1d31f59999c..199cd2f9b2d 100644 --- a/lib/gitlab/health_checks/base_abstract_check.rb +++ b/lib/gitlab/health_checks/base_abstract_check.rb @@ -15,10 +15,6 @@ module Gitlab raise NotImplementedError end - def liveness - HealthChecks::Result.new(true) - end - def metrics [] end diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index c5303dad558..1a3d848e692 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -187,7 +187,7 @@ module Gitlab .find_in_batches(batch_size: BATCH_SIZE) do |services| counts = services.group_by do |service| - # TODO: Simplify as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084 + # TODO: Simplify as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 service_url = service.data_fields&.url || (service.properties && service.properties['url']) service_url&.include?('.atlassian.net') ? :cloud : :server end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index cd7b27e81a1..c8b44197514 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1031,6 +1031,9 @@ msgstr "" msgid "Admin Section" msgstr "" +msgid "Admin mode disabled" +msgstr "" + msgid "Admin notes" msgstr "" @@ -5734,6 +5737,9 @@ msgstr "" msgid "Enter a number" msgstr "" +msgid "Enter admin mode" +msgstr "" + msgid "Enter at least three characters to search" msgstr "" @@ -9123,6 +9129,9 @@ msgstr "" msgid "Leave" msgstr "" +msgid "Leave admin mode" +msgstr "" + msgid "Leave edit mode? All unsaved changes will be lost." msgstr "" @@ -12739,6 +12748,9 @@ msgstr "" msgid "Raw blob request rate limit per minute" msgstr "" +msgid "Re-authentication required" +msgstr "" + msgid "Read more" msgstr "" diff --git a/public/robots.txt b/public/robots.txt index 7908297564e..2cda837d6f1 100644 --- a/public/robots.txt +++ b/public/robots.txt @@ -8,7 +8,7 @@ # Only some crawlers respect this setting, e.g. Googlebot does not # Crawl-delay: 1 -# Based on details in https://gitlab.com/gitlab-org/gitlab-ce/blob/master/config/routes.rb, https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/routing, and using application +# Based on details in https://gitlab.com/gitlab-org/gitlab/blob/master/config/routes.rb, https://gitlab.com/gitlab-org/gitlab/blob/master/spec/routing, and using application User-Agent: * Disallow: /autocomplete/users Disallow: /search diff --git a/spec/controllers/admin/sessions_controller_spec.rb b/spec/controllers/admin/sessions_controller_spec.rb new file mode 100644 index 00000000000..c1cb57c0b9d --- /dev/null +++ b/spec/controllers/admin/sessions_controller_spec.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Admin::SessionsController, :do_not_mock_admin_mode do + include_context 'custom session' + + let(:user) { create(:user) } + + before do + sign_in(user) + end + + describe '#new' do + context 'for regular users' do + it 'shows error page' do + get :new + + expect(response).to have_gitlab_http_status(:not_found) + expect(controller.send(:current_user_mode).admin_mode?).to be(false) + end + end + + context 'for admin users' do + let(:user) { create(:admin) } + + it 'renders a password form' do + get :new + + expect(response).to render_template :new + expect(controller.send(:current_user_mode).admin_mode?).to be(false) + end + end + end + + describe '#create' do + context 'for regular users' do + it 'shows error page' do + post :create + + expect(response).to have_gitlab_http_status(:not_found) + expect(controller.send(:current_user_mode).admin_mode?).to be(false) + end + end + + context 'for admin users' do + let(:user) { create(:admin) } + + it 'sets admin mode with a valid password' do + expect(controller.send(:current_user_mode).admin_mode?).to be(false) + + controller.store_location_for(:redirect, admin_root_path) + post :create, params: { password: user.password } + + expect(response).to redirect_to admin_root_path + expect(controller.send(:current_user_mode).admin_mode?).to be(true) + end + + it 'fails with an invalid password' do + expect(controller.send(:current_user_mode).admin_mode?).to be(false) + + controller.store_location_for(:redirect, admin_root_path) + + post :create, params: { password: '' } + + expect(response).to render_template :new + expect(controller.send(:current_user_mode).admin_mode?).to be(false) + end + end + end + + describe '#destroy' do + context 'for regular users' do + it 'shows error page' do + get :destroy + + expect(response).to have_gitlab_http_status(404) + expect(controller.send(:current_user_mode).admin_mode?).to be(false) + end + end + + context 'for admin users' do + let(:user) { create(:admin) } + + it 'disables admin mode and redirects to main page' do + expect(controller.send(:current_user_mode).admin_mode?).to be(false) + post :create, params: { password: user.password } + expect(controller.send(:current_user_mode).admin_mode?).to be(true) + + get :destroy + + expect(response).to have_gitlab_http_status(:found) + expect(response).to redirect_to(root_path) + expect(controller.send(:current_user_mode).admin_mode?).to be(false) + end + end + end +end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 5ef8a15d702..8c3642fb047 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -777,4 +777,48 @@ describe ApplicationController do end end end + + describe '#current_user_mode', :do_not_mock_admin_mode do + include_context 'custom session' + + controller(described_class) do + def index + render html: 'authenticated' + end + end + + before do + allow(ActiveSession).to receive(:list_sessions).with(user).and_return([session]) + + sign_in(user) + get :index + end + + context 'with a regular user' do + it 'admin mode is not set' do + expect(response).to have_gitlab_http_status(:ok) + expect(Gitlab::Auth::CurrentUserMode.new(user).admin_mode?).to be(false) + end + end + + context 'with an admin user' do + let(:user) { create(:admin) } + + it 'admin mode is not set' do + expect(response).to have_gitlab_http_status(:ok) + expect(Gitlab::Auth::CurrentUserMode.new(user).admin_mode?).to be(false) + end + + context 'that re-authenticated' do + before do + Gitlab::Auth::CurrentUserMode.new(user).enable_admin_mode!(password: user.password) + end + + it 'admin mode is set' do + expect(response).to have_gitlab_http_status(:ok) + expect(Gitlab::Auth::CurrentUserMode.new(user).admin_mode?).to be(true) + end + end + end + end end diff --git a/spec/controllers/concerns/enforces_admin_authentication_spec.rb b/spec/controllers/concerns/enforces_admin_authentication_spec.rb index e6a6702fdea..019a21e8cf0 100644 --- a/spec/controllers/concerns/enforces_admin_authentication_spec.rb +++ b/spec/controllers/concerns/enforces_admin_authentication_spec.rb @@ -2,7 +2,9 @@ require 'spec_helper' -describe EnforcesAdminAuthentication do +describe EnforcesAdminAuthentication, :do_not_mock_admin_mode do + include AdminModeHelper + let(:user) { create(:user) } before do @@ -10,30 +12,86 @@ describe EnforcesAdminAuthentication do end controller(ApplicationController) do - # `described_class` is not available in this context - include EnforcesAdminAuthentication # rubocop:disable RSpec/DescribedClass + include EnforcesAdminAuthentication def index head :ok end end - describe 'authenticate_admin!' do - context 'as an admin' do - let(:user) { create(:admin) } + context 'feature flag :user_mode_in_session is enabled' do + describe 'authenticate_admin!' do + context 'as an admin' do + let(:user) { create(:admin) } - it 'renders ok' do - get :index + it 'renders redirect for re-authentication and does not set admin mode' do + get :index + + expect(response).to redirect_to new_admin_session_path + expect(assigns(:current_user_mode)&.admin_mode?).to be(false) + end + + context 'when admin mode is active' do + before do + enable_admin_mode!(user) + end + + it 'renders ok' do + get :index + + expect(response).to have_gitlab_http_status(200) + end + end + end + + context 'as a user' do + it 'renders a 404' do + get :index + + expect(response).to have_gitlab_http_status(404) + end + + it 'does not set admin mode' do + get :index - expect(response).to have_gitlab_http_status(200) + # check for nil too since on 404, current_user_mode might not be initialized + expect(assigns(:current_user_mode)&.admin_mode?).to be_falsey + end end end + end + + context 'feature flag :user_mode_in_session is disabled' do + before do + stub_feature_flags(user_mode_in_session: false) + end - context 'as a user' do - it 'renders a 404' do + describe 'authenticate_admin!' do + before do get :index + end + + context 'as an admin' do + let(:user) { create(:admin) } + + it 'allows direct access to page' do + expect(response).to have_gitlab_http_status(200) + end + + it 'does not set admin mode' do + expect(assigns(:current_user_mode)&.admin_mode?).to be_falsey + end + end + + context 'as a user' do + it 'renders a 404' do + expect(response).to have_gitlab_http_status(404) + end - expect(response).to have_gitlab_http_status(404) + it 'does not set admin mode' do + # check for nil too since on 404, current_user_mode might not be initialized + expect(assigns(:current_user_mode)&.admin_mode?).to be_falsey + end end end end diff --git a/spec/controllers/health_controller_spec.rb b/spec/controllers/health_controller_spec.rb index e82dcfcdb64..8c44ae4e3de 100644 --- a/spec/controllers/health_controller_spec.rb +++ b/spec/controllers/health_controller_spec.rb @@ -76,10 +76,7 @@ describe HealthController do it 'responds with liveness checks data' do subject - expect(json_response['db_check']['status']).to eq('ok') - expect(json_response['cache_check']['status']).to eq('ok') - expect(json_response['queues_check']['status']).to eq('ok') - expect(json_response['shared_state_check']['status']).to eq('ok') + expect(json_response['status']).to eq('ok') end end diff --git a/spec/factories/services.rb b/spec/factories/services.rb index c4cb3f07fe7..e063b888d77 100644 --- a/spec/factories/services.rb +++ b/spec/factories/services.rb @@ -117,7 +117,7 @@ FactoryBot.define do end # this is for testing storing values inside properties, which is deprecated and will be removed in - # https://gitlab.com/gitlab-org/gitlab-ce/issues/63084 + # https://gitlab.com/gitlab-org/gitlab/issues/29404 trait :without_properties_callback do jira_tracker_data nil issue_tracker_data nil diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index ab58d9fb9d5..e1c9364067a 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -2,450 +2,529 @@ require 'spec_helper' -describe 'Admin updates settings' do +describe 'Admin updates settings', :clean_gitlab_redis_shared_state, :do_not_mock_admin_mode do include StubENV include TermsHelper let(:admin) { create(:admin) } - before do - stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') - sign_in(admin) - end - - context 'General page' do + context 'feature flag :user_mode_in_session is enabled' do before do - visit general_admin_application_settings_path + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + sign_in(admin) + gitlab_enable_admin_mode_sign_in(admin) end - it 'Change visibility settings' do - page.within('.as-visibility-access') do - choose "application_setting_default_project_visibility_20" - click_button 'Save changes' + context 'General page' do + before do + visit general_admin_application_settings_path end - expect(page).to have_content "Application settings saved successfully" - end + it 'Change visibility settings' do + page.within('.as-visibility-access') do + choose "application_setting_default_project_visibility_20" + click_button 'Save changes' + end - it 'Uncheck all restricted visibility levels' do - page.within('.as-visibility-access') do - find('#application_setting_visibility_level_0').set(false) - find('#application_setting_visibility_level_10').set(false) - find('#application_setting_visibility_level_20').set(false) - click_button 'Save changes' + expect(page).to have_content "Application settings saved successfully" end - expect(page).to have_content "Application settings saved successfully" - expect(find('#application_setting_visibility_level_0')).not_to be_checked - expect(find('#application_setting_visibility_level_10')).not_to be_checked - expect(find('#application_setting_visibility_level_20')).not_to be_checked - end + it 'Uncheck all restricted visibility levels' do + page.within('.as-visibility-access') do + find('#application_setting_visibility_level_0').set(false) + find('#application_setting_visibility_level_10').set(false) + find('#application_setting_visibility_level_20').set(false) + click_button 'Save changes' + end + + expect(page).to have_content "Application settings saved successfully" + expect(find('#application_setting_visibility_level_0')).not_to be_checked + expect(find('#application_setting_visibility_level_10')).not_to be_checked + expect(find('#application_setting_visibility_level_20')).not_to be_checked + end - it 'Modify import sources' do - expect(current_settings.import_sources).not_to be_empty + it 'Modify import sources' do + expect(current_settings.import_sources).not_to be_empty - page.within('.as-visibility-access') do - Gitlab::ImportSources.options.map do |name, _| - uncheck name + page.within('.as-visibility-access') do + Gitlab::ImportSources.options.map do |name, _| + uncheck name + end + + click_button 'Save changes' end - click_button 'Save changes' - end + expect(page).to have_content "Application settings saved successfully" + expect(current_settings.import_sources).to be_empty - expect(page).to have_content "Application settings saved successfully" - expect(current_settings.import_sources).to be_empty + page.within('.as-visibility-access') do + check "Repo by URL" + click_button 'Save changes' + end - page.within('.as-visibility-access') do - check "Repo by URL" - click_button 'Save changes' + expect(page).to have_content "Application settings saved successfully" + expect(current_settings.import_sources).to eq(['git']) end - expect(page).to have_content "Application settings saved successfully" - expect(current_settings.import_sources).to eq(['git']) - end + it 'Change Visibility and Access Controls' do + page.within('.as-visibility-access') do + uncheck 'Project export enabled' + click_button 'Save changes' + end - it 'Change Visibility and Access Controls' do - page.within('.as-visibility-access') do - uncheck 'Project export enabled' - click_button 'Save changes' + expect(current_settings.project_export_enabled).to be_falsey + expect(page).to have_content "Application settings saved successfully" end - expect(current_settings.project_export_enabled).to be_falsey - expect(page).to have_content "Application settings saved successfully" - end + it 'Change Keys settings' do + page.within('.as-visibility-access') do + select 'Are forbidden', from: 'RSA SSH keys' + select 'Are allowed', from: 'DSA SSH keys' + select 'Must be at least 384 bits', from: 'ECDSA SSH keys' + select 'Are forbidden', from: 'ED25519 SSH keys' + click_on 'Save changes' + end - it 'Change Keys settings' do - page.within('.as-visibility-access') do - select 'Are forbidden', from: 'RSA SSH keys' - select 'Are allowed', from: 'DSA SSH keys' - select 'Must be at least 384 bits', from: 'ECDSA SSH keys' - select 'Are forbidden', from: 'ED25519 SSH keys' - click_on 'Save changes' - end + forbidden = ApplicationSetting::FORBIDDEN_KEY_VALUE.to_s - forbidden = ApplicationSetting::FORBIDDEN_KEY_VALUE.to_s + expect(page).to have_content 'Application settings saved successfully' + expect(find_field('RSA SSH keys').value).to eq(forbidden) + expect(find_field('DSA SSH keys').value).to eq('0') + expect(find_field('ECDSA SSH keys').value).to eq('384') + expect(find_field('ED25519 SSH keys').value).to eq(forbidden) + end - expect(page).to have_content 'Application settings saved successfully' - expect(find_field('RSA SSH keys').value).to eq(forbidden) - expect(find_field('DSA SSH keys').value).to eq('0') - expect(find_field('ECDSA SSH keys').value).to eq('384') - expect(find_field('ED25519 SSH keys').value).to eq(forbidden) - end + it 'Change Account and Limit Settings' do + page.within('.as-account-limit') do + uncheck 'Gravatar enabled' + click_button 'Save changes' + end - it 'Change Account and Limit Settings' do - page.within('.as-account-limit') do - uncheck 'Gravatar enabled' - click_button 'Save changes' + expect(current_settings.gravatar_enabled).to be_falsey + expect(page).to have_content "Application settings saved successfully" end - expect(current_settings.gravatar_enabled).to be_falsey - expect(page).to have_content "Application settings saved successfully" - end + it 'Change New users set to external', :js do + user_internal_regex = find('#application_setting_user_default_internal_regex', visible: :all) - it 'Change New users set to external', :js do - user_internal_regex = find('#application_setting_user_default_internal_regex', visible: :all) + expect(user_internal_regex).to be_readonly + expect(user_internal_regex['placeholder']).to eq 'To define internal users, first enable new users set to external' - expect(user_internal_regex).to be_readonly - expect(user_internal_regex['placeholder']).to eq 'To define internal users, first enable new users set to external' + check 'application_setting_user_default_external' - check 'application_setting_user_default_external' + expect(user_internal_regex).not_to be_readonly + expect(user_internal_regex['placeholder']).to eq 'Regex pattern' + end - expect(user_internal_regex).not_to be_readonly - expect(user_internal_regex['placeholder']).to eq 'Regex pattern' - end + it 'Change Sign-in restrictions' do + page.within('.as-signin') do + fill_in 'Home page URL', with: 'https://about.gitlab.com/' + click_button 'Save changes' + end - it 'Change Sign-in restrictions' do - page.within('.as-signin') do - fill_in 'Home page URL', with: 'https://about.gitlab.com/' - click_button 'Save changes' + expect(current_settings.home_page_url).to eq "https://about.gitlab.com/" + expect(page).to have_content "Application settings saved successfully" end - expect(current_settings.home_page_url).to eq "https://about.gitlab.com/" - expect(page).to have_content "Application settings saved successfully" - end + it 'Terms of Service' do + # Already have the admin accept terms, so they don't need to accept in this spec. + _existing_terms = create(:term) + accept_terms(admin) - it 'Terms of Service' do - # Already have the admin accept terms, so they don't need to accept in this spec. - _existing_terms = create(:term) - accept_terms(admin) + page.within('.as-terms') do + check 'Require all users to accept Terms of Service and Privacy Policy when they access GitLab.' + fill_in 'Terms of Service Agreement', with: 'Be nice!' + click_button 'Save changes' + end - page.within('.as-terms') do - check 'Require all users to accept Terms of Service and Privacy Policy when they access GitLab.' - fill_in 'Terms of Service Agreement', with: 'Be nice!' - click_button 'Save changes' + expect(current_settings.enforce_terms).to be(true) + expect(current_settings.terms).to eq 'Be nice!' + expect(page).to have_content 'Application settings saved successfully' end - expect(current_settings.enforce_terms).to be(true) - expect(current_settings.terms).to eq 'Be nice!' - expect(page).to have_content 'Application settings saved successfully' - end + it 'Modify oauth providers' do + expect(current_settings.disabled_oauth_sign_in_sources).to be_empty - it 'Modify oauth providers' do - expect(current_settings.disabled_oauth_sign_in_sources).to be_empty + page.within('.as-signin') do + uncheck 'Google' + click_button 'Save changes' + end - page.within('.as-signin') do - uncheck 'Google' - click_button 'Save changes' - end + expect(page).to have_content "Application settings saved successfully" + expect(current_settings.disabled_oauth_sign_in_sources).to include('google_oauth2') - expect(page).to have_content "Application settings saved successfully" - expect(current_settings.disabled_oauth_sign_in_sources).to include('google_oauth2') + page.within('.as-signin') do + check "Google" + click_button 'Save changes' + end - page.within('.as-signin') do - check "Google" - click_button 'Save changes' + expect(page).to have_content "Application settings saved successfully" + expect(current_settings.disabled_oauth_sign_in_sources).not_to include('google_oauth2') end - expect(page).to have_content "Application settings saved successfully" - expect(current_settings.disabled_oauth_sign_in_sources).not_to include('google_oauth2') - end + it 'Oauth providers do not raise validation errors when saving unrelated changes' do + expect(current_settings.disabled_oauth_sign_in_sources).to be_empty - it 'Oauth providers do not raise validation errors when saving unrelated changes' do - expect(current_settings.disabled_oauth_sign_in_sources).to be_empty + page.within('.as-signin') do + uncheck 'Google' + click_button 'Save changes' + end - page.within('.as-signin') do - uncheck 'Google' - click_button 'Save changes' - end + expect(page).to have_content "Application settings saved successfully" + expect(current_settings.disabled_oauth_sign_in_sources).to include('google_oauth2') - expect(page).to have_content "Application settings saved successfully" - expect(current_settings.disabled_oauth_sign_in_sources).to include('google_oauth2') + # Remove google_oauth2 from the Omniauth strategies + allow(Devise).to receive(:omniauth_providers).and_return([]) - # Remove google_oauth2 from the Omniauth strategies - allow(Devise).to receive(:omniauth_providers).and_return([]) + # Save an unrelated setting + page.within('.as-terms') do + click_button 'Save changes' + end - # Save an unrelated setting - page.within('.as-terms') do - click_button 'Save changes' + expect(page).to have_content "Application settings saved successfully" + expect(current_settings.disabled_oauth_sign_in_sources).to include('google_oauth2') end - expect(page).to have_content "Application settings saved successfully" - expect(current_settings.disabled_oauth_sign_in_sources).to include('google_oauth2') + it 'Configure web terminal' do + page.within('.as-terminal') do + fill_in 'Max session time', with: 15 + click_button 'Save changes' + end + + expect(page).to have_content "Application settings saved successfully" + expect(current_settings.terminal_max_session_time).to eq(15) + end end - it 'Configure web terminal' do - page.within('.as-terminal') do - fill_in 'Max session time', with: 15 - click_button 'Save changes' + context 'Integrations page' do + before do + visit integrations_admin_application_settings_path end - expect(page).to have_content "Application settings saved successfully" - expect(current_settings.terminal_max_session_time).to eq(15) - end - end + it 'Enable hiding third party offers' do + page.within('.as-third-party-offers') do + check 'Do not display offers from third parties within GitLab' + click_button 'Save changes' + end - context 'Integrations page' do - before do - visit integrations_admin_application_settings_path - end + expect(page).to have_content "Application settings saved successfully" + expect(current_settings.hide_third_party_offers).to be true + end + + it 'Change Slack Notifications Service template settings' do + first(:link, 'Service Templates').click + click_link 'Slack notifications' + fill_in 'Webhook', with: 'http://localhost' + fill_in 'Username', with: 'test_user' + fill_in 'service_push_channel', with: '#test_channel' + page.check('Notify only broken pipelines') + page.select 'All branches', from: 'Branches to be notified' + + check_all_events + click_on 'Save' - it 'Enable hiding third party offers' do - page.within('.as-third-party-offers') do - check 'Do not display offers from third parties within GitLab' - click_button 'Save changes' + expect(page).to have_content 'Application settings saved successfully' + + click_link 'Slack notifications' + + page.all('input[type=checkbox]').each do |checkbox| + expect(checkbox).to be_checked + end + expect(find_field('Webhook').value).to eq 'http://localhost' + expect(find_field('Username').value).to eq 'test_user' + expect(find('#service_push_channel').value).to eq '#test_channel' end - expect(page).to have_content "Application settings saved successfully" - expect(current_settings.hide_third_party_offers).to be true + it 'defaults Deployment events to false for chat notification template settings' do + first(:link, 'Service Templates').click + click_link 'Slack notifications' + + expect(find_field('Deployment')).not_to be_checked + end end - it 'Change Slack Notifications Service template settings' do - first(:link, 'Service Templates').click - click_link 'Slack notifications' - fill_in 'Webhook', with: 'http://localhost' - fill_in 'Username', with: 'test_user' - fill_in 'service_push_channel', with: '#test_channel' - page.check('Notify only broken pipelines') - page.select 'All branches', from: 'Branches to be notified' + context 'CI/CD page' do + it 'Change CI/CD settings' do + visit ci_cd_admin_application_settings_path + + page.within('.as-ci-cd') do + check 'Default to Auto DevOps pipeline for all projects' + fill_in 'application_setting_auto_devops_domain', with: 'domain.com' + click_button 'Save changes' + end - check_all_events - click_on 'Save' + expect(current_settings.auto_devops_enabled?).to be true + expect(current_settings.auto_devops_domain).to eq('domain.com') + expect(page).to have_content "Application settings saved successfully" + end + end - expect(page).to have_content 'Application settings saved successfully' + context 'Reporting page' do + it 'Change Spam settings' do + visit reporting_admin_application_settings_path - click_link 'Slack notifications' + page.within('.as-spam') do + check 'Enable reCAPTCHA' + check 'Enable reCAPTCHA for login' + fill_in 'reCAPTCHA Site Key', with: 'key' + fill_in 'reCAPTCHA Private Key', with: 'key' + fill_in 'IPs per user', with: 15 + click_button 'Save changes' + end - page.all('input[type=checkbox]').each do |checkbox| - expect(checkbox).to be_checked + expect(page).to have_content "Application settings saved successfully" + expect(current_settings.recaptcha_enabled).to be true + expect(current_settings.login_recaptcha_protection_enabled).to be true + expect(current_settings.unique_ips_limit_per_user).to eq(15) end - expect(find_field('Webhook').value).to eq 'http://localhost' - expect(find_field('Username').value).to eq 'test_user' - expect(find('#service_push_channel').value).to eq '#test_channel' end - it 'defaults Deployment events to false for chat notification template settings' do - first(:link, 'Service Templates').click - click_link 'Slack notifications' + context 'Metrics and profiling page' do + before do + visit metrics_and_profiling_admin_application_settings_path + end - expect(find_field('Deployment')).not_to be_checked - end - end + it 'Change Influx settings' do + page.within('.as-influx') do + check 'Enable InfluxDB Metrics' + click_button 'Save changes' + end - context 'CI/CD page' do - it 'Change CI/CD settings' do - visit ci_cd_admin_application_settings_path + expect(current_settings.metrics_enabled?).to be true + expect(page).to have_content "Application settings saved successfully" + end + + it 'Change Prometheus settings' do + page.within('.as-prometheus') do + check 'Enable Prometheus Metrics' + click_button 'Save changes' + end - page.within('.as-ci-cd') do - check 'Default to Auto DevOps pipeline for all projects' - fill_in 'application_setting_auto_devops_domain', with: 'domain.com' - click_button 'Save changes' + expect(current_settings.prometheus_metrics_enabled?).to be true + expect(page).to have_content "Application settings saved successfully" end - expect(current_settings.auto_devops_enabled?).to be true - expect(current_settings.auto_devops_domain).to eq('domain.com') - expect(page).to have_content "Application settings saved successfully" - end - end + it 'Change Performance bar settings' do + group = create(:group) + + page.within('.as-performance-bar') do + check 'Enable access to the Performance Bar' + fill_in 'Allowed group', with: group.path + click_on 'Save changes' + end - context 'Reporting page' do - it 'Change Spam settings' do - visit reporting_admin_application_settings_path + expect(page).to have_content "Application settings saved successfully" + expect(find_field('Enable access to the Performance Bar')).to be_checked + expect(find_field('Allowed group').value).to eq group.path - page.within('.as-spam') do - check 'Enable reCAPTCHA' - check 'Enable reCAPTCHA for login' - fill_in 'reCAPTCHA Site Key', with: 'key' - fill_in 'reCAPTCHA Private Key', with: 'key' - fill_in 'IPs per user', with: 15 - click_button 'Save changes' + page.within('.as-performance-bar') do + uncheck 'Enable access to the Performance Bar' + click_on 'Save changes' + end + + expect(page).to have_content 'Application settings saved successfully' + expect(find_field('Enable access to the Performance Bar')).not_to be_checked + expect(find_field('Allowed group').value).to be_nil end - expect(page).to have_content "Application settings saved successfully" - expect(current_settings.recaptcha_enabled).to be true - expect(current_settings.login_recaptcha_protection_enabled).to be true - expect(current_settings.unique_ips_limit_per_user).to eq(15) - end - end + it 'loads usage ping payload on click', :js do + expect(page).to have_button 'Preview payload' - context 'Metrics and profiling page' do - before do - visit metrics_and_profiling_admin_application_settings_path - end + find('.js-usage-ping-payload-trigger').click - it 'Change Influx settings' do - page.within('.as-influx') do - check 'Enable InfluxDB Metrics' - click_button 'Save changes' + expect(page).to have_selector '.js-usage-ping-payload' + expect(page).to have_button 'Hide payload' end + end + + context 'Network page' do + it 'Changes Outbound requests settings' do + visit network_admin_application_settings_path - expect(current_settings.metrics_enabled?).to be true - expect(page).to have_content "Application settings saved successfully" + page.within('.as-outbound') do + check 'Allow requests to the local network from web hooks and services' + # Enabled by default + uncheck 'Allow requests to the local network from system hooks' + # Enabled by default + uncheck 'Enforce DNS rebinding attack protection' + click_button 'Save changes' + end + + expect(page).to have_content "Application settings saved successfully" + expect(current_settings.allow_local_requests_from_web_hooks_and_services).to be true + expect(current_settings.allow_local_requests_from_system_hooks).to be false + expect(current_settings.dns_rebinding_protection_enabled).to be false + end end - it 'Change Prometheus settings' do - page.within('.as-prometheus') do - check 'Enable Prometheus Metrics' - click_button 'Save changes' + context 'Preferences page' do + before do + visit preferences_admin_application_settings_path end - expect(current_settings.prometheus_metrics_enabled?).to be true - expect(page).to have_content "Application settings saved successfully" - end + it 'Change Help page' do + new_support_url = 'http://example.com/help' - it 'Change Performance bar settings' do - group = create(:group) + page.within('.as-help-page') do + fill_in 'Help page text', with: 'Example text' + check 'Hide marketing-related entries from help' + fill_in 'Support page URL', with: new_support_url + click_button 'Save changes' + end - page.within('.as-performance-bar') do - check 'Enable access to the Performance Bar' - fill_in 'Allowed group', with: group.path - click_on 'Save changes' + expect(current_settings.help_page_text).to eq "Example text" + expect(current_settings.help_page_hide_commercial_content).to be_truthy + expect(current_settings.help_page_support_url).to eq new_support_url + expect(page).to have_content "Application settings saved successfully" end - expect(page).to have_content "Application settings saved successfully" - expect(find_field('Enable access to the Performance Bar')).to be_checked - expect(find_field('Allowed group').value).to eq group.path + it 'Change Pages settings' do + page.within('.as-pages') do + fill_in 'Maximum size of pages (MB)', with: 15 + check 'Require users to prove ownership of custom domains' + click_button 'Save changes' + end - page.within('.as-performance-bar') do - uncheck 'Enable access to the Performance Bar' - click_on 'Save changes' + expect(current_settings.max_pages_size).to eq 15 + expect(current_settings.pages_domain_verification_enabled?).to be_truthy + expect(page).to have_content "Application settings saved successfully" end - expect(page).to have_content 'Application settings saved successfully' - expect(find_field('Enable access to the Performance Bar')).not_to be_checked - expect(find_field('Allowed group').value).to be_nil - end + it 'Change Real-time features settings' do + page.within('.as-realtime') do + fill_in 'Polling interval multiplier', with: 5.0 + click_button 'Save changes' + end - it 'loads usage ping payload on click', :js do - expect(page).to have_button 'Preview payload' + expect(current_settings.polling_interval_multiplier).to eq 5.0 + expect(page).to have_content "Application settings saved successfully" + end - find('.js-usage-ping-payload-trigger').click + it 'shows an error when validation fails' do + page.within('.as-realtime') do + fill_in 'Polling interval multiplier', with: -1.0 + click_button 'Save changes' + end - expect(page).to have_selector '.js-usage-ping-payload' - expect(page).to have_button 'Hide payload' - end - end + expect(current_settings.polling_interval_multiplier).not_to eq(-1.0) + expect(page) + .to have_content "The form contains the following error: Polling interval multiplier must be greater than or equal to 0" + end - context 'Network page' do - it 'Changes Outbound requests settings' do - visit network_admin_application_settings_path + it "Change Pages Let's Encrypt settings" do + visit preferences_admin_application_settings_path + page.within('.as-pages') do + fill_in 'Email', with: 'my@test.example.com' + check "I have read and agree to the Let's Encrypt Terms of Service" + click_button 'Save changes' + end - page.within('.as-outbound') do - check 'Allow requests to the local network from web hooks and services' - # Enabled by default - uncheck 'Allow requests to the local network from system hooks' - # Enabled by default - uncheck 'Enforce DNS rebinding attack protection' - click_button 'Save changes' + expect(current_settings.lets_encrypt_notification_email).to eq 'my@test.example.com' + expect(current_settings.lets_encrypt_terms_of_service_accepted).to eq true end - - expect(page).to have_content "Application settings saved successfully" - expect(current_settings.allow_local_requests_from_web_hooks_and_services).to be true - expect(current_settings.allow_local_requests_from_system_hooks).to be false - expect(current_settings.dns_rebinding_protection_enabled).to be false end - end - context 'Preferences page' do - before do - visit preferences_admin_application_settings_path - end + context 'Nav bar' do + it 'Shows default help links in nav' do + default_support_url = 'https://about.gitlab.com/getting-help/' - it 'Change Help page' do - new_support_url = 'http://example.com/help' + visit root_dashboard_path - page.within('.as-help-page') do - fill_in 'Help page text', with: 'Example text' - check 'Hide marketing-related entries from help' - fill_in 'Support page URL', with: new_support_url - click_button 'Save changes' + find('.header-help-dropdown-toggle').click + + page.within '.header-help' do + expect(page).to have_link(text: 'Help', href: help_path) + expect(page).to have_link(text: 'Support', href: default_support_url) + end end - expect(current_settings.help_page_text).to eq "Example text" - expect(current_settings.help_page_hide_commercial_content).to be_truthy - expect(current_settings.help_page_support_url).to eq new_support_url - expect(page).to have_content "Application settings saved successfully" - end + it 'Shows custom support url in nav when set' do + new_support_url = 'http://example.com/help' + stub_application_setting(help_page_support_url: new_support_url) - it 'Change Pages settings' do - page.within('.as-pages') do - fill_in 'Maximum size of pages (MB)', with: 15 - check 'Require users to prove ownership of custom domains' - click_button 'Save changes' - end + visit root_dashboard_path - expect(current_settings.max_pages_size).to eq 15 - expect(current_settings.pages_domain_verification_enabled?).to be_truthy - expect(page).to have_content "Application settings saved successfully" + find('.header-help-dropdown-toggle').click + + page.within '.header-help' do + expect(page).to have_link(text: 'Support', href: new_support_url) + end + end end - it 'Change Real-time features settings' do - page.within('.as-realtime') do - fill_in 'Polling interval multiplier', with: 5.0 - click_button 'Save changes' + context 'when in admin_mode' do + it 'contains link to leave admin mode' do + page.within('.navbar-sub-nav') do + expect(page).to have_link(href: destroy_admin_session_path) + end end - expect(current_settings.polling_interval_multiplier).to eq 5.0 - expect(page).to have_content "Application settings saved successfully" - end + it 'can leave admin mode' do + page.within('.navbar-sub-nav') do + # Select first, link is also included in mobile view list + click_on 'Leave admin mode', match: :first - it 'shows an error when validation fails' do - page.within('.as-realtime') do - fill_in 'Polling interval multiplier', with: -1.0 - click_button 'Save changes' + expect(page).to have_link(href: new_admin_session_path) + end end - expect(current_settings.polling_interval_multiplier).not_to eq(-1.0) - expect(page) - .to have_content "The form contains the following error: Polling interval multiplier must be greater than or equal to 0" + it 'can open pages not in admin scope' do + page.within('.navbar-sub-nav') do + find_all('a', text: 'Projects').first.click + + expect(page).to have_current_path(dashboard_projects_path) + end + end end - it "Change Pages Let's Encrypt settings" do - visit preferences_admin_application_settings_path - page.within('.as-pages') do - fill_in 'Email', with: 'my@test.example.com' - check "I have read and agree to the Let's Encrypt Terms of Service" - click_button 'Save changes' + context 'when not in admin mode' do + before do + page.within('.navbar-sub-nav') do + # Select first, link is also included in mobile view list + click_on 'Leave admin mode', match: :first + end end - expect(current_settings.lets_encrypt_notification_email).to eq 'my@test.example.com' - expect(current_settings.lets_encrypt_terms_of_service_accepted).to eq true - end - end + it 'has no leave admin mode button' do + page.within('.navbar-sub-nav') do + expect(page).not_to have_link(href: destroy_admin_session_path) + end + end - context 'Nav bar' do - it 'Shows default help links in nav' do - default_support_url = 'https://about.gitlab.com/getting-help/' + it 'is necessary to provide credentials again before opening admin settings' do + visit admin_application_settings_path # admin logged out because not in admin_mode - visit root_dashboard_path + expect(page).to have_current_path(new_admin_session_path) + end - find('.header-help-dropdown-toggle').click + it 'can open pages not in admin scope' do + page.within('.navbar-sub-nav') do + find_all('a', text: 'Projects').first.click + end - page.within '.header-help' do - expect(page).to have_link(text: 'Help', href: help_path) - expect(page).to have_link(text: 'Support', href: default_support_url) + expect(page).to have_current_path(dashboard_projects_path) end end + end - it 'Shows custom support url in nav when set' do - new_support_url = 'http://example.com/help' - stub_application_setting(help_page_support_url: new_support_url) + context 'feature flag :user_mode_in_session is disabled' do + before do + stub_feature_flags(user_mode_in_session: false) - visit root_dashboard_path + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') - find('.header-help-dropdown-toggle').click + sign_in(admin) + visit admin_application_settings_path + end + + it 'loads admin settings page without redirect for reauthentication' do + expect(current_path).to eq admin_application_settings_path + end - page.within '.header-help' do - expect(page).to have_link(text: 'Support', href: new_support_url) + it 'shows no admin mode buttons in navbar' do + page.within('.navbar-sub-nav') do + expect(page).not_to have_link(href: new_admin_session_path) + expect(page).not_to have_link(href: destroy_admin_session_path) end end end diff --git a/spec/frontend/jobs/store/utils_spec.js b/spec/frontend/jobs/store/utils_spec.js index bb45fcb7435..18f9026b487 100644 --- a/spec/frontend/jobs/store/utils_spec.js +++ b/spec/frontend/jobs/store/utils_spec.js @@ -3,6 +3,8 @@ import { updateIncrementalTrace, parseHeaderLine, parseLine, + addDurationToHeader, + isCollapsibleSection, findOffsetAndRemove, } from '~/jobs/store/utils'; import { @@ -43,6 +45,127 @@ describe('Jobs Store Utils', () => { }); }); + describe('addDurationToHeader', () => { + const duration = { + offset: 106, + content: [], + section: 'prepare-script', + section_duration: '00:03', + }; + + it('adds the section duration to the correct header', () => { + const parsed = [ + { + isClosed: true, + isHeader: true, + line: { + section: 'prepare-script', + content: [{ text: 'foo' }], + }, + lines: [], + }, + { + isClosed: true, + isHeader: true, + line: { + section: 'foo-bar', + content: [{ text: 'foo' }], + }, + lines: [], + }, + ]; + + addDurationToHeader(parsed, duration); + + expect(parsed[0].line.section_duration).toEqual(duration.section_duration); + expect(parsed[1].line.section_duration).toEqual(undefined); + }); + + it('does not add the section duration when the headers do not match', () => { + const parsed = [ + { + isClosed: true, + isHeader: true, + line: { + section: 'bar-foo', + content: [{ text: 'foo' }], + }, + lines: [], + }, + { + isClosed: true, + isHeader: true, + line: { + section: 'foo-bar', + content: [{ text: 'foo' }], + }, + lines: [], + }, + ]; + addDurationToHeader(parsed, duration); + + expect(parsed[0].line.section_duration).toEqual(undefined); + expect(parsed[1].line.section_duration).toEqual(undefined); + }); + + it('does not add when content has no headers', () => { + const parsed = [ + { + section: 'bar-foo', + content: [{ text: 'foo' }], + lineNumber: 1, + }, + { + section: 'foo-bar', + content: [{ text: 'foo' }], + lineNumber: 2, + }, + ]; + + addDurationToHeader(parsed, duration); + + expect(parsed[0].line).toEqual(undefined); + expect(parsed[1].line).toEqual(undefined); + }); + }); + + describe('isCollapsibleSection', () => { + const header = { + isHeader: true, + line: { + section: 'foo', + }, + }; + const line = { + lineNumber: 1, + section: 'foo', + content: [], + }; + + it('returns true when line belongs to the last section', () => { + expect(isCollapsibleSection([header], header, { section: 'foo', content: [] })).toEqual(true); + }); + + it('returns false when last line was not an header', () => { + expect(isCollapsibleSection([line], line, { section: 'bar' })).toEqual(false); + }); + + it('returns false when accumulator is empty', () => { + expect(isCollapsibleSection([], { isHeader: true }, { section: 'bar' })).toEqual(false); + }); + + it('returns false when section_duration is defined', () => { + expect(isCollapsibleSection([header], header, { section_duration: '10:00' })).toEqual(false); + }); + + it('returns false when `section` is not a match', () => { + expect(isCollapsibleSection([header], header, { section: 'bar' })).toEqual(false); + }); + + it('returns false when no parameters are provided', () => { + expect(isCollapsibleSection()).toEqual(false); + }); + }); describe('logLinesParser', () => { let result; @@ -75,7 +198,7 @@ describe('Jobs Store Utils', () => { describe('section duration', () => { it('adds the section information to the header section', () => { - expect(result[1].section_duration).toEqual(utilsMockData[4].section_duration); + expect(result[1].line.section_duration).toEqual(utilsMockData[4].section_duration); }); it('does not add section duration as a line', () => { diff --git a/spec/frontend/releases/mock_data.js b/spec/frontend/releases/mock_data.js index a0885813c7e..aff441e675e 100644 --- a/spec/frontend/releases/mock_data.js +++ b/spec/frontend/releases/mock_data.js @@ -89,7 +89,7 @@ export const release = { id: 2, name: 'my second link', url: - 'https://gitlab.com/gitlab-org/gitlab-ce/-/jobs/artifacts/v11.6.0-rc4/download?job=rspec-mysql+41%2F50', + 'https://gitlab.com/gitlab-org/gitlab-foss/-/jobs/artifacts/v11.6.0-rc4/download?job=rspec-mysql+41%2F50', external: false, }, ], diff --git a/spec/helpers/nav_helper_spec.rb b/spec/helpers/nav_helper_spec.rb index 979d89812f5..ff2be795d87 100644 --- a/spec/helpers/nav_helper_spec.rb +++ b/spec/helpers/nav_helper_spec.rb @@ -1,16 +1,20 @@ require 'spec_helper' -describe NavHelper do +describe NavHelper, :do_not_mock_admin_mode do describe '#header_links' do + include_context 'custom session' + before do - allow(helper).to receive(:session) { {} } + allow(helper).to receive(:session).and_return(session) end context 'when the user is logged in' do - let(:user) { build(:user) } + let(:user) { create(:user) } + let(:current_user_mode) { Gitlab::Auth::CurrentUserMode.new(user) } before do allow(helper).to receive(:current_user).and_return(user) + allow(helper).to receive(:current_user_mode).and_return(current_user_mode) allow(helper).to receive(:can?) { true } end @@ -26,6 +30,46 @@ describe NavHelper do expect(helper.header_links).to include(:admin_impersonation) end + context 'as admin' do + let(:user) { create(:user, :admin) } + + context 'feature flag :user_mode_in_session is enabled' do + it 'does not contain the admin mode link by default' do + expect(helper.header_links).not_to include(:admin_mode) + end + + context 'with admin mode enabled' do + before do + current_user_mode.enable_admin_mode!(password: user.password) + end + + it 'contains the admin mode link' do + expect(helper.header_links).to include(:admin_mode) + end + end + end + + context 'feature flag :user_mode_in_session is disabled' do + before do + stub_feature_flags(user_mode_in_session: false) + end + + it 'does not contain the admin mode link' do + expect(helper.header_links).not_to include(:admin_mode) + end + + context 'with admin mode enabled' do + before do + current_user_mode.enable_admin_mode!(password: user.password) + end + + it 'has no effect on header links' do + expect(helper.header_links).not_to include(:admin_mode) + end + end + end + end + context 'when the user cannot read cross project' do before do allow(helper).to receive(:can?).with(user, :read_cross_project) { false } diff --git a/spec/lib/gitlab/auth/current_user_mode_spec.rb b/spec/lib/gitlab/auth/current_user_mode_spec.rb new file mode 100644 index 00000000000..b93d460cf48 --- /dev/null +++ b/spec/lib/gitlab/auth/current_user_mode_spec.rb @@ -0,0 +1,159 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Auth::CurrentUserMode, :do_not_mock_admin_mode do + include_context 'custom session' + + let(:user) { build(:user) } + + subject { described_class.new(user) } + + before do + allow(ActiveSession).to receive(:list_sessions).with(user).and_return([session]) + end + + describe '#admin_mode?', :request_store do + context 'when the user is a regular user' do + it 'is false by default' do + expect(subject.admin_mode?).to be(false) + end + + it 'cannot be enabled with a valid password' do + subject.enable_admin_mode!(password: user.password) + + expect(subject.admin_mode?).to be(false) + end + + it 'cannot be enabled with an invalid password' do + subject.enable_admin_mode!(password: nil) + + expect(subject.admin_mode?).to be(false) + end + + it 'cannot be enabled with empty params' do + subject.enable_admin_mode! + + expect(subject.admin_mode?).to be(false) + end + + it 'disable has no effect' do + subject.enable_admin_mode! + subject.disable_admin_mode! + + expect(subject.admin_mode?).to be(false) + end + + context 'skipping password validation' do + it 'cannot be enabled with a valid password' do + subject.enable_admin_mode!(password: user.password, skip_password_validation: true) + + expect(subject.admin_mode?).to be(false) + end + + it 'cannot be enabled with an invalid password' do + subject.enable_admin_mode!(skip_password_validation: true) + + expect(subject.admin_mode?).to be(false) + end + end + end + + context 'when the user is an admin' do + let(:user) { build(:user, :admin) } + + it 'is false by default' do + expect(subject.admin_mode?).to be(false) + end + + it 'cannot be enabled with an invalid password' do + subject.enable_admin_mode!(password: nil) + + expect(subject.admin_mode?).to be(false) + end + + it 'can be enabled with a valid password' do + subject.enable_admin_mode!(password: user.password) + + expect(subject.admin_mode?).to be(true) + end + + it 'can be disabled' do + subject.enable_admin_mode!(password: user.password) + subject.disable_admin_mode! + + expect(subject.admin_mode?).to be(false) + end + + it 'will expire in the future' do + subject.enable_admin_mode!(password: user.password) + expect(subject.admin_mode?).to be(true), 'admin mode is not active in the present' + + Timecop.freeze(Gitlab::Auth::CurrentUserMode::MAX_ADMIN_MODE_TIME.from_now) do + # in the future this will be a new request, simulate by clearing the RequestStore + Gitlab::SafeRequestStore.clear! + + expect(subject.admin_mode?).to be(false), 'admin mode did not expire in the future' + end + end + + context 'skipping password validation' do + it 'can be enabled with a valid password' do + subject.enable_admin_mode!(password: user.password, skip_password_validation: true) + + expect(subject.admin_mode?).to be(true) + end + + it 'can be enabled with an invalid password' do + subject.enable_admin_mode!(skip_password_validation: true) + + expect(subject.admin_mode?).to be(true) + end + end + + context 'with two independent sessions' do + let(:another_session) { {} } + let(:another_subject) { described_class.new(user) } + + before do + allow(ActiveSession).to receive(:list_sessions).with(user).and_return([session, another_session]) + end + + it 'can be enabled in one and seen in the other' do + Gitlab::Session.with_session(another_session) do + another_subject.enable_admin_mode!(password: user.password) + end + + expect(subject.admin_mode?).to be(true) + end + end + end + end + + describe '#enable_admin_mode!' do + let(:user) { build(:user, :admin) } + + it 'creates a timestamp in the session' do + subject.enable_admin_mode!(password: user.password) + + expect(session).to include(expected_session_entry(be_within(1.second).of Time.now)) + end + end + + describe '#disable_admin_mode!' do + let(:user) { build(:user, :admin) } + + it 'sets the session timestamp to nil' do + subject.disable_admin_mode! + + expect(session).to include(expected_session_entry(be_nil)) + end + end + + def expected_session_entry(value_matcher) + { + Gitlab::Auth::CurrentUserMode::SESSION_STORE_KEY => a_hash_including( + Gitlab::Auth::CurrentUserMode::ADMIN_MODE_START_TIME_KEY => value_matcher) + } + end +end diff --git a/spec/lib/gitlab/health_checks/simple_check_shared.rb b/spec/lib/gitlab/health_checks/simple_check_shared.rb index e2643458aca..b4a8a4e6ab6 100644 --- a/spec/lib/gitlab/health_checks/simple_check_shared.rb +++ b/spec/lib/gitlab/health_checks/simple_check_shared.rb @@ -58,9 +58,4 @@ shared_context 'simple_check' do |metrics_prefix, check_name, success_result| it { is_expected.to have_attributes(success: false, message: "#{described_class.human_name} check timed out") } end end - - describe '#liveness' do - subject { described_class.readiness } - it { is_expected.to eq(Gitlab::HealthChecks::Result.new(true)) } - end end diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 29dc84c1924..a1a007811fe 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -343,6 +343,8 @@ describe API::Helpers do end context 'sudo' do + include_context 'custom session' + shared_examples 'successful sudo' do it 'sets current_user' do expect(current_user).to eq(user) diff --git a/spec/serializers/build_action_entity_spec.rb b/spec/serializers/build_action_entity_spec.rb index ea88951ebc6..4d335e8fb77 100644 --- a/spec/serializers/build_action_entity_spec.rb +++ b/spec/serializers/build_action_entity_spec.rb @@ -3,9 +3,14 @@ require 'spec_helper' describe BuildActionEntity do let(:job) { create(:ci_build, name: 'test_job') } let(:request) { double('request') } + let(:user) { create(:user) } let(:entity) do - described_class.new(job, request: spy('request')) + described_class.new(job, request: request) + end + + before do + allow(request).to receive(:current_user).and_return(user) end describe '#as_json' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 18bfe5a2be7..364c033900f 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -160,6 +160,25 @@ RSpec.configure do |config| allow(Gitlab::Git::KeepAround).to receive(:execute) Gitlab::ThreadMemoryCache.cache_backend.clear + + # Temporary patch to force admin mode to be active by default in tests when + # using the feature flag :user_mode_in_session, since this will require + # modifying a significant number of specs to test both states for admin + # mode enabled / disabled. + # + # See https://gitlab.com/gitlab-org/gitlab/issues/31511 + # See gitlab/spec/support/helpers/admin_mode_helpers.rb + # + # If it is required to have the real behaviour that an admin is signed in + # with normal user mode and needs to switch to admin mode, it is possible to + # mark such tests with the `do_not_mock_admin_mode` metadata tag, e.g: + # + # context 'some test with normal user mode', :do_not_mock_admin_mode do ... end + unless example.metadata[:do_not_mock_admin_mode] + allow_any_instance_of(Gitlab::Auth::CurrentUserMode).to receive(:admin_mode?) do |current_user_mode| + current_user_mode.send(:user)&.admin? + end + end end config.around(:example, :quarantine) do |example| diff --git a/spec/support/helpers/admin_mode_helpers.rb b/spec/support/helpers/admin_mode_helpers.rb new file mode 100644 index 00000000000..de8ffe40536 --- /dev/null +++ b/spec/support/helpers/admin_mode_helpers.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# Helper for enabling admin mode in tests + +module AdminModeHelper + # Users are logged in by default in user mode and have to switch to admin + # mode for accessing any administrative functionality. This helper lets a user + # be in admin mode without requiring a second authentication step (provided + # the user is an admin) + def enable_admin_mode!(user) + fake_user_mode = instance_double(Gitlab::Auth::CurrentUserMode) + + allow(Gitlab::Auth::CurrentUserMode).to receive(:new).with(user).and_return(fake_user_mode) + allow(fake_user_mode).to receive(:admin_mode?).and_return(user&.admin?) + end +end diff --git a/spec/support/helpers/login_helpers.rb b/spec/support/helpers/login_helpers.rb index 140ad173d38..94e23f08f5d 100644 --- a/spec/support/helpers/login_helpers.rb +++ b/spec/support/helpers/login_helpers.rb @@ -48,6 +48,14 @@ module LoginHelpers @current_user = user end + def gitlab_enable_admin_mode_sign_in(user) + visit new_admin_session_path + + fill_in 'password', with: user.password + + click_button 'Enter admin mode' + end + def gitlab_sign_in_via(provider, user, uid, saml_response = nil) mock_auth_hash_with_saml_xml(provider, uid, user.email, saml_response) visit new_user_session_path diff --git a/spec/support/shared_contexts/session_shared_context.rb b/spec/support/shared_contexts/session_shared_context.rb new file mode 100644 index 00000000000..86c145a8360 --- /dev/null +++ b/spec/support/shared_contexts/session_shared_context.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +# the session is empty by default; you can overwrite it by defining your own +# let(:session) variable +# we do not use a parameter such as |session| because it does not play nice +# with let variables +shared_context 'custom session' do + let!(:session) { {} } + + around do |example| + Gitlab::Session.with_session(session) do + example.run + end + end +end diff --git a/spec/views/admin/sessions/new.html.haml_spec.rb b/spec/views/admin/sessions/new.html.haml_spec.rb new file mode 100644 index 00000000000..eb431a0b6de --- /dev/null +++ b/spec/views/admin/sessions/new.html.haml_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe 'admin/sessions/new.html.haml' do + context 'admin has password set' do + before do + allow(view).to receive(:password_authentication_enabled_for_web?).and_return(true) + end + + it "shows enter password form" do + render + + expect(rendered).to have_css('#login-pane.active') + expect(rendered).to have_selector('input[name="password"]') + end + end + + context 'admin has no password set' do + before do + allow(view).to receive(:password_authentication_enabled_for_web?).and_return(false) + end + + it "warns authentication not possible" do + render + + expect(rendered).not_to have_css('#login-pane') + expect(rendered).to have_content 'No authentication methods configured' + end + end +end |