diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-08-08 21:10:59 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-08-08 21:10:59 +0300 |
commit | 3849f5bb99b2f46f4ffeda56b9b7ae8afe954ed3 (patch) | |
tree | 0c7e45228a4737222e57bc45ed64932056b25ac4 | |
parent | bfb24e1685fb574d3144865da29a21b38cb52883 (diff) |
Add latest changes from gitlab-org/gitlab@master
37 files changed, 271 insertions, 102 deletions
diff --git a/.gitlab/ci/global.gitlab-ci.yml b/.gitlab/ci/global.gitlab-ci.yml index b4a3232fcf7..dd615fe5e9d 100644 --- a/.gitlab/ci/global.gitlab-ci.yml +++ b/.gitlab/ci/global.gitlab-ci.yml @@ -36,7 +36,17 @@ .ruby-gems-cache-push: &ruby-gems-cache-push <<: *ruby-gems-cache - policy: push # We want to rebuild the cache from scratch to ensure stale dependencies are cleaned up. + policy: push # We want to rebuild the cache from scratch to ensure stale dependencies are cleaned up. + +.ruby-coverage-gems-cache: &ruby-coverage-gems-cache + key: "ruby-coverage-gems-debian-${DEBIAN_VERSION}-ruby-${RUBY_VERSION}" + paths: + - vendor/ruby/ + policy: pull + +.ruby-coverage-gems-cache-push: &ruby-coverage-gems-cache-push + <<: *ruby-coverage-gems-cache + policy: push # We want to rebuild the cache from scratch to ensure stale dependencies are cleaned up. .gitaly-binaries-cache: &gitaly-binaries-cache key: @@ -161,6 +171,15 @@ - *ruby-gems-cache # We don't push this cache as it's already rebuilt by `update-setup-test-env-cache` - *rubocop-cache-push +.ruby-gems-coverage-cache: + cache: + - *ruby-coverage-gems-cache + +.ruby-gems-coverage-cache-push: + cache: + - *ruby-coverage-gems-cache-push + +# This cache should eventually be replaced by .ruby-gems-coverage-cache. .coverage-cache: cache: - *ruby-gems-cache diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml index e11b81d9ddd..ca5c1319735 100644 --- a/.gitlab/ci/rails.gitlab-ci.yml +++ b/.gitlab/ci/rails.gitlab-ci.yml @@ -56,9 +56,27 @@ update-gitaly-binaries-cache: artifacts: paths: [] # This job's purpose is only to update the cache. +update-ruby-gems-coverage-cache-push: + extends: + - .ruby-gems-coverage-cache-push + - .shared:rules:update-cache + variables: + BUNDLE_WITHOUT: "" # This is to override the variable defined in .gitlab-ci.yml + BUNDLE_ONLY: "coverage" + script: + - source scripts/utils.sh + - bundle_install_script + +# Used in: +# - rspec:undercoverage +# - rspec:feature-flags +# - rspec:merge-auto-explain-logs +# +# TODO: Consider making rspec:coverage reuse this base job, or split into two base jobs. .coverage-base: extends: - .default-retry + # TODO: If applicable for all children jobs, delete/replace this cache with .ruby-gems-coverage-cache, as it is much smaller. - .coverage-cache before_script: - source scripts/utils.sh @@ -446,7 +464,7 @@ rspec:artifact-collector ee: rspec:coverage: extends: - - .coverage-base + - .ruby-gems-coverage-cache - .fast-no-clone-job - .rails:rules:rspec-coverage stage: post-test @@ -487,10 +505,12 @@ rspec:coverage: scripts/merge-simplecov spec/simplecov_env_core.rb spec/simplecov_env.rb + BUNDLE_WITHOUT: "" # This is to override the variable defined in .gitlab-ci.yml + BUNDLE_ONLY: "coverage" before_script: - !reference [".fast-no-clone-job", before_script] - run_timed_command "download_local_gems" - - !reference [".coverage-base", before_script] + - bundle_install_script script: - chmod u+x scripts/merge-simplecov # Not the right permissions when downloading the script via the API. - run_timed_command "bundle exec scripts/merge-simplecov" diff --git a/.gitlab/route-map.yml b/.gitlab/route-map.yml index cc0c2856d49..7f6cbee9469 100644 --- a/.gitlab/route-map.yml +++ b/.gitlab/route-map.yml @@ -1,3 +1,3 @@ # Documentation - source: /doc/(.+?)\.md/ # doc/administration/build_artifacts.md - public: '\1.html' # doc/administration/build_artifacts.html + public: '\1.html' # administration/build_artifacts.html diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 8588273a41f..08e4f4956df 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -181,7 +181,7 @@ class ApplicationController < ActionController::Base payload[:queue_duration_s] = request.env[::Gitlab::Middleware::RailsQueueDuration::GITLAB_RAILS_QUEUE_DURATION_KEY] - payload[:response_bytes] = response.body_parts.sum(&:bytesize) + payload[:response_bytes] = response.body_parts.sum(&:bytesize) if Feature.enabled?(:log_response_length) store_cloudflare_headers!(payload, request) end diff --git a/app/controllers/concerns/enforces_two_factor_authentication.rb b/app/controllers/concerns/enforces_two_factor_authentication.rb index 8068913eea2..539feb3cf1c 100644 --- a/app/controllers/concerns/enforces_two_factor_authentication.rb +++ b/app/controllers/concerns/enforces_two_factor_authentication.rb @@ -77,7 +77,7 @@ module EnforcesTwoFactorAuthentication end def two_factor_verifier - @two_factor_verifier ||= Gitlab::Auth::TwoFactorAuthVerifier.new(current_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables + @two_factor_verifier ||= Gitlab::Auth::TwoFactorAuthVerifier.new(current_user, request) # rubocop:disable Gitlab/ModuleWithInstanceVariables end def mfa_help_page_url diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb index 4e34094b52c..93cf1d15086 100644 --- a/app/controllers/concerns/notes_actions.rb +++ b/app/controllers/concerns/notes_actions.rb @@ -264,7 +264,7 @@ module NotesActions end def require_last_fetched_at_header! - return if request.headers['X-Last-Fetched-At'].present? || Feature.disabled?(:require_notes_last_fetched_at) + return if request.headers['X-Last-Fetched-At'].present? render json: { message: 'X-Last-Fetched-At header is required' }, status: :bad_request end diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index eda72400f17..72b3516ae3f 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -130,6 +130,8 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController link_identity(identity_linker) set_remember_me(current_user) + store_idp_two_factor_status(build_auth_user(auth_module::User).bypass_two_factor?) + if identity_linker.changed? redirect_identity_linked elsif identity_linker.failed? @@ -159,7 +161,9 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController end def build_auth_user(auth_user_class) - auth_user_class.new(oauth) + strong_memoize_with(:build_auth_user, auth_user_class) do + auth_user_class.new(oauth) + end end def sign_in_user_flow(auth_user_class) @@ -179,12 +183,16 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController if user.two_factor_enabled? && !auth_user.bypass_two_factor? prompt_for_two_factor(user) + store_idp_two_factor_status(false) else if user.deactivated? user.activate flash[:notice] = _('Welcome back! Your account had been deactivated due to inactivity but is now reactivated.') end + # session variable for storing bypass two-factor request from IDP + store_idp_two_factor_status(true) + accept_pending_invitations(user: user) if new_user persist_accepted_terms_if_required(user) if new_user @@ -323,6 +331,14 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController def sign_in_and_redirect_or_verify_identity(user, _, _) sign_in_and_redirect(user, event: :authentication) end + + def store_idp_two_factor_status(bypass_2fa) + if Feature.enabled?(:by_pass_two_factor_for_current_session) + session[:provider_2FA] = true if bypass_2fa + else + session.delete(:provider_2FA) + end + end end OmniauthCallbacksController.prepend_mod_with('OmniauthCallbacksController') diff --git a/config/feature_flags/development/raise_error_for_missing_audit_event_yml.yml b/config/feature_flags/development/by_pass_two_factor_for_current_session.yml index 14835496d39..d60b5bdc234 100644 --- a/config/feature_flags/development/raise_error_for_missing_audit_event_yml.yml +++ b/config/feature_flags/development/by_pass_two_factor_for_current_session.yml @@ -1,8 +1,8 @@ --- -name: raise_error_for_missing_audit_event_yml -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/127193 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/419377 +name: by_pass_two_factor_for_current_session +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/122109 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/416535 milestone: '16.3' type: development -group: group::compliance +group: group::authentication and authorization default_enabled: false diff --git a/config/feature_flags/development/require_notes_last_fetched_at.yml b/config/feature_flags/development/log_response_length.yml index 7ded6f16543..689a262bec1 100644 --- a/config/feature_flags/development/require_notes_last_fetched_at.yml +++ b/config/feature_flags/development/log_response_length.yml @@ -1,8 +1,8 @@ --- -name: require_notes_last_fetched_at -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/127763 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/419829 -milestone: '16.3' +name: log_response_length +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/91448 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/366854 +milestone: '15.3' type: development -group: group::project management +group: group::tenant scale default_enabled: false diff --git a/doc/administration/auth/test_oidc_oauth.md b/doc/administration/auth/test_oidc_oauth.md index 95cca1ced86..be0ea5c963e 100644 --- a/doc/administration/auth/test_oidc_oauth.md +++ b/doc/administration/auth/test_oidc_oauth.md @@ -4,7 +4,7 @@ group: Authentication and Authorization info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# Test OIDC/OAuth in GitLab **(FREE)** +# Test OIDC/OAuth in GitLab **(FREE SELF)** To test OIDC/OAuth in GitLab, you must: diff --git a/doc/administration/compliance.md b/doc/administration/compliance.md index 5562d808bef..7889bc9f4ff 100644 --- a/doc/administration/compliance.md +++ b/doc/administration/compliance.md @@ -4,7 +4,7 @@ group: Compliance info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# Compliance features **(FREE)** +# Compliance features **(FREE SELF)** GitLab compliance features ensure your GitLab instance meets common compliance standards, and are available at various pricing tiers. For more information about compliance management, see the compliance management [solutions page](https://about.gitlab.com/solutions/compliance/). diff --git a/doc/administration/dedicated/index.md b/doc/administration/dedicated/index.md index 828ea110d23..e9211f7283c 100644 --- a/doc/administration/dedicated/index.md +++ b/doc/administration/dedicated/index.md @@ -233,6 +233,9 @@ Specify a comma separated list of IP addresses that can access your GitLab Dedic ### SAML +NOTE: +GitLab Dedicated supports a limited number of SAML parameters. Parameters not shown in the configuration below are unavailable for GitLab Dedicated tenant instances. + Prerequisites: - You must configure the identity provider before sending the required data to GitLab. @@ -240,11 +243,40 @@ Prerequisites: To activate SAML for your GitLab Dedicated instance: 1. To make the necessary changes, include the desired [SAML configuration block](../../integration/saml.md#configure-saml-support-in-gitlab) for your GitLab application in your [support ticket](https://support.gitlab.com/hc/en-us/requests/new?ticket_form_id=4414917877650). At a minimum, GitLab needs the following information to enable SAML for your instance: - - Assertion consumer service URL + - IDP SSO Target URL - Certificate fingerprint or certificate - NameID format - SSO login button description + ```json + "saml": { + "attribute_statements": { + //optional + }, + "enabled": true, + "groups_attribute": "", + "admin_groups": [ + // optional + ], + "idp_cert_fingerprint": "43:51:43:a1:b5:fc:8b:b7:0a:3a:a9:b1:0f:66:73:a8", + "idp_sso_target_url": "https://login.example.com/idp", + "label": "IDP Name", + "name_identifier_format": "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent", + "security": { + // optional + }, + "auditor_groups": [ + // optional + ], + "external_groups": [ + // optional + ], + "required_groups": [ + // optional + ], + } + ``` + 1. After GitLab deploys the SAML configuration to your instance, you are notified on your support ticket. 1. To verify the SAML configuration is successful: - Check that the SSO login button description is displayed on your instance's login page. @@ -253,7 +285,7 @@ To activate SAML for your GitLab Dedicated instance: #### Request signing If [SAML request signing](../../integration/saml.md#sign-saml-authentication-requests-optional) is desired, a certificate must be obtained. This certificate can be self-signed which has the advantage of not having to prove ownership of an arbitrary Common Name (CN) to a public Certificate Authority (CA)). - +If you choose to enable SAML request signing, the manual steps below will need to be completed before you are able to use SAML, since it requires certificate signing to happen. To enable SAML request signing, indicate on your SAML [support ticket](https://support.gitlab.com/hc/en-us/requests/new?ticket_form_id=4414917877650) that you want request signing enabled. GitLab works with you on sending the Certificate Signing Request (CSR) for you to sign. Alternatively, the CSR can be signed with a public CA. After the certificate is signed, GitLab adds the certificate and its associated private key to the `security` section of the SAML configuration. Authentication requests from GitLab to your identity provider can then be signed. #### SAML groups diff --git a/doc/administration/get_started.md b/doc/administration/get_started.md index bf3d38657f8..d56287c44ac 100644 --- a/doc/administration/get_started.md +++ b/doc/administration/get_started.md @@ -4,7 +4,7 @@ stage: none group: Tutorials --- -# Get started administering GitLab **(FREE)** +# Get started administering GitLab **(FREE SELF)** Get started with GitLab administration. Configure your organization and its authentication, then secure, monitor, and back up GitLab. diff --git a/doc/administration/git_protocol.md b/doc/administration/git_protocol.md index 1ece7d773ee..7fb241b8d1f 100644 --- a/doc/administration/git_protocol.md +++ b/doc/administration/git_protocol.md @@ -5,7 +5,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w description: "Set and configure Git protocol v2" --- -# Configuring Git Protocol v2 **(FREE)** +# Configuring Git Protocol v2 **(FREE SELF)** > [Re-enabled](https://gitlab.com/gitlab-org/gitlab/-/issues/27828) in GitLab 12.8. diff --git a/doc/administration/integration/diagrams_net.md b/doc/administration/integration/diagrams_net.md index 335b26565e6..7f8c01ad7bd 100644 --- a/doc/administration/integration/diagrams_net.md +++ b/doc/administration/integration/diagrams_net.md @@ -5,7 +5,7 @@ info: "To determine the technical writer assigned to the Stage/Group associated type: reference, howto --- -# Diagrams.net **(FREE)** +# Diagrams.net **(FREE SELF)** With the [diagrams.net](https://www.diagrams.net/) integration, you can create and embed SVG diagrams in wikis. The diagram editor is available in both the plain text editor and the rich text editor. diff --git a/doc/administration/integration/plantuml.md b/doc/administration/integration/plantuml.md index 6755d5c8f85..5e499e302db 100644 --- a/doc/administration/integration/plantuml.md +++ b/doc/administration/integration/plantuml.md @@ -5,7 +5,7 @@ info: "To determine the technical writer assigned to the Stage/Group associated type: reference, howto --- -# PlantUML **(FREE)** +# PlantUML **(FREE SELF)** With the [PlantUML](https://plantuml.com) integration, you can create diagrams in snippets, wikis, and repositories. This integration is enabled on GitLab.com for all SaaS users and does not require any additional configuration. diff --git a/doc/administration/integration/terminal.md b/doc/administration/integration/terminal.md index add036ea5ec..2939e227a04 100644 --- a/doc/administration/integration/terminal.md +++ b/doc/administration/integration/terminal.md @@ -4,7 +4,7 @@ group: Environments info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# Web terminals (deprecated) **(FREE)** +# Web terminals (deprecated) **(FREE SELF)** > - [Deprecated](https://gitlab.com/groups/gitlab-org/configure/-/epics/8) in GitLab 14.5. > - [Disabled on self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/353410) in GitLab 15.0. diff --git a/doc/administration/invalidate_markdown_cache.md b/doc/administration/invalidate_markdown_cache.md index 366cbea5711..5db08075449 100644 --- a/doc/administration/invalidate_markdown_cache.md +++ b/doc/administration/invalidate_markdown_cache.md @@ -5,7 +5,7 @@ info: "To determine the technical writer assigned to the Stage/Group associated type: reference --- -# Markdown cache **(FREE)** +# Markdown cache **(FREE SELF)** For performance reasons, GitLab caches the HTML version of Markdown text in fields such as: diff --git a/doc/administration/secure_files.md b/doc/administration/secure_files.md index a14033bdf00..9005bcea319 100644 --- a/doc/administration/secure_files.md +++ b/doc/administration/secure_files.md @@ -4,7 +4,7 @@ group: Mobile DevOps info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# Secure Files administration **(FREE)** +# Secure Files administration **(FREE SELF)** > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78227) in GitLab 14.8 [with a flag](feature_flags.md) named `ci_secure_files`. Disabled by default. > - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/350748) in GitLab 15.7. Feature flag `ci_secure_files` removed. diff --git a/doc/administration/settings/project_integration_management.md b/doc/administration/settings/project_integration_management.md index d05381cb777..95dddf34182 100644 --- a/doc/administration/settings/project_integration_management.md +++ b/doc/administration/settings/project_integration_management.md @@ -4,7 +4,7 @@ group: Import and Integrate info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# Project integration management **(FREE)** +# Project integration management **(FREE SELF)** Project integrations can be configured and enabled by project administrators. As a GitLab instance administrator, you can set default configuration parameters for a given integration that all projects diff --git a/doc/administration/settings/push_event_activities_limit.md b/doc/administration/settings/push_event_activities_limit.md index 117e7322e30..ff924e0d208 100644 --- a/doc/administration/settings/push_event_activities_limit.md +++ b/doc/administration/settings/push_event_activities_limit.md @@ -5,7 +5,7 @@ info: "To determine the technical writer assigned to the Stage/Group associated type: reference --- -# Push event activities limit and bulk push events **(FREE)** +# Push event activities limit and bulk push events **(FREE SELF)** > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/31007) in GitLab 12.4. diff --git a/doc/administration/static_objects_external_storage.md b/doc/administration/static_objects_external_storage.md index c7a22da38de..6b232ddc25f 100644 --- a/doc/administration/static_objects_external_storage.md +++ b/doc/administration/static_objects_external_storage.md @@ -5,7 +5,7 @@ info: "To determine the technical writer assigned to the Stage/Group associated type: reference --- -# External storage for static objects **(FREE)** +# External storage for static objects **(FREE SELF)** > [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/31025) in GitLab 12.3. diff --git a/doc/administration/terraform_state.md b/doc/administration/terraform_state.md index ec4bf3e16da..c0f1e8ebc12 100644 --- a/doc/administration/terraform_state.md +++ b/doc/administration/terraform_state.md @@ -4,7 +4,7 @@ group: Environments info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# Terraform state administration **(FREE)** +# Terraform state administration **(FREE SELF)** > [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/2673) in GitLab 12.10. diff --git a/doc/architecture/blueprints/observability_tracing/index.md b/doc/architecture/blueprints/observability_tracing/index.md index 4291683f83f..71e03d81bcf 100644 --- a/doc/architecture/blueprints/observability_tracing/index.md +++ b/doc/architecture/blueprints/observability_tracing/index.md @@ -135,7 +135,7 @@ All requests from GitLab.com will then include the GOB session cookie for observ The new UI will be built using the Pajamas Design System in accordance with GitLab UX design standards. The UI will interact with the GOB query service directly from vue.js (see architecture diagram above) by sending a fetch to the subdomain `observe.gitLab.com/v1/query` with `{withCredentials: true}`. See the Authentication and Authorization section above for more details on how this is enabled. -[**TODO Figma UI designs and commentary**] +**TODO Figma UI designs and commentary** ## Iterations diff --git a/doc/architecture/blueprints/runner_admission_controller/index.md b/doc/architecture/blueprints/runner_admission_controller/index.md index d73ffb21ef3..92c824527ec 100644 --- a/doc/architecture/blueprints/runner_admission_controller/index.md +++ b/doc/architecture/blueprints/runner_admission_controller/index.md @@ -229,7 +229,7 @@ be rare in typical circumstances. ### Implementation Details -1. [placeholder for steps required to code the admissions controller MVC] +1. _placeholder for steps required to code the admissions controller MVC_ ## Technical issues to resolve diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 0aee0c70203..7033856a42e 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -128,7 +128,7 @@ module API end def two_factor_required_but_not_setup?(user) - verifier = Gitlab::Auth::TwoFactorAuthVerifier.new(user) + verifier = Gitlab::Auth::TwoFactorAuthVerifier.new(user, request) if verifier.two_factor_authentication_required? && verifier.current_user_needs_to_setup_two_factor? verifier.two_factor_grace_period_expired? diff --git a/lib/gitlab/audit/auditor.rb b/lib/gitlab/audit/auditor.rb index c8dc804c4a1..a035b6face9 100644 --- a/lib/gitlab/audit/auditor.rb +++ b/lib/gitlab/audit/auditor.rb @@ -78,16 +78,9 @@ module Gitlab return if @is_audit_event_yaml_defined - if Feature.enabled?(:raise_error_for_missing_audit_event_yml) - raise StandardError, "Audit event type YML file is not defined for #{@name}. Please read " \ - "https://docs.gitlab.com/ee/development/audit_event_guide/" \ - "#how-to-instrument-new-audit-events for adding a new audit event" - else - message = 'Logging audit events without an event type definition will be deprecated soon ' \ - '(https://docs.gitlab.com/ee/development/audit_event_guide/#event-type-definitions)' - - Gitlab::AppLogger.warn(message: message, event_type: @name) - end + raise StandardError, "Audit event type YML file is not defined for #{@name}. Please read " \ + "https://docs.gitlab.com/ee/development/audit_event_guide/" \ + "#how-to-instrument-new-audit-events for adding a new audit event" end def single_audit diff --git a/lib/gitlab/auth/two_factor_auth_verifier.rb b/lib/gitlab/auth/two_factor_auth_verifier.rb index 5a203a1fe9c..fbdfd105ee3 100644 --- a/lib/gitlab/auth/two_factor_auth_verifier.rb +++ b/lib/gitlab/auth/two_factor_auth_verifier.rb @@ -3,10 +3,11 @@ module Gitlab module Auth class TwoFactorAuthVerifier - attr_reader :current_user + attr_reader :current_user, :request - def initialize(current_user) + def initialize(current_user, request = nil) @current_user = current_user + @request = request end def two_factor_authentication_enforced? @@ -14,6 +15,8 @@ module Gitlab end def two_factor_authentication_required? + return false if allow_2fa_bypass_for_provider + Gitlab::CurrentSettings.require_two_factor_authentication? || current_user&.require_two_factor_authentication_from_group? end @@ -35,6 +38,12 @@ module Gitlab two_factor_grace_period.hours.since(time) < Time.current end + + def allow_2fa_bypass_for_provider + return false if Feature.disabled?(:by_pass_two_factor_for_current_session) + + request.session[:provider_2FA].present? if request + end end end end diff --git a/lib/gitlab/grape_logging/loggers/response_logger.rb b/lib/gitlab/grape_logging/loggers/response_logger.rb index b87566a62b0..767c282d62e 100644 --- a/lib/gitlab/grape_logging/loggers/response_logger.rb +++ b/lib/gitlab/grape_logging/loggers/response_logger.rb @@ -5,6 +5,8 @@ module Gitlab module Loggers class ResponseLogger < ::GrapeLogging::Loggers::Base def parameters(_, response) + return {} unless Feature.enabled?(:log_response_length) + response_bytes = 0 case response diff --git a/lib/gitlab/lograge/custom_options.rb b/lib/gitlab/lograge/custom_options.rb index 9abad44b10e..f8ec58cf217 100644 --- a/lib/gitlab/lograge/custom_options.rb +++ b/lib/gitlab/lograge/custom_options.rb @@ -36,6 +36,10 @@ module Gitlab payload[:feature_flag_states] = Feature.logged_states.map { |key, state| "#{key}:#{state ? 1 : 0}" } end + if Feature.disabled?(:log_response_length) + payload.delete(:response_bytes) + end + payload end end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 0beaae7a2d7..58125f3a831 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -567,6 +567,20 @@ RSpec.describe ApplicationController, feature_category: :shared do expect(controller.last_payload[:response_bytes]).to eq('authenticated'.bytesize) end + + context 'with log_response_length disabled' do + before do + stub_feature_flags(log_response_length: false) + end + + it 'logs response length' do + sign_in user + + get :index + + expect(controller.last_payload).not_to include(:response_bytes) + end + end end describe '#access_denied' do diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb index 75bdad8b19e..8d2face0233 100644 --- a/spec/controllers/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/omniauth_callbacks_controller_spec.rb @@ -5,6 +5,32 @@ require 'spec_helper' RSpec.describe OmniauthCallbacksController, type: :controller, feature_category: :system_access do include LoginHelpers + shared_examples 'store provider2FA value in session' do + before do + stub_omniauth_setting(allow_bypass_two_factor: true) + saml_config.args[:upstream_two_factor_authn_contexts] << "urn:oasis:names:tc:SAML:2.0:ac:classes:Password" + sign_in user + end + + it "sets the session varible for provider 2FA" do + post :saml, params: { SAMLResponse: mock_saml_response } + + expect(session[:provider_2FA]).to eq(true) + end + + context 'when by_pass_two_factor_for_current_session feature flag is false' do + before do + stub_feature_flags(by_pass_two_factor_for_current_session: false) + end + + it "does not set the session variable for provider 2FA" do + post :saml, params: { SAMLResponse: mock_saml_response } + + expect(session[:provider_2FA]).to be_nil + end + end + end + describe 'omniauth' do let(:user) { create(:omniauth_user, extern_uid: extern_uid, provider: provider) } let(:additional_info) { {} } @@ -593,6 +619,12 @@ RSpec.describe OmniauthCallbacksController, type: :controller, feature_category: post :saml, params: { SAMLResponse: mock_saml_response } end + + context 'with IDP bypass two factor request' do + let(:user) { create(:omniauth_user, extern_uid: 'my-uid', provider: 'saml') } + + it_behaves_like 'store provider2FA value in session' + end end context 'with a blocked user trying to log in when there are hooks set up' do @@ -623,9 +655,14 @@ RSpec.describe OmniauthCallbacksController, type: :controller, feature_category: it 'authenticate with SAML module' do expect(@controller).to receive(:omniauth_flow).with(Gitlab::Auth::Saml).and_call_original post :saml_okta, params: { SAMLResponse: mock_saml_response } + expect(request.env['warden']).to be_authenticated end end + + context 'with IDP bypass two factor request' do + it_behaves_like 'store provider2FA value in session' + end end describe 'enable admin mode' do diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 3dcd6b04040..500fab471ef 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -68,20 +68,6 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : expect(response).to have_gitlab_http_status(:bad_request) end - context 'when require_notes_last_fetched_at is disabled' do - before do - stub_feature_flags(require_notes_last_fetched_at: false) - end - - it 'returns status 200 when last_fetched_at is not present' do - request.headers['X-Last-Fetched-At'] = nil - - get :index, params: request_params - - expect(response).to have_gitlab_http_status(:ok) - end - end - context 'when user notes_filter is present' do let(:notes_json) { parsed_response[:notes] } let!(:comment) { create(:note, noteable: issue, project: project) } diff --git a/spec/lib/gitlab/audit/auditor_spec.rb b/spec/lib/gitlab/audit/auditor_spec.rb index 1a45235a4e7..bde72a656b8 100644 --- a/spec/lib/gitlab/audit/auditor_spec.rb +++ b/spec/lib/gitlab/audit/auditor_spec.rb @@ -34,52 +34,24 @@ RSpec.describe Gitlab::Audit::Auditor, feature_category: :audit_events do before do allow(Gitlab::Audit::Type::Definition).to receive(:defined?).and_call_original allow(Gitlab::Audit::Type::Definition).to receive(:defined?).with(name).and_return(false) - allow(Gitlab::AppLogger).to receive(:warn).and_return(app_logger) end - context 'when feature flag raise_error_for_missing_audit_event_yml is enabled' do - before do - stub_feature_flags(raise_error_for_missing_audit_event_yml: true) - end - - it 'raises an error' do - expected_error = "Audit event type YML file is not defined for audit_operation. " \ - "Please read https://docs.gitlab.com/ee/development/audit_event_guide/" \ - "#how-to-instrument-new-audit-events for adding a new audit event" - - expect { audit! }.to raise_error(StandardError, expected_error) - end - end - - context 'when feature flag raise_error_for_missing_audit_event_yml is disabled' do - before do - stub_feature_flags(raise_error_for_missing_audit_event_yml: false) - end + it 'raises an error' do + expected_error = "Audit event type YML file is not defined for audit_operation. " \ + "Please read https://docs.gitlab.com/ee/development/audit_event_guide/" \ + "#how-to-instrument-new-audit-events for adding a new audit event" - it 'logs a warning when YAML is not defined' do - expected_warning = { - message: 'Logging audit events without an event type definition will be deprecated soon ' \ - '(https://docs.gitlab.com/ee/development/audit_event_guide/#event-type-definitions)', - event_type: name - } - - audit! - - expect(Gitlab::AppLogger).to have_received(:warn).with(expected_warning) - end + expect { audit! }.to raise_error(StandardError, expected_error) end end context 'when yaml definition is defined' do before do allow(Gitlab::Audit::Type::Definition).to receive(:defined?).and_return(true) - allow(Gitlab::AppLogger).to receive(:warn).and_return(app_logger) end - it 'does not log a warning when YAML is defined' do - audit! - - expect(Gitlab::AppLogger).not_to have_received(:warn) + it 'does not raise an error' do + expect { audit! }.not_to raise_error end end diff --git a/spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb b/spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb index 876c23a91bd..e0ef45d5621 100644 --- a/spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb +++ b/spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb @@ -5,10 +5,13 @@ require 'spec_helper' RSpec.describe Gitlab::Auth::TwoFactorAuthVerifier do using RSpec::Parameterized::TableSyntax - subject(:verifier) { described_class.new(user) } + let(:request) { instance_double(ActionDispatch::Request, session: session) } + let(:session) { {} } let(:user) { build_stubbed(:user, otp_grace_period_started_at: Time.zone.now) } + subject(:verifier) { described_class.new(user, request) } + describe '#two_factor_authentication_enforced?' do subject { verifier.two_factor_authentication_enforced? } @@ -34,25 +37,69 @@ RSpec.describe Gitlab::Auth::TwoFactorAuthVerifier do describe '#two_factor_authentication_required?' do subject { verifier.two_factor_authentication_required? } - where(:instance_level_enabled, :group_level_enabled, :should_be_required) do - true | false | true - false | true | true - false | false | false + where(:instance_level_enabled, :group_level_enabled, :should_be_required, :provider_2FA) do + true | false | true | false + false | true | false | true + false | true | true | false + false | false | false | true end with_them do before do stub_application_setting(require_two_factor_authentication: instance_level_enabled) allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(group_level_enabled) + session[:provider_2FA] = provider_2FA end it { is_expected.to eq(should_be_required) } end + + context 'when feature by_pass_two_factor_for_current_session is disabled' do + where(:instance_level_enabled, :group_level_enabled, :should_be_required, :provider_2FA) do + true | false | true | false + false | true | true | true + false | false | false | true + end + + with_them do + before do + allow(request).to receive(:session).and_return(session) + stub_feature_flags(by_pass_two_factor_for_current_session: false) + stub_application_setting(require_two_factor_authentication: instance_level_enabled) + allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(group_level_enabled) + session[:provider_2FA] = provider_2FA + end + + it { is_expected.to eq(should_be_required) } + end + end + + context 'when request is nil' do + let(:request) { nil } + + where(:instance_level_enabled, :group_level_enabled, :should_be_required, :provider_2FA) do + true | false | true | false + false | true | true | true + false | false | false | true + end + + with_them do + before do + allow(request).to receive(:session).and_return(session) + stub_feature_flags(bypass_two_factor: false) + stub_application_setting(require_two_factor_authentication: instance_level_enabled) + allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(group_level_enabled) + session[:provider_2FA] = provider_2FA + end + + it { is_expected.to eq(should_be_required) } + end + end end describe '#current_user_needs_to_setup_two_factor?' do it 'returns false when current_user is nil' do - expect(described_class.new(nil).current_user_needs_to_setup_two_factor?).to be_falsey + expect(described_class.new(nil, request).current_user_needs_to_setup_two_factor?).to be_falsey end it 'returns false when current_user does not have temp email' do diff --git a/spec/lib/gitlab/grape_logging/loggers/response_logger_spec.rb b/spec/lib/gitlab/grape_logging/loggers/response_logger_spec.rb index 1bf97e87708..449096a6faf 100644 --- a/spec/lib/gitlab/grape_logging/loggers/response_logger_spec.rb +++ b/spec/lib/gitlab/grape_logging/loggers/response_logger_spec.rb @@ -20,6 +20,14 @@ RSpec.describe Gitlab::GrapeLogging::Loggers::ResponseLogger do it { expect(subject).to eq({ response_bytes: response1.bytesize + response2.bytesize }) } end + context 'with log_response_length disabled' do + before do + stub_feature_flags(log_response_length: false) + end + + it { expect(subject).to eq({}) } + end + context 'when response is a String' do let(:response) { response1 } diff --git a/spec/lib/gitlab/lograge/custom_options_spec.rb b/spec/lib/gitlab/lograge/custom_options_spec.rb index 3460a8fb080..090b79c5d3c 100644 --- a/spec/lib/gitlab/lograge/custom_options_spec.rb +++ b/spec/lib/gitlab/lograge/custom_options_spec.rb @@ -60,6 +60,16 @@ RSpec.describe Gitlab::Lograge::CustomOptions do expect(subject[:response_bytes]).to eq(1234) end + context 'with log_response_length disabled' do + before do + stub_feature_flags(log_response_length: false) + end + + it 'does not add the response length' do + expect(subject).not_to include(:response_bytes) + end + end + it 'adds Cloudflare headers' do expect(subject[:cf_ray]).to eq(event.payload[:cf_ray]) expect(subject[:cf_request_id]).to eq(event.payload[:cf_request_id]) |