diff options
136 files changed, 1950 insertions, 447 deletions
diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index 47a1e8341b7..aedcd4f71a0 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -761,7 +761,7 @@ changes: *code-qa-patterns when: manual allow_failure: true - - <<: *if-master-refs + - <<: *if-dot-com-gitlab-org-schedule allow_failure: true .review:rules:danger: diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index d01b3fb91da..6c46ad5dffc 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -18,20 +18,6 @@ Capybara/CurrentPathExpectation: Layout/ArgumentAlignment: Enabled: false -# Offense count: 72 -# Cop supports --auto-correct. -Layout/EmptyLinesAroundArguments: - Exclude: - - 'app/models/concerns/discussion_on_diff.rb' - - 'app/models/concerns/resolvable_discussion.rb' - - 'app/models/diff_discussion.rb' - - 'app/models/discussion.rb' - - 'ee/spec/models/geo/project_registry_spec.rb' - - 'lib/banzai/pipeline/broadcast_message_pipeline.rb' - - 'lib/banzai/pipeline/gfm_pipeline.rb' - - 'lib/banzai/pipeline/single_line_pipeline.rb' - - 'spec/features/markdown/copy_as_gfm_spec.rb' - # Offense count: 413 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, IndentationWidth. @@ -101,14 +87,6 @@ Layout/SpaceInsideParens: Lint/MissingCopEnableDirective: Enabled: false -# Offense count: 11 -# Cop supports --auto-correct. -Lint/NonDeterministicRequireOrder: - Exclude: - - 'ee/spec/spec_helper.rb' - - 'qa/spec/spec_helper.rb' - - 'spec/spec_helper.rb' - # Offense count: 27 # Cop supports --auto-correct. Lint/RedundantCopDisableDirective: @@ -229,15 +207,6 @@ RSpec/ExpectChange: RSpec/ExpectInHook: Enabled: false -# Offense count: 9 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: it_behaves_like, it_should_behave_like -RSpec/ItBehavesLike: - Exclude: - - 'spec/lib/gitlab/git/commit_spec.rb' - - 'spec/services/notification_service_spec.rb' - # Offense count: 68 # Cop supports --auto-correct. RSpec/LetBeforeExamples: @@ -600,17 +569,6 @@ Style/EmptyLambdaParameter: - 'app/models/ci/build.rb' - 'app/models/ci/runner.rb' -# Offense count: 7 -# Cop supports --auto-correct. -Style/EmptyLiteral: - Exclude: - - 'lib/gitlab/fogbugz_import/importer.rb' - - 'lib/gitlab/git/diff_collection.rb' - - 'lib/gitlab/gitaly_client.rb' - - 'spec/helpers/merge_requests_helper_spec.rb' - - 'spec/lib/gitlab/workhorse_spec.rb' - - 'spec/requests/api/jobs_spec.rb' - # Offense count: 170 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle. diff --git a/Gemfile.lock b/Gemfile.lock index ae678d2321b..53ede64312b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1197,7 +1197,7 @@ GEM railties (>= 3.2.0) websocket-driver (0.7.1) websocket-extensions (>= 0.1.0) - websocket-extensions (0.1.4) + websocket-extensions (0.1.5) wikicloth (0.8.1) builder expression_parser diff --git a/app/assets/javascripts/filtered_search/issuable_filtered_search_token_keys.js b/app/assets/javascripts/filtered_search/issuable_filtered_search_token_keys.js index 9bea7aa7b04..d2ac80fa190 100644 --- a/app/assets/javascripts/filtered_search/issuable_filtered_search_token_keys.js +++ b/app/assets/javascripts/filtered_search/issuable_filtered_search_token_keys.js @@ -86,6 +86,11 @@ export const conditions = flattenDeep( value: __('Any'), }, { + url: 'author_username=support-bot', + tokenKey: 'author', + value: 'support-bot', + }, + { url: 'milestone_title=None', tokenKey: 'milestone', value: __('None'), diff --git a/app/controllers/clusters/clusters_controller.rb b/app/controllers/clusters/clusters_controller.rb index b6d5eb7e5c0..7006c23321c 100644 --- a/app/controllers/clusters/clusters_controller.rb +++ b/app/controllers/clusters/clusters_controller.rb @@ -38,8 +38,7 @@ class Clusters::ClustersController < Clusters::BaseController def new if params[:provider] == 'aws' - @aws_role = current_user.aws_role || Aws::Role.new - @aws_role.ensure_role_external_id! + @aws_role = Aws::Role.create_or_find_by!(user: current_user) @instance_types = load_instance_types.to_json elsif params[:provider] == 'gcp' @@ -274,7 +273,7 @@ class Clusters::ClustersController < Clusters::BaseController end def aws_role_params - params.require(:cluster).permit(:role_arn, :role_external_id) + params.require(:cluster).permit(:role_arn) end def generate_gcp_authorize_url diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb index 4b4bcc8d37e..2cc51c65c26 100644 --- a/app/controllers/concerns/authenticates_with_two_factor.rb +++ b/app/controllers/concerns/authenticates_with_two_factor.rb @@ -22,6 +22,8 @@ module AuthenticatesWithTwoFactor return handle_locked_user(user) unless user.can?(:log_in) session[:otp_user_id] = user.id + session[:user_updated_at] = user.updated_at + setup_u2f_authentication(user) render 'devise/sessions/two_factor' end @@ -39,6 +41,7 @@ module AuthenticatesWithTwoFactor def authenticate_with_two_factor user = self.resource = find_user return handle_locked_user(user) unless user.can?(:log_in) + return handle_changed_user(user) if user_changed?(user) if user_params[:otp_attempt].present? && session[:otp_user_id] authenticate_with_two_factor_via_otp(user) @@ -63,12 +66,14 @@ module AuthenticatesWithTwoFactor def clear_two_factor_attempt! session.delete(:otp_user_id) + session.delete(:user_updated_at) + session.delete(:challenge) end def authenticate_with_two_factor_via_otp(user) if valid_otp_attempt?(user) # Remove any lingering user data from login - session.delete(:otp_user_id) + clear_two_factor_attempt! remember_me(user) if user_params[:remember_me] == '1' user.save! @@ -85,8 +90,7 @@ module AuthenticatesWithTwoFactor def authenticate_with_two_factor_via_u2f(user) if U2fRegistration.authenticate(user, u2f_app_id, user_params[:device_response], session[:challenge]) # Remove any lingering user data from login - session.delete(:otp_user_id) - session.delete(:challenge) + clear_two_factor_attempt! remember_me(user) if user_params[:remember_me] == '1' sign_in(user, message: :two_factor_authenticated, event: :authentication) @@ -113,4 +117,18 @@ module AuthenticatesWithTwoFactor end end # rubocop: enable CodeReuse/ActiveRecord + + def handle_changed_user(user) + clear_two_factor_attempt! + + redirect_to new_user_session_path, alert: _('An error occurred. Please sign in again.') + end + + # If user has been updated since we validated the password, + # the password might have changed. + def user_changed?(user) + return false unless session[:user_updated_at] + + user.updated_at != session[:user_updated_at] + end end diff --git a/app/controllers/concerns/enforces_two_factor_authentication.rb b/app/controllers/concerns/enforces_two_factor_authentication.rb index f1dd46648f1..02082f81598 100644 --- a/app/controllers/concerns/enforces_two_factor_authentication.rb +++ b/app/controllers/concerns/enforces_two_factor_authentication.rb @@ -29,12 +29,11 @@ module EnforcesTwoFactorAuthentication end def two_factor_authentication_required? - Gitlab::CurrentSettings.require_two_factor_authentication? || - current_user.try(:require_two_factor_authentication_from_group?) + two_factor_verifier.two_factor_authentication_required? end def current_user_requires_two_factor? - current_user && !current_user.temp_oauth_email? && !current_user.two_factor_enabled? && !skip_two_factor? + two_factor_verifier.current_user_needs_to_setup_two_factor? && !skip_two_factor? end # rubocop: disable CodeReuse/ActiveRecord @@ -43,7 +42,7 @@ module EnforcesTwoFactorAuthentication if Gitlab::CurrentSettings.require_two_factor_authentication? global.call else - groups = current_user.expanded_groups_requiring_two_factor_authentication.reorder(name: :asc) + groups = current_user.source_groups_of_two_factor_authentication_requirement.reorder(name: :asc) group.call(groups) end end @@ -51,14 +50,11 @@ module EnforcesTwoFactorAuthentication # rubocop: enable CodeReuse/ActiveRecord def two_factor_grace_period - periods = [Gitlab::CurrentSettings.two_factor_grace_period] - periods << current_user.two_factor_grace_period if current_user.try(:require_two_factor_authentication_from_group?) - periods.min + two_factor_verifier.two_factor_grace_period end def two_factor_grace_period_expired? - date = current_user.otp_grace_period_started_at - date && (date + two_factor_grace_period.hours) < Time.current + two_factor_verifier.two_factor_grace_period_expired? end def two_factor_skippable? @@ -70,6 +66,10 @@ module EnforcesTwoFactorAuthentication def skip_two_factor? session[:skip_two_factor] && session[:skip_two_factor] > Time.current end + + def two_factor_verifier + @two_factor_verifier ||= Gitlab::Auth::TwoFactorAuthVerifier.new(current_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables + end end EnforcesTwoFactorAuthentication.prepend_if_ee('EE::EnforcesTwoFactorAuthentication') diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 6a393405e4d..a558b01f0c6 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -50,12 +50,6 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController redirect_unverified_saml_initiation end - def omniauth_error - @provider = params[:provider] - @error = params[:error] - render 'errors/omniauth_error', layout: "oauth_error", status: :unprocessable_entity - end - def cas3 ticket = params['ticket'] if ticket @@ -205,9 +199,10 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController def fail_login(user) log_failed_login(user.username, oauth['provider']) - error_message = user.errors.full_messages.to_sentence + @provider = Gitlab::Auth::OAuth::Provider.label_for(params[:action]) + @error = user.errors.full_messages.to_sentence - redirect_to omniauth_error_path(oauth['provider'], error: error_message) + render 'errors/omniauth_error', layout: "oauth_error", status: :unprocessable_entity end def fail_auth0_login diff --git a/app/controllers/profiles/active_sessions_controller.rb b/app/controllers/profiles/active_sessions_controller.rb index f409193aefc..d9ec3195fd1 100644 --- a/app/controllers/profiles/active_sessions_controller.rb +++ b/app/controllers/profiles/active_sessions_controller.rb @@ -7,6 +7,7 @@ class Profiles::ActiveSessionsController < Profiles::ApplicationController def destroy ActiveSession.destroy_with_public_id(current_user, params[:id]) + current_user.forget_me! respond_to do |format| format.html { redirect_to profile_active_sessions_url, status: :found } diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb index a88c5ca4fa1..f600c34ca75 100644 --- a/app/controllers/profiles/two_factor_auths_controller.rb +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -4,7 +4,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController skip_before_action :check_two_factor_requirement def show - unless current_user.otp_secret + unless current_user.two_factor_enabled? current_user.otp_secret = User.generate_otp_secret(32) end @@ -38,6 +38,8 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController def create if current_user.validate_and_consume_otp!(params[:pin_code]) + ActiveSession.destroy_all_but_current(current_user, session) + Users::UpdateService.new(current_user, user: current_user, otp_required_for_login: true).execute! do |user| @codes = user.generate_otp_backup_codes! end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index baf7a05f8ba..5da2bafbcb3 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -8,6 +8,7 @@ class SessionsController < Devise::SessionsController include Recaptcha::Verify include RendersLdapServers include KnownSignIn + include Gitlab::Utils::StrongMemoize skip_before_action :check_two_factor_requirement, only: [:destroy] skip_before_action :check_password_expiration, only: [:destroy] @@ -199,10 +200,14 @@ class SessionsController < Devise::SessionsController end def find_user - if session[:otp_user_id] - User.find(session[:otp_user_id]) - elsif user_params[:login] - User.by_login(user_params[:login]) + strong_memoize(:find_user) do + if session[:otp_user_id] && user_params[:login] + User.by_id_and_login(session[:otp_user_id], user_params[:login]).first + elsif session[:otp_user_id] + User.find(session[:otp_user_id]) + elsif user_params[:login] + User.by_login(user_params[:login]) + end end end diff --git a/app/finders/ci/auth_job_finder.rb b/app/finders/ci/auth_job_finder.rb new file mode 100644 index 00000000000..aee7dd16341 --- /dev/null +++ b/app/finders/ci/auth_job_finder.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true +module Ci + class AuthJobFinder + AuthError = Class.new(StandardError) + NotRunningJobError = Class.new(AuthError) + ErasedJobError = Class.new(AuthError) + DeletedProjectError = Class.new(AuthError) + + def initialize(token:) + @token = token + end + + def execute! + find_job_by_token.tap do |job| + next unless job + + validate_job!(job) + end + end + + def execute + execute! + rescue AuthError + end + + private + + attr_reader :token, :require_running, :raise_on_missing + + def find_job_by_token + ::Ci::Build.find_by_token(token) + end + + def validate_job!(job) + validate_running_job!(job) + validate_job_not_erased!(job) + validate_project_presence!(job) + + true + end + + def validate_running_job!(job) + raise NotRunningJobError, 'Job is not running' unless job.running? + end + + def validate_job_not_erased!(job) + raise ErasedJobError, 'Job has been erased!' if job.erased? + end + + def validate_project_presence!(job) + if job.project.nil? || job.project.pending_delete? + raise DeletedProjectError, 'Project has been deleted!' + end + end + end +end diff --git a/app/finders/user_recent_events_finder.rb b/app/finders/user_recent_events_finder.rb index 3f2e813d381..f376b26ab9c 100644 --- a/app/finders/user_recent_events_finder.rb +++ b/app/finders/user_recent_events_finder.rb @@ -6,6 +6,7 @@ # WARNING: does not consider project feature visibility! # - user: The user for which to load the events # - params: +# - limit: Number of items that to be returned. Defaults to 20 and limited to 100. # - offset: The page of events to return class UserRecentEventsFinder prepend FinderWithCrossProjectAccess @@ -16,7 +17,8 @@ class UserRecentEventsFinder attr_reader :current_user, :target_user, :params - LIMIT = 20 + DEFAULT_LIMIT = 20 + MAX_LIMIT = 100 def initialize(current_user, target_user, params = {}) @current_user = current_user @@ -31,7 +33,7 @@ class UserRecentEventsFinder recent_events(params[:offset] || 0) .joins(:project) .with_associations - .limit_recent(params[:limit].presence || LIMIT, params[:offset]) + .limit_recent(limit, params[:offset]) end # rubocop: enable CodeReuse/ActiveRecord @@ -59,4 +61,10 @@ class UserRecentEventsFinder def projects target_user.project_interactions.to_sql end + + def limit + return DEFAULT_LIMIT unless params[:limit].present? + + [params[:limit].to_i, MAX_LIMIT].min + end end diff --git a/app/graphql/mutations/boards/lists/base.rb b/app/graphql/mutations/boards/lists/base.rb index 34b271ba3b8..d244d6bf8dd 100644 --- a/app/graphql/mutations/boards/lists/base.rb +++ b/app/graphql/mutations/boards/lists/base.rb @@ -8,7 +8,7 @@ module Mutations argument :board_id, ::Types::GlobalIDType[::Board], required: true, - description: 'The Global ID of the issue board to mutate' + description: 'Global ID of the issue board to mutate' field :list, Types::BoardListType, diff --git a/app/graphql/mutations/boards/lists/create.rb b/app/graphql/mutations/boards/lists/create.rb index 4f545709ee9..3fe1052315f 100644 --- a/app/graphql/mutations/boards/lists/create.rb +++ b/app/graphql/mutations/boards/lists/create.rb @@ -12,7 +12,7 @@ module Mutations argument :label_id, ::Types::GlobalIDType[::Label], required: false, - description: 'ID of an existing label' + description: 'Global ID of an existing label' def ready?(**args) if args.slice(*mutually_exclusive_args).size != 1 @@ -39,6 +39,7 @@ module Mutations private + # Overridden in EE def authorize_list_type_resource!(board, params) return unless params[:label_id] @@ -57,13 +58,15 @@ module Mutations create_list_service.execute(board) end + # Overridden in EE def create_list_params(args) params = args.slice(*mutually_exclusive_args).with_indifferent_access - params[:label_id] = GitlabSchema.parse_gid(params[:label_id]).model_id if params[:label_id] + params[:label_id] &&= ::GitlabSchema.parse_gid(params[:label_id], expected_type: ::Label).model_id params end + # Overridden in EE def mutually_exclusive_args [:backlog, :label_id] end @@ -71,3 +74,5 @@ module Mutations end end end + +Mutations::Boards::Lists::Create.prepend_if_ee('::EE::Mutations::Boards::Lists::Create') diff --git a/app/graphql/mutations/snippets/base.rb b/app/graphql/mutations/snippets/base.rb index c8cc721b2e0..023f876d035 100644 --- a/app/graphql/mutations/snippets/base.rb +++ b/app/graphql/mutations/snippets/base.rb @@ -11,7 +11,7 @@ module Mutations private def find_object(id:) - GitlabSchema.object_from_id(id) + GitlabSchema.object_from_id(id, expected_type: ::Snippet) end def authorized_resource?(snippet) diff --git a/app/models/active_session.rb b/app/models/active_session.rb index be07c221f32..4908290e06b 100644 --- a/app/models/active_session.rb +++ b/app/models/active_session.rb @@ -105,6 +105,19 @@ class ActiveSession end end + def self.destroy_all_but_current(user, current_session) + session_ids = not_impersonated(user) + session_ids.reject! { |session| session.current?(current_session) } if current_session + + Gitlab::Redis::SharedState.with do |redis| + destroy_sessions(redis, user, session_ids.map(&:session_id)) if session_ids.any? + end + end + + def self.not_impersonated(user) + list(user).reject(&:is_impersonated) + end + def self.key_name(user_id, session_id = '*') "#{Gitlab::Redis::SharedState::USER_SESSIONS_NAMESPACE}:#{user_id}:#{session_id}" end diff --git a/app/models/aws/role.rb b/app/models/aws/role.rb index 54132be749d..7d34665082d 100644 --- a/app/models/aws/role.rb +++ b/app/models/aws/role.rb @@ -9,6 +9,7 @@ module Aws validates :role_external_id, uniqueness: true, length: { in: 1..64 } validates :role_arn, length: 1..2048, + allow_nil: true, format: { with: Gitlab::Regex.aws_arn_regex, message: Gitlab::Regex.aws_arn_regex_message diff --git a/app/models/clusters/applications/runner.rb b/app/models/clusters/applications/runner.rb index f33d54544ca..4983de83800 100644 --- a/app/models/clusters/applications/runner.rb +++ b/app/models/clusters/applications/runner.rb @@ -3,7 +3,7 @@ module Clusters module Applications class Runner < ApplicationRecord - VERSION = '0.20.0' + VERSION = '0.20.1' self.table_name = 'clusters_applications_runners' diff --git a/app/models/concerns/discussion_on_diff.rb b/app/models/concerns/discussion_on_diff.rb index 8542c48f366..40891073738 100644 --- a/app/models/concerns/discussion_on_diff.rb +++ b/app/models/concerns/discussion_on_diff.rb @@ -13,14 +13,12 @@ module DiscussionOnDiff :diff_line, :active?, :created_at_diff?, - to: :first_note delegate :file_path, :blob, :highlighted_diff_lines, :diff_lines, - to: :diff_file, allow_nil: true end diff --git a/app/models/concerns/resolvable_discussion.rb b/app/models/concerns/resolvable_discussion.rb index 5174ae05d15..3e1e5faee54 100644 --- a/app/models/concerns/resolvable_discussion.rb +++ b/app/models/concerns/resolvable_discussion.rb @@ -31,7 +31,6 @@ module ResolvableDiscussion delegate :resolved_at, :resolved_by, :resolved_by_push?, - to: :last_resolved_note, allow_nil: true end diff --git a/app/models/diff_discussion.rb b/app/models/diff_discussion.rb index f9e2f00b9f3..6806008d676 100644 --- a/app/models/diff_discussion.rb +++ b/app/models/diff_discussion.rb @@ -16,7 +16,6 @@ class DiffDiscussion < Discussion :diff_note_positions, :on_text?, :on_image?, - to: :first_note def legacy_diff_discussion? diff --git a/app/models/discussion.rb b/app/models/discussion.rb index adcb2217d85..793cdb5dece 100644 --- a/app/models/discussion.rb +++ b/app/models/discussion.rb @@ -24,7 +24,6 @@ class Discussion :system_note_with_references_visible_for?, :resource_parent, :save, - to: :first_note def declarative_policy_delegate diff --git a/app/models/member.rb b/app/models/member.rb index a27076b49d7..1913df61614 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -76,6 +76,8 @@ class Member < ApplicationRecord scope :request, -> { where.not(requested_at: nil) } scope :non_request, -> { where(requested_at: nil) } + scope :not_accepted_invitations_by_user, -> (user) { invite.where(invite_accepted_at: nil, created_by: user) } + scope :has_access, -> { active.where('access_level > 0') } scope :guests, -> { active.where(access_level: GUEST) } diff --git a/app/models/user.rb b/app/models/user.rb index 355a174ba9a..7574f0865a0 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -364,6 +364,7 @@ class User < ApplicationRecord scope :order_oldest_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'ASC')) } scope :order_recent_last_activity, -> { reorder(Gitlab::Database.nulls_last_order('last_activity_on', 'DESC')) } scope :order_oldest_last_activity, -> { reorder(Gitlab::Database.nulls_first_order('last_activity_on', 'ASC')) } + scope :by_id_and_login, ->(id, login) { where(id: id).where('username = LOWER(:login) OR email = LOWER(:login)', login: login) } def preferred_language read_attribute('preferred_language') || @@ -887,6 +888,12 @@ class User < ApplicationRecord all_expanded_groups.where(require_two_factor_authentication: true) end + def source_groups_of_two_factor_authentication_requirement + Gitlab::ObjectHierarchy.new(expanded_groups_requiring_two_factor_authentication) + .all_objects + .where(id: groups) + end + # rubocop: disable CodeReuse/ServiceClass def refresh_authorized_projects Users::RefreshAuthorizedProjectsService.new(self).execute diff --git a/app/services/applications/create_service.rb b/app/services/applications/create_service.rb index 500db1e172a..92500fbc254 100644 --- a/app/services/applications/create_service.rb +++ b/app/services/applications/create_service.rb @@ -11,7 +11,16 @@ module Applications # EE would override and use `request` arg def execute(request) - Doorkeeper::Application.create(params) + @application = Doorkeeper::Application.new(params) + + unless params[:scopes].present? + @application.errors.add(:base, _("Scopes can't be blank")) + + return @application + end + + @application.save + @application end end end diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 44a434f4402..831a25a637e 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -132,6 +132,7 @@ module Auth def can_access?(requested_project, requested_action) return false unless requested_project.container_registry_enabled? + return false if requested_project.repository_access_level == ::ProjectFeature::DISABLED case requested_action when 'pull' diff --git a/app/services/branches/delete_service.rb b/app/services/branches/delete_service.rb index 9bd5b343448..6efbdd161a1 100644 --- a/app/services/branches/delete_service.rb +++ b/app/services/branches/delete_service.rb @@ -26,7 +26,7 @@ module Branches message: 'Failed to remove branch', http_status: 400) end - rescue Gitlab::Git::PreReceiveError => ex + rescue Gitlab::Git::PreReceiveError, Gitlab::Git::CommandError => ex ServiceResponse.error(message: ex.message, http_status: 400) end diff --git a/app/services/ci/pipeline_trigger_service.rb b/app/services/ci/pipeline_trigger_service.rb index 37b9b4c362c..d9f41b7040e 100644 --- a/app/services/ci/pipeline_trigger_service.rb +++ b/app/services/ci/pipeline_trigger_service.rb @@ -10,6 +10,9 @@ module Ci elsif job_from_token create_pipeline_from_job(job_from_token) end + + rescue Ci::AuthJobFinder::AuthError => e + error(e.message, 401) end private @@ -41,8 +44,6 @@ module Ci # this check is to not leak the presence of the project if user cannot read it return unless can?(job.user, :read_project, project) - return error("400 Job has to be running", 400) unless job.running? - pipeline = Ci::CreatePipelineService.new(project, job.user, ref: params[:ref]) .execute(:pipeline, ignore_skip_ci: true) do |pipeline| source = job.sourced_pipelines.build( @@ -64,7 +65,7 @@ module Ci def job_from_token strong_memoize(:job) do - Ci::Build.find_by_token(params[:token].to_s) + Ci::AuthJobFinder.new(token: params[:token].to_s).execute! end end diff --git a/app/services/clusters/aws/authorize_role_service.rb b/app/services/clusters/aws/authorize_role_service.rb index fb620f77b9f..2712a4b05bb 100644 --- a/app/services/clusters/aws/authorize_role_service.rb +++ b/app/services/clusters/aws/authorize_role_service.rb @@ -9,6 +9,7 @@ module Clusters ERRORS = [ ActiveRecord::RecordInvalid, + ActiveRecord::RecordNotFound, Clusters::Aws::FetchCredentialsService::MissingRoleError, ::Aws::Errors::MissingCredentialsError, ::Aws::STS::Errors::ServiceError @@ -20,7 +21,8 @@ module Clusters end def execute - @role = create_or_update_role! + ensure_role_exists! + update_role_arn! Response.new(:ok, credentials) rescue *ERRORS => e @@ -33,14 +35,12 @@ module Clusters attr_reader :role, :params - def create_or_update_role! - if role = user.aws_role - role.update!(params) + def ensure_role_exists! + @role = ::Aws::Role.find_by_user_id!(user.id) + end - role - else - user.create_aws_role!(params) - end + def update_role_arn! + role.update!(params) end def credentials diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index fdd43260521..8cad065e6cc 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -18,6 +18,7 @@ module Members end delete_subresources(member) unless skip_subresources + delete_project_invitations_by(member) unless skip_subresources enqueue_delete_todos(member) enqueue_unassign_issuables(member) if unassign_issuables @@ -39,24 +40,48 @@ module Members delete_project_members(member) delete_subgroup_members(member) + delete_invited_members(member) end def delete_project_members(member) groups = member.group.self_and_descendants - ProjectMember.in_namespaces(groups).with_user(member.user).each do |project_member| - self.class.new(current_user).execute(project_member, skip_authorization: @skip_auth) - end + destroy_project_members(ProjectMember.in_namespaces(groups).with_user(member.user)) end def delete_subgroup_members(member) groups = member.group.descendants - GroupMember.of_groups(groups).with_user(member.user).each do |group_member| + destroy_group_members(GroupMember.of_groups(groups).with_user(member.user)) + end + + def delete_invited_members(member) + groups = member.group.self_and_descendants + + destroy_group_members(GroupMember.of_groups(groups).not_accepted_invitations_by_user(member.user)) + + destroy_project_members(ProjectMember.in_namespaces(groups).not_accepted_invitations_by_user(member.user)) + end + + def destroy_project_members(members) + members.each do |project_member| + self.class.new(current_user).execute(project_member, skip_authorization: @skip_auth) + end + end + + def destroy_group_members(members) + members.each do |group_member| self.class.new(current_user).execute(group_member, skip_authorization: @skip_auth, skip_subresources: true) end end + def delete_project_invitations_by(member) + return unless member.is_a?(ProjectMember) && member.user && member.project + + members_to_delete = member.project.members.not_accepted_invitations_by_user(member.user) + destroy_project_members(members_to_delete) + end + def can_destroy_member?(member) can?(current_user, destroy_member_permission(member), member) end diff --git a/app/services/projects/update_remote_mirror_service.rb b/app/services/projects/update_remote_mirror_service.rb index fe2610f89fb..7961f689259 100644 --- a/app/services/projects/update_remote_mirror_service.rb +++ b/app/services/projects/update_remote_mirror_service.rb @@ -7,6 +7,10 @@ module Projects def execute(remote_mirror, tries) return success unless remote_mirror.enabled? + if Gitlab::UrlBlocker.blocked_url?(CGI.unescape(Gitlab::UrlSanitizer.sanitize(remote_mirror.url))) + return error("The remote mirror URL is invalid.") + end + update_mirror(remote_mirror) success diff --git a/app/views/profiles/active_sessions/_active_session.html.haml b/app/views/profiles/active_sessions/_active_session.html.haml index 9ae75fe6b8e..c4b3fa80d75 100644 --- a/app/views/profiles/active_sessions/_active_session.html.haml +++ b/app/views/profiles/active_sessions/_active_session.html.haml @@ -27,6 +27,6 @@ - unless is_current_session .float-right - = link_to profile_active_session_path(active_session.public_id), data: { confirm: _('Are you sure? The device will be signed out of GitLab.') }, method: :delete, class: "btn btn-danger gl-ml-3" do + = link_to profile_active_session_path(active_session.public_id), data: { confirm: _('Are you sure? The device will be signed out of GitLab and all remember me tokens revoked.') }, method: :delete, class: "btn btn-danger gl-ml-3" do %span.sr-only= _('Revoke') = _('Revoke') diff --git a/app/views/projects/issues/service_desk.html.haml b/app/views/projects/issues/service_desk.html.haml index bd260bdf143..65580a94cd0 100644 --- a/app/views/projects/issues/service_desk.html.haml +++ b/app/views/projects/issues/service_desk.html.haml @@ -7,7 +7,7 @@ - support_bot_attrs = { service_desk_enabled: @project.service_desk_enabled?, **UserSerializer.new.represent(User.support_bot) }.to_json -- data_endpoint = "#{expose_path(api_v4_projects_issues_path(id: @project.id))}?author_id=#{User.support_bot.id}" +- data_endpoint = "#{expose_path(api_v4_projects_issues_path(id: @project.id))}?author_username=#{User.support_bot.username}" %div{ class: "js-service-desk-issues service-desk-issues", data: { support_bot: support_bot_attrs, service_desk_meta: service_desk_meta(@project) } } .top-area diff --git a/changelogs/unreleased/217908-handle-git-errors.yml b/changelogs/unreleased/217908-handle-git-errors.yml new file mode 100644 index 00000000000..bb6c4918428 --- /dev/null +++ b/changelogs/unreleased/217908-handle-git-errors.yml @@ -0,0 +1,5 @@ +--- +title: Fix unfinished merge by Merge Train process +merge_request: 41106 +author: +type: fixed diff --git a/changelogs/unreleased/235440-search-query-is-added-without-query-parameter-name-in-the-service-.yml b/changelogs/unreleased/235440-search-query-is-added-without-query-parameter-name-in-the-service-.yml new file mode 100644 index 00000000000..09b4181ee32 --- /dev/null +++ b/changelogs/unreleased/235440-search-query-is-added-without-query-parameter-name-in-the-service-.yml @@ -0,0 +1,5 @@ +--- +title: Fix the filtered search bar to work in the service desk issue list +merge_request: 40797 +author: +type: fixed diff --git a/changelogs/unreleased/eb-track-group-coverage-csv-download.yml b/changelogs/unreleased/eb-track-group-coverage-csv-download.yml new file mode 100644 index 00000000000..603d01bad2c --- /dev/null +++ b/changelogs/unreleased/eb-track-group-coverage-csv-download.yml @@ -0,0 +1,5 @@ +--- +title: Track downloads of group code coverage CSV in snowplow +merge_request: 40754 +author: +type: added diff --git a/changelogs/unreleased/empty-lines-cop-fix.yml b/changelogs/unreleased/empty-lines-cop-fix.yml new file mode 100644 index 00000000000..f5ed1955266 --- /dev/null +++ b/changelogs/unreleased/empty-lines-cop-fix.yml @@ -0,0 +1,5 @@ +--- +title: Fix Layout/EmptyLinesAroundArguments cop +merge_request: 41086 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/empty-literal-cop.yml b/changelogs/unreleased/empty-literal-cop.yml new file mode 100644 index 00000000000..1d5b4857193 --- /dev/null +++ b/changelogs/unreleased/empty-literal-cop.yml @@ -0,0 +1,5 @@ +--- +title: Fix Style/EmptyLiteral cop +merge_request: 41110 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/it-behaves-cop.yml b/changelogs/unreleased/it-behaves-cop.yml new file mode 100644 index 00000000000..95bb412c55a --- /dev/null +++ b/changelogs/unreleased/it-behaves-cop.yml @@ -0,0 +1,5 @@ +--- +title: Fix RSpec/ItBehavesLike cop +merge_request: 41111 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/non-deter-cop.yml b/changelogs/unreleased/non-deter-cop.yml new file mode 100644 index 00000000000..18cd89ccceb --- /dev/null +++ b/changelogs/unreleased/non-deter-cop.yml @@ -0,0 +1,5 @@ +--- +title: Fix Lint/NonDeterministicRequireOrder cop +merge_request: 41098 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/security-205-dblessing-oauth-token-brute-force.yml b/changelogs/unreleased/security-205-dblessing-oauth-token-brute-force.yml new file mode 100644 index 00000000000..e53ef7ba09a --- /dev/null +++ b/changelogs/unreleased/security-205-dblessing-oauth-token-brute-force.yml @@ -0,0 +1,5 @@ +--- +title: Protect OAuth endpoints from brute force/password stuffing +merge_request: +author: +type: security diff --git a/changelogs/unreleased/update-gitlab-runner-helm-chart-to-0-20-1.yml b/changelogs/unreleased/update-gitlab-runner-helm-chart-to-0-20-1.yml new file mode 100644 index 00000000000..67cddaa4539 --- /dev/null +++ b/changelogs/unreleased/update-gitlab-runner-helm-chart-to-0-20-1.yml @@ -0,0 +1,5 @@ +--- +title: Update GitLab Runner Helm Chart to 0.20.1 +merge_request: +author: +type: security diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 76e29fb6c02..ad0b0c2008f 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -17,7 +17,7 @@ Doorkeeper.configure do end resource_owner_from_credentials do |routes| - user = Gitlab::Auth.find_with_user_password(params[:username], params[:password]) + user = Gitlab::Auth.find_with_user_password(params[:username], params[:password], increment_failed_attempts: true) user unless user.try(:two_factor_enabled?) end diff --git a/config/routes/user.rb b/config/routes/user.rb index 3bf13908d39..c7a5a56d9ed 100644 --- a/config/routes/user.rb +++ b/config/routes/user.rb @@ -25,7 +25,6 @@ devise_for :users, controllers: { omniauth_callbacks: :omniauth_callbacks, confirmations: :confirmations } devise_scope :user do - get '/users/auth/:provider/omniauth_error' => 'omniauth_callbacks#omniauth_error', as: :omniauth_error get '/users/almost_there' => 'confirmations#almost_there' end diff --git a/db/migrate/20200717040735_change_aws_roles_role_arn_null.rb b/db/migrate/20200717040735_change_aws_roles_role_arn_null.rb new file mode 100644 index 00000000000..707cfe96a7c --- /dev/null +++ b/db/migrate/20200717040735_change_aws_roles_role_arn_null.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class ChangeAwsRolesRoleArnNull < ActiveRecord::Migration[6.0] + DOWNTIME = false + EXAMPLE_ARN = 'arn:aws:iam::000000000000:role/example-role' + + def up + change_column_null :aws_roles, :role_arn, true + end + + def down + # Records may now exist with nulls, so we must fill them with a dummy value + change_column_null :aws_roles, :role_arn, false, EXAMPLE_ARN + end +end diff --git a/db/migrate/20200728182311_add_o_auth_paths_to_protected_paths.rb b/db/migrate/20200728182311_add_o_auth_paths_to_protected_paths.rb new file mode 100644 index 00000000000..7a5af0135fa --- /dev/null +++ b/db/migrate/20200728182311_add_o_auth_paths_to_protected_paths.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +class AddOAuthPathsToProtectedPaths < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + ADD_PROTECTED_PATHS = [ + '/oauth/authorize', + '/oauth/token' + ].freeze + + EXISTING_DEFAULT_PROTECTED_PATHS = [ + '/users/password', + '/users/sign_in', + '/api/v3/session.json', + '/api/v3/session', + '/api/v4/session.json', + '/api/v4/session', + '/users', + '/users/confirmation', + '/unsubscribes/', + '/import/github/personal_access_token', + '/admin/session' + ].freeze + + NEW_DEFAULT_PROTECTED_PATHS = (EXISTING_DEFAULT_PROTECTED_PATHS + ADD_PROTECTED_PATHS).freeze + + class ApplicationSetting < ActiveRecord::Base + self.table_name = 'application_settings' + end + + def up + change_column_default :application_settings, :protected_paths, NEW_DEFAULT_PROTECTED_PATHS + + ApplicationSetting.reset_column_information + + ApplicationSetting.where.not(protected_paths: nil).each do |application_setting| + missing_paths = ADD_PROTECTED_PATHS - application_setting.protected_paths + + next if missing_paths.empty? + + updated_protected_paths = application_setting.protected_paths + missing_paths + application_setting.update!(protected_paths: updated_protected_paths) + end + end + + def down + change_column_default :application_settings, :protected_paths, EXISTING_DEFAULT_PROTECTED_PATHS + + ApplicationSetting.reset_column_information + + ApplicationSetting.where.not(protected_paths: nil).each do |application_setting| + paths_to_remove = application_setting.protected_paths - EXISTING_DEFAULT_PROTECTED_PATHS + + next if paths_to_remove.empty? + + updated_protected_paths = application_setting.protected_paths - paths_to_remove + application_setting.update!(protected_paths: updated_protected_paths) + end + end +end diff --git a/db/schema_migrations/20200717040735 b/db/schema_migrations/20200717040735 new file mode 100644 index 00000000000..6bfa1dd7261 --- /dev/null +++ b/db/schema_migrations/20200717040735 @@ -0,0 +1 @@ +6b8fa09c9700c494eeb5151f43064f1656eaaea804742629b7bd66483e2b04cb
\ No newline at end of file diff --git a/db/schema_migrations/20200728182311 b/db/schema_migrations/20200728182311 new file mode 100644 index 00000000000..6bb5a869513 --- /dev/null +++ b/db/schema_migrations/20200728182311 @@ -0,0 +1 @@ +2aab4599404312ddcc5bc9af11b0a21dfd6aa8aa10d4b4b5086a93ce1ffe77b6
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 61a9dadba08..97d74287694 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9190,7 +9190,7 @@ CREATE TABLE public.application_settings ( throttle_protected_paths_enabled boolean DEFAULT false NOT NULL, throttle_protected_paths_requests_per_period integer DEFAULT 10 NOT NULL, throttle_protected_paths_period_in_seconds integer DEFAULT 60 NOT NULL, - protected_paths character varying(255)[] DEFAULT '{/users/password,/users/sign_in,/api/v3/session.json,/api/v3/session,/api/v4/session.json,/api/v4/session,/users,/users/confirmation,/unsubscribes/,/import/github/personal_access_token,/admin/session}'::character varying[], + protected_paths character varying(255)[] DEFAULT '{/users/password,/users/sign_in,/api/v3/session.json,/api/v3/session,/api/v4/session.json,/api/v4/session,/users,/users/confirmation,/unsubscribes/,/import/github/personal_access_token,/admin/session,/oauth/authorize,/oauth/token}'::character varying[], throttle_incident_management_notification_enabled boolean DEFAULT false NOT NULL, throttle_incident_management_notification_period_in_seconds integer DEFAULT 3600, throttle_incident_management_notification_per_period integer DEFAULT 3600, @@ -9557,7 +9557,7 @@ CREATE TABLE public.aws_roles ( user_id integer NOT NULL, created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, - role_arn character varying(2048) NOT NULL, + role_arn character varying(2048), role_external_id character varying(64) NOT NULL ); diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index f66a897c1c2..6d73c0d594a 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -1310,12 +1310,17 @@ Autogenerated input type of BoardListCreate """ input BoardListCreateInput { """ + Global ID of an existing user + """ + assigneeId: UserID + + """ Create the backlog list """ backlog: Boolean """ - The Global ID of the issue board to mutate + Global ID of the issue board to mutate """ boardId: BoardID! @@ -1325,9 +1330,14 @@ input BoardListCreateInput { clientMutationId: String """ - ID of an existing label + Global ID of an existing label """ labelId: LabelID + + """ + Global ID of an existing milestone + """ + milestoneId: MilestoneID } """ @@ -17210,6 +17220,11 @@ type UserEdge { node: User } +""" +Identifier of User +""" +scalar UserID + type UserPermissions { """ Indicates the user can perform `create_snippet` on this resource diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index ecf3cb3e38c..7a58ee5d54b 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -3505,7 +3505,7 @@ "inputFields": [ { "name": "boardId", - "description": "The Global ID of the issue board to mutate", + "description": "Global ID of the issue board to mutate", "type": { "kind": "NON_NULL", "name": null, @@ -3529,7 +3529,7 @@ }, { "name": "labelId", - "description": "ID of an existing label", + "description": "Global ID of an existing label", "type": { "kind": "SCALAR", "name": "LabelID", @@ -3538,6 +3538,26 @@ "defaultValue": null }, { + "name": "milestoneId", + "description": "Global ID of an existing milestone", + "type": { + "kind": "SCALAR", + "name": "MilestoneID", + "ofType": null + }, + "defaultValue": null + }, + { + "name": "assigneeId", + "description": "Global ID of an existing user", + "type": { + "kind": "SCALAR", + "name": "UserID", + "ofType": null + }, + "defaultValue": null + }, + { "name": "clientMutationId", "description": "A unique identifier for the client performing the mutation.", "type": { @@ -50479,6 +50499,16 @@ "possibleTypes": null }, { + "kind": "SCALAR", + "name": "UserID", + "description": "Identifier of User", + "fields": null, + "inputFields": null, + "interfaces": null, + "enumValues": null, + "possibleTypes": null + }, + { "kind": "OBJECT", "name": "UserPermissions", "description": null, diff --git a/doc/development/sidekiq_style_guide.md b/doc/development/sidekiq_style_guide.md index c5dfc5731e6..fdea27f43ca 100644 --- a/doc/development/sidekiq_style_guide.md +++ b/doc/development/sidekiq_style_guide.md @@ -615,42 +615,51 @@ Jobs need to be backward and forward compatible between consecutive versions of the application. Adding or removing an argument may cause problems during deployment before all Rails and Sidekiq nodes have the updated code. -#### Remove an argument +#### Deprecate and remove an argument -**Do not remove arguments from the `perform` function.**. Instead, use the -following approach: +**Before you remove arguments from the `perform_async` and `perform` methods.**, deprecate them. The +following example deprecates and then removes `arg2` from the `perform_async` method: 1. Provide a default value (usually `nil`) and use a comment to mark the - argument as deprecated -1. Stop using the argument in `perform_async`. -1. Ignore the value in the worker class, but do not remove it until the next - major release. + argument as deprecated in the coming minor release. (Release M) -In the following example, if you want to remove `arg2`, first set a `nil` default value, -and then update locations where `ExampleWorker.perform_async` is called. + ```ruby + class ExampleWorker + # Keep arg2 parameter for backwards compatibility. + def perform(object_id, arg1, arg2 = nil) + # ... + end + end + ``` -```ruby -class ExampleWorker - def perform(object_id, arg1, arg2 = nil) - # ... - end -end -``` +1. One minor release later, stop using the argument in `perform_async`. (Release M+1) + + ```ruby + ExampleWorker.perform_async(object_id, arg1) + ``` + +1. At the next major release, remove the value from the worker class. (Next major release) + + ```ruby + class ExampleWorker + def perform(object_id, arg1) + # ... + end + end + ``` #### Add an argument There are two options for safely adding new arguments to Sidekiq workers: -1. Set up a [multi-step deployment](#multi-step-deployment) in which the new argument is first added to the worker +1. Set up a [multi-step deployment](#multi-step-deployment) in which the new argument is first added to the worker. 1. Use a [parameter hash](#parameter-hash) for additional arguments. This is perhaps the most flexible option. ##### Multi-step deployment -This approach requires multiple merge requests and for the first merge request -to be merged and deployed before additional changes are merged. +This approach requires multiple releases. -1. In an initial merge request, add the argument to the worker with a default - value: +1. Add the argument to the worker with a default value (Release M). ```ruby class ExampleWorker @@ -660,16 +669,28 @@ to be merged and deployed before additional changes are merged. end ``` -1. Merge and deploy the worker with the new argument. -1. In a further merge request, update `ExampleWorker.perform_async` calls to - use the new argument. +1. Add the new argument to all the invocations of the worker (Release M+1). + + ```ruby + ExampleWorker.perform_async(object_id, new_arg) + ``` + +1. Remove the default value (Release M+2). + + ```ruby + class ExampleWorker + def perform(object_id, new_arg) + # ... + end + end + ``` ##### Parameter hash -This approach will not require multiple deployments if an existing worker already +This approach will not require multiple releases if an existing worker already utilizes a parameter hash. -1. Use a parameter hash in the worker to allow for future flexibility: +1. Use a parameter hash in the worker to allow future flexibility. ```ruby class ExampleWorker diff --git a/doc/integration/elasticsearch.md b/doc/integration/elasticsearch.md index daf7a8f6703..4d0ed035ad5 100644 --- a/doc/integration/elasticsearch.md +++ b/doc/integration/elasticsearch.md @@ -165,7 +165,7 @@ instances](#indexing-large-instances) below. In order to enable Elasticsearch, you need to have admin access in GitLab. -1. Navigate to **Admin Area** (wrench icon), then **Settings > Integrations** +1. Navigate to **Admin Area** (wrench icon), then **Settings > General** and expand the **Elasticsearch** section. 1. Configure the [Elasticsearch settings](#elasticsearch-configuration) for your Elasticsearch cluster. Do not enable **Elasticsearch indexing** or @@ -183,7 +183,7 @@ In order to enable Elasticsearch, you need to have admin access in GitLab. ``` 1. Now enable `Elasticsearch indexing` in **Admin Area > Settings > - Integrations > Elasticsearch** and click **Save changes**. + General > Elasticsearch** and click **Save changes**. 1. Click **Index all projects**. 1. Click **Check progress** in the confirmation message to see the status of the background jobs. @@ -198,7 +198,7 @@ In order to enable Elasticsearch, you need to have admin access in GitLab. ``` 1. After the indexing has completed, enable **Search with Elasticsearch** in - **Admin Area > Settings > Integrations > Elasticsearch** and click **Save + **Admin Area > Settings > General > Elasticsearch** and click **Save changes**. ### Elasticsearch configuration @@ -251,7 +251,7 @@ from the Elasticsearch index as expected. To disable the Elasticsearch integration: -1. Navigate to the **Admin Area** (wrench icon), then **Settings > Integrations**. +1. Navigate to the **Admin Area** (wrench icon), then **Settings > General**. 1. Expand the **Elasticsearch** section and uncheck **Elasticsearch indexing** and **Search with Elasticsearch enabled**. 1. Click **Save changes** for the changes to take effect. @@ -439,7 +439,7 @@ used by the GitLab Elasticsearch integration. ### Pause the indexing -In the **Admin Area > Integration > Elasticsearch** section, select the +In the **Admin Area > Settings > General > Elasticsearch** section, select the **Pause Elasticsearch Indexing** setting, and then save your change. With this, all updates that should happen on your Elasticsearch index will be @@ -455,7 +455,7 @@ This process involves several shell commands and curl invocations, so a good initial setup will help for later: ```shell -# You can find this value under Admin Area > Integration > Elasticsearch > URL +# You can find this value under Admin Area > Settings > General > Elasticsearch > URL export CLUSTER_URL="http://localhost:9200" export PRIMARY_INDEX="gitlab-production" export SECONDARY_INDEX="gitlab-production-$(date +%s)" @@ -556,14 +556,14 @@ To trigger the re-index from `primary` index: 1. Unpause the indexing - Under **Admin Area > Integration > Elasticsearch**, uncheck the **Pause Elasticsearch Indexing** setting and save. + Under **Admin Area > Settings > General > Elasticsearch**, uncheck the **Pause Elasticsearch Indexing** setting and save. ### Trigger the reindex via the Elasticsearch administration > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/34069) in [GitLab Starter](https://about.gitlab.com/pricing/) 13.2. > - A scheduled index deletion and the ability to cancel it was [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38914) in GitLab Starter 13.3. -Under **Admin Area > Integration > Elasticsearch zero-downtime reindexing**, click on **Trigger cluster reindexing**. +Under **Admin Area > Settings > General > Elasticsearch > Elasticsearch zero-downtime reindexing**, click on **Trigger cluster reindexing**. NOTE: **Note:** Reindexing can be a lengthy process depending on the size of your Elasticsearch cluster. diff --git a/doc/user/profile/active_sessions.md b/doc/user/profile/active_sessions.md index 4dbb11b581d..a5b15a7880c 100644 --- a/doc/user/profile/active_sessions.md +++ b/doc/user/profile/active_sessions.md @@ -29,6 +29,11 @@ exceeds 100, the oldest ones are deleted. 1. Use the previous steps to navigate to **Active Sessions**. 1. Click on **Revoke** besides a session. The current session cannot be revoked, as this would sign you out of GitLab. +NOTE: **Note:** +When any session is revoked all **Remember me** tokens for all +devices will be revoked. See ['Why do I keep getting signed out?'](index.md#why-do-i-keep-getting-signed-out) +for more information about the **Remember me** feature. + <!-- ## Troubleshooting Include any troubleshooting steps that you can foresee. If you know beforehand what issues diff --git a/doc/user/profile/index.md b/doc/user/profile/index.md index b6ef6d7fdb7..894494da513 100644 --- a/doc/user/profile/index.md +++ b/doc/user/profile/index.md @@ -255,6 +255,12 @@ to get you a new `_gitlab_session` and keep you signed in through browser restar After your `remember_user_token` expires and your `_gitlab_session` is cleared/expired, you are asked to sign in again to verify your identity for security reasons. +NOTE: **Note:** +When any session is signed out, or when a session is revoked +via [Active Sessions](active_sessions.md), all **Remember me** tokens are revoked. +While other sessions will remain active, the **Remember me** feature will not restore +a session if the browser is closed or the existing session expires. + ### Increased sign-in time > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/20340) in GitLab 13.1. @@ -264,7 +270,7 @@ The `remember_user_token` lifetime of a cookie can now extend beyond the deadlin GitLab uses both session and persistent cookies: - Session cookie: Session cookies are normally removed at the end of the browser session when the browser is closed. The `_gitlab_session` cookie has no expiration date. -- Persistent cookie: The `remember_me_token` is a cookie with an expiration date of two weeks. GitLab activates this cookie if you click Remember Me when you sign in. +- Persistent cookie: The `remember_user_token` is a cookie with an expiration date of two weeks. GitLab activates this cookie if you click Remember Me when you sign in. By default, the server sets a time-to-live (TTL) of 1-week on any session that is used. diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 59978962b1d..46b69214877 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -69,7 +69,7 @@ module API deploy_token_from_request || find_user_from_bearer_token || find_user_from_job_token || - find_user_from_warden + user_from_warden end end @@ -103,6 +103,25 @@ module API def user_allowed_or_deploy_token?(user) Gitlab::UserAccess.new(user).allowed? || user.is_a?(DeployToken) end + + def user_from_warden + user = find_user_from_warden + + return unless user + return if two_factor_required_but_not_setup?(user) + + user + end + + def two_factor_required_but_not_setup?(user) + verifier = Gitlab::Auth::TwoFactorAuthVerifier.new(user) + + if verifier.two_factor_authentication_required? && verifier.current_user_needs_to_setup_two_factor? + verifier.two_factor_grace_period_expired? + else + false + end + end end class_methods do diff --git a/lib/api/badges.rb b/lib/api/badges.rb index f6cd3f83ff3..f9728ffc446 100644 --- a/lib/api/badges.rb +++ b/lib/api/badges.rb @@ -109,9 +109,10 @@ module API end put ":id/badges/:badge_id" do source = find_source_if_admin(source_type) + badge = find_badge(source) badge = ::Badges::UpdateService.new(declared_params(include_missing: false)) - .execute(find_badge(source)) + .execute(badge) if badge.valid? present_badges(source, badge) @@ -130,10 +131,6 @@ module API source = find_source_if_admin(source_type) badge = find_badge(source) - if badge.is_a?(GroupBadge) && source.is_a?(Project) - error!('To delete a Group badge please use the Group endpoint', 403) - end - destroy_conditionally!(badge) end end diff --git a/lib/api/conan_packages.rb b/lib/api/conan_packages.rb index c98812c16ce..cf91d8ac74a 100644 --- a/lib/api/conan_packages.rb +++ b/lib/api/conan_packages.rb @@ -26,6 +26,8 @@ module API PACKAGE_COMPONENT_REGEX = Gitlab::Regex.conan_recipe_component_regex CONAN_REVISION_REGEX = Gitlab::Regex.conan_revision_regex + CONAN_FILES = (Gitlab::Regex::Packages::CONAN_RECIPE_FILES + Gitlab::Regex::Packages::CONAN_PACKAGE_FILES).freeze + before do require_packages_enabled! @@ -259,7 +261,7 @@ module API end params do - requires :file_name, type: String, desc: 'Package file name', regexp: Gitlab::Regex.conan_file_name_regex + requires :file_name, type: String, desc: 'Package file name', values: CONAN_FILES end namespace 'export/:file_name', requirements: FILE_NAME_REQUIREMENTS do desc 'Download recipe files' do @@ -277,7 +279,7 @@ module API end params do - use :workhorse_upload_params + requires :file, type: ::API::Validations::Types::WorkhorseFile, desc: 'The package file to be published (generated by Multipart middleware)' end route_setting :authentication, job_token_allowed: true, basic_auth_personal_access_token: true @@ -300,7 +302,7 @@ module API params do requires :conan_package_reference, type: String, desc: 'Conan Package ID' requires :package_revision, type: String, desc: 'Conan Package Revision' - requires :file_name, type: String, desc: 'Package file name', regexp: Gitlab::Regex.conan_file_name_regex + requires :file_name, type: String, desc: 'Package file name', values: CONAN_FILES end namespace 'package/:conan_package_reference/:package_revision/:file_name', requirements: FILE_NAME_REQUIREMENTS do desc 'Download package files' do @@ -328,7 +330,7 @@ module API end params do - use :workhorse_upload_params + requires :file, type: ::API::Validations::Types::WorkhorseFile, desc: 'The package file to be published (generated by Multipart middleware)' end route_setting :authentication, job_token_allowed: true, basic_auth_personal_access_token: true diff --git a/lib/api/helpers/badges_helpers.rb b/lib/api/helpers/badges_helpers.rb index 46ce5b4e7b5..f402c603c87 100644 --- a/lib/api/helpers/badges_helpers.rb +++ b/lib/api/helpers/badges_helpers.rb @@ -6,7 +6,13 @@ module API include ::API::Helpers::MembersHelpers def find_badge(source) - source.badges.find(params[:badge_id]) + badge_id = params[:badge_id] + + if source.is_a?(Project) + source.project_badges.find(badge_id) + else + source.badges.find(badge_id) + end end def present_badges(source, records, options = {}) diff --git a/lib/api/helpers/packages/conan/api_helpers.rb b/lib/api/helpers/packages/conan/api_helpers.rb index 69c10440283..65769d7d8ec 100644 --- a/lib/api/helpers/packages/conan/api_helpers.rb +++ b/lib/api/helpers/packages/conan/api_helpers.rb @@ -134,7 +134,7 @@ module API end def track_push_package_event - if params[:file_name] == ::Packages::Conan::FileMetadatum::PACKAGE_BINARY && params['file.size'] > 0 + if params[:file_name] == ::Packages::Conan::FileMetadatum::PACKAGE_BINARY && params[:file].size > 0 # rubocop: disable Style/ZeroLengthPredicate track_event('push_package') end end @@ -148,9 +148,9 @@ module API end def create_package_file_with_type(file_type, current_package) - unless params['file.size'] == 0 + unless params[:file].size == 0 # rubocop: disable Style/ZeroLengthPredicate # conan sends two upload requests, the first has no file, so we skip record creation if file.size == 0 - ::Packages::Conan::CreatePackageFileService.new(current_package, uploaded_package_file, params.merge(conan_file_type: file_type)).execute + ::Packages::Conan::CreatePackageFileService.new(current_package, params[:file], params.merge(conan_file_type: file_type)).execute end end @@ -222,7 +222,7 @@ module API return unless token - ::Ci::Build.find_by_token(token.access_token_id.to_s) + ::Ci::AuthJobFinder.new(token: token.access_token_id.to_s).execute end def decode_oauth_token_from_jwt diff --git a/lib/api/helpers/packages_manager_clients_helpers.rb b/lib/api/helpers/packages_manager_clients_helpers.rb index 2c0598ae7dc..a30396bf07a 100644 --- a/lib/api/helpers/packages_manager_clients_helpers.rb +++ b/lib/api/helpers/packages_manager_clients_helpers.rb @@ -23,7 +23,7 @@ module API return unless token - ::Ci::Build.find_by_token(token) + ::Ci::AuthJobFinder.new(token: token).execute end def find_deploy_token_from_http_basic_auth diff --git a/lib/banzai/pipeline/broadcast_message_pipeline.rb b/lib/banzai/pipeline/broadcast_message_pipeline.rb index e31795e673c..27118269bd0 100644 --- a/lib/banzai/pipeline/broadcast_message_pipeline.rb +++ b/lib/banzai/pipeline/broadcast_message_pipeline.rb @@ -7,7 +7,6 @@ module Banzai @filters ||= FilterArray[ Filter::MarkdownFilter, Filter::BroadcastMessageSanitizationFilter, - Filter::EmojiFilter, Filter::ColorFilter, Filter::AutolinkFilter, diff --git a/lib/banzai/pipeline/gfm_pipeline.rb b/lib/banzai/pipeline/gfm_pipeline.rb index 10ac813ea15..0d6f36cd6e9 100644 --- a/lib/banzai/pipeline/gfm_pipeline.rb +++ b/lib/banzai/pipeline/gfm_pipeline.rb @@ -12,14 +12,11 @@ module Banzai def self.filters @filters ||= FilterArray[ Filter::PlantumlFilter, - # Must always be before the SanitizationFilter to prevent XSS attacks Filter::SpacedLinkFilter, - Filter::SanitizationFilter, Filter::AssetProxyFilter, Filter::SyntaxHighlightFilter, - Filter::MathFilter, Filter::ColorFilter, Filter::MermaidFilter, @@ -34,13 +31,10 @@ module Banzai Filter::ExternalLinkFilter, Filter::SuggestionFilter, Filter::FootnoteFilter, - *reference_filters, - Filter::EmojiFilter, Filter::TaskListFilter, Filter::InlineDiffFilter, - Filter::SetDirectionFilter ] end diff --git a/lib/banzai/pipeline/single_line_pipeline.rb b/lib/banzai/pipeline/single_line_pipeline.rb index 7fe13100ec2..b06cb907025 100644 --- a/lib/banzai/pipeline/single_line_pipeline.rb +++ b/lib/banzai/pipeline/single_line_pipeline.rb @@ -8,11 +8,9 @@ module Banzai Filter::HtmlEntityFilter, Filter::SanitizationFilter, Filter::AssetProxyFilter, - Filter::EmojiFilter, Filter::AutolinkFilter, Filter::ExternalLinkFilter, - *reference_filters ] end diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 332d0bc1478..ece4946383d 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -65,7 +65,15 @@ module Gitlab raise Gitlab::Auth::MissingPersonalAccessTokenError end - def find_with_user_password(login, password) + # Find and return a user if the provided password is valid for various + # authenticators (OAuth, LDAP, Local Database). + # + # Specify `increment_failed_attempts: true` to increment Devise `failed_attempts`. + # CAUTION: Avoid incrementing failed attempts when authentication falls through + # different mechanisms, as in `.find_for_git_client`. This may lead to + # unwanted access locks when the value provided for `password` was actually + # a PAT, deploy token, etc. + def find_with_user_password(login, password, increment_failed_attempts: false) # Avoid resource intensive checks if login credentials are not provided return unless login.present? && password.present? @@ -96,10 +104,14 @@ module Gitlab authenticators.compact! # return found user that was authenticated first for given login credentials - authenticators.find do |auth| + authenticated_user = authenticators.find do |auth| authenticated_user = auth.login(login, password) break authenticated_user if authenticated_user end + + user_auth_attempt!(user, success: !!authenticated_user) if increment_failed_attempts + + authenticated_user end end @@ -222,6 +234,8 @@ module Gitlab # Registry access (with jwt) does not have access to project return if project && !token.has_access_to?(project) + # When repository is disabled, no resources are accessible via Deploy Token + return if project&.repository_access_level == ::ProjectFeature::DISABLED scopes = abilities_for_scopes(token.scopes) @@ -355,6 +369,13 @@ module Gitlab def find_build_by_token(token) ::Ci::Build.running.find_by_token(token) end + + def user_auth_attempt!(user, success:) + return unless user && Gitlab::Database.read_write? + return user.unlock_access! if success + + user.increment_failed_attempts! + end end end end diff --git a/lib/gitlab/auth/auth_finders.rb b/lib/gitlab/auth/auth_finders.rb index f3d0c053880..ccf52bae9a5 100644 --- a/lib/gitlab/auth/auth_finders.rb +++ b/lib/gitlab/auth/auth_finders.rb @@ -69,9 +69,7 @@ module Gitlab current_request.env[JOB_TOKEN_HEADER].presence return unless token - job = ::Ci::Build.find_by_token(token) - raise UnauthorizedError unless job - + job = find_valid_running_job_by_token!(token) @current_authenticated_job = job # rubocop:disable Gitlab/ModuleWithInstanceVariables job.user @@ -84,9 +82,7 @@ module Gitlab return unless login.present? && password.present? return unless ::Gitlab::Auth::CI_JOB_USER == login - job = ::Ci::Build.find_by_token(password) - raise UnauthorizedError unless job - + job = find_valid_running_job_by_token!(password) job.user end @@ -179,7 +175,7 @@ module Gitlab token = parsed_oauth_token return unless token - job = ::Ci::Build.find_by_token(token) + job = ::Ci::AuthJobFinder.new(token: token).execute return unless job @current_authenticated_job = job # rubocop:disable Gitlab/ModuleWithInstanceVariables @@ -304,6 +300,12 @@ module Gitlab def blob_request? current_request.path.include?('/raw/') end + + def find_valid_running_job_by_token!(token) + ::Ci::AuthJobFinder.new(token: token).execute.tap do |job| + raise UnauthorizedError unless job + end + end end end end diff --git a/lib/gitlab/auth/two_factor_auth_verifier.rb b/lib/gitlab/auth/two_factor_auth_verifier.rb new file mode 100644 index 00000000000..86552ef1267 --- /dev/null +++ b/lib/gitlab/auth/two_factor_auth_verifier.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module Gitlab + module Auth + class TwoFactorAuthVerifier + attr_reader :current_user + + def initialize(current_user) + @current_user = current_user + end + + def two_factor_authentication_required? + Gitlab::CurrentSettings.require_two_factor_authentication? || + current_user&.require_two_factor_authentication_from_group? + end + + def current_user_needs_to_setup_two_factor? + current_user && !current_user.temp_oauth_email? && !current_user.two_factor_enabled? + end + + def two_factor_grace_period + periods = [Gitlab::CurrentSettings.two_factor_grace_period] + periods << current_user.two_factor_grace_period if current_user&.require_two_factor_authentication_from_group? + periods.min + end + + def two_factor_grace_period_expired? + time = current_user&.otp_grace_period_started_at + + return false unless time + + two_factor_grace_period.hours.since(time) < Time.current + end + end + end +end diff --git a/lib/gitlab/fogbugz_import/importer.rb b/lib/gitlab/fogbugz_import/importer.rb index 2c53f9b026d..bd5d2e53180 100644 --- a/lib/gitlab/fogbugz_import/importer.rb +++ b/lib/gitlab/fogbugz_import/importer.rb @@ -39,7 +39,7 @@ module Gitlab def user_map @user_map ||= begin - user_map = Hash.new + user_map = {} import_data = project.import_data.try(:data) stored_user_map = import_data['user_map'] if import_data user_map.update(stored_user_map) if stored_user_map diff --git a/lib/gitlab/git/diff_collection.rb b/lib/gitlab/git/diff_collection.rb index 46896961867..e6121d688ba 100644 --- a/lib/gitlab/git/diff_collection.rb +++ b/lib/gitlab/git/diff_collection.rb @@ -37,7 +37,7 @@ module Gitlab @byte_count = 0 @overflow = false @empty = true - @array = Array.new + @array = [] end def each(&block) diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index f3b53a2ba0b..b67b3a37440 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -98,6 +98,13 @@ module Gitlab Guest.can?(download_ability, container) end + def deploy_key_can_download_code? + authentication_abilities.include?(:download_code) && + deploy_key? && + deploy_key.has_access_to?(container) && + (project? && project&.repository_access_level != ::Featurable::DISABLED) + end + def user_can_download_code? authentication_abilities.include?(:download_code) && user_access.can_do_action?(download_ability) @@ -257,7 +264,7 @@ module Gitlab end def check_download_access! - passed = deploy_key? || + passed = deploy_key_can_download_code? || deploy_token? || user_can_download_code? || build_can_download_code? || diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 131c00db612..e1324530412 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -450,7 +450,7 @@ module Gitlab stack_string = Gitlab::BacktraceCleaner.clean_backtrace(caller).drop(1).join("\n") - Gitlab::SafeRequestStore[:stack_counter] ||= Hash.new + Gitlab::SafeRequestStore[:stack_counter] ||= {} count = Gitlab::SafeRequestStore[:stack_counter][stack_string] || 0 Gitlab::SafeRequestStore[:stack_counter][stack_string] = count + 1 diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index 0f52d8ab95d..06e8a9ec649 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -6,11 +6,6 @@ module Gitlab CONAN_RECIPE_FILES = %w[conanfile.py conanmanifest.txt conan_sources.tgz conan_export.tgz].freeze CONAN_PACKAGE_FILES = %w[conaninfo.txt conanmanifest.txt conan_package.tgz].freeze - def conan_file_name_regex - @conan_file_name_regex ||= - %r{\A#{(CONAN_RECIPE_FILES + CONAN_PACKAGE_FILES).join("|")}\z}.freeze - end - def conan_package_reference_regex @conan_package_reference_regex ||= %r{\A[A-Za-z0-9]+\z}.freeze end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 06a802bcac4..469e959ad54 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2873,6 +2873,9 @@ msgstr "" msgid "An error occurred while validating username" msgstr "" +msgid "An error occurred. Please sign in again." +msgstr "" + msgid "An error occurred. Please try again." msgstr "" @@ -3310,7 +3313,7 @@ msgstr "" msgid "Are you sure? Removing this GPG key does not affect already signed commits." msgstr "" -msgid "Are you sure? The device will be signed out of GitLab." +msgid "Are you sure? The device will be signed out of GitLab and all remember me tokens revoked." msgstr "" msgid "Are you sure? This will invalidate your registered applications and U2F devices." @@ -17212,6 +17215,9 @@ msgstr "" msgid "Only project members will be imported. Group members will be skipped." msgstr "" +msgid "Only projects created under a Gold license are available in Security Dashboards." +msgstr "" + msgid "Only verified users with an email address in any of these domains can be added to the group." msgstr "" @@ -19135,6 +19141,9 @@ msgstr "" msgid "Project visibility level will be changed to match namespace rules when transferring to a group." msgstr "" +msgid "Project was not found or you do not have permission to add this project to Security Dashboards." +msgstr "" + msgid "Project: %{name}" msgstr "" @@ -24689,6 +24698,9 @@ msgstr "" msgid "The project can be accessed without any authentication." msgstr "" +msgid "The project has already been added to your dashboard." +msgstr "" + msgid "The project is accessible only by members of the project. Access must be granted explicitly to each user." msgstr "" diff --git a/package.json b/package.json index a794d41db92..70c88e901f1 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ "@babel/preset-env": "^7.10.1", "@gitlab/at.js": "1.5.5", "@gitlab/svgs": "1.161.0", - "@gitlab/ui": "20.16.0", + "@gitlab/ui": "20.17.0", "@gitlab/visual-review-tools": "1.6.1", "@rails/actioncable": "^6.0.3-1", "@sentry/browser": "^5.10.2", diff --git a/qa/spec/spec_helper.rb b/qa/spec/spec_helper.rb index b3251c739a9..81730c3ab13 100644 --- a/qa/spec/spec_helper.rb +++ b/qa/spec/spec_helper.rb @@ -13,9 +13,9 @@ QA::Runtime::Browser.configure! QA::Runtime::Scenario.from_env(QA::Runtime::Env.runtime_scenario_attributes) if QA::Runtime::Env.runtime_scenario_attributes -Dir[::File.join(__dir__, "support/helpers/*.rb")].each { |f| require f } -Dir[::File.join(__dir__, "support/shared_contexts/*.rb")].each { |f| require f } -Dir[::File.join(__dir__, "support/shared_examples/*.rb")].each { |f| require f } +Dir[::File.join(__dir__, "support/helpers/*.rb")].sort.each { |f| require f } +Dir[::File.join(__dir__, "support/shared_contexts/*.rb")].sort.each { |f| require f } +Dir[::File.join(__dir__, "support/shared_examples/*.rb")].sort.each { |f| require f } RSpec.configure do |config| QA::Specs::Helpers::Quarantine.configure_rspec diff --git a/spec/controllers/admin/applications_controller_spec.rb b/spec/controllers/admin/applications_controller_spec.rb index 732d20666cb..6c423097e70 100644 --- a/spec/controllers/admin/applications_controller_spec.rb +++ b/spec/controllers/admin/applications_controller_spec.rb @@ -40,7 +40,7 @@ RSpec.describe Admin::ApplicationsController do describe 'POST #create' do it 'creates the application' do - create_params = attributes_for(:application, trusted: true, confidential: false) + create_params = attributes_for(:application, trusted: true, confidential: false, scopes: ['api']) expect do post :create, params: { doorkeeper_application: create_params } @@ -63,7 +63,7 @@ RSpec.describe Admin::ApplicationsController do context 'when the params are for a confidential application' do it 'creates a confidential application' do - create_params = attributes_for(:application, confidential: true) + create_params = attributes_for(:application, confidential: true, scopes: ['read_user']) expect do post :create, params: { doorkeeper_application: create_params } @@ -75,6 +75,18 @@ RSpec.describe Admin::ApplicationsController do expect(application).to have_attributes(create_params.except(:uid, :owner_type)) end end + + context 'when scopes are not present' do + it 'renders the application form on errors' do + create_params = attributes_for(:application, trusted: true, confidential: false) + + expect do + post :create, params: { doorkeeper_application: create_params } + end.not_to change { Doorkeeper::Application.count } + + expect(response).to render_template :new + end + end end describe 'PATCH #update' do diff --git a/spec/controllers/admin/clusters_controller_spec.rb b/spec/controllers/admin/clusters_controller_spec.rb index 2e0ee671d3f..d2a569a9d48 100644 --- a/spec/controllers/admin/clusters_controller_spec.rb +++ b/spec/controllers/admin/clusters_controller_spec.rb @@ -99,7 +99,9 @@ RSpec.describe Admin::ClustersController do end describe 'GET #new' do - def get_new(provider: 'gcp') + let(:user) { admin } + + def go(provider: 'gcp') get :new, params: { provider: provider } end @@ -112,7 +114,7 @@ RSpec.describe Admin::ClustersController do context 'when selected provider is gke and no valid gcp token exists' do it 'redirects to gcp authorize_url' do - get_new + go expect(response).to redirect_to(assigns(:authorize_url)) end @@ -125,7 +127,7 @@ RSpec.describe Admin::ClustersController do end it 'does not have authorize_url' do - get_new + go expect(assigns(:authorize_url)).to be_nil end @@ -137,7 +139,7 @@ RSpec.describe Admin::ClustersController do end it 'has new object' do - get_new + go expect(assigns(:gcp_cluster)).to be_an_instance_of(Clusters::ClusterPresenter) end @@ -158,16 +160,18 @@ RSpec.describe Admin::ClustersController do describe 'functionality for existing cluster' do it 'has new object' do - get_new + go expect(assigns(:user_cluster)).to be_an_instance_of(Clusters::ClusterPresenter) end end + include_examples 'GET new cluster shared examples' + describe 'security' do - it { expect { get_new }.to be_allowed_for(:admin) } - it { expect { get_new }.to be_denied_for(:user) } - it { expect { get_new }.to be_denied_for(:external) } + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } end end @@ -424,14 +428,13 @@ RSpec.describe Admin::ClustersController do end describe 'POST authorize AWS role for EKS cluster' do - let(:role_arn) { 'arn:aws:iam::123456789012:role/role-name' } - let(:role_external_id) { '12345' } + let!(:role) { create(:aws_role, user: admin) } + let(:role_arn) { 'arn:new-role' } let(:params) do { cluster: { - role_arn: role_arn, - role_external_id: role_external_id + role_arn: role_arn } } end @@ -445,28 +448,32 @@ RSpec.describe Admin::ClustersController do .and_return(double(execute: double)) end - it 'creates an Aws::Role record' do - expect { go }.to change { Aws::Role.count } + it 'updates the associated role with the supplied ARN' do + go expect(response).to have_gitlab_http_status(:ok) - - role = Aws::Role.last - expect(role.user).to eq admin - expect(role.role_arn).to eq role_arn - expect(role.role_external_id).to eq role_external_id + expect(role.reload.role_arn).to eq(role_arn) end - context 'role cannot be created' do + context 'supplied role is invalid' do let(:role_arn) { 'invalid-role' } - it 'does not create a record' do - expect { go }.not_to change { Aws::Role.count } + it 'does not update the associated role' do + expect { go }.not_to change { role.role_arn } expect(response).to have_gitlab_http_status(:unprocessable_entity) end end describe 'security' do + before do + allow_next_instance_of(Clusters::Aws::AuthorizeRoleService) do |service| + response = double(status: :ok, body: double) + + allow(service).to receive(:execute).and_return(response) + end + end + it { expect { go }.to be_allowed_for(:admin) } it { expect { go }.to be_denied_for(:user) } it { expect { go }.to be_denied_for(:external) } diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 357044a144c..f8d4690e9ce 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -229,6 +229,7 @@ RSpec.describe ApplicationController do it 'does not redirect if 2FA is not required' do allow(controller).to receive(:two_factor_authentication_required?).and_return(false) + allow(controller).to receive(:current_user).and_return(create(:user)) expect(controller).not_to receive(:redirect_to) @@ -346,13 +347,17 @@ RSpec.describe ApplicationController do let(:user) { create :user, otp_grace_period_started_at: 2.hours.ago } it 'returns true if the grace period has expired' do - allow(controller).to receive(:two_factor_grace_period).and_return(1) + allow_next_instance_of(Gitlab::Auth::TwoFactorAuthVerifier) do |verifier| + allow(verifier).to receive(:two_factor_grace_period).and_return(2) + end expect(subject).to be_truthy end it 'returns false if the grace period is still active' do - allow(controller).to receive(:two_factor_grace_period).and_return(3) + allow_next_instance_of(Gitlab::Auth::TwoFactorAuthVerifier) do |verifier| + allow(verifier).to receive(:two_factor_grace_period).and_return(3) + end expect(subject).to be_falsey end diff --git a/spec/controllers/groups/clusters_controller_spec.rb b/spec/controllers/groups/clusters_controller_spec.rb index 1593e1290c4..81d5bc7770f 100644 --- a/spec/controllers/groups/clusters_controller_spec.rb +++ b/spec/controllers/groups/clusters_controller_spec.rb @@ -180,6 +180,8 @@ RSpec.describe Groups::ClustersController do end end + include_examples 'GET new cluster shared examples' + describe 'security' do it { expect { go }.to be_allowed_for(:admin) } it { expect { go }.to be_allowed_for(:owner).of(group) } @@ -493,14 +495,13 @@ RSpec.describe Groups::ClustersController do end describe 'POST authorize AWS role for EKS cluster' do - let(:role_arn) { 'arn:aws:iam::123456789012:role/role-name' } - let(:role_external_id) { '12345' } + let!(:role) { create(:aws_role, user: user) } + let(:role_arn) { 'arn:new-role' } let(:params) do { cluster: { - role_arn: role_arn, - role_external_id: role_external_id + role_arn: role_arn } } end @@ -514,28 +515,32 @@ RSpec.describe Groups::ClustersController do .and_return(double(execute: double)) end - it 'creates an Aws::Role record' do - expect { go }.to change { Aws::Role.count } + it 'updates the associated role with the supplied ARN' do + go expect(response).to have_gitlab_http_status(:ok) - - role = Aws::Role.last - expect(role.user).to eq user - expect(role.role_arn).to eq role_arn - expect(role.role_external_id).to eq role_external_id + expect(role.reload.role_arn).to eq(role_arn) end - context 'role cannot be created' do + context 'supplied role is invalid' do let(:role_arn) { 'invalid-role' } - it 'does not create a record' do - expect { go }.not_to change { Aws::Role.count } + it 'does not update the associated role' do + expect { go }.not_to change { role.role_arn } expect(response).to have_gitlab_http_status(:unprocessable_entity) end end describe 'security' do + before do + allow_next_instance_of(Clusters::Aws::AuthorizeRoleService) do |service| + response = double(status: :ok, body: double) + + allow(service).to receive(:execute).and_return(response) + end + end + it { expect { go }.to be_allowed_for(:admin) } it { expect { go }.to be_allowed_for(:owner).of(group) } it { expect { go }.to be_allowed_for(:maintainer).of(group) } diff --git a/spec/controllers/oauth/applications_controller_spec.rb b/spec/controllers/oauth/applications_controller_spec.rb index 0a7975b8c1b..f21ef324884 100644 --- a/spec/controllers/oauth/applications_controller_spec.rb +++ b/spec/controllers/oauth/applications_controller_spec.rb @@ -123,7 +123,8 @@ RSpec.describe Oauth::ApplicationsController do invalid_uri_params = { doorkeeper_application: { name: 'foo', - redirect_uri: 'javascript://alert()' + redirect_uri: 'javascript://alert()', + scopes: ['api'] } } @@ -133,6 +134,23 @@ RSpec.describe Oauth::ApplicationsController do end end + context 'when scopes are not present' do + render_views + + it 'shows an error for blank scopes' do + invalid_uri_params = { + doorkeeper_application: { + name: 'foo', + redirect_uri: 'http://example.org' + } + } + + post :create, params: invalid_uri_params + + expect(response.body).to include 'Scopes can't be blank' + end + end + it_behaves_like 'redirects to login page when the user is not signed in' it_behaves_like 'redirects to 2fa setup page when the user requires it' end @@ -172,7 +190,8 @@ RSpec.describe Oauth::ApplicationsController do { doorkeeper_application: { name: 'foo', - redirect_uri: 'http://example.org' + redirect_uri: 'http://example.org', + scopes: ['api'] } } end diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb index dce996b977d..3f7f0c55f38 100644 --- a/spec/controllers/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/omniauth_callbacks_controller_spec.rb @@ -40,6 +40,22 @@ RSpec.describe OmniauthCallbacksController, type: :controller do end end + context 'when sign in is not valid' do + let(:provider) { :github } + let(:extern_uid) { 'my-uid' } + + it 'renders omniauth error page' do + allow_next_instance_of(Gitlab::Auth::OAuth::User) do |instance| + allow(instance).to receive(:valid_sign_in?).and_return(false) + end + + post provider + + expect(response).to render_template("errors/omniauth_error") + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end + end + context 'when the user is on the last sign in attempt' do let(:extern_uid) { 'my-uid' } diff --git a/spec/controllers/profiles/active_sessions_controller_spec.rb b/spec/controllers/profiles/active_sessions_controller_spec.rb new file mode 100644 index 00000000000..f54f69d853d --- /dev/null +++ b/spec/controllers/profiles/active_sessions_controller_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Profiles::ActiveSessionsController do + describe 'DELETE destroy' do + let_it_be(:user) { create(:user) } + + before do + sign_in(user) + end + + it 'invalidates all remember user tokens' do + ActiveSession.set(user, request) + session_id = request.session.id.public_id + user.remember_me! + + delete :destroy, params: { id: session_id } + + expect(user.reload.remember_created_at).to be_nil + end + end +end diff --git a/spec/controllers/profiles/two_factor_auths_controller_spec.rb b/spec/controllers/profiles/two_factor_auths_controller_spec.rb index e6839f54c5d..59eb33f4bc6 100644 --- a/spec/controllers/profiles/two_factor_auths_controller_spec.rb +++ b/spec/controllers/profiles/two_factor_auths_controller_spec.rb @@ -14,10 +14,9 @@ RSpec.describe Profiles::TwoFactorAuthsController do let(:user) { create(:user) } it 'generates otp_secret for user' do - expect(User).to receive(:generate_otp_secret).with(32).and_return('secret').once + expect(User).to receive(:generate_otp_secret).with(32).and_call_original.once get :show - get :show # Second hit shouldn't re-generate it end it 'assigns qr_code' do @@ -27,6 +26,14 @@ RSpec.describe Profiles::TwoFactorAuthsController do get :show expect(assigns[:qr_code]).to eq code end + + it 'generates a unique otp_secret every time the page is loaded' do + expect(User).to receive(:generate_otp_secret).with(32).and_call_original.twice + + 2.times do + get :show + end + end end describe 'POST create' do @@ -57,6 +64,12 @@ RSpec.describe Profiles::TwoFactorAuthsController do expect(assigns[:codes]).to match_array %w(a b c) end + it 'calls to delete other sessions' do + expect(ActiveSession).to receive(:destroy_all_but_current) + + go + end + it 'renders create' do go expect(response).to render_template(:create) diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index da4faad2a39..51a451570c5 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -183,6 +183,8 @@ RSpec.describe Projects::ClustersController do end end + include_examples 'GET new cluster shared examples' + describe 'security' do it 'is allowed for admin when admin mode enabled', :enable_admin_mode do expect { go }.to be_allowed_for(:admin) @@ -521,14 +523,13 @@ RSpec.describe Projects::ClustersController do end describe 'POST authorize AWS role for EKS cluster' do - let(:role_arn) { 'arn:aws:iam::123456789012:role/role-name' } - let(:role_external_id) { '12345' } + let!(:role) { create(:aws_role, user: user) } + let(:role_arn) { 'arn:new-role' } let(:params) do { cluster: { - role_arn: role_arn, - role_external_id: role_external_id + role_arn: role_arn } } end @@ -542,28 +543,32 @@ RSpec.describe Projects::ClustersController do .and_return(double(execute: double)) end - it 'creates an Aws::Role record' do - expect { go }.to change { Aws::Role.count } + it 'updates the associated role with the supplied ARN' do + go expect(response).to have_gitlab_http_status(:ok) - - role = Aws::Role.last - expect(role.user).to eq user - expect(role.role_arn).to eq role_arn - expect(role.role_external_id).to eq role_external_id + expect(role.reload.role_arn).to eq(role_arn) end - context 'role cannot be created' do + context 'supplied role is invalid' do let(:role_arn) { 'invalid-role' } - it 'does not create a record' do - expect { go }.not_to change { Aws::Role.count } + it 'does not update the associated role' do + expect { go }.not_to change { role.role_arn } expect(response).to have_gitlab_http_status(:unprocessable_entity) end end describe 'security' do + before do + allow_next_instance_of(Clusters::Aws::AuthorizeRoleService) do |service| + response = double(status: :ok, body: double) + + allow(service).to receive(:execute).and_return(response) + end + end + it 'is allowed for admin when admin mode enabled', :enable_admin_mode do expect { go }.to be_allowed_for(:admin) end diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index d66a0246c2d..4d325f55fb0 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -149,9 +149,15 @@ RSpec.describe SearchController do expect(assigns[:search_objects].first).to eq note end - it_behaves_like 'tracking unique hll events', :show do - let(:request_params) { { scope: 'projects', search: 'term' } } - let(:target_id) { 'i_search_total' } + context 'unique users tracking' do + before do + allow(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event) + end + + it_behaves_like 'tracking unique hll events', :show do + let(:request_params) { { scope: 'projects', search: 'term' } } + let(:target_id) { 'i_search_total' } + end end context 'on restricted projects' do diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 7754fac2fb2..acec1cf22fd 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -261,8 +261,8 @@ RSpec.describe SessionsController do context 'when using two-factor authentication via OTP' do let(:user) { create(:user, :two_factor) } - def authenticate_2fa(user_params) - post(:create, params: { user: user_params }, session: { otp_user_id: user.id }) + def authenticate_2fa(user_params, otp_user_id: user.id) + post(:create, params: { user: user_params }, session: { otp_user_id: otp_user_id }) end context 'remember_me field' do @@ -299,8 +299,22 @@ RSpec.describe SessionsController do end end + # See issue gitlab-org/gitlab#20302. + context 'when otp_user_id is stale' do + render_views + + it 'favors login over otp_user_id when password is present and does not authenticate the user' do + authenticate_2fa( + { login: 'random_username', password: user.password }, + otp_user_id: user.id + ) + + expect(response).to set_flash.now[:alert].to /Invalid Login or password/ + end + end + ## - # See #14900 issue + # See issue gitlab-org/gitlab-foss#14900 # context 'when authenticating with login and OTP of another user' do context 'when another user has 2FA enabled' do @@ -386,18 +400,6 @@ RSpec.describe SessionsController do end end end - - context 'when another user does not have 2FA enabled' do - let(:another_user) { create(:user) } - - it 'does not leak that 2FA is disabled for another user' do - authenticate_2fa(login: another_user.username, - otp_attempt: 'invalid') - - expect(response).to set_flash.now[:alert] - .to /Invalid two-factor code/ - end - end end end diff --git a/spec/features/admin/admin_manage_applications_spec.rb b/spec/features/admin/admin_manage_applications_spec.rb index 3f3d71e842c..7a9a6f2ccb8 100644 --- a/spec/features/admin/admin_manage_applications_spec.rb +++ b/spec/features/admin/admin_manage_applications_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'admin manage applications' do sign_in(create(:admin)) end - it do + it 'creates new oauth application' do visit admin_applications_path click_on 'New application' @@ -16,6 +16,7 @@ RSpec.describe 'admin manage applications' do fill_in :doorkeeper_application_name, with: 'test' fill_in :doorkeeper_application_redirect_uri, with: 'https://test.com' check :doorkeeper_application_trusted + check :doorkeeper_application_scopes_read_user click_on 'Submit' expect(page).to have_content('Application: test') expect(page).to have_content('Application ID') @@ -43,4 +44,19 @@ RSpec.describe 'admin manage applications' do end expect(page.find('.oauth-applications')).not_to have_content('test_changed') end + + context 'when scopes are blank' do + it 'returns an error' do + visit admin_applications_path + + click_on 'New application' + expect(page).to have_content('New application') + + fill_in :doorkeeper_application_name, with: 'test' + fill_in :doorkeeper_application_redirect_uri, with: 'https://test.com' + click_on 'Submit' + + expect(page).to have_content("Scopes can't be blank") + end + end end diff --git a/spec/features/issues/service_desk_spec.rb b/spec/features/issues/service_desk_spec.rb index 666fbe84fae..1512d539dec 100644 --- a/spec/features/issues/service_desk_spec.rb +++ b/spec/features/issues/service_desk_spec.rb @@ -109,6 +109,14 @@ RSpec.describe 'Service Desk Issue Tracker', :js do find('.input-token .filtered-search').native.send_key(:backspace) expect(page).to have_selector('.js-visual-token', count: 1) end + + it 'support bot author token has been properly added' do + within('.filtered-search-token') do + expect(page).to have_selector('.name', count: 1, visible: false) + expect(page).to have_selector('.operator', count: 1, visible: false) + expect(page).to have_selector('.value-container', count: 1, visible: false) + end + end end end end diff --git a/spec/features/markdown/copy_as_gfm_spec.rb b/spec/features/markdown/copy_as_gfm_spec.rb index 57362ed2d54..fbf4e531db1 100644 --- a/spec/features/markdown/copy_as_gfm_spec.rb +++ b/spec/features/markdown/copy_as_gfm_spec.rb @@ -30,13 +30,11 @@ RSpec.describe 'Copy as GFM', :js do it 'works', :aggregate_failures do verify( 'nesting', - '> 1. [x] **[$`2 + 2`$ {-=-}{+=+} 2^2 ~~:thumbsup:~~](http://google.com)**' ) verify( 'a real world example from the gitlab-ce README', - <<~GFM # GitLab @@ -103,19 +101,16 @@ RSpec.describe 'Copy as GFM', :js do verify( 'InlineDiffFilter', - '{-Deleted text-}', '{+Added text+}' ) verify( 'TaskListFilter', - <<~GFM, * [ ] Unchecked task * [x] Checked task GFM - <<~GFM 1. [ ] Unchecked ordered task 1. [x] Checked ordered task @@ -124,7 +119,6 @@ RSpec.describe 'Copy as GFM', :js do verify( 'ReferenceFilter', - # issue reference @feat.issue.to_reference, # full issue reference @@ -141,13 +135,11 @@ RSpec.describe 'Copy as GFM', :js do verify( 'AutolinkFilter', - 'https://example.com' ) verify( 'TableOfContentsFilter', - <<~GFM, [[_TOC_]] @@ -155,64 +147,53 @@ RSpec.describe 'Copy as GFM', :js do ## Heading 2 GFM - pipeline: :wiki, wiki: @project.wiki ) verify( 'EmojiFilter', - ':thumbsup:' ) verify( 'ImageLinkFilter', - '' ) verify_media_with_partial_path( '[test.txt](/uploads/a123/image.txt)', - project_media_uri(@project, '/uploads/a123/image.txt') ) verify_media_with_partial_path( '', - project_media_uri(@project, '/uploads/a123/image.png') ) verify( 'VideoLinkFilter', - '' ) verify_media_with_partial_path( '', - project_media_uri(@project, '/uploads/a123/video.mp4') ) verify( 'AudioLinkFilter', - '' ) verify_media_with_partial_path( '', - project_media_uri(@project, '/uploads/a123/audio.wav') ) verify( 'MathFilter: math as converted from GFM to HTML', - '$`c = \pm\sqrt{a^2 + b^2}`$', - # math block <<~GFM ```math @@ -334,7 +315,6 @@ RSpec.describe 'Copy as GFM', :js do verify( 'MermaidFilter: mermaid as converted from GFM to HTML', - <<~GFM ```mermaid graph TD; @@ -429,7 +409,6 @@ RSpec.describe 'Copy as GFM', :js do verify( 'SuggestionFilter: suggestion as converted from GFM to HTML', - <<~GFM ```suggestion New @@ -491,7 +470,6 @@ RSpec.describe 'Copy as GFM', :js do verify( 'SanitizationFilter', - <<~GFM <sub>sub</sub> @@ -527,13 +505,11 @@ RSpec.describe 'Copy as GFM', :js do verify( 'SanitizationFilter', - <<~GFM, ``` Plain text ``` GFM - <<~GFM, ```ruby def foo @@ -541,7 +517,6 @@ RSpec.describe 'Copy as GFM', :js do end ``` GFM - <<~GFM Foo @@ -553,27 +528,19 @@ RSpec.describe 'Copy as GFM', :js do verify( 'MarkdownFilter', - "Line with two spaces at the end \nto insert a linebreak", - '`code`', '`` code with ` ticks ``', - '> Quote', - # multiline quote <<~GFM, > Multiline Quote > > With multiple paragraphs GFM - '', - '# Heading with no anchor link', - '[Link](https://example.com)', - <<~GFM, * List item * List item 2 @@ -598,7 +565,6 @@ RSpec.describe 'Copy as GFM', :js do > Blockquote GFM - <<~GFM, 1. Ordered list item 1. Ordered list item 2 @@ -623,22 +589,16 @@ RSpec.describe 'Copy as GFM', :js do --- GFM - '# Heading', '## Heading', '### Heading', '#### Heading', '##### Heading', '###### Heading', - '**Bold**', - '*Italics*', - '~~Strikethrough~~', - '---', - # table <<~GFM, | Centered | Right | Left | @@ -696,9 +656,7 @@ RSpec.describe 'Copy as GFM', :js do it 'copies as inline code' do verify( '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"] .line .no', - '`RuntimeError`', - target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]' ) end @@ -708,9 +666,7 @@ RSpec.describe 'Copy as GFM', :js do it 'copies as inline code' do verify( '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]', - '`raise RuntimeError, "System commands must be given as an array of strings"`', - target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]' ) end @@ -720,14 +676,12 @@ RSpec.describe 'Copy as GFM', :js do it 'copies as a code block' do verify( '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10"]', - <<~GFM, ```ruby raise RuntimeError, "System commands must be given as an array of strings" end ``` GFM - target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]' ) end @@ -755,7 +709,6 @@ RSpec.describe 'Copy as GFM', :js do it 'copies as a code block' do verify( '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10"]', - <<~GFM, ```ruby unless cmd.is_a?(Array) @@ -763,7 +716,6 @@ RSpec.describe 'Copy as GFM', :js do end ``` GFM - target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"].left-side' ) end @@ -773,7 +725,6 @@ RSpec.describe 'Copy as GFM', :js do it 'copies as a code block' do verify( '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10"]', - <<~GFM, ```ruby unless cmd.is_a?(Array) @@ -781,7 +732,6 @@ RSpec.describe 'Copy as GFM', :js do end ``` GFM - target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"].right-side' ) end @@ -799,7 +749,6 @@ RSpec.describe 'Copy as GFM', :js do it 'copies as inline code' do verify( '.line[id="LC9"] .no', - '`RuntimeError`' ) end @@ -809,7 +758,6 @@ RSpec.describe 'Copy as GFM', :js do it 'copies as inline code' do verify( '.line[id="LC9"]', - '`raise RuntimeError, "System commands must be given as an array of strings"`' ) end @@ -819,7 +767,6 @@ RSpec.describe 'Copy as GFM', :js do it 'copies as a code block' do verify( '.line[id="LC9"], .line[id="LC10"]', - <<~GFM ```ruby raise RuntimeError, "System commands must be given as an array of strings" @@ -841,7 +788,6 @@ RSpec.describe 'Copy as GFM', :js do it 'copies as inline code' do verify( '.line[id="LC27"] .nl', - '`"bio"`' ) end @@ -851,7 +797,6 @@ RSpec.describe 'Copy as GFM', :js do it 'copies as inline code' do verify( '.line[id="LC27"]', - '`"bio": null,`' ) end @@ -861,7 +806,6 @@ RSpec.describe 'Copy as GFM', :js do it 'copies as a code block with the correct language' do verify( '.line[id="LC27"], .line[id="LC28"]', - <<~GFM ```json "bio": null, diff --git a/spec/features/profiles/user_manages_applications_spec.rb b/spec/features/profiles/user_manages_applications_spec.rb index d65365db880..22eed748c00 100644 --- a/spec/features/profiles/user_manages_applications_spec.rb +++ b/spec/features/profiles/user_manages_applications_spec.rb @@ -15,6 +15,7 @@ RSpec.describe 'User manages applications' do fill_in :doorkeeper_application_name, with: 'test' fill_in :doorkeeper_application_redirect_uri, with: 'https://test.com' + check :doorkeeper_application_scopes_read_user click_on 'Save application' expect(page).to have_content 'Application: test' @@ -41,4 +42,16 @@ RSpec.describe 'User manages applications' do end expect(page.find('.oauth-applications')).not_to have_content 'test_changed' end + + context 'when scopes are blank' do + it 'returns an error' do + expect(page).to have_content 'Add new application' + + fill_in :doorkeeper_application_name, with: 'test' + fill_in :doorkeeper_application_redirect_uri, with: 'https://test.com' + click_on 'Save application' + + expect(page).to have_content("Scopes can't be blank") + end + end end diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index a2fd1caf6a3..853c381fe6b 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -177,6 +177,14 @@ RSpec.describe 'Login' do expect(page).not_to have_content(I18n.t('devise.failure.already_authenticated')) end + it 'does not allow sign-in if the user password is updated before entering a one-time code' do + user.update!(password: 'new_password') + + enter_code(user.current_otp) + + expect(page).to have_content('An error occurred. Please sign in again.') + end + context 'using one-time code' do it 'allows login with valid code' do expect(authentication_metrics) @@ -232,7 +240,7 @@ RSpec.describe 'Login' do expect(codes.size).to eq 10 # Ensure the generated codes get saved - user.save + user.save(touch: false) end context 'with valid code' do @@ -290,7 +298,7 @@ RSpec.describe 'Login' do code = codes.sample expect(user.invalidate_otp_backup_code!(code)).to eq true - user.save! + user.save!(touch: false) expect(user.reload.otp_backup_codes.size).to eq 9 enter_code(code) diff --git a/spec/finders/ci/auth_job_finder_spec.rb b/spec/finders/ci/auth_job_finder_spec.rb new file mode 100644 index 00000000000..6cd58f5cd01 --- /dev/null +++ b/spec/finders/ci/auth_job_finder_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe Ci::AuthJobFinder do + let_it_be(:job, reload: true) { create(:ci_build, status: :running) } + + let(:token) { job.token } + + subject(:finder) do + described_class.new(token: token) + end + + describe '#execute!' do + subject(:execute) { finder.execute! } + + it { is_expected.to eq(job) } + + it 'raises error if the job is not running' do + job.success! + + expect { execute }.to raise_error described_class::NotRunningJobError, 'Job is not running' + end + + it 'raises error if the job is erased' do + expect(::Ci::Build).to receive(:find_by_token).with(job.token).and_return(job) + expect(job).to receive(:erased?).and_return(true) + + expect { execute }.to raise_error described_class::ErasedJobError, 'Job has been erased!' + end + + it 'raises error if the the project is missing' do + expect(::Ci::Build).to receive(:find_by_token).with(job.token).and_return(job) + expect(job).to receive(:project).and_return(nil) + + expect { execute }.to raise_error described_class::DeletedProjectError, 'Project has been deleted!' + end + + it 'raises error if the the project is being removed' do + project = double(Project) + + expect(::Ci::Build).to receive(:find_by_token).with(job.token).and_return(job) + expect(job).to receive(:project).twice.and_return(project) + expect(project).to receive(:pending_delete?).and_return(true) + + expect { execute }.to raise_error described_class::DeletedProjectError, 'Project has been deleted!' + end + + context 'with wrong job token' do + let(:token) { 'missing' } + + it { is_expected.to be_nil } + end + end + + describe '#execute' do + subject(:execute) { finder.execute } + + before do + job.success! + end + + it { is_expected.to be_nil } + end +end diff --git a/spec/finders/user_recent_events_finder_spec.rb b/spec/finders/user_recent_events_finder_spec.rb index 559d1004b4b..ddba9b595a4 100644 --- a/spec/finders/user_recent_events_finder_spec.rb +++ b/spec/finders/user_recent_events_finder_spec.rb @@ -11,8 +11,10 @@ RSpec.describe UserRecentEventsFinder do let!(:private_event) { create(:event, project: private_project, author: project_owner) } let!(:internal_event) { create(:event, project: internal_project, author: project_owner) } let!(:public_event) { create(:event, project: public_project, author: project_owner) } + let(:limit) { nil } + let(:params) { { limit: limit } } - subject(:finder) { described_class.new(current_user, project_owner) } + subject(:finder) { described_class.new(current_user, project_owner, params) } describe '#execute' do context 'when profile is public' do @@ -48,5 +50,38 @@ RSpec.describe UserRecentEventsFinder do expect(events).to include(event_b) end end + + context 'limits' do + before do + stub_const("#{described_class}::DEFAULT_LIMIT", 1) + stub_const("#{described_class}::MAX_LIMIT", 3) + end + + context 'when limit is not set' do + it 'returns events limited to DEFAULT_LIMIT' do + expect(finder.execute.size).to eq(described_class::DEFAULT_LIMIT) + end + end + + context 'when limit is set' do + let(:limit) { 2 } + + it 'returns events limited to specified limit' do + expect(finder.execute.size).to eq(limit) + end + end + + context 'when limit is set to a number that exceeds maximum limit' do + let(:limit) { 4 } + + before do + create(:event, project: public_project, author: project_owner) + end + + it 'returns events limited to MAX_LIMIT' do + expect(finder.execute.size).to eq(described_class::MAX_LIMIT) + end + end + end end end diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb index 5d6aa863994..4db12643069 100644 --- a/spec/graphql/gitlab_schema_spec.rb +++ b/spec/graphql/gitlab_schema_spec.rb @@ -212,6 +212,55 @@ RSpec.describe GitlabSchema do end end + describe '.parse_gid' do + let_it_be(:global_id) { 'gid://gitlab/TestOne/2147483647' } + + before do + test_base = Class.new + test_one = Class.new(test_base) + test_two = Class.new(test_base) + + stub_const('TestBase', test_base) + stub_const('TestOne', test_one) + stub_const('TestTwo', test_two) + end + + it 'parses the gid' do + gid = described_class.parse_gid(global_id) + + expect(gid.model_id).to eq '2147483647' + expect(gid.model_class).to eq TestOne + end + + context 'when gid is malformed' do + let_it_be(:global_id) { 'malformed://gitlab/TestOne/2147483647' } + + it 'raises an error' do + expect { described_class.parse_gid(global_id) } + .to raise_error(Gitlab::Graphql::Errors::ArgumentError, "#{global_id} is not a valid GitLab ID.") + end + end + + context 'when using expected_type' do + it 'accepts a single type' do + gid = described_class.parse_gid(global_id, expected_type: TestOne) + + expect(gid.model_class).to eq TestOne + end + + it 'accepts an ancestor type' do + gid = described_class.parse_gid(global_id, expected_type: TestBase) + + expect(gid.model_class).to eq TestOne + end + + it 'rejects an unknown type' do + expect { described_class.parse_gid(global_id, expected_type: TestTwo) } + .to raise_error(Gitlab::Graphql::Errors::ArgumentError, "#{global_id} is not a valid ID for TestTwo.") + end + end + end + def field_instrumenters described_class.instrumenters[:field] + described_class.instrumenters[:field_after_built_ins] end diff --git a/spec/graphql/mutations/boards/lists/create_spec.rb b/spec/graphql/mutations/boards/lists/create_spec.rb index 1a881ac81e8..b1fe9911c7b 100644 --- a/spec/graphql/mutations/boards/lists/create_spec.rb +++ b/spec/graphql/mutations/boards/lists/create_spec.rb @@ -24,14 +24,12 @@ RSpec.describe Mutations::Boards::Lists::Create do describe '#ready?' do it 'raises an error if required arguments are missing' do expect { mutation.ready?({ board_id: 'some id' }) } - .to raise_error(Gitlab::Graphql::Errors::ArgumentError, - 'one and only one of backlog or labelId is required') + .to raise_error(Gitlab::Graphql::Errors::ArgumentError, /one and only one of/) end it 'raises an error if too many required arguments are specified' do expect { mutation.ready?({ board_id: 'some id', backlog: true, label_id: 'some label' }) } - .to raise_error(Gitlab::Graphql::Errors::ArgumentError, - 'one and only one of backlog or labelId is required') + .to raise_error(Gitlab::Graphql::Errors::ArgumentError, /one and only one of/) end end @@ -66,6 +64,15 @@ RSpec.describe Mutations::Boards::Lists::Create do expect(new_list.title).to eq dev_label.title expect(new_list.position).to eq 0 end + + context 'when label not found' do + let(:list_create_params) { { label_id: "gid://gitlab/Label/#{non_existing_record_id}" } } + + it 'raises an error' do + expect { subject } + .to raise_error(Gitlab::Graphql::Errors::ArgumentError, 'Label not found!') + end + end end end diff --git a/spec/helpers/merge_requests_helper_spec.rb b/spec/helpers/merge_requests_helper_spec.rb index fcb9efa39d5..153dc19335b 100644 --- a/spec/helpers/merge_requests_helper_spec.rb +++ b/spec/helpers/merge_requests_helper_spec.rb @@ -75,7 +75,7 @@ RSpec.describe MergeRequestsHelper do describe '#tab_link_for' do let(:merge_request) { create(:merge_request, :simple) } - let(:options) { Hash.new } + let(:options) { {} } subject { tab_link_for(merge_request, :show, options) { 'Discussion' } } diff --git a/spec/lib/api/helpers/packages_manager_clients_helpers_spec.rb b/spec/lib/api/helpers/packages_manager_clients_helpers_spec.rb index 70a95d7ba7b..c19b7b63eec 100644 --- a/spec/lib/api/helpers/packages_manager_clients_helpers_spec.rb +++ b/spec/lib/api/helpers/packages_manager_clients_helpers_spec.rb @@ -50,7 +50,7 @@ RSpec.describe API::Helpers::PackagesManagerClientsHelpers do describe '#find_job_from_http_basic_auth' do let_it_be(:user) { personal_access_token.user } - let(:job) { create(:ci_build, user: user) } + let(:job) { create(:ci_build, user: user, status: :running) } let(:password) { job.token } subject { helper.find_job_from_http_basic_auth } @@ -60,6 +60,16 @@ RSpec.describe API::Helpers::PackagesManagerClientsHelpers do end it_behaves_like 'invalid auth header' + + context 'when the job is not running' do + before do + job.update!(status: :failed) + end + + it_behaves_like 'valid auth header' do + let(:expected_result) { nil } + end + end end describe '#find_deploy_token_from_http_basic_auth' do diff --git a/spec/lib/gitlab/auth/auth_finders_spec.rb b/spec/lib/gitlab/auth/auth_finders_spec.rb index a73ac0b34af..1ac8ebe1369 100644 --- a/spec/lib/gitlab/auth/auth_finders_spec.rb +++ b/spec/lib/gitlab/auth/auth_finders_spec.rb @@ -37,11 +37,29 @@ RSpec.describe Gitlab::Auth::AuthFinders do expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) end - it "return user if token is valid" do - set_token(job.token) + context 'with a running job' do + before do + job.update!(status: :running) + end + + it 'return user if token is valid' do + set_token(job.token) + + expect(subject).to eq(user) + expect(@current_authenticated_job).to eq job + end + end - expect(subject).to eq(user) - expect(@current_authenticated_job).to eq job + context 'with a job that is not running' do + before do + job.update!(status: :failed) + end + + it 'returns an Unauthorized exception' do + set_token(job.token) + + expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) + end end end end @@ -557,7 +575,7 @@ RSpec.describe Gitlab::Auth::AuthFinders do context 'with CI username' do let(:username) { ::Gitlab::Auth::CI_JOB_USER } let(:user) { create(:user) } - let(:build) { create(:ci_build, user: user) } + let(:build) { create(:ci_build, user: user, status: :running) } it 'returns nil without password' do set_basic_auth_header(username, nil) @@ -576,6 +594,13 @@ RSpec.describe Gitlab::Auth::AuthFinders do expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) end + + it 'returns exception if the job is not running' do + set_basic_auth_header(username, build.token) + build.success! + + expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) + end end end @@ -586,7 +611,7 @@ RSpec.describe Gitlab::Auth::AuthFinders do context 'with a job token' do let(:route_authentication_setting) { { job_token_allowed: true } } - let(:job) { create(:ci_build, user: user) } + let(:job) { create(:ci_build, user: user, status: :running) } before do env['HTTP_AUTHORIZATION'] = "Bearer #{job.token}" @@ -641,7 +666,7 @@ RSpec.describe Gitlab::Auth::AuthFinders do end describe '#find_user_from_job_token' do - let(:job) { create(:ci_build, user: user) } + let(:job) { create(:ci_build, user: user, status: :running) } let(:route_authentication_setting) { { job_token_allowed: true } } subject { find_user_from_job_token } @@ -666,6 +691,13 @@ RSpec.describe Gitlab::Auth::AuthFinders do expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) end + it 'returns exception if the job is not running' do + set_header(described_class::JOB_TOKEN_HEADER, job.token) + job.success! + + expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) + end + context 'when route is not allowed to be authenticated' do let(:route_authentication_setting) { { job_token_allowed: false } } diff --git a/spec/lib/gitlab/auth/request_authenticator_spec.rb b/spec/lib/gitlab/auth/request_authenticator_spec.rb index ef83321cc0e..b89ceb37076 100644 --- a/spec/lib/gitlab/auth/request_authenticator_spec.rb +++ b/spec/lib/gitlab/auth/request_authenticator_spec.rb @@ -88,7 +88,7 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do describe '#find_user_from_job_token' do let!(:user) { build(:user) } - let!(:job) { build(:ci_build, user: user) } + let!(:job) { build(:ci_build, user: user, status: :running) } before do env[Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER] = 'token' @@ -97,13 +97,18 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do context 'with API requests' do before do env['SCRIPT_NAME'] = '/api/endpoint' + expect(::Ci::Build).to receive(:find_by_token).with('token').and_return(job) end it 'tries to find the user' do - expect(::Ci::Build).to receive(:find_by_token).and_return(job) - expect(subject.find_sessionless_user([:api])).to eq user end + + it 'returns nil if the job is not running' do + job.status = :success + + expect(subject.find_sessionless_user([:api])).to be_blank + end end context 'without API requests' do diff --git a/spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb b/spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb new file mode 100644 index 00000000000..f906870195a --- /dev/null +++ b/spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb @@ -0,0 +1,112 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Auth::TwoFactorAuthVerifier do + let(:user) { create(:user) } + + subject { described_class.new(user) } + + describe '#two_factor_authentication_required?' do + describe 'when it is required on application level' do + it 'returns true' do + stub_application_setting require_two_factor_authentication: true + + expect(subject.two_factor_authentication_required?).to be_truthy + end + end + + describe 'when it is required on group level' do + it 'returns true' do + allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(true) + + expect(subject.two_factor_authentication_required?).to be_truthy + end + end + + describe 'when it is not required' do + it 'returns false when not required on group level' do + allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(false) + + expect(subject.two_factor_authentication_required?).to be_falsey + 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 + end + + it 'returns false when current_user does not have temp email' do + allow(user).to receive(:two_factor_enabled?).and_return(false) + allow(user).to receive(:temp_oauth_email?).and_return(true) + + expect(subject.current_user_needs_to_setup_two_factor?).to be_falsey + end + + it 'returns false when current_user has 2fa disabled' do + allow(user).to receive(:temp_oauth_email?).and_return(false) + allow(user).to receive(:two_factor_enabled?).and_return(true) + + expect(subject.current_user_needs_to_setup_two_factor?).to be_falsey + end + + it 'returns true when user requires 2fa authentication' do + allow(user).to receive(:two_factor_enabled?).and_return(false) + allow(user).to receive(:temp_oauth_email?).and_return(false) + + expect(subject.current_user_needs_to_setup_two_factor?).to be_truthy + end + end + + describe '#two_factor_grace_period' do + it 'returns grace period from settings if there is no period from groups' do + stub_application_setting two_factor_grace_period: 2 + allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(false) + + expect(subject.two_factor_grace_period).to eq(2) + end + + it 'returns grace period from groups if there is no period from settings' do + allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(true) + allow(user).to receive(:two_factor_grace_period).and_return(3) + + expect(subject.two_factor_grace_period).to eq(3) + end + + it 'returns minimal grace period if there is grace period from settings and from group' do + allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(true) + allow(user).to receive(:two_factor_grace_period).and_return(3) + stub_application_setting two_factor_grace_period: 2 + + expect(subject.two_factor_grace_period).to eq(2) + end + end + + describe '#two_factor_grace_period_expired?' do + before do + allow(user).to receive(:otp_grace_period_started_at).and_return(4.hours.ago) + end + + it 'returns true if the grace period has expired' do + allow(subject).to receive(:two_factor_grace_period).and_return(2) + + expect(subject.two_factor_grace_period_expired?).to be_truthy + end + + it 'returns false if the grace period has not expired' do + allow(subject).to receive(:two_factor_grace_period).and_return(6) + + expect(subject.two_factor_grace_period_expired?).to be_falsey + end + + context 'when otp_grace_period_started_at is nil' do + it 'returns false' do + allow(user).to receive(:otp_grace_period_started_at).and_return(nil) + + expect(subject.two_factor_grace_period_expired?).to be_falsey + end + end + end +end diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index dcaaa8d4188..b6a8ac31074 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -441,7 +441,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do end end - shared_examples 'deploy token with disabled registry' do + shared_examples 'deploy token with disabled feature' do context 'when registry disabled' do before do stub_container_registry_config(enabled: false) @@ -452,6 +452,15 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do .to eq(auth_failure) end end + + context 'when repository is disabled' do + let(:project) { create(:project, :repository_disabled) } + + it 'fails when login and token are valid' do + expect(gl_auth.find_for_git_client(login, deploy_token.token, project: project, ip: 'ip')) + .to eq(auth_failure) + end + end end context 'when deploy token and user have the same username' do @@ -604,7 +613,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do it_behaves_like 'registry token scope' end - it_behaves_like 'deploy token with disabled registry' + it_behaves_like 'deploy token with disabled feature' end context 'when the deploy token has write_registry as a scope' do @@ -626,7 +635,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do it_behaves_like 'registry token scope' end - it_behaves_like 'deploy token with disabled registry' + it_behaves_like 'deploy token with disabled feature' end end end @@ -682,12 +691,69 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do expect( gl_auth.find_with_user_password(username, password) ).not_to eql user end + it 'does not find user in locked state' do + user.lock_access! + + expect(gl_auth.find_with_user_password(username, password)).not_to eql user + end + it "does not find user in ldap_blocked state" do user.ldap_block expect( gl_auth.find_with_user_password(username, password) ).not_to eql user end + context 'with increment_failed_attempts' do + wrong_password = 'incorrect_password' + + it 'increments failed_attempts when true and password is incorrect' do + expect do + gl_auth.find_with_user_password(username, wrong_password, increment_failed_attempts: true) + user.reload + end.to change(user, :failed_attempts).from(0).to(1) + end + + it 'resets failed_attempts when true and password is correct' do + user.failed_attempts = 2 + user.save + + expect do + gl_auth.find_with_user_password(username, password, increment_failed_attempts: true) + user.reload + end.to change(user, :failed_attempts).from(2).to(0) + end + + it 'does not increment failed_attempts by default' do + expect do + gl_auth.find_with_user_password(username, wrong_password) + user.reload + end.not_to change(user, :failed_attempts) + end + + context 'when the database is read only' do + before do + allow(Gitlab::Database).to receive(:read_only?).and_return(true) + end + + it 'does not increment failed_attempts when true and password is incorrect' do + expect do + gl_auth.find_with_user_password(username, wrong_password, increment_failed_attempts: true) + user.reload + end.not_to change(user, :failed_attempts) + end + + it 'does not reset failed_attempts when true and password is correct' do + user.failed_attempts = 2 + user.save + + expect do + gl_auth.find_with_user_password(username, password, increment_failed_attempts: true) + user.reload + end.not_to change(user, :failed_attempts) + end + end + end + context "with ldap enabled" do before do allow(Gitlab::Auth::Ldap::Config).to receive(:enabled?).and_return(true) diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 491437856d4..8961cdcae7d 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -192,7 +192,7 @@ RSpec.describe Gitlab::Git::Commit, :seed_helper do end describe '.find with Gitaly enabled' do - it_should_behave_like '.find' + it_behaves_like '.find' end describe '.find with Rugged enabled', :enable_rugged do @@ -204,7 +204,7 @@ RSpec.describe Gitlab::Git::Commit, :seed_helper do described_class.find(repository, SeedRepo::Commit::ID) end - it_should_behave_like '.find' + it_behaves_like '.find' end describe '.last_for_path' do @@ -474,7 +474,7 @@ RSpec.describe Gitlab::Git::Commit, :seed_helper do end describe '.batch_by_oid with Gitaly enabled' do - it_should_behave_like '.batch_by_oid' + it_behaves_like '.batch_by_oid' context 'when oids is empty' do it 'makes no Gitaly request' do @@ -486,7 +486,7 @@ RSpec.describe Gitlab::Git::Commit, :seed_helper do end describe '.batch_by_oid with Rugged enabled', :enable_rugged do - it_should_behave_like '.batch_by_oid' + it_behaves_like '.batch_by_oid' it 'calls out to the Rugged implementation' do allow_next_instance_of(Rugged) do |instance| diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 8153886a2ab..85567ab2e55 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -472,31 +472,135 @@ RSpec.describe Gitlab::GitAccess do let(:actor) { key } context 'pull code' do - context 'when project is authorized' do - before do - key.projects << project + context 'when project is public' do + let(:project) { create(:project, :public, :repository, *options) } + + context 'when deploy key exists in the project' do + before do + key.projects << project + end + + context 'when the repository is public' do + let(:options) { %i[repository_enabled] } + + it { expect { pull_access_check }.not_to raise_error } + end + + context 'when the repository is private' do + let(:options) { %i[repository_private] } + + it { expect { pull_access_check }.not_to raise_error } + end + + context 'when the repository is disabled' do + let(:options) { %i[repository_disabled] } + + it { expect { pull_access_check }.to raise_error('You are not allowed to download code from this project.') } + end end - it { expect { pull_access_check }.not_to raise_error } + context 'when deploy key does not exist in the project' do + context 'when the repository is public' do + let(:options) { %i[repository_enabled] } + + it { expect { pull_access_check }.not_to raise_error } + end + + context 'when the repository is private' do + let(:options) { %i[repository_private] } + + it { expect { pull_access_check }.to raise_error('You are not allowed to download code from this project.') } + end + + context 'when the repository is disabled' do + let(:options) { %i[repository_disabled] } + + it { expect { pull_access_check }.to raise_error('You are not allowed to download code from this project.') } + end + end end - context 'when unauthorized' do - context 'from public project' do - let(:project) { create(:project, :public, :repository) } + context 'when project is internal' do + let(:project) { create(:project, :internal, :repository, *options) } - it { expect { pull_access_check }.not_to raise_error } + context 'when deploy key exists in the project' do + before do + key.projects << project + end + + context 'when the repository is public' do + let(:options) { %i[repository_enabled] } + + it { expect { pull_access_check }.not_to raise_error } + end + + context 'when the repository is private' do + let(:options) { %i[repository_private] } + + it { expect { pull_access_check }.not_to raise_error } + end + + context 'when the repository is disabled' do + let(:options) { %i[repository_disabled] } + + it { expect { pull_access_check }.to raise_error('You are not allowed to download code from this project.') } + end end - context 'from internal project' do - let(:project) { create(:project, :internal, :repository) } + context 'when deploy key does not exist in the project' do + context 'when the repository is public' do + let(:options) { %i[repository_enabled] } - it { expect { pull_access_check }.to raise_not_found } + it { expect { pull_access_check }.to raise_error('The project you were looking for could not be found.') } + end + + context 'when the repository is private' do + let(:options) { %i[repository_private] } + + it { expect { pull_access_check }.to raise_error('The project you were looking for could not be found.') } + end + + context 'when the repository is disabled' do + let(:options) { %i[repository_disabled] } + + it { expect { pull_access_check }.to raise_error('The project you were looking for could not be found.') } + end end + end - context 'from private project' do - let(:project) { create(:project, :private, :repository) } + context 'when project is private' do + let(:project) { create(:project, :private, :repository, *options) } - it { expect { pull_access_check }.to raise_not_found } + context 'when deploy key exists in the project' do + before do + key.projects << project + end + + context 'when the repository is private' do + let(:options) { %i[repository_private] } + + it { expect { pull_access_check }.not_to raise_error } + end + + context 'when the repository is disabled' do + let(:options) { %i[repository_disabled] } + + it { expect { pull_access_check }.to raise_error('You are not allowed to download code from this project.') } + end + end + + context 'when deploy key does not exist in the project' do + context 'when the repository is private' do + let(:options) { %i[repository_private] } + + it { expect { pull_access_check }.to raise_error('The project you were looking for could not be found.') } + end + + context 'when the repository is disabled' do + let(:options) { %i[repository_disabled] } + + it { expect { pull_access_check }.to raise_error('The project you were looking for could not be found.') } + end end end end diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index f736f19ca9a..88c3315150b 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -195,15 +195,6 @@ RSpec.describe Gitlab::Regex do it { is_expected.not_to match('foo/bar') } end - describe '.conan_file_name_regex' do - subject { described_class.conan_file_name_regex } - - it { is_expected.to match('conanfile.py') } - it { is_expected.to match('conan_package.tgz') } - it { is_expected.not_to match('foo.txt') } - it { is_expected.not_to match('!!()()') } - end - describe '.conan_package_reference_regex' do subject { described_class.conan_package_reference_regex } diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index cac2c410132..e9733851590 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -71,7 +71,7 @@ RSpec.describe Gitlab::Workhorse do context "when the repository doesn't have an archive file path" do before do - allow(project.repository).to receive(:archive_metadata).and_return(Hash.new) + allow(project.repository).to receive(:archive_metadata).and_return({}) end it "raises an error" do diff --git a/spec/migrations/20200728182311_add_o_auth_paths_to_protected_paths_spec.rb b/spec/migrations/20200728182311_add_o_auth_paths_to_protected_paths_spec.rb new file mode 100644 index 00000000000..e12519e15b8 --- /dev/null +++ b/spec/migrations/20200728182311_add_o_auth_paths_to_protected_paths_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20200728182311_add_o_auth_paths_to_protected_paths.rb') + +RSpec.describe AddOAuthPathsToProtectedPaths do + subject(:migration) { described_class.new } + + let(:application_settings) { table(:application_settings) } + let(:new_paths) do + [ + '/oauth/authorize', + '/oauth/token' + ] + end + + it 'appends new OAuth paths' do + application_settings.create! + + protected_paths_before = application_settings.first.protected_paths + protected_paths_after = protected_paths_before + new_paths + + expect { migrate! }.to change { application_settings.first.protected_paths }.from(protected_paths_before).to(protected_paths_after) + end + + it 'new default includes new paths' do + settings_before = application_settings.create! + + expect(settings_before.protected_paths).not_to include(*new_paths) + + migrate! + + application_settings.reset_column_information + settings_after = application_settings.create! + + expect(settings_after.protected_paths).to include(*new_paths) + end + + it 'does not change the value when the new paths are already included' do + application_settings.create!(protected_paths: %w(/users/sign_in /users/password) + new_paths) + + expect { migrate! }.not_to change { application_settings.first.protected_paths } + end + + it 'adds one value when the other is already present' do + application_settings.create!(protected_paths: %W(/users/sign_in /users/password #{new_paths.first})) + + migrate! + + expect(application_settings.first.protected_paths).to include(new_paths.second) + end +end diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb index 24b47be3c69..de39c8c7c5c 100644 --- a/spec/models/active_session_spec.rb +++ b/spec/models/active_session_spec.rb @@ -296,6 +296,59 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do end end + describe '.destroy_all_but_current' do + it 'gracefully handles a nil session ID' do + expect(described_class).not_to receive(:destroy_sessions) + + ActiveSession.destroy_all_but_current(user, nil) + end + + context 'with user sessions' do + let(:current_session_id) { '6919a6f1bb119dd7396fadc38fd18d0d' } + + before do + Gitlab::Redis::SharedState.with do |redis| + redis.set(described_class.key_name(user.id, current_session_id), + Marshal.dump(ActiveSession.new(session_id: Rack::Session::SessionId.new(current_session_id)))) + redis.set(described_class.key_name(user.id, '59822c7d9fcdfa03725eff41782ad97d'), + Marshal.dump(ActiveSession.new(session_id: Rack::Session::SessionId.new('59822c7d9fcdfa03725eff41782ad97d')))) + redis.set(described_class.key_name(9999, '5c8611e4f9c69645ad1a1492f4131358'), + Marshal.dump(ActiveSession.new(session_id: Rack::Session::SessionId.new('5c8611e4f9c69645ad1a1492f4131358')))) + redis.sadd(described_class.lookup_key_name(user.id), '59822c7d9fcdfa03725eff41782ad97d') + redis.sadd(described_class.lookup_key_name(user.id), current_session_id) + redis.sadd(described_class.lookup_key_name(9999), '5c8611e4f9c69645ad1a1492f4131358') + end + end + + it 'removes the entry associated with the all user sessions but current' do + expect { ActiveSession.destroy_all_but_current(user, request.session) }.to change { ActiveSession.session_ids_for_user(user.id).size }.from(2).to(1) + + expect(ActiveSession.session_ids_for_user(9999).size).to eq(1) + end + + it 'removes the lookup entry of deleted sessions' do + ActiveSession.destroy_all_but_current(user, request.session) + + Gitlab::Redis::SharedState.with do |redis| + expect(redis.smembers(described_class.lookup_key_name(user.id))).to eq [current_session_id] + end + end + + it 'does not remove impersonated sessions' do + impersonated_session_id = '6919a6f1bb119dd7396fadc38fd18eee' + Gitlab::Redis::SharedState.with do |redis| + redis.set(described_class.key_name(user.id, impersonated_session_id), + Marshal.dump(ActiveSession.new(session_id: Rack::Session::SessionId.new(impersonated_session_id), is_impersonated: true))) + redis.sadd(described_class.lookup_key_name(user.id), impersonated_session_id) + end + + expect { ActiveSession.destroy_all_but_current(user, request.session) }.to change { ActiveSession.session_ids_for_user(user.id).size }.from(3).to(2) + + expect(ActiveSession.session_ids_for_user(9999).size).to eq(1) + end + end + end + describe '.cleanup' do before do stub_const("ActiveSession::ALLOWED_NUMBER_OF_ACTIVE_SESSIONS", 5) diff --git a/spec/models/aws/role_spec.rb b/spec/models/aws/role_spec.rb index 612868f6eb0..ee93c9d6fad 100644 --- a/spec/models/aws/role_spec.rb +++ b/spec/models/aws/role_spec.rb @@ -29,6 +29,12 @@ RSpec.describe Aws::Role do it { is_expected.to be_truthy } end + + context 'ARN is nil' do + let(:role_arn) { } + + it { is_expected.to be_truthy } + end end end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 8da164e041c..b83c769284a 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -195,6 +195,19 @@ RSpec.describe Member do it { expect(described_class.non_request).to include @accepted_request_member } end + describe '.not_accepted_invitations_by_user' do + let(:invited_by_user) { create(:project_member, :invited, project: project, created_by: @owner_user) } + + before do + create(:project_member, :invited, invite_email: 'test@test.com', project: project, created_by: @owner_user, invite_accepted_at: Time.zone.now) + create(:project_member, :invited, invite_email: 'test2@test.com', project: project, created_by: @maintainer_user) + end + + subject { described_class.not_accepted_invitations_by_user(@owner_user) } + + it { is_expected.to contain_exactly(invited_by_user) } + end + describe '.search_invite_email' do it 'returns only members the matching e-mail' do create(:group_member, :invited) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 98f1c5bb2cc..74a36daa727 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -895,6 +895,20 @@ RSpec.describe User do expect(described_class.without_ghosts).to match_array([user1, user2]) end end + + describe '.by_id_and_login' do + let_it_be(:user) { create(:user) } + + it 'finds a user regardless of case' do + expect(described_class.by_id_and_login(user.id, user.username.upcase)) + .to contain_exactly(user) + end + + it 'finds a user when login is an email address regardless of case' do + expect(described_class.by_id_and_login(user.id, user.email.upcase)) + .to contain_exactly(user) + end + end end describe "Respond to" do @@ -3580,6 +3594,42 @@ RSpec.describe User do end end + describe '#source_groups_of_two_factor_authentication_requirement' do + let_it_be(:group_not_requiring_2FA) { create :group } + let(:user) { create :user } + + before do + group.add_user(user, GroupMember::OWNER) + group_not_requiring_2FA.add_user(user, GroupMember::OWNER) + end + + context 'when user is direct member of group requiring 2FA' do + let_it_be(:group) { create :group, require_two_factor_authentication: true } + + it 'returns group requiring 2FA' do + expect(user.source_groups_of_two_factor_authentication_requirement).to contain_exactly(group) + end + end + + context 'when user is member of group which parent requires 2FA' do + let_it_be(:parent_group) { create :group, require_two_factor_authentication: true } + let_it_be(:group) { create :group, parent: parent_group } + + it 'returns group requiring 2FA' do + expect(user.source_groups_of_two_factor_authentication_requirement).to contain_exactly(group) + end + end + + context 'when user is member of group which child requires 2FA' do + let_it_be(:group) { create :group } + let_it_be(:child_group) { create :group, require_two_factor_authentication: true, parent: group } + + it 'returns group requiring 2FA' do + expect(user.source_groups_of_two_factor_authentication_requirement).to contain_exactly(group) + end + end + end + describe '.active' do before do described_class.ghost diff --git a/spec/requests/api/badges_spec.rb b/spec/requests/api/badges_spec.rb index 99d224cb8e9..d8a345a79b0 100644 --- a/spec/requests/api/badges_spec.rb +++ b/spec/requests/api/badges_spec.rb @@ -332,10 +332,32 @@ RSpec.describe API::Badges do context 'when deleting a badge' do context 'and the source is a project' do + let(:badge) { project.group.badges.first } + it 'cannot delete badges owned by the project group' do - delete api("/projects/#{project.id}/badges/#{project_group.badges.first.id}", maintainer) + expect do + delete api("/projects/#{project.id}/badges/#{badge.id}", maintainer) + + expect(response).to have_gitlab_http_status(:not_found) + end.not_to change { badge.reload.persisted? } + end + end + end + + context 'when updating a badge' do + context 'and the source is a project' do + let(:badge) { project.group.badges.first } + let(:example_name) { 'BadgeName' } + let(:example_url) { 'http://www.example.com' } + let(:example_url2) { 'http://www.example1.com' } + + it 'cannot update badges owned by the project group' do + expect do + put api("/projects/#{project.id}/badges/#{badge.id}", maintainer), + params: { name: example_name, link_url: example_url, image_url: example_url2 } - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:not_found) + end.not_to change { badge.reload.updated_at } end end end diff --git a/spec/requests/api/conan_packages_spec.rb b/spec/requests/api/conan_packages_spec.rb index 58c360d055b..d1ddaa277fc 100644 --- a/spec/requests/api/conan_packages_spec.rb +++ b/spec/requests/api/conan_packages_spec.rb @@ -14,7 +14,7 @@ RSpec.describe API::ConanPackages do let(:base_secret) { SecureRandom.base64(64) } let(:auth_token) { personal_access_token.token } - let(:job) { create(:ci_build, user: user) } + let(:job) { create(:ci_build, user: user, status: :running) } let(:job_token) { job.token } let(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } let(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } @@ -94,6 +94,14 @@ RSpec.describe API::ConanPackages do expect(response).to have_gitlab_http_status(:unauthorized) end + it 'responds with 401 Unauthorized when the job is not running' do + job.update!(status: :failed) + jwt = build_jwt_from_job(job) + get api('/packages/conan/v1/ping'), headers: build_token_auth_header(jwt.encoded) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + context 'packages feature disabled' do it 'responds with 404 Not Found' do stub_packages_setting(enabled: false) @@ -234,6 +242,18 @@ RSpec.describe API::ConanPackages do end end + shared_examples 'rejects invalid file_name' do |invalid_file_name| + let(:file_name) { invalid_file_name } + + context 'with invalid file_name' do + it 'returns 400' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end + shared_examples 'rejects recipe for invalid project' do context 'with invalid recipe path' do let(:recipe_path) { 'aa/bb/not-existing-project/ccc' } @@ -717,8 +737,6 @@ RSpec.describe API::ConanPackages do context 'without a file from workhorse' do let(:params) { { file: nil } } - it_behaves_like 'package workhorse uploads' - it 'rejects the request' do subject @@ -726,6 +744,10 @@ RSpec.describe API::ConanPackages do end end + context 'with a file' do + it_behaves_like 'package workhorse uploads' + end + context 'without a token' do it 'rejects request without a token' do headers_with_token.delete('HTTP_AUTHORIZATION') @@ -884,16 +906,22 @@ RSpec.describe API::ConanPackages do end describe 'PUT /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:file_name/authorize' do - subject { put api("/packages/conan/v1/files/#{recipe_path}/0/export/conanfile.py/authorize"), headers: headers_with_token } + let(:file_name) { 'conanfile.py' } + + subject { put api("/packages/conan/v1/files/#{recipe_path}/0/export/#{file_name}/authorize"), headers: headers_with_token } it_behaves_like 'rejects invalid recipe' + it_behaves_like 'rejects invalid file_name', 'conanfile.py.git%2fgit-upload-pack' it_behaves_like 'workhorse authorization' end describe 'PUT /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:conan_package_reference/:package_revision/:file_name/authorize' do - subject { put api("/packages/conan/v1/files/#{recipe_path}/0/package/123456789/0/conaninfo.txt/authorize"), headers: headers_with_token } + let(:file_name) { 'conaninfo.txt' } + + subject { put api("/packages/conan/v1/files/#{recipe_path}/0/package/123456789/0/#{file_name}/authorize"), headers: headers_with_token } it_behaves_like 'rejects invalid recipe' + it_behaves_like 'rejects invalid file_name', 'conaninfo.txttest' it_behaves_like 'workhorse authorization' end @@ -907,11 +935,13 @@ RSpec.describe API::ConanPackages do method: :put, file_key: :file, params: params, + send_rewritten_field: true, headers: headers_with_token ) end it_behaves_like 'rejects invalid recipe' + it_behaves_like 'rejects invalid file_name', 'conanfile.py.git%2fgit-upload-pack' it_behaves_like 'uploads a package file' end @@ -925,12 +955,15 @@ RSpec.describe API::ConanPackages do method: :put, file_key: :file, params: params, - headers: headers_with_token + headers: headers_with_token, + send_rewritten_field: true ) end it_behaves_like 'rejects invalid recipe' + it_behaves_like 'rejects invalid file_name', 'conaninfo.txttest' it_behaves_like 'uploads a package file' + context 'tracking the conan_package.tgz upload' do let(:file_name) { ::Packages::Conan::FileMetadatum::PACKAGE_BINARY } diff --git a/spec/requests/api/generic_packages_spec.rb b/spec/requests/api/generic_packages_spec.rb index e08637629cc..ed852fe75c7 100644 --- a/spec/requests/api/generic_packages_spec.rb +++ b/spec/requests/api/generic_packages_spec.rb @@ -55,7 +55,7 @@ RSpec.describe API::GenericPackages do context 'authenticating using job token' do it 'responds with 200 OK when valid job token is provided' do - job_token = create(:ci_build, user: user).token + job_token = create(:ci_build, :running, user: user).token ping(job_token: job_token) diff --git a/spec/requests/api/go_proxy_spec.rb b/spec/requests/api/go_proxy_spec.rb index 2d7e319b0be..9d422ebbce2 100644 --- a/spec/requests/api/go_proxy_spec.rb +++ b/spec/requests/api/go_proxy_spec.rb @@ -11,7 +11,7 @@ RSpec.describe API::GoProxy do let_it_be(:base) { "#{Settings.build_gitlab_go_url}/#{project.full_path}" } let_it_be(:oauth) { create :oauth_access_token, scopes: 'api', resource_owner: user } - let_it_be(:job) { create :ci_build, user: user } + let_it_be(:job) { create :ci_build, user: user, status: :running } let_it_be(:pa_token) { create :personal_access_token, user: user } let_it_be(:modules) do @@ -393,6 +393,13 @@ RSpec.describe API::GoProxy do expect(response).to have_gitlab_http_status(:ok) end + it 'returns unauthorized with a failed job token' do + job.update!(status: :failed) + get_resource(oauth_access_token: job) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + it 'returns unauthorized with no authentication' do get_resource diff --git a/spec/requests/api/graphql/mutations/boards/lists/create_spec.rb b/spec/requests/api/graphql/mutations/boards/lists/create_spec.rb new file mode 100644 index 00000000000..328f4fb7b6e --- /dev/null +++ b/spec/requests/api/graphql/mutations/boards/lists/create_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Create a label or backlog board list' do + include GraphqlHelpers + + let_it_be(:group) { create(:group, :private) } + let_it_be(:board) { create(:board, group: group) } + let_it_be(:user) { create(:user) } + let_it_be(:dev_label) do + create(:group_label, title: 'Development', color: '#FFAABB', group: group) + end + + let(:current_user) { user } + let(:mutation) { graphql_mutation(:board_list_create, input) } + let(:mutation_response) { graphql_mutation_response(:board_list_create) } + + context 'the user is not allowed to read board lists' do + let(:input) { { board_id: board.to_global_id.to_s, backlog: true } } + + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when user has permissions to admin board lists' do + before do + group.add_reporter(current_user) + end + + describe 'backlog list' do + let(:input) { { board_id: board.to_global_id.to_s, backlog: true } } + + it 'creates the list' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['list']) + .to include('position' => nil, 'listType' => 'backlog') + end + end + + describe 'label list' do + let(:input) { { board_id: board.to_global_id.to_s, label_id: dev_label.to_global_id.to_s } } + + it 'creates the list' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['list']) + .to include('position' => 0, 'listType' => 'label', 'label' => include('title' => 'Development')) + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/snippets/destroy_spec.rb b/spec/requests/api/graphql/mutations/snippets/destroy_spec.rb index 8ade72635af..b71f87d2702 100644 --- a/spec/requests/api/graphql/mutations/snippets/destroy_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/destroy_spec.rb @@ -46,6 +46,31 @@ RSpec.describe 'Destroying a Snippet' do expect(mutation_response).to have_key('snippet') expect(mutation_response['snippet']).to be_nil end + + context 'when a bad gid is given' do + let!(:project) { create(:project, :private) } + let!(:snippet) { create(:project_snippet, :private, project: project, author: create(:user)) } + let!(:snippet_gid) { project.to_gid.to_s } + + it 'returns an error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(graphql_errors) + .to include(a_hash_including('message' => "#{snippet_gid} is not a valid ID for Snippet.")) + end + + it 'does not destroy the Snippet' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { Snippet.count } + end + + it 'does not destroy the Project' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { Project.count } + end + end end end diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 090ff3c1b5a..9c0ea14e3e3 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -85,6 +85,27 @@ RSpec.describe API::Helpers do end it { is_expected.to eq(user) } + + context 'when user should have 2fa enabled' do + before do + allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(true) + allow_next_instance_of(Gitlab::Auth::TwoFactorAuthVerifier) do |verifier| + allow(verifier).to receive(:two_factor_grace_period_expired?).and_return(true) + end + end + + context 'when 2fa is not enabled' do + it { is_expected.to be_nil } + end + + context 'when 2fa is enabled' do + before do + allow(user).to receive(:two_factor_enabled?).and_return(true) + end + + it { is_expected.to eq(user) } + end + end end context "PUT request" do diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 6555a055678..f90e04bbd2e 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -56,7 +56,7 @@ RSpec.describe API::Jobs do end describe 'GET /projects/:id/jobs' do - let(:query) { Hash.new } + let(:query) { {} } before do |example| unless example.metadata[:skip_before_request] @@ -167,7 +167,7 @@ RSpec.describe API::Jobs do end describe 'GET /projects/:id/pipelines/:pipeline_id/jobs' do - let(:query) { Hash.new } + let(:query) { {} } before do |example| unless example.metadata[:skip_before_request] @@ -296,7 +296,7 @@ RSpec.describe API::Jobs do project: downstream_pipeline.project) end - let(:query) { Hash.new } + let(:query) { {} } before do |example| unless example.metadata[:skip_before_request] diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb index 99588694bab..04d1d2cf1e8 100644 --- a/spec/requests/api/maven_packages_spec.rb +++ b/spec/requests/api/maven_packages_spec.rb @@ -12,7 +12,7 @@ RSpec.describe API::MavenPackages do let_it_be(:package_file) { package.package_files.with_file_name_like('%.xml').first } let_it_be(:jar_file) { package.package_files.with_file_name_like('%.jar').first } let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } - let_it_be(:job) { create(:ci_build, user: user) } + let_it_be(:job, reload: true) { create(:ci_build, user: user, status: :running) } let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } @@ -102,11 +102,25 @@ RSpec.describe API::MavenPackages do end shared_examples 'downloads with a job token' do - it 'allows download with job token' do - download_file(package_file.file_name, job_token: job.token) + context 'with a running job' do + it 'allows download with job token' do + download_file(package_file.file_name, job_token: job.token) - expect(response).to have_gitlab_http_status(:ok) - expect(response.media_type).to eq('application/octet-stream') + expect(response).to have_gitlab_http_status(:ok) + expect(response.media_type).to eq('application/octet-stream') + end + end + + context 'with a finished job' do + before do + job.update!(status: :failed) + end + + it 'returns unauthorized error' do + download_file(package_file.file_name, job_token: job.token) + + expect(response).to have_gitlab_http_status(:unauthorized) + end end end @@ -569,13 +583,20 @@ RSpec.describe API::MavenPackages do expect(jar_file.file_name).to eq(file_upload.original_filename) end - it 'allows upload with job token' do + it 'allows upload with running job token' do upload_file(params.merge(job_token: job.token)) expect(response).to have_gitlab_http_status(:ok) expect(project.reload.packages.last.build_info.pipeline).to eq job.pipeline end + it 'rejects upload without running job token' do + job.update!(status: :failed) + upload_file(params.merge(job_token: job.token)) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + it 'allows upload with deploy token' do upload_file(params, headers_with_deploy_token) diff --git a/spec/requests/api/npm_packages_spec.rb b/spec/requests/api/npm_packages_spec.rb index 94647123df0..108ea84b7e6 100644 --- a/spec/requests/api/npm_packages_spec.rb +++ b/spec/requests/api/npm_packages_spec.rb @@ -12,7 +12,7 @@ RSpec.describe API::NpmPackages do let_it_be(:package, reload: true) { create(:npm_package, project: project) } let_it_be(:token) { create(:oauth_access_token, scopes: 'api', resource_owner: user) } let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } - let_it_be(:job) { create(:ci_build, user: user) } + let_it_be(:job, reload: true) { create(:ci_build, user: user, status: :running) } let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } @@ -27,12 +27,19 @@ RSpec.describe API::NpmPackages do expect_a_valid_package_response end - it 'returns the package info with job token' do + it 'returns the package info with running job token' do get_package_with_job_token(package) expect_a_valid_package_response end + it 'denies request without running job token' do + job.update!(status: :success) + get_package_with_job_token(package) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + it 'denies request without oauth token' do get_package(package) diff --git a/spec/requests/api/nuget_packages_spec.rb b/spec/requests/api/nuget_packages_spec.rb index 37170592b60..62f244c433b 100644 --- a/spec/requests/api/nuget_packages_spec.rb +++ b/spec/requests/api/nuget_packages_spec.rb @@ -80,7 +80,7 @@ RSpec.describe API::NugetPackages do end with_them do - let(:job) { user_token ? create(:ci_build, project: project, user: user) : double(token: 'wrong') } + let(:job) { user_token ? create(:ci_build, project: project, user: user, status: :running) : double(token: 'wrong') } let(:headers) { user_role == :anonymous ? {} : job_basic_auth_header(job) } subject { get api(url), headers: headers } diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index a9a92a4d3cd..779ae983886 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -671,12 +671,20 @@ RSpec.describe API::Releases do end context 'when a valid token is provided' do - it 'creates the release' do + it 'creates the release for a running job' do + job.update!(status: :running) post api("/projects/#{project.id}/releases"), params: params.merge(job_token: job.token) expect(response).to have_gitlab_http_status(:created) expect(project.releases.last.description).to eq('Another nice release') end + + it 'returns an :unauthorized error for a completed job' do + job.success! + post api("/projects/#{project.id}/releases"), params: params.merge(job_token: job.token) + + expect(response).to have_gitlab_http_status(:unauthorized) + end end end diff --git a/spec/requests/api/terraform/state_spec.rb b/spec/requests/api/terraform/state_spec.rb index c6cba39314b..c47a12456c3 100644 --- a/spec/requests/api/terraform/state_spec.rb +++ b/spec/requests/api/terraform/state_spec.rb @@ -72,7 +72,7 @@ RSpec.describe API::Terraform::State do let(:auth_header) { job_basic_auth_header(job) } context 'with maintainer permissions' do - let(:job) { create(:ci_build, project: project, user: maintainer) } + let(:job) { create(:ci_build, status: :running, project: project, user: maintainer) } it 'returns terraform state belonging to a project of given state name' do request @@ -81,6 +81,13 @@ RSpec.describe API::Terraform::State do expect(response.body).to eq(state.file.read) end + it 'returns unauthorized if the the job is not running' do + job.update!(status: :failed) + request + + expect(response).to have_gitlab_http_status(:unauthorized) + end + context 'for a project that does not exist' do let(:project_id) { '0000' } @@ -93,7 +100,7 @@ RSpec.describe API::Terraform::State do end context 'with developer permissions' do - let(:job) { create(:ci_build, project: project, user: developer) } + let(:job) { create(:ci_build, status: :running, project: project, user: developer) } it 'returns terraform state belonging to a project of given state name' do request diff --git a/spec/services/applications/create_service_spec.rb b/spec/services/applications/create_service_spec.rb index 58ac723ee55..8b8beb057a9 100644 --- a/spec/services/applications/create_service_spec.rb +++ b/spec/services/applications/create_service_spec.rb @@ -6,9 +6,24 @@ RSpec.describe ::Applications::CreateService do include TestRequestHelpers let(:user) { create(:user) } - let(:params) { attributes_for(:application) } subject { described_class.new(user, params) } - it { expect { subject.execute(test_request) }.to change { Doorkeeper::Application.count }.by(1) } + context 'when scopes are present' do + let(:params) { attributes_for(:application, scopes: ['read_user']) } + + it { expect { subject.execute(test_request) }.to change { Doorkeeper::Application.count }.by(1) } + end + + context 'when scopes are missing' do + let(:params) { attributes_for(:application) } + + it { expect { subject.execute(test_request) }.not_to change { Doorkeeper::Application.count } } + + it 'includes blank scopes error message' do + application = subject.execute(test_request) + + expect(application.errors.full_messages).to include "Scopes can't be blank" + end + end end diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index 8d58c4b27e1..bc85f4f0087 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -654,6 +654,19 @@ RSpec.describe Auth::ContainerRegistryAuthenticationService do it_behaves_like 'not a container repository factory' end end + + context 'for project that disables repository' do + let(:project) { create(:project, :public, :repository_disabled) } + + context 'disallow when pulling' do + let(:current_params) do + { scopes: ["repository:#{project.full_path}:pull"] } + end + + it_behaves_like 'an inaccessible' + it_behaves_like 'not a container repository factory' + end + end end context 'registry catalog browsing authorized as admin' do diff --git a/spec/services/branches/delete_service_spec.rb b/spec/services/branches/delete_service_spec.rb index f1e7c9340b1..291431c1723 100644 --- a/spec/services/branches/delete_service_spec.rb +++ b/spec/services/branches/delete_service_spec.rb @@ -37,6 +37,21 @@ RSpec.describe Branches::DeleteService do end it_behaves_like 'a deleted branch', 'feature' + + context 'when Gitlab::Git::CommandError is raised' do + before do + allow(repository).to receive(:rm_branch) do + raise Gitlab::Git::CommandError.new('Could not update patch') + end + end + + it 'handles and returns error' do + result = service.execute('feature') + + expect(result.status).to eq(:error) + expect(result.message).to eq('Could not update patch') + end + end end context 'when user does not have access to push to repository' do diff --git a/spec/services/ci/pipeline_trigger_service_spec.rb b/spec/services/ci/pipeline_trigger_service_spec.rb index 18fab9623ec..ac077e3c30e 100644 --- a/spec/services/ci/pipeline_trigger_service_spec.rb +++ b/spec/services/ci/pipeline_trigger_service_spec.rb @@ -106,9 +106,23 @@ RSpec.describe Ci::PipelineTriggerService do let(:params) { { token: job.token, ref: 'master', variables: nil } } let(:job) { create(:ci_build, :success, pipeline: pipeline, user: user) } - it 'does nothing' do + it 'does nothing', :aggregate_failures do + expect { result }.not_to change { Ci::Pipeline.count } + expect(result[:message]).to eq('Job is not running') + expect(result[:http_status]).to eq(401) + end + end + + context 'when job does not have a project' do + let(:params) { { token: job.token, ref: 'master', variables: nil } } + let(:job) { create(:ci_build, status: :running, pipeline: pipeline, user: user) } + + it 'does nothing', :aggregate_failures do + job.update!(project: nil) + expect { result }.not_to change { Ci::Pipeline.count } - expect(result[:message]).to eq('400 Job has to be running') + expect(result[:message]).to eq('Project has been deleted!') + expect(result[:http_status]).to eq(401) end end diff --git a/spec/services/clusters/aws/authorize_role_service_spec.rb b/spec/services/clusters/aws/authorize_role_service_spec.rb index 3d12400a47b..5b47cf0ecde 100644 --- a/spec/services/clusters/aws/authorize_role_service_spec.rb +++ b/spec/services/clusters/aws/authorize_role_service_spec.rb @@ -3,47 +3,34 @@ require 'spec_helper' RSpec.describe Clusters::Aws::AuthorizeRoleService do - let(:user) { create(:user) } + subject { described_class.new(user, params: params).execute } + + let(:role) { create(:aws_role) } + let(:user) { role.user } let(:credentials) { instance_double(Aws::Credentials) } let(:credentials_service) { instance_double(Clusters::Aws::FetchCredentialsService, execute: credentials) } + let(:role_arn) { 'arn:my-role' } let(:params) do params = ActionController::Parameters.new({ cluster: { - role_arn: 'arn:my-role', - role_external_id: 'external-id' + role_arn: role_arn } }) - params.require(:cluster).permit(:role_arn, :role_external_id) + params.require(:cluster).permit(:role_arn) end - subject { described_class.new(user, params: params).execute } - before do allow(Clusters::Aws::FetchCredentialsService).to receive(:new) .with(instance_of(Aws::Role)).and_return(credentials_service) end - context 'role does not exist' do - it 'creates an Aws::Role record and returns a set of credentials' do - expect(user).to receive(:create_aws_role!) - .with(params).and_call_original - - expect(subject.status).to eq(:ok) - expect(subject.body).to eq(credentials) - end - end - - context 'role already exists' do - let(:role) { create(:aws_role, user: user) } - + context 'role exists' do it 'updates the existing Aws::Role record and returns a set of credentials' do - expect(role).to receive(:update!) - .with(params).and_call_original - expect(subject.status).to eq(:ok) expect(subject.body).to eq(credentials) + expect(role.reload.role_arn).to eq(role_arn) end end @@ -61,11 +48,14 @@ RSpec.describe Clusters::Aws::AuthorizeRoleService do end end - context 'cannot create role' do - before do - allow(user).to receive(:create_aws_role!) - .and_raise(ActiveRecord::RecordInvalid.new(user)) - end + context 'role does not exist' do + let(:user) { create(:user) } + + include_examples 'bad request' + end + + context 'supplied ARN is invalid' do + let(:role_arn) { 'invalid' } include_examples 'bad request' end diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb index 13e7b4c1006..5c90f1f54ea 100644 --- a/spec/services/members/destroy_service_spec.rb +++ b/spec/services/members/destroy_service_spec.rb @@ -292,6 +292,10 @@ RSpec.describe Members::DestroyService do before do create(:group_member, :developer, group: subsubgroup, user: member_user) + create(:project_member, :invited, project: group_project, created_by: member_user) + create(:group_member, :invited, group: group, created_by: member_user) + create(:project_member, :invited, project: subsubproject, created_by: member_user) + create(:group_member, :invited, group: subgroup, created_by: member_user) subsubproject.add_developer(member_user) control_project.add_maintainer(user) @@ -325,5 +329,41 @@ RSpec.describe Members::DestroyService do it 'does not remove the user from the control project' do expect(control_project.members.map(&:user)).to include(user) end + + it 'removes group members invited by deleted user' do + expect(group.members.not_accepted_invitations_by_user(member_user)).to be_empty + end + + it 'removes project members invited by deleted user' do + expect(group_project.members.not_accepted_invitations_by_user(member_user)).to be_empty + end + + it 'removes subgroup members invited by deleted user' do + expect(subgroup.members.not_accepted_invitations_by_user(member_user)).to be_empty + end + + it 'removes subproject members invited by deleted user' do + expect(subsubproject.members.not_accepted_invitations_by_user(member_user)).to be_empty + end + end + + context 'deletion of invitations created by deleted project member' do + let(:user) { project.owner } + let(:member_user) { create(:user) } + let(:opts) { {} } + + let(:project) { create(:project) } + + before do + create(:project_member, :invited, project: project, created_by: member_user) + + project_member = create(:project_member, :maintainer, user: member_user, project: project) + + described_class.new(user).execute(project_member, opts) + end + + it 'removes project members invited by deleted user' do + expect(project.members.not_accepted_invitations_by_user(member_user)).to be_empty + end end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 47bc6bcd989..87d6f99e8aa 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -151,9 +151,9 @@ RSpec.describe NotificationService, :mailer do end shared_examples_for 'participating notifications' do - it_should_behave_like 'participating by note notification' - it_should_behave_like 'participating by author notification' - it_should_behave_like 'participating by assignee notification' + it_behaves_like 'participating by note notification' + it_behaves_like 'participating by author notification' + it_behaves_like 'participating by assignee notification' end describe '#async' do @@ -1682,13 +1682,13 @@ RSpec.describe NotificationService, :mailer do end context 'participating' do - it_should_behave_like 'participating by assignee notification' do + it_behaves_like 'participating by assignee notification' do let(:participant) { create(:user, username: 'user-participant')} let(:issuable) { merge_request } let(:notification_trigger) { notification.new_merge_request(merge_request, @u_disabled) } end - it_should_behave_like 'participating by note notification' do + it_behaves_like 'participating by note notification' do let(:participant) { create(:user, username: 'user-participant')} let(:issuable) { merge_request } let(:notification_trigger) { notification.new_merge_request(merge_request, @u_disabled) } diff --git a/spec/services/projects/update_remote_mirror_service_spec.rb b/spec/services/projects/update_remote_mirror_service_spec.rb index 09244db8010..a452f88c75f 100644 --- a/spec/services/projects/update_remote_mirror_service_spec.rb +++ b/spec/services/projects/update_remote_mirror_service_spec.rb @@ -56,6 +56,39 @@ RSpec.describe Projects::UpdateRemoteMirrorService do expect(remote_mirror.last_error).to include('Badly broken') end + context 'when the URL is blocked' do + before do + allow(Gitlab::UrlBlocker).to receive(:blocked_url?).and_return(true) + end + + it 'fails and returns error status' do + expect(execute!).to eq(status: :error, message: 'The remote mirror URL is invalid.') + end + end + + context "when given URLs containing escaped elements" do + using RSpec::Parameterized::TableSyntax + + where(:url, :result_status) do + "https://user:0a%23@test.example.com/project.git" | :success + "https://git.example.com:1%2F%2F@source.developers.google.com/project.git" | :success + CGI.escape("git://localhost:1234/some-path?some-query=some-val\#@example.com/") | :error + CGI.escape(CGI.escape("https://user:0a%23@test.example.com/project.git")) | :error + end + + with_them do + before do + allow(remote_mirror).to receive(:url).and_return(url) + end + + it "returns expected status" do + result = execute! + + expect(result[:status]).to eq(result_status) + end + end + end + context 'when the update fails because of a `Gitlab::Git::CommandError`' do before do allow(remote_mirror).to receive(:update_repository) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 395930c8cbc..06ec97d65b2 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -47,10 +47,10 @@ require_relative('../ee/spec/spec_helper') if Gitlab.ee? require Rails.root.join("spec/support/helpers/git_helpers.rb") # Then the rest -Dir[Rails.root.join("spec/support/helpers/*.rb")].each { |f| require f } -Dir[Rails.root.join("spec/support/shared_contexts/*.rb")].each { |f| require f } -Dir[Rails.root.join("spec/support/shared_examples/*.rb")].each { |f| require f } -Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f } +Dir[Rails.root.join("spec/support/helpers/*.rb")].sort.each { |f| require f } +Dir[Rails.root.join("spec/support/shared_contexts/*.rb")].sort.each { |f| require f } +Dir[Rails.root.join("spec/support/shared_examples/*.rb")].sort.each { |f| require f } +Dir[Rails.root.join("spec/support/**/*.rb")].sort.each { |f| require f } quality_level = Quality::TestLevel.new diff --git a/spec/support/helpers/stub_object_storage.rb b/spec/support/helpers/stub_object_storage.rb index 0486eccf48c..476b7d34ee5 100644 --- a/spec/support/helpers/stub_object_storage.rb +++ b/spec/support/helpers/stub_object_storage.rb @@ -21,7 +21,7 @@ module StubObjectStorage proxy_download: false, background_upload: false, direct_upload: false - ) + ) allow(config).to receive(:enabled) { enabled } allow(config).to receive(:proxy_download) { proxy_download } allow(config).to receive(:background_upload) { background_upload } diff --git a/spec/support/shared_examples/controllers/clusters_controller_shared_examples.rb b/spec/support/shared_examples/controllers/clusters_controller_shared_examples.rb new file mode 100644 index 00000000000..aa17e72d08e --- /dev/null +++ b/spec/support/shared_examples/controllers/clusters_controller_shared_examples.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'GET new cluster shared examples' do + describe 'EKS cluster' do + context 'user already has an associated AWS role' do + let!(:role) { create(:aws_role, user: user) } + + it 'does not create an Aws::Role record' do + expect { go(provider: 'aws') }.not_to change { Aws::Role.count } + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:aws_role)).to eq(role) + end + end + + context 'user does not have an associated AWS role' do + it 'creates an Aws::Role record' do + expect { go(provider: 'aws') }.to change { Aws::Role.count } + + expect(response).to have_gitlab_http_status(:ok) + + role = assigns(:aws_role) + expect(role.user).to eq(user) + expect(role.role_arn).to be_nil + expect(role.role_external_id).to be_present + end + end + end +end diff --git a/yarn.lock b/yarn.lock index 62102a3cbec..50f38b57dc6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -848,10 +848,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.161.0.tgz#661e8d19862dfba0e4c558e2eb6d64b402c1453e" integrity sha512-qsbboEICn08ZoEoAX/TuYygsFaXlzsCY+CfmdOzqvJbOdfHhVXmrJBxd2hP2qqjTZm2PkbRRmn+03+ce1jvatQ== -"@gitlab/ui@20.16.0": - version "20.16.0" - resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-20.16.0.tgz#45bad5500e6f47bc4bebb95a17b446f254f9ee21" - integrity sha512-k1+X8RgBXknCu40dEWTjE0Sd9jzGmpoPWPlgmoo28tE1vewhRmCOa8aL0tAcm6VNg5av5ZEqoQEJvgFKxLQB1Q== +"@gitlab/ui@20.17.0": + version "20.17.0" + resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-20.17.0.tgz#5a148c778247efc08df5c093acf5e82069823970" + integrity sha512-yDZ+B9/rq7t6OqIHMxPk120KzAO7xM1JKlE9ahkDyijC7lohQy9ygQGFlvVHo1Bic4Y/IW7nzeeZErjzJ3PPtA== dependencies: "@babel/standalone" "^7.0.0" "@gitlab/vue-toasted" "^1.3.0" |