From 0519a37195a1214bdee12972741b147f10966a01 Mon Sep 17 00:00:00 2001 From: Pirate Praveen Date: Wed, 4 Aug 2021 16:29:09 +0530 Subject: [PATCH 1/2] New upstream version 13.12.9+ds1 --- .gitlab/ci/frontend.gitlab-ci.yml | 7 ++ .gitlab/ci/rails.gitlab-ci.yml | 3 + CHANGELOG.md | 20 +++++ GITALY_SERVER_VERSION | 2 +- GITLAB_SHELL_VERSION | 2 +- Gemfile.lock | 2 +- VERSION | 2 +- .../behaviors/markdown/render_mermaid.js | 1 + .../admin/impersonation_tokens_controller.rb | 5 ++ app/controllers/dashboard/todos_controller.rb | 2 + app/controllers/invites_controller.rb | 4 +- .../projects/pipelines_controller.rb | 4 +- .../project_pipeline_statistics_resolver.rb | 4 + app/graphql/resolvers/todo_resolver.rb | 2 +- .../types/alert_management/alert_type.rb | 8 +- app/graphql/types/user_interface.rb | 7 +- app/models/alert_management/alert.rb | 4 + app/models/application_record.rb | 8 ++ app/models/commit.rb | 4 + app/models/design_management/design.rb | 4 - app/models/group.rb | 4 - app/models/issue.rb | 34 ++++---- app/models/note.rb | 10 --- app/models/project.rb | 4 - app/policies/personal_access_token_policy.rb | 2 +- app/policies/project_policy.rb | 3 + app/policies/todo_policy.rb | 7 +- app/services/issues/base_service.rb | 3 + .../todos/allowed_target_filter_service.rb | 18 ++++ app/views/admin/users/_head.html.haml | 5 +- app/views/dashboard/todos/index.html.haml | 8 +- app/views/invites/show.html.haml | 41 ++++----- config/initializers/omniauth.rb | 4 +- lib/api/personal_access_tokens.rb | 2 +- lib/api/todos.rb | 1 + lib/gitlab/auth.rb | 5 +- lib/gitlab/ci/pipeline/chain/command.rb | 30 ++++++- .../field_extension.rb | 26 ++++++ lib/gitlab/omniauth_logging/json_formatter.rb | 13 --- lib/sidebars/projects/menus/analytics_menu.rb | 1 + locale/gitlab.pot | 36 ++++---- spec/controllers/invites_controller_spec.rb | 84 ++++++++++++------- .../projects/pipelines_controller_spec.rb | 51 ++++++----- .../admin_users_impersonation_tokens_spec.rb | 12 +++ .../dashboard/todos/target_state_spec.rb | 20 +++-- .../dashboard/todos/todos_filtering_spec.rb | 2 +- spec/features/dashboard/todos/todos_spec.rb | 38 +++++++-- spec/features/invites_spec.rb | 39 +-------- spec/features/markdown/mermaid_spec.rb | 31 +++++-- .../projects/pipelines/pipeline_spec.rb | 5 +- spec/frontend/members/store/mutations_spec.js | 6 +- .../graphql/mutations/todos/mark_done_spec.rb | 10 ++- spec/graphql/mutations/todos/restore_spec.rb | 10 ++- ...oject_pipeline_statistics_resolver_spec.rb | 24 +++++- spec/graphql/resolvers/todo_resolver_spec.rb | 39 ++++++--- spec/lib/gitlab/auth_spec.rb | 9 ++ .../gitlab/ci/pipeline/chain/build_spec.rb | 12 ++- .../gitlab/ci/pipeline/chain/command_spec.rb | 24 ++++++ .../omniauth_logging/json_formatter_spec.rb | 12 --- .../projects/menus/analytics_menu_spec.rb | 16 ++-- spec/models/diff_note_spec.rb | 6 ++ spec/models/discussion_note_spec.rb | 11 +++ spec/models/legacy_diff_note_spec.rb | 11 +++ spec/models/synthetic_note_spec.rb | 11 +++ .../personal_access_token_policy_spec.rb | 7 ++ spec/policies/project_policy_spec.rb | 53 ++++++++++-- spec/policies/todo_policy_spec.rb | 24 ++++-- .../impersonation_tokens_controller_spec.rb | 38 +++++++++ .../graphql/current_user/todos_query_spec.rb | 45 ++++++++-- .../mutations/todos/mark_all_done_spec.rb | 12 ++- .../graphql/mutations/todos/mark_done_spec.rb | 10 ++- .../mutations/todos/restore_many_spec.rb | 10 ++- .../graphql/mutations/todos/restore_spec.rb | 10 ++- .../api/personal_access_tokens_spec.rb | 12 ++- spec/requests/api/todos_spec.rb | 71 ++++++++++++---- spec/requests/git_http_spec.rb | 26 ++++++ .../ci/create_pipeline_service_spec.rb | 67 +++++++++++++++ .../git/process_ref_changes_service_spec.rb | 10 ++- spec/services/issues/create_service_spec.rb | 21 +++++ spec/services/issues/update_service_spec.rb | 25 ++++++ .../allowed_target_filter_service_spec.rb | 59 +++++++++++++ 81 files changed, 1015 insertions(+), 320 deletions(-) create mode 100644 app/services/todos/allowed_target_filter_service.rb create mode 100644 lib/gitlab/graphql/todos_project_permission_preloader/field_extension.rb delete mode 100644 lib/gitlab/omniauth_logging/json_formatter.rb delete mode 100644 spec/lib/gitlab/omniauth_logging/json_formatter_spec.rb create mode 100644 spec/models/discussion_note_spec.rb create mode 100644 spec/models/legacy_diff_note_spec.rb create mode 100644 spec/models/synthetic_note_spec.rb create mode 100644 spec/requests/admin/impersonation_tokens_controller_spec.rb create mode 100644 spec/services/todos/allowed_target_filter_service_spec.rb diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml index 33aab8554e..dc76a872cd 100644 --- a/.gitlab/ci/frontend.gitlab-ci.yml +++ b/.gitlab/ci/frontend.gitlab-ci.yml @@ -11,6 +11,9 @@ variables: SETUP_DB: "false" WEBPACK_VENDOR_DLL: "true" + # Disable warnings in browserslist which can break on backports + # https://github.com/browserslist/browserslist/blob/a287ec6/node.js#L367-L384 + BROWSERSLIST_IGNORE_OLD_DATA: "true" stage: prepare script: - *yarn-install @@ -142,6 +145,10 @@ graphql-schema-dump: extends: - .default-retry - .yarn-cache + variables: + # Disable warnings in browserslist which can break on backports + # https://github.com/browserslist/browserslist/blob/a287ec6/node.js#L367-L384 + BROWSERSLIST_IGNORE_OLD_DATA: "true" stage: test eslint-as-if-foss: diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml index 68804b0f4c..126cb73f52 100644 --- a/.gitlab/ci/rails.gitlab-ci.yml +++ b/.gitlab/ci/rails.gitlab-ci.yml @@ -210,6 +210,9 @@ update-setup-test-env-cache: variables: SETUP_DB: "false" ENABLE_SPRING: "1" + # Disable warnings in browserslist which can break on backports + # https://github.com/browserslist/browserslist/blob/a287ec6/node.js#L367-L384 + BROWSERSLIST_IGNORE_OLD_DATA: "true" update-static-analysis-cache: extends: diff --git a/CHANGELOG.md b/CHANGELOG.md index 0842a1c217..384b8aac32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,26 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 13.12.9 (2021-08-03) + +### Security (15 changes) + +- [Add project member validation for domain limitation](gitlab-org/security/gitlab@8aff1815f897c2c454c87b1ccdd98c7a2c9eedb3) ([merge request](gitlab-org/security/gitlab!1562)) +- [Block impersonation token use if it is not permitted](gitlab-org/security/gitlab@99ab170ae5a2d991600dec9e7dfd8b5ca502c437) ([merge request](gitlab-org/security/gitlab!1585)) +- [Hide project-level CI/CD Analytics for Guests](gitlab-org/security/gitlab@740395d9663be41d52d831b8f90e271c08137220) ([merge request](gitlab-org/security/gitlab!1575)) +- [Only allow invite to be accepted by user with matching email](gitlab-org/security/gitlab@ae7ade09920486f6124496d800bf5f63f5a909eb) ([merge request](gitlab-org/security/gitlab!1634)) +- [Configure OmniAuth to use GitLab AppLogger](gitlab-org/security/gitlab@ed5e7742173878e59d760744e3f4f6686268584b) ([merge request](gitlab-org/security/gitlab!1617)) +- [Fix Protected Environment Accesses Cleanup](gitlab-org/security/gitlab@79eb0cb13a35864267c30663fd6033e8c6224cac) ([merge request](gitlab-org/security/gitlab!1608)) **GitLab Enterprise Edition** +- [Add permissions check to pipelines#show action](gitlab-org/security/gitlab@1a293b409226ce743527f1ac5ac5d216998339e1) ([merge request](gitlab-org/security/gitlab!1618)) +- [Prevent impersonation in gitlab-shell SSH certs](gitlab-org/security/gitlab@42521d9e7e72047bac09bd42779203ae6e508227) ([merge request](gitlab-org/security/gitlab!1611)) +- [Prevent guests from linking issues with errors](gitlab-org/security/gitlab@da799b0c7bcade058d4b57e065b1a1bebf903fa3) ([merge request](gitlab-org/security/gitlab!1599)) +- [Do not show email address in error message](gitlab-org/security/gitlab@2c3318edaa39ed0837b8fb30acae9f2cdc3d158f) ([merge request](gitlab-org/security/gitlab!1598)) **GitLab Enterprise Edition** +- [Updates oauth to 0.5.6](gitlab-org/security/gitlab@33df3791b646026016303a9d64661fbee7563630) ([merge request](gitlab-org/security/gitlab!1569)) +- [Remove impersonation token from api response for non-admin user](gitlab-org/security/gitlab@b56ae1953b2cd6b9d12c584e0f2c298a931f6f08) ([merge request](gitlab-org/security/gitlab!1567)) +- [Filter todos whose target users no longer have access to](gitlab-org/security/gitlab@ba613574b12e40fb61e5fbae8b1159f9ad037e84) ([merge request](gitlab-org/security/gitlab!1555)) +- [Fix tag ref detection for pipelines](gitlab-org/security/gitlab@4c36e98bcecd6e42e23ec5e20443f41de7f5bf18) ([merge request](gitlab-org/security/gitlab!1549)) +- [Fix XSS in Mermaid Markdown rendering](gitlab-org/security/gitlab@b27425816723b53db2f65b39f4702711b858cdfc) ([merge request](gitlab-org/security/gitlab!1487)) + ## 13.12.8 (2021-07-07) ### Security (1 change) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 21edccba21..01452f6e28 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -13.12.8 \ No newline at end of file +13.12.9 \ No newline at end of file diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index ce0b279568..41ea3a819a 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -13.18.0 +13.18.1 diff --git a/Gemfile.lock b/Gemfile.lock index 351e7ec94a..ac21ea32dc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -791,7 +791,7 @@ GEM nenv (~> 0.1) shellany (~> 0.0) numerizer (0.2.0) - oauth (0.5.4) + oauth (0.5.6) oauth2 (1.4.4) faraday (>= 0.8, < 2.0) jwt (>= 1.0, < 3.0) diff --git a/VERSION b/VERSION index 21edccba21..01452f6e28 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -13.12.8 \ No newline at end of file +13.12.9 \ No newline at end of file diff --git a/app/assets/javascripts/behaviors/markdown/render_mermaid.js b/app/assets/javascripts/behaviors/markdown/render_mermaid.js index f5b2d266c1..d18a1a72ee 100644 --- a/app/assets/javascripts/behaviors/markdown/render_mermaid.js +++ b/app/assets/javascripts/behaviors/markdown/render_mermaid.js @@ -48,6 +48,7 @@ export function initMermaid(mermaid) { useMaxWidth: true, htmlLabels: false, }, + secure: ['secure', 'securityLevel', 'startOnLoad', 'maxTextSize', 'htmlLabels'], securityLevel: 'strict', }); diff --git a/app/controllers/admin/impersonation_tokens_controller.rb b/app/controllers/admin/impersonation_tokens_controller.rb index c3166d5dd8..eb279298ba 100644 --- a/app/controllers/admin/impersonation_tokens_controller.rb +++ b/app/controllers/admin/impersonation_tokens_controller.rb @@ -2,6 +2,7 @@ class Admin::ImpersonationTokensController < Admin::ApplicationController before_action :user + before_action :verify_impersonation_enabled! feature_category :authentication_and_authorization @@ -41,6 +42,10 @@ class Admin::ImpersonationTokensController < Admin::ApplicationController end # rubocop: enable CodeReuse/ActiveRecord + def verify_impersonation_enabled! + access_denied! unless helpers.impersonation_enabled? + end + def finder(options = {}) PersonalAccessTokensFinder.new({ user: user, impersonation: true }.merge(options)) end diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb index 782c8c293f..5948d9b118 100644 --- a/app/controllers/dashboard/todos_controller.rb +++ b/app/controllers/dashboard/todos_controller.rb @@ -17,6 +17,8 @@ class Dashboard::TodosController < Dashboard::ApplicationController @todos = @todos.with_entity_associations return if redirect_out_of_range(@todos, todos_page_count(@todos)) + + @allowed_todos = ::Todos::AllowedTargetFilterService.new(@todos, current_user).execute end def destroy diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 0a9a9e03e9..cec2f0e4d8 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -20,7 +20,7 @@ class InvitesController < ApplicationController end def accept - if member.accept_invite!(current_user) + if current_user_matches_invite? && member.accept_invite!(current_user) redirect_to invite_details[:path], notice: helpers.invite_accepted_notice(member) else redirect_back_or_default(options: { alert: _("The invitation could not be accepted.") }) @@ -52,7 +52,7 @@ class InvitesController < ApplicationController end def current_user_matches_invite? - @member.invite_email == current_user.email + current_user.verified_emails.include?(@member.invite_email) end def member? diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 0de8dc597a..e14fe7d064 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -8,8 +8,8 @@ class Projects::PipelinesController < Projects::ApplicationController before_action :pipeline, except: [:index, :new, :create, :charts, :config_variables] before_action :set_pipeline_path, only: [:show] before_action :authorize_read_pipeline! - before_action :authorize_read_build!, only: [:index] - before_action :authorize_read_analytics!, only: [:charts] + before_action :authorize_read_build!, only: [:index, :show] + before_action :authorize_read_ci_cd_analytics!, only: [:charts] before_action :authorize_create_pipeline!, only: [:new, :create, :config_variables] before_action :authorize_update_pipeline!, only: [:retry, :cancel] before_action do diff --git a/app/graphql/resolvers/project_pipeline_statistics_resolver.rb b/app/graphql/resolvers/project_pipeline_statistics_resolver.rb index 29ab9402f5..79d01b9bf2 100644 --- a/app/graphql/resolvers/project_pipeline_statistics_resolver.rb +++ b/app/graphql/resolvers/project_pipeline_statistics_resolver.rb @@ -2,8 +2,12 @@ module Resolvers class ProjectPipelineStatisticsResolver < BaseResolver + include Gitlab::Graphql::Authorize::AuthorizeResource type Types::Ci::AnalyticsType, null: true + authorizes_object! + authorize :read_ci_cd_analytics + def resolve weekly_stats = Gitlab::Ci::Charts::WeekChart.new(object) monthly_stats = Gitlab::Ci::Charts::MonthChart.new(object) diff --git a/app/graphql/resolvers/todo_resolver.rb b/app/graphql/resolvers/todo_resolver.rb index 8966285fcc..af48ceefd6 100644 --- a/app/graphql/resolvers/todo_resolver.rb +++ b/app/graphql/resolvers/todo_resolver.rb @@ -34,7 +34,7 @@ module Resolvers return Todo.none unless current_user.present? && target.present? return Todo.none if target.is_a?(User) && target != current_user - TodosFinder.new(current_user, todo_finder_params(args)).execute + TodosFinder.new(current_user, todo_finder_params(args)).execute.with_entity_associations end private diff --git a/app/graphql/types/alert_management/alert_type.rb b/app/graphql/types/alert_management/alert_type.rb index 5a2a5c68c8..7cad62b9bf 100644 --- a/app/graphql/types/alert_management/alert_type.rb +++ b/app/graphql/types/alert_management/alert_type.rb @@ -115,11 +115,9 @@ module Types null: true, description: 'Runbook for the alert as defined in alert details.' - field :todos, - Types::TodoType.connection_type, - null: true, - description: 'To-do items of the current user for the alert.', - resolver: Resolvers::TodoResolver + field :todos, description: 'To-do items of the current user for the alert.', resolver: Resolvers::TodoResolver do + extension(::Gitlab::Graphql::TodosProjectPermissionPreloader::FieldExtension) + end field :details_url, GraphQL::STRING_TYPE, diff --git a/app/graphql/types/user_interface.rb b/app/graphql/types/user_interface.rb index e5abc03315..7d61b296ea 100644 --- a/app/graphql/types/user_interface.rb +++ b/app/graphql/types/user_interface.rb @@ -55,9 +55,6 @@ module Types type: GraphQL::STRING_TYPE, null: false, description: 'Web path of the user.' - field :todos, - resolver: Resolvers::TodoResolver, - description: 'To-do items of the user.' field :group_memberships, type: Types::GroupMemberType.connection_type, null: true, @@ -81,6 +78,10 @@ module Types description: 'Projects starred by the user.', resolver: Resolvers::UserStarredProjectsResolver + field :todos, resolver: Resolvers::TodoResolver, description: 'To-do items of the user.' do + extension(::Gitlab::Graphql::TodosProjectPermissionPreloader::FieldExtension) + end + # Merge request field: MRs can be authored, assigned, or assigned-for-review: field :authored_merge_requests, resolver: Resolvers::AuthoredMergeRequestsResolver, diff --git a/app/models/alert_management/alert.rb b/app/models/alert_management/alert.rb index 156111ffaf..8ab9048b85 100644 --- a/app/models/alert_management/alert.rb +++ b/app/models/alert_management/alert.rb @@ -262,6 +262,10 @@ module AlertManagement end end + def to_ability_name + 'alert_management_alert' + end + private def hook_data diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 5e5bc00458..4c869e9c6e 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -84,6 +84,14 @@ class ApplicationRecord < ActiveRecord::Base values = enum_mod.definition.transform_values { |v| v[:value] } enum(enum_mod.key => values) end + + def readable_by?(user) + Ability.allowed?(user, "read_#{to_ability_name}".to_sym, self) + end + + def to_ability_name + model_name.element + end end ApplicationRecord.prepend_mod_with('ApplicationRecordHelpers') diff --git a/app/models/commit.rb b/app/models/commit.rb index 09e43bb8f2..c7edfa6c5c 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -530,6 +530,10 @@ class Commit expire_note_etag_cache_for_related_mrs end + def readable_by?(user) + Ability.allowed?(user, :read_commit, self) + end + private def expire_note_etag_cache_for_related_mrs diff --git a/app/models/design_management/design.rb b/app/models/design_management/design.rb index e2d10cc7e7..79f5a63bcb 100644 --- a/app/models/design_management/design.rb +++ b/app/models/design_management/design.rb @@ -182,10 +182,6 @@ module DesignManagement File.join(DesignManagement.designs_directory, "issue-#{issue.iid}", design.filename) end - def to_ability_name - 'design' - end - def description '' end diff --git a/app/models/group.rb b/app/models/group.rb index da795651c6..8114d75f68 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -720,10 +720,6 @@ class Group < Namespace Gitlab::ServiceDesk.supported? && all_projects.service_desk_enabled.exists? end - def to_ability_name - model_name.singular - end - def activity_path Gitlab::Routing.url_helpers.activity_group_path(self) end diff --git a/app/models/issue.rb b/app/models/issue.rb index 2077f9bfdb..5338e2e667 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -476,23 +476,6 @@ class Issue < ApplicationRecord issue_assignees.pluck(:user_id) end - private - - # Ensure that the metrics association is safely created and respecting the unique constraint on issue_id - override :ensure_metrics - def ensure_metrics - if !association(:metrics).loaded? || metrics.blank? - metrics_record = Issue::Metrics.safe_find_or_create_by(issue: self) - self.metrics = metrics_record - end - - metrics.record! - end - - def record_create_action - Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_created_action(author: author) - end - # Returns `true` if the given User can read the current Issue. # # This method duplicates the same check of issue_policy.rb @@ -512,6 +495,23 @@ class Issue < ApplicationRecord end end + private + + # Ensure that the metrics association is safely created and respecting the unique constraint on issue_id + override :ensure_metrics + def ensure_metrics + if !association(:metrics).loaded? || metrics.blank? + metrics_record = Issue::Metrics.safe_find_or_create_by(issue: self) + self.metrics = metrics_record + end + + metrics.record! + end + + def record_create_action + Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_created_action(author: author) + end + # Returns `true` if this Issue is visible to everybody. def publicly_visible? project.public? && !confidential? && !::Gitlab::ExternalAuthorization.enabled? diff --git a/app/models/note.rb b/app/models/note.rb index ae4a8859d4..96977f3c6c 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -378,12 +378,6 @@ class Note < ApplicationRecord super end - # This method is to be used for checking read permissions on a note instead of `system_note_with_references_visible_for?` - def readable_by?(user) - # note_policy accounts for #system_note_with_references_visible_for?(user) check when granting read access - Ability.allowed?(user, :read_note, self) - end - def award_emoji? can_be_award_emoji? && contains_emoji_only? end @@ -400,10 +394,6 @@ class Note < ApplicationRecord note =~ /\A#{Banzai::Filter::EmojiFilter.emoji_pattern}\s?\Z/ end - def to_ability_name - model_name.singular - end - def noteable_ability_name if for_snippet? 'snippet' diff --git a/app/models/project.rb b/app/models/project.rb index 9d572b7e2f..fb6a6dbf24 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1496,10 +1496,6 @@ class Project < ApplicationRecord end end - def to_ability_name - model_name.singular - end - # rubocop: disable CodeReuse/ServiceClass def execute_hooks(data, hooks_scope = :push_hooks) run_after_commit_or_now do diff --git a/app/policies/personal_access_token_policy.rb b/app/policies/personal_access_token_policy.rb index 1e5404b782..31c973f575 100644 --- a/app/policies/personal_access_token_policy.rb +++ b/app/policies/personal_access_token_policy.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class PersonalAccessTokenPolicy < BasePolicy - condition(:is_owner) { user && subject.user_id == user.id } + condition(:is_owner) { user && subject.user_id == user.id && !subject.impersonation } rule { (is_owner | admin) & ~blocked }.policy do enable :read_token diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 1ce19511be..a3ccee150c 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -264,6 +264,7 @@ class ProjectPolicy < BasePolicy enable :read_package enable :read_product_analytics enable :read_group_timelogs + enable :read_ci_cd_analytics end # We define `:public_user_access` separately because there are cases in gitlab-ee @@ -460,6 +461,7 @@ class ProjectPolicy < BasePolicy prevent(:read_insights) prevent(:read_cycle_analytics) prevent(:read_repository_graphs) + prevent(:read_ci_cd_analytics) end rule { wiki_disabled }.policy do @@ -533,6 +535,7 @@ class ProjectPolicy < BasePolicy enable :read_cycle_analytics enable :read_pages_content enable :read_analytics + enable :read_ci_cd_analytics enable :read_insights # NOTE: may be overridden by IssuePolicy diff --git a/app/policies/todo_policy.rb b/app/policies/todo_policy.rb index d01a046c34..6237fbc50f 100644 --- a/app/policies/todo_policy.rb +++ b/app/policies/todo_policy.rb @@ -5,7 +5,10 @@ class TodoPolicy < BasePolicy condition(:own_todo) do @user && @subject.user_id == @user.id end + condition(:can_read_target) do + @user && @subject.target&.readable_by?(@user) + end - rule { own_todo }.enable :read_todo - rule { own_todo }.enable :update_todo + rule { own_todo & can_read_target }.enable :read_todo + rule { own_todo & can_read_target }.enable :update_todo end diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 72e906e20f..dae1e55724 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -47,6 +47,9 @@ module Issues params.delete(:created_at) unless moved_issue || current_user.can?(:set_issue_created_at, project) params.delete(:updated_at) unless moved_issue || current_user.can?(:set_issue_updated_at, project) + # Only users with permission to handle error data can add it to issues + params.delete(:sentry_issue_attributes) unless current_user.can?(:update_sentry_issue, project) + issue.system_note_timestamp = params[:created_at] || params[:updated_at] end diff --git a/app/services/todos/allowed_target_filter_service.rb b/app/services/todos/allowed_target_filter_service.rb new file mode 100644 index 0000000000..dfed616710 --- /dev/null +++ b/app/services/todos/allowed_target_filter_service.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Todos + class AllowedTargetFilterService + include Gitlab::Allowable + + def initialize(todos, current_user) + @todos = todos + @current_user = current_user + end + + def execute + Preloaders::UserMaxAccessLevelInProjectsPreloader.new(@todos.map(&:project).compact, @current_user).execute + + @todos.select { |todo| can?(@current_user, :read_todo, todo) } + end + end +end diff --git a/app/views/admin/users/_head.html.haml b/app/views/admin/users/_head.html.haml index be04e87f8b..a7f821a273 100644 --- a/app/views/admin/users/_head.html.haml +++ b/app/views/admin/users/_head.html.haml @@ -37,6 +37,7 @@ = link_to _("SSH keys"), keys_admin_user_path(@user) = nav_link(controller: :identities) do = link_to _("Identities"), admin_user_identities_path(@user) - = nav_link(controller: :impersonation_tokens) do - = link_to _("Impersonation Tokens"), admin_user_impersonation_tokens_path(@user) + - if impersonation_enabled? + = nav_link(controller: :impersonation_tokens) do + = link_to _("Impersonation Tokens"), admin_user_impersonation_tokens_path(@user) .gl-mb-3 diff --git a/app/views/dashboard/todos/index.html.haml b/app/views/dashboard/todos/index.html.haml index 52e41946ed..2a32fc96ca 100644 --- a/app/views/dashboard/todos/index.html.haml +++ b/app/views/dashboard/todos/index.html.haml @@ -25,7 +25,7 @@ = number_with_delimiter(todos_done_count) .nav-controls - - if @todos.any?(&:pending?) + - if @allowed_todos.any?(&:pending?) .gl-mr-3 = link_to destroy_all_dashboard_todos_path(todos_filter_params), class: 'gl-button btn btn-default btn-loading align-items-center js-todos-mark-all', method: :delete, data: { href: destroy_all_dashboard_todos_path(todos_filter_params) } do Mark all as done @@ -82,11 +82,11 @@ = sort_title_oldest_created .row.js-todos-all - - if @todos.any? + - if @allowed_todos.any? .col.js-todos-list-container{ data: { qa_selector: "todos_list_container" } } - .js-todos-options{ data: { per_page: @todos.limit_value, current_page: @todos.current_page, total_pages: @todos.total_pages } } + .js-todos-options{ data: { per_page: @allowed_todos.count, current_page: @todos.current_page, total_pages: @todos.total_pages } } %ul.content-list.todos-list - = render @todos + = render @allowed_todos = paginate @todos, theme: "gitlab" .js-nothing-here-container.empty-state.hidden .svg-content diff --git a/app/views/invites/show.html.haml b/app/views/invites/show.html.haml index ae13ef831d..3622fc4698 100644 --- a/app/views/invites/show.html.haml +++ b/app/views/invites/show.html.haml @@ -1,29 +1,30 @@ - page_title _("Invitation") %h3.page-title= _("Invitation") -%p - = _("You have been invited") - - inviter = @member.created_by - - if inviter - = _("by") - = link_to inviter.name, user_url(inviter) - = _("to join %{source_name}") % { source_name: @invite_details[:title] } - %strong - = link_to @invite_details[:name], @invite_details[:url] - = _("as %{role}.") % { role: @member.human_access } +- if current_user_matches_invite? + - if member? + %p + = _("You are already a member of this %{member_source}.") % { member_source: @invite_details[:title] } + .actions + = link_to _("Go to %{source_name}") % { source_name: @invite_details[:title] }, @invite_details[:url], class: "btn gl-button btn-confirm" -- if member? - %p - = _("However, you are already a member of this %{member_source}. Sign in using a different account to accept the invitation.") % { member_source: @invite_details[:title] } + - else + %p + - inviter = @member.created_by + - link_to_inviter = link_to(inviter.name, user_url(inviter)) + - link_to_source = link_to(@invite_details[:name], @invite_details[:url]) -- if !current_user_matches_invite? + = html_escape(_("You have been invited by %{link_to_inviter} to join %{source_name} %{strong_open}%{link_to_source}%{strong_close} as %{role}")) % { link_to_inviter: link_to_inviter, source_name: @invite_details[:title], strong_open: ''.html_safe, link_to_source: link_to_source, strong_close: ''.html_safe, role: @member.human_access } + + .actions + = link_to _("Accept invitation"), accept_invite_url(@token), method: :post, class: "btn gl-button btn-confirm" + = link_to _("Decline"), decline_invite_url(@token), method: :post, class: "btn gl-button btn-danger gl-ml-3" + +- else %p - mail_to_invite_email = mail_to(@member.invite_email) - mail_to_current_user = mail_to(current_user.email) - link_to_current_user = link_to(current_user.to_reference, user_url(current_user)) - = _("Note that this invitation was sent to %{mail_to_invite_email}, but you are signed in as %{link_to_current_user} with email %{mail_to_current_user}.").html_safe % { mail_to_invite_email: mail_to_invite_email, mail_to_current_user: mail_to_current_user, link_to_current_user: link_to_current_user } - -- if !member? - .actions - = link_to _("Accept invitation"), accept_invite_url(@token), method: :post, class: "btn gl-button btn-confirm" - = link_to _("Decline"), decline_invite_url(@token), method: :post, class: "btn gl-button btn-danger gl-ml-3" + = _("This invitation was sent to %{mail_to_invite_email}, but you are signed in as %{link_to_current_user} with email %{mail_to_current_user}.").html_safe % { mail_to_invite_email: mail_to_invite_email, mail_to_current_user: mail_to_current_user, link_to_current_user: link_to_current_user } + %p + = _("Sign in as a user with the matching email address, add the email to this account, or sign-up for a new account using the matching email.") diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 85984772d0..478a582880 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -19,6 +19,4 @@ OmniAuth.config.before_request_phase do |env| Gitlab::RequestForgeryProtection.call(env) end -# Use json formatter -OmniAuth.config.logger.formatter = Gitlab::OmniauthLogging::JSONFormatter.new -OmniAuth.config.logger.level = Logger::ERROR if Rails.env.production? +OmniAuth.config.logger = Gitlab::AppLogger diff --git a/lib/api/personal_access_tokens.rb b/lib/api/personal_access_tokens.rb index 2c60938b75..56590bb9a8 100644 --- a/lib/api/personal_access_tokens.rb +++ b/lib/api/personal_access_tokens.rb @@ -23,7 +23,7 @@ module API helpers do def finder_params(current_user) - current_user.admin? ? { user: user(params[:user_id]) } : { user: current_user } + current_user.admin? ? { user: user(params[:user_id]) } : { user: current_user, impersonation: false } end def user(user_id) diff --git a/lib/api/todos.rb b/lib/api/todos.rb index a001313a11..e0e5ca615a 100644 --- a/lib/api/todos.rb +++ b/lib/api/todos.rb @@ -92,6 +92,7 @@ module API end get do todos = paginate(find_todos.with_entity_associations) + todos = ::Todos::AllowedTargetFilterService.new(todos, current_user).execute options = { with: Entities::Todo, current_user: current_user } batch_load_issuable_metadata(todos, options) diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 7cae7e4ea8..e3b6e575f5 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -192,7 +192,10 @@ module Gitlab def personal_access_token_check(password, project) return unless password.present? - token = PersonalAccessTokensFinder.new(state: 'active').find_by_token(password) + finder_options = { state: 'active' } + finder_options[:impersonation] = false unless Gitlab.config.gitlab.impersonation_enabled + + token = PersonalAccessTokensFinder.new(finder_options).find_by_token(password) return unless token diff --git a/lib/gitlab/ci/pipeline/chain/command.rb b/lib/gitlab/ci/pipeline/chain/command.rb index c3c1728602..7564d0c3ed 100644 --- a/lib/gitlab/ci/pipeline/chain/command.rb +++ b/lib/gitlab/ci/pipeline/chain/command.rb @@ -26,13 +26,13 @@ module Gitlab def branch_exists? strong_memoize(:is_branch) do - project.repository.branch_exists?(ref) + branch_ref? && project.repository.branch_exists?(ref) end end def tag_exists? strong_memoize(:is_tag) do - project.repository.tag_exists?(ref) + tag_ref? && project.repository.tag_exists?(ref) end end @@ -105,6 +105,32 @@ module Gitlab def dangling_build? %i[ondemand_dast_scan webide].include?(source) end + + private + + # Verifies that origin_ref is a fully qualified tag reference (refs/tags/) + # + # Fallbacks to `true` for backward compatibility reasons + # if origin_ref is a short ref + def tag_ref? + return true if full_git_ref_name_unavailable? + + Gitlab::Git.tag_ref?(origin_ref).present? + end + + # Verifies that origin_ref is a fully qualified branch reference (refs/heads/) + # + # Fallbacks to `true` for backward compatibility reasons + # if origin_ref is a short ref + def branch_ref? + return true if full_git_ref_name_unavailable? + + Gitlab::Git.branch_ref?(origin_ref).present? + end + + def full_git_ref_name_unavailable? + ref == origin_ref + end end end end diff --git a/lib/gitlab/graphql/todos_project_permission_preloader/field_extension.rb b/lib/gitlab/graphql/todos_project_permission_preloader/field_extension.rb new file mode 100644 index 0000000000..77f3b1ac71 --- /dev/null +++ b/lib/gitlab/graphql/todos_project_permission_preloader/field_extension.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + module TodosProjectPermissionPreloader + class FieldExtension < ::GraphQL::Schema::FieldExtension + def after_resolve(value:, memo:, **rest) + todos = value.to_a + + Preloaders::UserMaxAccessLevelInProjectsPreloader.new( + todos.map(&:project).compact, + current_user(rest) + ).execute + + value + end + + private + + def current_user(options) + options.dig(:context, :current_user) + end + end + end + end +end diff --git a/lib/gitlab/omniauth_logging/json_formatter.rb b/lib/gitlab/omniauth_logging/json_formatter.rb deleted file mode 100644 index cdd4da3180..0000000000 --- a/lib/gitlab/omniauth_logging/json_formatter.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -require 'json' - -module Gitlab - module OmniauthLogging - class JSONFormatter - def call(severity, datetime, progname, msg) - { severity: severity, timestamp: datetime.utc.iso8601(3), pid: $$, progname: progname, message: msg }.to_json << "\n" - end - end - end -end diff --git a/lib/sidebars/projects/menus/analytics_menu.rb b/lib/sidebars/projects/menus/analytics_menu.rb index 660965005c..ea3a25d513 100644 --- a/lib/sidebars/projects/menus/analytics_menu.rb +++ b/lib/sidebars/projects/menus/analytics_menu.rb @@ -46,6 +46,7 @@ module Sidebars def ci_cd_analytics_menu_item if !context.project.feature_available?(:builds, context.current_user) || !can?(context.current_user, :read_build, context.project) || + !can?(context.current_user, :read_ci_cd_analytics, context.project) || context.project.empty_repo? return ::Sidebars::NilMenuItem.new(item_id: :ci_cd_analytics) end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9f94ae98a6..48b2158386 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -15374,6 +15374,9 @@ msgstr "" msgid "Go full screen" msgstr "" +msgid "Go to %{source_name}" +msgstr "" + msgid "Go to Webhooks" msgstr "" @@ -16470,9 +16473,6 @@ msgstr "" msgid "How many users will be evaluating the trial?" msgstr "" -msgid "However, you are already a member of this %{member_source}. Sign in using a different account to accept the invitation." -msgstr "" - msgid "I accept the %{terms_link}" msgstr "" @@ -22499,9 +22499,6 @@ msgstr "" msgid "Note that pushing to GitLab requires write access to this repository." msgstr "" -msgid "Note that this invitation was sent to %{mail_to_invite_email}, but you are signed in as %{link_to_current_user} with email %{mail_to_current_user}." -msgstr "" - msgid "Note: As an administrator you may like to configure %{github_integration_link}, which will allow login via GitHub and allow connecting repositories without generating a Personal Access Token." msgstr "" @@ -30052,6 +30049,9 @@ msgstr "" msgid "Sign in / Register" msgstr "" +msgid "Sign in as a user with the matching email address, add the email to this account, or sign-up for a new account using the matching email." +msgstr "" + msgid "Sign in preview" msgstr "" @@ -33166,6 +33166,9 @@ msgstr "" msgid "This group, its subgroups and projects will be removed on %{date} since its parent group '%{parent_group_name}'' has been scheduled for removal." msgstr "" +msgid "This invitation was sent to %{mail_to_invite_email}, but you are signed in as %{link_to_current_user} with email %{mail_to_current_user}." +msgstr "" + msgid "This is a \"Ghost User\", created to hold all issues authored by users that have since been deleted. This user cannot be removed." msgstr "" @@ -36958,6 +36961,9 @@ msgstr "" msgid "You are about to transfer the control of your account to %{group_name} group. This action is NOT reversible, you won't be able to access any of your groups and projects outside of %{group_name} once this transfer is complete." msgstr "" +msgid "You are already a member of this %{member_source}." +msgstr "" + msgid "You are an admin, which means granting access to %{client_name} will allow them to interact with GitLab as an admin as well. Proceed with caution." msgstr "" @@ -37291,7 +37297,7 @@ msgstr "" msgid "You have been granted %{member_human_access} access to project %{name}." msgstr "" -msgid "You have been invited" +msgid "You have been invited by %{link_to_inviter} to join %{source_name} %{strong_open}%{link_to_source}%{strong_close} as %{role}" msgstr "" msgid "You have been redirected to the only result; see the %{a_start}search results%{a_end} instead." @@ -37887,9 +37893,6 @@ msgstr "" msgid "archived:" msgstr "" -msgid "as %{role}." -msgstr "" - msgid "assign yourself" msgstr "" @@ -38343,14 +38346,14 @@ msgstr "" msgid "element is not a hierarchy" msgstr "" -msgid "email '%{email}' does not match the allowed domain of %{email_domains}" -msgid_plural "email '%{email}' does not match the allowed domains: %{email_domains}" -msgstr[0] "" -msgstr[1] "" - msgid "email '%{email}' is not a verified email." msgstr "" +msgid "email does not match the allowed domain of %{email_domains}" +msgid_plural "email does not match the allowed domains: %{email_domains}" +msgstr[0] "" +msgstr[1] "" + msgid "enabled" msgstr "" @@ -39383,9 +39386,6 @@ msgstr "" msgid "to help your contributors communicate effectively!" msgstr "" -msgid "to join %{source_name}" -msgstr "" - msgid "toggle collapse" msgstr "" diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb index 6b94d186d5..bca59813a2 100644 --- a/spec/controllers/invites_controller_spec.rb +++ b/spec/controllers/invites_controller_spec.rb @@ -24,9 +24,64 @@ RSpec.describe InvitesController do end end + shared_examples 'invite email match enforcement' do |error_status:, flash_alert: nil| + it 'accepts user if invite email matches signed in user' do + expect do + request + end.to change { project_members.include?(user) }.from(false).to(true) + + expect(response).to have_gitlab_http_status(:found) + expect(flash[:notice]).to include 'You have been granted' + end + + it 'accepts invite if invite email matches confirmed secondary email' do + secondary_email = create(:email, :confirmed, user: user) + member.update!(invite_email: secondary_email.email) + + expect do + request + end.to change { project_members.include?(user) }.from(false).to(true) + + expect(response).to have_gitlab_http_status(:found) + expect(flash[:notice]).to include 'You have been granted' + end + + it 'does not accept if invite email matches unconfirmed secondary email' do + secondary_email = create(:email, user: user) + member.update!(invite_email: secondary_email.email) + + expect do + request + end.not_to change { project_members.include?(user) } + + expect(response).to have_gitlab_http_status(error_status) + expect(flash[:alert]).to eq(flash_alert) + end + + it 'does not accept if invite email does not match signed in user' do + member.update!(invite_email: 'bogus@email.com') + + expect do + request + end.not_to change { project_members.include?(user) } + + expect(response).to have_gitlab_http_status(error_status) + expect(flash[:alert]).to eq(flash_alert) + end + end + describe 'GET #show' do subject(:request) { get :show, params: params } + context 'when logged in' do + before do + sign_in(user) + end + + it_behaves_like 'invite email match enforcement', error_status: :ok + it_behaves_like 'invalid token' + end + context 'when it is part of our invite email experiment' do let(:extra_params) { { invite_type: 'initial_email' } } @@ -58,34 +113,6 @@ RSpec.describe InvitesController do end end - context 'when logged in' do - before do - sign_in(user) - end - - it 'accepts user if invite email matches signed in user' do - expect do - request - end.to change { project_members.include?(user) }.from(false).to(true) - - expect(response).to have_gitlab_http_status(:found) - expect(flash[:notice]).to include 'You have been granted' - end - - it 'forces re-confirmation if email does not match signed in user' do - member.update!(invite_email: 'bogus@email.com') - - expect do - request - end.not_to change { project_members.include?(user) } - - expect(response).to have_gitlab_http_status(:ok) - expect(flash[:notice]).to be_nil - end - - it_behaves_like 'invalid token' - end - context 'when not logged in' do context 'when invite token belongs to a valid member' do context 'when instance allows sign up' do @@ -239,6 +266,7 @@ RSpec.describe InvitesController do subject(:request) { post :accept, params: params } + it_behaves_like 'invite email match enforcement', error_status: :redirect, flash_alert: 'The invitation could not be accepted.' it_behaves_like 'invalid token' end diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 0e6b5e84d8..7faf641294 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -302,35 +302,46 @@ RSpec.describe Projects::PipelinesController do end describe 'GET #show' do - render_views - - let_it_be(:pipeline) { create(:ci_pipeline, project: project) } - - subject { get_pipeline_html } - def get_pipeline_html get :show, params: { namespace_id: project.namespace, project_id: project, id: pipeline }, format: :html end - def create_build_with_artifacts(stage, stage_idx, name) - create(:ci_build, :artifacts, :tags, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name) + context 'when the project is public' do + render_views + + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + + def create_build_with_artifacts(stage, stage_idx, name) + create(:ci_build, :artifacts, :tags, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name) + end + + before do + create_build_with_artifacts('build', 0, 'job1') + create_build_with_artifacts('build', 0, 'job2') + end + + it 'avoids N+1 database queries', :request_store do + control_count = ActiveRecord::QueryRecorder.new { get_pipeline_html }.count + expect(response).to have_gitlab_http_status(:ok) + + create_build_with_artifacts('build', 0, 'job3') + + expect { get_pipeline_html }.not_to exceed_query_limit(control_count) + expect(response).to have_gitlab_http_status(:ok) + end end - before do - create_build_with_artifacts('build', 0, 'job1') - create_build_with_artifacts('build', 0, 'job2') - end + context 'when the project is private' do + let(:project) { create(:project, :private, :repository) } + let(:pipeline) { create(:ci_pipeline, project: project) } - it 'avoids N+1 database queries', :request_store do - get_pipeline_html + it 'returns `not_found` when the user does not have access' do + sign_in(create(:user)) - control_count = ActiveRecord::QueryRecorder.new { get_pipeline_html }.count - expect(response).to have_gitlab_http_status(:ok) + get_pipeline_html - create_build_with_artifacts('build', 0, 'job3') - - expect { get_pipeline_html }.not_to exceed_query_limit(control_count) - expect(response).to have_gitlab_http_status(:ok) + expect(response).to have_gitlab_http_status(:not_found) + end end end diff --git a/spec/features/admin/admin_users_impersonation_tokens_spec.rb b/spec/features/admin/admin_users_impersonation_tokens_spec.rb index dc528dd92d..0e9fe35faf 100644 --- a/spec/features/admin/admin_users_impersonation_tokens_spec.rb +++ b/spec/features/admin/admin_users_impersonation_tokens_spec.rb @@ -83,4 +83,16 @@ RSpec.describe 'Admin > Users > Impersonation Tokens', :js do expect(no_personal_access_tokens_message).to have_text("This user has no active impersonation tokens.") end end + + describe "impersonation disabled state" do + before do + stub_config_setting(impersonation_enabled: false) + end + + it "does not show impersonation tokens tab" do + visit admin_user_path(user) + + expect(page).not_to have_content("Impersonation Tokens") + end + end end diff --git a/spec/features/dashboard/todos/target_state_spec.rb b/spec/features/dashboard/todos/target_state_spec.rb index 4c43948201..b0aafdda59 100644 --- a/spec/features/dashboard/todos/target_state_spec.rb +++ b/spec/features/dashboard/todos/target_state_spec.rb @@ -3,16 +3,20 @@ require 'spec_helper' RSpec.describe 'Dashboard > Todo target states' do - let(:user) { create(:user) } - let(:author) { create(:user) } - let(:project) { create(:project, :public) } + let_it_be(:user) { create(:user) } + let_it_be(:author) { create(:user) } + let_it_be(:project) { create(:project, :public) } + + before_all do + project.add_developer(user) + end before do sign_in(user) end it 'on a closed issue todo has closed label' do - issue_closed = create(:issue, state: 'closed') + issue_closed = create(:issue, state: 'closed', project: project) create_todo issue_closed visit dashboard_todos_path @@ -22,7 +26,7 @@ RSpec.describe 'Dashboard > Todo target states' do end it 'on an open issue todo does not have an open label' do - issue_open = create(:issue) + issue_open = create(:issue, project: project) create_todo issue_open visit dashboard_todos_path @@ -32,7 +36,7 @@ RSpec.describe 'Dashboard > Todo target states' do end it 'on a merged merge request todo has merged label' do - mr_merged = create(:merge_request, :simple, :merged, author: user) + mr_merged = create(:merge_request, :simple, :merged, author: user, source_project: project) create_todo mr_merged visit dashboard_todos_path @@ -42,7 +46,7 @@ RSpec.describe 'Dashboard > Todo target states' do end it 'on a closed merge request todo has closed label' do - mr_closed = create(:merge_request, :simple, :closed, author: user) + mr_closed = create(:merge_request, :simple, :closed, author: user, source_project: project) create_todo mr_closed visit dashboard_todos_path @@ -52,7 +56,7 @@ RSpec.describe 'Dashboard > Todo target states' do end it 'on an open merge request todo does not have an open label' do - mr_open = create(:merge_request, :simple, author: user) + mr_open = create(:merge_request, :simple, author: user, source_project: project) create_todo mr_open visit dashboard_todos_path diff --git a/spec/features/dashboard/todos/todos_filtering_spec.rb b/spec/features/dashboard/todos/todos_filtering_spec.rb index b1464af419..53209db310 100644 --- a/spec/features/dashboard/todos/todos_filtering_spec.rb +++ b/spec/features/dashboard/todos/todos_filtering_spec.rb @@ -128,7 +128,7 @@ RSpec.describe 'Dashboard > User filters todos', :js do describe 'filter by action' do before do - create(:todo, :build_failed, user: user_1, author: user_2, project: project_1) + create(:todo, :build_failed, user: user_1, author: user_2, project: project_1, target: merge_request) create(:todo, :marked, user: user_1, author: user_2, project: project_1, target: issue1) create(:todo, :review_requested, user: user_1, author: user_2, project: project_1, target: issue1) end diff --git a/spec/features/dashboard/todos/todos_spec.rb b/spec/features/dashboard/todos/todos_spec.rb index 0bc6cc9c01..7345bfa19e 100644 --- a/spec/features/dashboard/todos/todos_spec.rb +++ b/spec/features/dashboard/todos/todos_spec.rb @@ -3,10 +3,16 @@ require 'spec_helper' RSpec.describe 'Dashboard Todos' do + include DesignManagementTestHelpers + let_it_be(:user) { create(:user, username: 'john') } let_it_be(:author) { create(:user) } let_it_be(:project) { create(:project, :public) } - let_it_be(:issue) { create(:issue, due_date: Date.today, title: "Fix bug") } + let_it_be(:issue) { create(:issue, project: project, due_date: Date.today, title: "Fix bug") } + + before_all do + project.add_developer(user) + end context 'User does not have todos' do before do @@ -21,8 +27,8 @@ RSpec.describe 'Dashboard Todos' do context 'when the todo references a merge request' do let(:referenced_mr) { create(:merge_request, source_project: project) } - let(:note) { create(:note, project: project, note: "Check out #{referenced_mr.to_reference}") } - let!(:todo) { create(:todo, :mentioned, user: user, project: project, author: author, note: note) } + let(:note) { create(:note, project: project, note: "Check out #{referenced_mr.to_reference}", noteable: create(:issue, project: project)) } + let!(:todo) { create(:todo, :mentioned, user: user, project: project, author: author, note: note, target: note.noteable) } before do sign_in(user) @@ -39,9 +45,26 @@ RSpec.describe 'Dashboard Todos' do end end - context 'User has a todo', :js do + context 'user has an unauthorized todo' do before do + sign_in(user) + end + + it 'does not render the todo' do + unauthorized_issue = create(:issue) + create(:todo, :mentioned, user: user, project: unauthorized_issue.project, target: unauthorized_issue, author: author) create(:todo, :mentioned, user: user, project: project, target: issue, author: author) + + visit dashboard_todos_path + + expect(page).to have_selector('.todos-list .todo', count: 1) + end + end + + context 'User has a todo', :js do + let_it_be(:user_todo) { create(:todo, :mentioned, user: user, project: project, target: issue, author: author) } + + before do sign_in(user) visit dashboard_todos_path @@ -183,7 +206,7 @@ RSpec.describe 'Dashboard Todos' do end context 'approval todo' do - let(:merge_request) { create(:merge_request, title: "Fixes issue") } + let(:merge_request) { create(:merge_request, title: "Fixes issue", source_project: project) } before do create(:todo, :approval_required, user: user, project: project, target: merge_request, author: user) @@ -199,7 +222,7 @@ RSpec.describe 'Dashboard Todos' do end context 'review request todo' do - let(:merge_request) { create(:merge_request, title: "Fixes issue") } + let(:merge_request) { create(:merge_request, title: "Fixes issue", source_project: project) } before do create(:todo, :review_requested, user: user, project: project, target: merge_request, author: user) @@ -355,7 +378,7 @@ RSpec.describe 'Dashboard Todos' do end context 'User has a Build Failed todo' do - let!(:todo) { create(:todo, :build_failed, user: user, project: project, author: author) } + let!(:todo) { create(:todo, :build_failed, user: user, project: project, author: author, target: create(:merge_request, source_project: project)) } before do sign_in(user) @@ -386,6 +409,7 @@ RSpec.describe 'Dashboard Todos' do end before do + enable_design_management project.add_developer(user) sign_in(user) diff --git a/spec/features/invites_spec.rb b/spec/features/invites_spec.rb index a72cf033d6..be01488e3f 100644 --- a/spec/features/invites_spec.rb +++ b/spec/features/invites_spec.rb @@ -90,48 +90,17 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do end context 'when signed in and an invite link is clicked' do - context 'when an invite email is a secondary email for the user' do - let(:invite_email) { 'user_secondary@example.com' } - - before do - sign_in(user) - visit invite_path(group_invite.raw_invite_token) - end - - it 'sends user to the invite url and allows them to decline' do - expect(current_path).to eq(invite_path(group_invite.raw_invite_token)) - expect(page).to have_content("Note that this invitation was sent to #{invite_email}") - expect(page).to have_content("but you are signed in as #{user.to_reference} with email #{user.email}") - - click_link('Decline') - - expect(page).to have_content('You have declined the invitation') - expect(current_path).to eq(dashboard_projects_path) - expect { group_invite.reload }.to raise_error ActiveRecord::RecordNotFound - end - - it 'sends uer to the invite url and allows them to accept' do - expect(current_path).to eq(invite_path(group_invite.raw_invite_token)) - expect(page).to have_content("Note that this invitation was sent to #{invite_email}") - expect(page).to have_content("but you are signed in as #{user.to_reference} with email #{user.email}") - - click_link('Accept invitation') - - expect(page).to have_content('You have been granted') - expect(current_path).to eq(activity_group_path(group)) - end - end - context 'when user is an existing member' do before do - sign_in(owner) + group.add_developer(user) + sign_in(user) visit invite_path(group_invite.raw_invite_token) end it 'shows message user already a member' do expect(current_path).to eq(invite_path(group_invite.raw_invite_token)) - expect(page).to have_link(owner.name, href: user_url(owner)) - expect(page).to have_content('However, you are already a member of this group.') + expect(page).to have_link(user.name, href: user_path(user)) + expect(page).to have_content('You are already a member of this group.') end end end diff --git a/spec/features/markdown/mermaid_spec.rb b/spec/features/markdown/mermaid_spec.rb index 207678e07c..d0abd404a5 100644 --- a/spec/features/markdown/mermaid_spec.rb +++ b/spec/features/markdown/mermaid_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe 'Mermaid rendering', :js do + let_it_be(:project) { create(:project, :public) } + it 'renders Mermaid diagrams correctly' do description = <<~MERMAID ```mermaid @@ -14,7 +16,6 @@ RSpec.describe 'Mermaid rendering', :js do ``` MERMAID - project = create(:project, :public) issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -36,7 +37,6 @@ RSpec.describe 'Mermaid rendering', :js do ``` MERMAID - project = create(:project, :public) issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -64,7 +64,6 @@ RSpec.describe 'Mermaid rendering', :js do ``` MERMAID - project = create(:project, :public) issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -94,7 +93,6 @@ RSpec.describe 'Mermaid rendering', :js do MERMAID - project = create(:project, :public) issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -123,7 +121,6 @@ RSpec.describe 'Mermaid rendering', :js do ``` MERMAID - project = create(:project, :public) issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -144,7 +141,6 @@ RSpec.describe 'Mermaid rendering', :js do ``` MERMAID - project = create(:project, :public) issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -183,8 +179,6 @@ RSpec.describe 'Mermaid rendering', :js do description *= 51 - project = create(:project, :public) - issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -200,6 +194,27 @@ RSpec.describe 'Mermaid rendering', :js do expect(page).to have_selector('.js-lazy-render-mermaid-container') end end + + it 'does not allow HTML injection' do + description = <<~MERMAID + ```mermaid + %%{init: {"flowchart": {"htmlLabels": "false"}} }%% + flowchart + A[""] + ``` + MERMAID + + issue = create(:issue, project: project, description: description) + + visit project_issue_path(project, issue) + + wait_for_requests + wait_for_mermaid + + page.within('.description') do + expect(page).not_to have_xpath("//iframe") + end + end end def wait_for_mermaid diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index 70dc0bd04e..b93cbddf55 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -361,9 +361,8 @@ RSpec.describe 'Pipeline', :js do let(:project) { create(:project, :public, :repository, public_builds: false) } let(:role) { :guest } - it 'does not show failed jobs tab pane' do - expect(page).to have_link('Pipeline') - expect(page).not_to have_content('Failed Jobs') + it 'does not show the pipeline details page' do + expect(page).to have_content('Not Found') end end end diff --git a/spec/frontend/members/store/mutations_spec.js b/spec/frontend/members/store/mutations_spec.js index 7ad7034eb6..78bbad394a 100644 --- a/spec/frontend/members/store/mutations_spec.js +++ b/spec/frontend/members/store/mutations_spec.js @@ -44,8 +44,7 @@ describe('Vuex members mutations', () => { describe('when error has a message', () => { it('shows error message', () => { const error = new Error('Request failed with status code 422'); - const message = - 'User email "john.smith@gmail.com" does not match the allowed domain of example.com'; + const message = 'User email does not match the allowed domain of example.com'; error.response = { data: { message }, @@ -88,8 +87,7 @@ describe('Vuex members mutations', () => { describe('when error has a message', () => { it('shows error message', () => { const error = new Error('Request failed with status code 422'); - const message = - 'User email "john.smith@gmail.com" does not match the allowed domain of example.com'; + const message = 'User email does not match the allowed domain of example.com'; error.response = { data: { message }, diff --git a/spec/graphql/mutations/todos/mark_done_spec.rb b/spec/graphql/mutations/todos/mark_done_spec.rb index b5f2ff5d04..9723ac8af4 100644 --- a/spec/graphql/mutations/todos/mark_done_spec.rb +++ b/spec/graphql/mutations/todos/mark_done_spec.rb @@ -5,17 +5,23 @@ require 'spec_helper' RSpec.describe Mutations::Todos::MarkDone do include GraphqlHelpers + let_it_be(:project) { create(:project) } + let_it_be(:issue) { create(:issue, project: project) } let_it_be(:current_user) { create(:user) } let_it_be(:author) { create(:user) } let_it_be(:other_user) { create(:user) } - let_it_be(:todo1) { create(:todo, user: current_user, author: author, state: :pending) } - let_it_be(:todo2) { create(:todo, user: current_user, author: author, state: :done) } + let_it_be(:todo1) { create(:todo, user: current_user, author: author, state: :pending, target: issue) } + let_it_be(:todo2) { create(:todo, user: current_user, author: author, state: :done, target: issue) } let_it_be(:other_user_todo) { create(:todo, user: other_user, author: author, state: :pending) } let(:mutation) { described_class.new(object: nil, context: { current_user: current_user }, field: nil) } + before_all do + project.add_developer(current_user) + end + specify { expect(described_class).to require_graphql_authorizations(:update_todo) } describe '#resolve' do diff --git a/spec/graphql/mutations/todos/restore_spec.rb b/spec/graphql/mutations/todos/restore_spec.rb index 22fb1bba7a..954bb3db66 100644 --- a/spec/graphql/mutations/todos/restore_spec.rb +++ b/spec/graphql/mutations/todos/restore_spec.rb @@ -5,17 +5,23 @@ require 'spec_helper' RSpec.describe Mutations::Todos::Restore do include GraphqlHelpers + let_it_be(:project) { create(:project) } + let_it_be(:issue) { create(:issue, project: project) } let_it_be(:current_user) { create(:user) } let_it_be(:author) { create(:user) } let_it_be(:other_user) { create(:user) } - let_it_be(:todo1) { create(:todo, user: current_user, author: author, state: :done) } - let_it_be(:todo2) { create(:todo, user: current_user, author: author, state: :pending) } + let_it_be(:todo1) { create(:todo, user: current_user, author: author, state: :done, target: issue) } + let_it_be(:todo2) { create(:todo, user: current_user, author: author, state: :pending, target: issue) } let_it_be(:other_user_todo) { create(:todo, user: other_user, author: author, state: :done) } let(:mutation) { described_class.new(object: nil, context: { current_user: current_user }, field: nil) } + before_all do + project.add_developer(current_user) + end + specify { expect(described_class).to require_graphql_authorizations(:update_todo) } describe '#resolve' do diff --git a/spec/graphql/resolvers/project_pipeline_statistics_resolver_spec.rb b/spec/graphql/resolvers/project_pipeline_statistics_resolver_spec.rb index c0367f7d42..ccc861baae 100644 --- a/spec/graphql/resolvers/project_pipeline_statistics_resolver_spec.rb +++ b/spec/graphql/resolvers/project_pipeline_statistics_resolver_spec.rb @@ -5,14 +5,24 @@ require 'spec_helper' RSpec.describe Resolvers::ProjectPipelineStatisticsResolver do include GraphqlHelpers - let_it_be(:project) { create(:project) } + let_it_be(:project) { create(:project, :private) } + let_it_be(:guest) { create(:user) } + let_it_be(:reporter) { create(:user) } + + let(:current_user) { reporter } + + before_all do + project.add_guest(guest) + project.add_reporter(reporter) + end specify do expect(described_class).to have_nullable_graphql_type(::Types::Ci::AnalyticsType) end def resolve_statistics(project, args) - resolve(described_class, obj: project, args: args) + ctx = { current_user: current_user } + resolve(described_class, obj: project, args: args, ctx: ctx) end describe '#resolve' do @@ -32,5 +42,15 @@ RSpec.describe Resolvers::ProjectPipelineStatisticsResolver do :pipeline_times_values ) end + + context 'when the user does not have access to the CI/CD analytics data' do + let(:current_user) { guest } + + it 'returns nil' do + result = resolve_statistics(project, {}) + + expect(result).to be_nil + end + end end end diff --git a/spec/graphql/resolvers/todo_resolver_spec.rb b/spec/graphql/resolvers/todo_resolver_spec.rb index ac14852b36..0760935a2f 100644 --- a/spec/graphql/resolvers/todo_resolver_spec.rb +++ b/spec/graphql/resolvers/todo_resolver_spec.rb @@ -4,19 +4,28 @@ require 'spec_helper' RSpec.describe Resolvers::TodoResolver do include GraphqlHelpers + include DesignManagementTestHelpers specify do expect(described_class).to have_nullable_graphql_type(Types::TodoType.connection_type) end describe '#resolve' do + let_it_be(:project) { create(:project) } let_it_be(:current_user) { create(:user) } + let_it_be(:issue) { create(:issue, project: project) } let_it_be(:author1) { create(:user) } let_it_be(:author2) { create(:user) } - let_it_be(:merge_request_todo_pending) { create(:todo, user: current_user, target_type: 'MergeRequest', state: :pending, action: Todo::MENTIONED, author: author1) } - let_it_be(:issue_todo_done) { create(:todo, user: current_user, state: :done, action: Todo::ASSIGNED, author: author2) } - let_it_be(:issue_todo_pending) { create(:todo, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1) } + let_it_be(:issue_todo_done) { create(:todo, user: current_user, state: :done, action: Todo::ASSIGNED, author: author2, target: issue) } + let_it_be(:issue_todo_pending) { create(:todo, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1, target: issue) } + + let(:merge_request) { create(:merge_request, source_project: project) } + let!(:merge_request_todo_pending) { create(:todo, user: current_user, target: merge_request, state: :pending, action: Todo::MENTIONED, author: author1) } + + before_all do + project.add_developer(current_user) + end it 'calls TodosFinder' do expect_next_instance_of(TodosFinder) do |finder| @@ -40,7 +49,9 @@ RSpec.describe Resolvers::TodoResolver do end it 'returns the todos for multiple filters' do - design_todo_pending = create(:todo, target_type: 'DesignManagement::Design', user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1) + enable_design_management + design = create(:design, issue: issue) + design_todo_pending = create(:todo, target: design, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1) todos = resolve_todos(type: ['MergeRequest', 'DesignManagement::Design']) @@ -59,11 +70,15 @@ RSpec.describe Resolvers::TodoResolver do group3 = create(:group) group1.add_developer(current_user) + issue1 = create(:issue, project: create(:project, group: group1)) group2.add_developer(current_user) + issue2 = create(:issue, project: create(:project, group: group2)) + group3.add_developer(current_user) + issue3 = create(:issue, project: create(:project, group: group3)) - todo4 = create(:todo, group: group1, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1) - todo5 = create(:todo, group: group2, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1) - create(:todo, group: group3, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1) + todo4 = create(:todo, group: group1, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1, target: issue1) + todo5 = create(:todo, group: group2, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1, target: issue2) + create(:todo, group: group3, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1, target: issue3) todos = resolve_todos(group_id: [group2.id, group1.id]) @@ -93,9 +108,13 @@ RSpec.describe Resolvers::TodoResolver do project2 = create(:project) project3 = create(:project) - todo4 = create(:todo, project: project1, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1) - todo5 = create(:todo, project: project2, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1) - create(:todo, project: project3, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1) + project1.add_developer(current_user) + project2.add_developer(current_user) + project3.add_developer(current_user) + + todo4 = create(:todo, project: project1, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1, target: create(:issue, project: project1)) + todo5 = create(:todo, project: project2, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1, target: create(:issue, project: project2)) + create(:todo, project: project3, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1, target: create(:issue, project: project3)) todos = resolve_todos(project_id: [project2.id, project1.id]) diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 7f06e66ad5..2d1caf53bc 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -336,6 +336,15 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do expect_results_with_abilities(impersonation_token, described_class.full_authentication_abilities) end + it 'fails if it is an impersonation token but impersonation is blocked' do + stub_config_setting(impersonation_enabled: false) + + impersonation_token = create(:personal_access_token, :impersonation, scopes: ['api']) + + expect(gl_auth.find_for_git_client('', impersonation_token.token, project: nil, ip: 'ip')) + .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil)) + end + it 'limits abilities based on scope' do personal_access_token = create(:personal_access_token, scopes: %w[read_user sudo]) diff --git a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb index 6019318a40..7771289abe 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb @@ -136,7 +136,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Build do let(:command) do Gitlab::Ci::Pipeline::Chain::Command.new( source: :push, - origin_ref: 'mytag', + origin_ref: origin_ref, checkout_sha: project.commit.id, after_sha: nil, before_sha: nil, @@ -147,6 +147,8 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Build do current_user: user) end + let(:origin_ref) { 'mytag' } + before do allow_any_instance_of(Repository).to receive(:tag_exists?).with('mytag').and_return(true) @@ -156,6 +158,14 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Build do it 'correctly indicated that this is a tagged pipeline' do expect(pipeline).to be_tag end + + context 'when origin_ref is branch but tag ref with the same name exists' do + let(:origin_ref) { 'refs/heads/mytag' } + + it 'correctly indicated that a pipeline is not tagged' do + expect(pipeline).not_to be_tag + end + end end context 'when pipeline is running for a merge request' do diff --git a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb index 900dfec38e..2e73043e30 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb @@ -27,6 +27,18 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Command do it { is_expected.to eq(true) } end + context 'for fully described tag ref' do + let(:origin_ref) { 'refs/tags/master' } + + it { is_expected.to eq(false) } + end + + context 'for fully described branch ref' do + let(:origin_ref) { 'refs/heads/master' } + + it { is_expected.to eq(true) } + end + context 'for invalid branch' do let(:origin_ref) { 'something' } @@ -43,6 +55,18 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Command do it { is_expected.to eq(true) } end + context 'for fully described tag ref' do + let(:origin_ref) { 'refs/tags/v1.0.0' } + + it { is_expected.to eq(true) } + end + + context 'for fully described branch ref' do + let(:origin_ref) { 'refs/heads/v1.0.0' } + + it { is_expected.to eq(false) } + end + context 'for invalid ref' do let(:origin_ref) { 'something' } diff --git a/spec/lib/gitlab/omniauth_logging/json_formatter_spec.rb b/spec/lib/gitlab/omniauth_logging/json_formatter_spec.rb deleted file mode 100644 index f65b247d5d..0000000000 --- a/spec/lib/gitlab/omniauth_logging/json_formatter_spec.rb +++ /dev/null @@ -1,12 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::OmniauthLogging::JSONFormatter do - it "generates log in json format" do - Timecop.freeze(Time.utc(2019, 12, 04, 9, 10, 11, 123456)) do - expect(subject.call(:info, Time.now, 'omniauth', 'log message')) - .to eq %Q({"severity":"info","timestamp":"2019-12-04T09:10:11.123Z","pid":#{Process.pid},"progname":"omniauth","message":"log message"}\n) - end - end -end diff --git a/spec/lib/sidebars/projects/menus/analytics_menu_spec.rb b/spec/lib/sidebars/projects/menus/analytics_menu_spec.rb index ed94b81520..9d5f029fff 100644 --- a/spec/lib/sidebars/projects/menus/analytics_menu_spec.rb +++ b/spec/lib/sidebars/projects/menus/analytics_menu_spec.rb @@ -4,15 +4,19 @@ require 'spec_helper' RSpec.describe Sidebars::Projects::Menus::AnalyticsMenu do let_it_be(:project) { create(:project, :repository) } + let_it_be(:guest) do + create(:user).tap { |u| project.add_guest(u) } + end - let(:user) { project.owner } - let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project, current_ref: project.repository.root_ref) } + let(:owner) { project.owner } + let(:current_user) { owner } + let(:context) { Sidebars::Projects::Context.new(current_user: current_user, container: project, current_ref: project.repository.root_ref) } subject { described_class.new(context) } describe '#render?' do context 'whe user cannot read analytics' do - let(:user) { nil } + let(:current_user) { nil } it 'returns false' do expect(subject.render?).to be false @@ -79,7 +83,7 @@ RSpec.describe Sidebars::Projects::Menus::AnalyticsMenu do end describe 'when the user does not have access' do - let(:user) { nil } + let(:current_user) { guest } specify { is_expected.to be_nil } end @@ -99,7 +103,7 @@ RSpec.describe Sidebars::Projects::Menus::AnalyticsMenu do end describe 'when the user does not have access' do - let(:user) { nil } + let(:current_user) { nil } specify { is_expected.to be_nil } end @@ -111,7 +115,7 @@ RSpec.describe Sidebars::Projects::Menus::AnalyticsMenu do specify { is_expected.not_to be_nil } describe 'when the user does not have access' do - let(:user) { nil } + let(:current_user) { nil } specify { is_expected.to be_nil } end diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb index 215c733f26..2be7ad5f8c 100644 --- a/spec/models/diff_note_spec.rb +++ b/spec/models/diff_note_spec.rb @@ -552,4 +552,10 @@ RSpec.describe DiffNote do expect(subject.on_image?).to be_truthy end end + + describe '#to_ability_name' do + subject { described_class.new.to_ability_name } + + it { is_expected.to eq('note') } + end end diff --git a/spec/models/discussion_note_spec.rb b/spec/models/discussion_note_spec.rb new file mode 100644 index 0000000000..6e1b39cc43 --- /dev/null +++ b/spec/models/discussion_note_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe DiscussionNote do + describe '#to_ability_name' do + subject { described_class.new.to_ability_name } + + it { is_expected.to eq('note') } + end +end diff --git a/spec/models/legacy_diff_note_spec.rb b/spec/models/legacy_diff_note_spec.rb new file mode 100644 index 0000000000..ee3bbf186b --- /dev/null +++ b/spec/models/legacy_diff_note_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe LegacyDiffNote do + describe '#to_ability_name' do + subject { described_class.new.to_ability_name } + + it { is_expected.to eq('note') } + end +end diff --git a/spec/models/synthetic_note_spec.rb b/spec/models/synthetic_note_spec.rb new file mode 100644 index 0000000000..55fa4f7c33 --- /dev/null +++ b/spec/models/synthetic_note_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe SyntheticNote do + describe '#to_ability_name' do + subject { described_class.new.to_ability_name } + + it { is_expected.to eq('note') } + end +end diff --git a/spec/policies/personal_access_token_policy_spec.rb b/spec/policies/personal_access_token_policy_spec.rb index b5e8d40b13..e146133429 100644 --- a/spec/policies/personal_access_token_policy_spec.rb +++ b/spec/policies/personal_access_token_policy_spec.rb @@ -41,6 +41,13 @@ RSpec.describe PersonalAccessTokenPolicy do it { is_expected.to be_allowed(:read_token) } it { is_expected.to be_allowed(:revoke_token) } end + + context 'subject of the impersonated token' do + let_it_be(:token) { build_stubbed(:personal_access_token, user: current_user, impersonation: true) } + + it { is_expected.to be_disallowed(:read_token) } + it { is_expected.to be_disallowed(:revoke_token) } + end end context 'current_user is a blocked administrator', :enable_admin_mode do diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 46da42a478..17f4f95bef 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -1099,12 +1099,20 @@ RSpec.describe ProjectPolicy do let_it_be(:project_with_analytics_enabled) { create(:project, :analytics_enabled) } before do + project_with_analytics_disabled.add_guest(guest) + project_with_analytics_private.add_guest(guest) + project_with_analytics_enabled.add_guest(guest) + + project_with_analytics_disabled.add_reporter(reporter) + project_with_analytics_private.add_reporter(reporter) + project_with_analytics_enabled.add_reporter(reporter) + project_with_analytics_disabled.add_developer(developer) project_with_analytics_private.add_developer(developer) project_with_analytics_enabled.add_developer(developer) end - context 'when analytics is enabled for the project' do + context 'when analytics is disabled for the project' do let(:project) { project_with_analytics_disabled } context 'for guest user' do @@ -1113,6 +1121,16 @@ RSpec.describe ProjectPolicy do it { is_expected.to be_disallowed(:read_cycle_analytics) } it { is_expected.to be_disallowed(:read_insights) } it { is_expected.to be_disallowed(:read_repository_graphs) } + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + end + + context 'for reporter user' do + let(:current_user) { reporter } + + it { is_expected.to be_disallowed(:read_cycle_analytics) } + it { is_expected.to be_disallowed(:read_insights) } + it { is_expected.to be_disallowed(:read_repository_graphs) } + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } end context 'for developer' do @@ -1121,6 +1139,7 @@ RSpec.describe ProjectPolicy do it { is_expected.to be_disallowed(:read_cycle_analytics) } it { is_expected.to be_disallowed(:read_insights) } it { is_expected.to be_disallowed(:read_repository_graphs) } + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } end end @@ -1130,9 +1149,19 @@ RSpec.describe ProjectPolicy do context 'for guest user' do let(:current_user) { guest } - it { is_expected.to be_disallowed(:read_cycle_analytics) } - it { is_expected.to be_disallowed(:read_insights) } + it { is_expected.to be_allowed(:read_cycle_analytics) } + it { is_expected.to be_allowed(:read_insights) } it { is_expected.to be_disallowed(:read_repository_graphs) } + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + end + + context 'for reporter user' do + let(:current_user) { reporter } + + it { is_expected.to be_allowed(:read_cycle_analytics) } + it { is_expected.to be_allowed(:read_insights) } + it { is_expected.to be_allowed(:read_repository_graphs) } + it { is_expected.to be_allowed(:read_ci_cd_analytics) } end context 'for developer' do @@ -1141,18 +1170,29 @@ RSpec.describe ProjectPolicy do it { is_expected.to be_allowed(:read_cycle_analytics) } it { is_expected.to be_allowed(:read_insights) } it { is_expected.to be_allowed(:read_repository_graphs) } + it { is_expected.to be_allowed(:read_ci_cd_analytics) } end end context 'when analytics is enabled for the project' do - let(:project) { project_with_analytics_private } + let(:project) { project_with_analytics_enabled } context 'for guest user' do let(:current_user) { guest } - it { is_expected.to be_disallowed(:read_cycle_analytics) } - it { is_expected.to be_disallowed(:read_insights) } + it { is_expected.to be_allowed(:read_cycle_analytics) } + it { is_expected.to be_allowed(:read_insights) } it { is_expected.to be_disallowed(:read_repository_graphs) } + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + end + + context 'for reporter user' do + let(:current_user) { reporter } + + it { is_expected.to be_allowed(:read_cycle_analytics) } + it { is_expected.to be_allowed(:read_insights) } + it { is_expected.to be_allowed(:read_repository_graphs) } + it { is_expected.to be_allowed(:read_ci_cd_analytics) } end context 'for developer' do @@ -1161,6 +1201,7 @@ RSpec.describe ProjectPolicy do it { is_expected.to be_allowed(:read_cycle_analytics) } it { is_expected.to be_allowed(:read_insights) } it { is_expected.to be_allowed(:read_repository_graphs) } + it { is_expected.to be_allowed(:read_ci_cd_analytics) } end end end diff --git a/spec/policies/todo_policy_spec.rb b/spec/policies/todo_policy_spec.rb index b4876baa50..16435b2166 100644 --- a/spec/policies/todo_policy_spec.rb +++ b/spec/policies/todo_policy_spec.rb @@ -9,22 +9,28 @@ RSpec.describe TodoPolicy do let_it_be(:user2) { create(:user) } let_it_be(:user3) { create(:user) } - let_it_be(:todo1) { create(:todo, author: author, user: user1) } - let_it_be(:todo2) { create(:todo, author: author, user: user2) } + let_it_be(:project) { create(:project) } + let_it_be(:issue) { create(:issue, project: project) } + + let_it_be(:todo1) { create(:todo, author: author, user: user1, issue: issue) } + let_it_be(:todo2) { create(:todo, author: author, user: user2, issue: issue) } let_it_be(:todo3) { create(:todo, author: author, user: user2) } - let_it_be(:todo4) { create(:todo, author: author, user: user3) } + let_it_be(:todo4) { create(:todo, author: author, user: user3, issue: issue) } def permissions(user, todo) described_class.new(user, todo) end + before_all do + project.add_developer(user1) + project.add_developer(user2) + end + describe 'own_todo' do - it 'allows owners to access their own todos' do + it 'allows owners to access their own todos if they can read todo target' do [ [user1, todo1], - [user2, todo2], - [user2, todo3], - [user3, todo4] + [user2, todo2] ].each do |user, todo| expect(permissions(user, todo)).to be_allowed(:read_todo) end @@ -38,7 +44,9 @@ RSpec.describe TodoPolicy do [user2, todo4], [user3, todo1], [user3, todo2], - [user3, todo3] + [user3, todo3], + [user2, todo3], + [user3, todo4] ].each do |user, todo| expect(permissions(user, todo)).to be_disallowed(:read_todo) end diff --git a/spec/requests/admin/impersonation_tokens_controller_spec.rb b/spec/requests/admin/impersonation_tokens_controller_spec.rb new file mode 100644 index 0000000000..018f497e7e --- /dev/null +++ b/spec/requests/admin/impersonation_tokens_controller_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Admin::ImpersonationTokensController, :enable_admin_mode do + let(:admin) { create(:admin) } + let!(:user) { create(:user) } + + before do + sign_in(admin) + end + + context "when impersonation is disabled" do + before do + stub_config_setting(impersonation_enabled: false) + end + + it "shows error page for index page" do + get admin_user_impersonation_tokens_path(user_id: user.username) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it "responds with 404 for create action" do + post admin_user_impersonation_tokens_path(user_id: user.username) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it "responds with 404 for revoke action" do + token = create(:personal_access_token, :impersonation, user: user) + + put revoke_admin_user_impersonation_token_path(user_id: user.username, id: token.id) + + expect(response).to have_gitlab_http_status(:not_found) + end + end +end diff --git a/spec/requests/api/graphql/current_user/todos_query_spec.rb b/spec/requests/api/graphql/current_user/todos_query_spec.rb index e298de0df0..981b10a746 100644 --- a/spec/requests/api/graphql/current_user/todos_query_spec.rb +++ b/spec/requests/api/graphql/current_user/todos_query_spec.rb @@ -4,12 +4,17 @@ require 'spec_helper' RSpec.describe 'Query current user todos' do include GraphqlHelpers + include DesignManagementTestHelpers let_it_be(:current_user) { create(:user) } - let_it_be(:commit_todo) { create(:on_commit_todo, user: current_user, project: create(:project, :repository)) } - let_it_be(:issue_todo) { create(:todo, user: current_user, target: create(:issue)) } - let_it_be(:merge_request_todo) { create(:todo, user: current_user, target: create(:merge_request)) } - let_it_be(:design_todo) { create(:todo, user: current_user, target: create(:design)) } + let_it_be(:project) { create(:project, :repository) } + let_it_be(:unauthorize_project) { create(:project) } + let_it_be(:commit_todo) { create(:on_commit_todo, user: current_user, project: project) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:issue_todo) { create(:todo, project: project, user: current_user, target: issue) } + let_it_be(:merge_request_todo) { create(:todo, project: project, user: current_user, target: create(:merge_request, source_project: project)) } + let_it_be(:design_todo) { create(:todo, project: project, user: current_user, target: create(:design, issue: issue)) } + let_it_be(:unauthorized_todo) { create(:todo, user: current_user, project: unauthorize_project, target: create(:issue, project: unauthorize_project)) } let(:fields) do <<~QUERY @@ -23,16 +28,22 @@ RSpec.describe 'Query current user todos' do graphql_query_for('currentUser', {}, query_graphql_field('todos', {}, fields)) end + before_all do + project.add_developer(current_user) + end + subject { graphql_data.dig('currentUser', 'todos', 'nodes') } before do + enable_design_management + post_graphql(query, current_user: current_user) end it_behaves_like 'a working graphql query' it 'contains the expected ids' do - is_expected.to include( + is_expected.to contain_exactly( a_hash_including('id' => commit_todo.to_global_id.to_s), a_hash_including('id' => issue_todo.to_global_id.to_s), a_hash_including('id' => merge_request_todo.to_global_id.to_s), @@ -41,11 +52,33 @@ RSpec.describe 'Query current user todos' do end it 'returns Todos for all target types' do - is_expected.to include( + is_expected.to contain_exactly( a_hash_including('targetType' => 'COMMIT'), a_hash_including('targetType' => 'ISSUE'), a_hash_including('targetType' => 'MERGEREQUEST'), a_hash_including('targetType' => 'DESIGN') ) end + + context 'when requesting a single field' do + let(:fields) do + <<~QUERY + nodes { + id + } + QUERY + end + + it 'avoids N+1 queries', :request_store do + control = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } + + project2 = create(:project) + project2.add_developer(current_user) + issue2 = create(:issue, project: project2) + create(:todo, user: current_user, target: issue2, project: project2) + + # An additional query is made for each different group that owns a todo through a project + expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(control).with_threshold(2) + end + end end diff --git a/spec/requests/api/graphql/mutations/todos/mark_all_done_spec.rb b/spec/requests/api/graphql/mutations/todos/mark_all_done_spec.rb index 705ef28ffd..f2bee8cc30 100644 --- a/spec/requests/api/graphql/mutations/todos/mark_all_done_spec.rb +++ b/spec/requests/api/graphql/mutations/todos/mark_all_done_spec.rb @@ -5,14 +5,16 @@ require 'spec_helper' RSpec.describe 'Marking all todos done' do include GraphqlHelpers + let_it_be(:project) { create(:project) } + let_it_be(:issue) { create(:issue, project: project) } let_it_be(:current_user) { create(:user) } let_it_be(:author) { create(:user) } let_it_be(:other_user) { create(:user) } let_it_be(:other_user2) { create(:user) } - let_it_be(:todo1) { create(:todo, user: current_user, author: author, state: :pending) } - let_it_be(:todo2) { create(:todo, user: current_user, author: author, state: :done) } - let_it_be(:todo3) { create(:todo, user: current_user, author: author, state: :pending) } + let_it_be(:todo1) { create(:todo, user: current_user, author: author, state: :pending, target: issue) } + let_it_be(:todo2) { create(:todo, user: current_user, author: author, state: :done, target: issue) } + let_it_be(:todo3) { create(:todo, user: current_user, author: author, state: :pending, target: issue) } let_it_be(:other_user_todo) { create(:todo, user: other_user, author: author, state: :pending) } @@ -28,6 +30,10 @@ RSpec.describe 'Marking all todos done' do ) end + before_all do + project.add_developer(current_user) + end + def mutation_response graphql_mutation_response(:todos_mark_all_done) end diff --git a/spec/requests/api/graphql/mutations/todos/mark_done_spec.rb b/spec/requests/api/graphql/mutations/todos/mark_done_spec.rb index 8a9a0b9e84..7f5ea71c76 100644 --- a/spec/requests/api/graphql/mutations/todos/mark_done_spec.rb +++ b/spec/requests/api/graphql/mutations/todos/mark_done_spec.rb @@ -5,12 +5,14 @@ require 'spec_helper' RSpec.describe 'Marking todos done' do include GraphqlHelpers + let_it_be(:project) { create(:project) } + let_it_be(:issue) { create(:issue, project: project) } let_it_be(:current_user) { create(:user) } let_it_be(:author) { create(:user) } let_it_be(:other_user) { create(:user) } - let_it_be(:todo1) { create(:todo, user: current_user, author: author, state: :pending) } - let_it_be(:todo2) { create(:todo, user: current_user, author: author, state: :done) } + let_it_be(:todo1) { create(:todo, user: current_user, author: author, state: :pending, target: issue) } + let_it_be(:todo2) { create(:todo, user: current_user, author: author, state: :done, target: issue) } let_it_be(:other_user_todo) { create(:todo, user: other_user, author: author, state: :pending) } @@ -29,6 +31,10 @@ RSpec.describe 'Marking todos done' do ) end + before_all do + project.add_developer(current_user) + end + def mutation_response graphql_mutation_response(:todo_mark_done) end diff --git a/spec/requests/api/graphql/mutations/todos/restore_many_spec.rb b/spec/requests/api/graphql/mutations/todos/restore_many_spec.rb index 3e96d5c505..d44b9419b4 100644 --- a/spec/requests/api/graphql/mutations/todos/restore_many_spec.rb +++ b/spec/requests/api/graphql/mutations/todos/restore_many_spec.rb @@ -5,12 +5,14 @@ require 'spec_helper' RSpec.describe 'Restoring many Todos' do include GraphqlHelpers + let_it_be(:project) { create(:project) } + let_it_be(:issue) { create(:issue, project: project) } let_it_be(:current_user) { create(:user) } let_it_be(:author) { create(:user) } let_it_be(:other_user) { create(:user) } - let_it_be(:todo1) { create(:todo, user: current_user, author: author, state: :done) } - let_it_be(:todo2) { create(:todo, user: current_user, author: author, state: :done) } + let_it_be(:todo1) { create(:todo, user: current_user, author: author, state: :done, target: issue) } + let_it_be(:todo2) { create(:todo, user: current_user, author: author, state: :done, target: issue) } let_it_be(:other_user_todo) { create(:todo, user: other_user, author: author, state: :done) } @@ -31,6 +33,10 @@ RSpec.describe 'Restoring many Todos' do ) end + before_all do + project.add_developer(current_user) + end + def mutation_response graphql_mutation_response(:todo_restore_many) end diff --git a/spec/requests/api/graphql/mutations/todos/restore_spec.rb b/spec/requests/api/graphql/mutations/todos/restore_spec.rb index a58c7fc69f..d995191c97 100644 --- a/spec/requests/api/graphql/mutations/todos/restore_spec.rb +++ b/spec/requests/api/graphql/mutations/todos/restore_spec.rb @@ -5,12 +5,14 @@ require 'spec_helper' RSpec.describe 'Restoring Todos' do include GraphqlHelpers + let_it_be(:project) { create(:project) } + let_it_be(:issue) { create(:issue, project: project) } let_it_be(:current_user) { create(:user) } let_it_be(:author) { create(:user) } let_it_be(:other_user) { create(:user) } - let_it_be(:todo1) { create(:todo, user: current_user, author: author, state: :done) } - let_it_be(:todo2) { create(:todo, user: current_user, author: author, state: :pending) } + let_it_be(:todo1) { create(:todo, user: current_user, author: author, state: :done, target: issue) } + let_it_be(:todo2) { create(:todo, user: current_user, author: author, state: :pending, target: issue) } let_it_be(:other_user_todo) { create(:todo, user: other_user, author: author, state: :done) } @@ -29,6 +31,10 @@ RSpec.describe 'Restoring Todos' do ) end + before_all do + project.add_developer(current_user) + end + def mutation_response graphql_mutation_response(:todo_restore) end diff --git a/spec/requests/api/personal_access_tokens_spec.rb b/spec/requests/api/personal_access_tokens_spec.rb index ccc5f322ff..0ff2c46e69 100644 --- a/spec/requests/api/personal_access_tokens_spec.rb +++ b/spec/requests/api/personal_access_tokens_spec.rb @@ -6,6 +6,7 @@ RSpec.describe API::PersonalAccessTokens do let_it_be(:path) { '/personal_access_tokens' } let_it_be(:token1) { create(:personal_access_token) } let_it_be(:token2) { create(:personal_access_token) } + let_it_be(:token_impersonated) { create(:personal_access_token, impersonation: true, user: token1.user) } let_it_be(:current_user) { create(:user) } describe 'GET /personal_access_tokens' do @@ -24,8 +25,9 @@ RSpec.describe API::PersonalAccessTokens do get api(path, current_user), params: { user_id: token1.user.id } expect(response).to have_gitlab_http_status(:ok) - expect(json_response.count).to eq(1) + expect(json_response.count).to eq(2) expect(json_response.first['user_id']).to eq(token1.user.id) + expect(json_response.last['id']).to eq(token_impersonated.id) end end @@ -34,6 +36,7 @@ RSpec.describe API::PersonalAccessTokens do let_it_be(:user) { create(:user) } let_it_be(:token) { create(:personal_access_token, user: current_user)} let_it_be(:other_token) { create(:personal_access_token, user: user) } + let_it_be(:token_impersonated) { create(:personal_access_token, impersonation: true, user: current_user) } it 'returns all PATs belonging to the signed-in user' do get api(path, current_user, personal_access_token: token) @@ -95,6 +98,7 @@ RSpec.describe API::PersonalAccessTokens do context 'when current_user is not an administrator' do let_it_be(:user_token) { create(:personal_access_token, user: current_user) } let_it_be(:user_token_path) { "/personal_access_tokens/#{user_token.id}" } + let_it_be(:token_impersonated) { create(:personal_access_token, impersonation: true, user: current_user) } it 'fails revokes a different users token' do delete api(path, current_user) @@ -107,6 +111,12 @@ RSpec.describe API::PersonalAccessTokens do expect(response).to have_gitlab_http_status(:no_content) end + + it 'cannot revoke impersonation token' do + delete api("/personal_access_tokens/#{token_impersonated.id}", current_user) + + expect(response).to have_gitlab_http_status(:bad_request) + end end end end diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb index 00de1ef596..d31f571e63 100644 --- a/spec/requests/api/todos_spec.rb +++ b/spec/requests/api/todos_spec.rb @@ -3,18 +3,22 @@ require 'spec_helper' RSpec.describe API::Todos do + include DesignManagementTestHelpers + let_it_be(:group) { create(:group) } let_it_be(:project_1) { create(:project, :repository, group: group) } let_it_be(:project_2) { create(:project) } let_it_be(:author_1) { create(:user) } let_it_be(:author_2) { create(:user) } let_it_be(:john_doe) { create(:user, username: 'john_doe') } + let_it_be(:issue) { create(:issue, project: project_1) } let_it_be(:merge_request) { create(:merge_request, source_project: project_1) } let_it_be(:merge_request_todo) { create(:todo, project: project_1, author: author_2, user: john_doe, target: merge_request) } - let_it_be(:pending_1) { create(:todo, :mentioned, project: project_1, author: author_1, user: john_doe) } - let_it_be(:pending_2) { create(:todo, project: project_2, author: author_2, user: john_doe) } + let_it_be(:pending_1) { create(:todo, :mentioned, project: project_1, author: author_1, user: john_doe, target: issue) } + let_it_be(:pending_2) { create(:todo, project: project_2, author: author_2, user: john_doe, target: issue) } let_it_be(:pending_3) { create(:on_commit_todo, project: project_1, author: author_2, user: john_doe) } - let_it_be(:done) { create(:todo, :done, project: project_1, author: author_1, user: john_doe) } + let_it_be(:pending_4) { create(:on_commit_todo, project: project_1, author: author_2, user: john_doe, commit_id: 'invalid_id') } + let_it_be(:done) { create(:todo, :done, project: project_1, author: author_1, user: john_doe, target: issue) } let_it_be(:award_emoji_1) { create(:award_emoji, awardable: merge_request, user: author_1, name: 'thumbsup') } let_it_be(:award_emoji_2) { create(:award_emoji, awardable: pending_1.target, user: author_1, name: 'thumbsup') } let_it_be(:award_emoji_3) { create(:award_emoji, awardable: pending_2.target, user: author_2, name: 'thumbsdown') } @@ -77,13 +81,13 @@ RSpec.describe API::Todos do expect(json_response[0]['target_type']).to eq('Commit') expect(json_response[1]['target_type']).to eq('Issue') - expect(json_response[1]['target']['upvotes']).to eq(0) + expect(json_response[1]['target']['upvotes']).to eq(1) expect(json_response[1]['target']['downvotes']).to eq(1) expect(json_response[1]['target']['merge_requests_count']).to eq(0) expect(json_response[2]['target_type']).to eq('Issue') expect(json_response[2]['target']['upvotes']).to eq(1) - expect(json_response[2]['target']['downvotes']).to eq(0) + expect(json_response[2]['target']['downvotes']).to eq(1) expect(json_response[2]['target']['merge_requests_count']).to eq(0) expect(json_response[3]['target_type']).to eq('MergeRequest') @@ -93,6 +97,19 @@ RSpec.describe API::Todos do expect(json_response[3]['target']['downvotes']).to eq(0) end + context "when current user does not have access to one of the TODO's target" do + it 'filters out unauthorized todos' do + no_access_project = create(:project, :repository, group: group) + no_access_merge_request = create(:merge_request, source_project: no_access_project) + no_access_todo = create(:todo, project: no_access_project, author: author_2, user: john_doe, target: no_access_merge_request) + + get api('/todos', john_doe) + + expect(json_response.count).to eq(4) + expect(json_response.map { |t| t['id'] }).not_to include(no_access_todo.id, pending_4.id) + end + end + context 'and using the author filter' do it 'filters based on author_id param' do get api('/todos', john_doe), params: { author_id: author_2.id } @@ -163,23 +180,31 @@ RSpec.describe API::Todos do end it 'avoids N+1 queries', :request_store do + create_issue_todo_for(john_doe) create(:todo, project: project_1, author: author_2, user: john_doe, target: merge_request) get api('/todos', john_doe) - control = ActiveRecord::QueryRecorder.new { get api('/todos', john_doe) } + control1 = ActiveRecord::QueryRecorder.new { get api('/todos', john_doe) } - merge_request_2 = create(:merge_request, source_project: project_2) - create(:todo, project: project_2, author: author_2, user: john_doe, target: merge_request_2) + create_issue_todo_for(john_doe) + create_mr_todo_for(john_doe, project_2) + create(:todo, :mentioned, project: project_1, author: author_1, user: john_doe, target: merge_request) + new_todo = create_mr_todo_for(john_doe) + merge_request_3 = create(:merge_request, :jira_branch, source_project: new_todo.project) + create(:on_commit_todo, project: new_todo.project, author: author_1, user: john_doe, target: merge_request_3) + create(:todo, project: new_todo.project, author: author_2, user: john_doe, target: merge_request_3) - project_3 = create(:project, :repository) - project_3.add_developer(john_doe) - merge_request_3 = create(:merge_request, source_project: project_3) - create(:todo, project: project_3, author: author_2, user: john_doe, target: merge_request_3) - create(:todo, :mentioned, project: project_1, author: author_1, user: john_doe) - create(:on_commit_todo, project: project_3, author: author_1, user: john_doe) + expect { get api('/todos', john_doe) }.not_to exceed_query_limit(control1).with_threshold(4) + control2 = ActiveRecord::QueryRecorder.new { get api('/todos', john_doe) } + + create_issue_todo_for(john_doe) + create_issue_todo_for(john_doe, project_1) + create_issue_todo_for(john_doe, project_1) + + # Additional query only when target belongs to project from different group + expect { get api('/todos', john_doe) }.not_to exceed_query_limit(control2).with_threshold(1) - expect { get api('/todos', john_doe) }.not_to exceed_query_limit(control) expect(response).to have_gitlab_http_status(:ok) end @@ -201,6 +226,8 @@ RSpec.describe API::Todos do end before do + enable_design_management + api_request end @@ -222,6 +249,20 @@ RSpec.describe API::Todos do ) end end + + def create_mr_todo_for(user, project = nil) + new_project = project || create(:project, group: create(:group)) + new_project.add_developer(user) if project.blank? + new_merge_request = create(:merge_request, source_project: new_project) + create(:todo, project: new_project, author: user, user: user, target: new_merge_request) + end + + def create_issue_todo_for(user, project = nil) + new_project = project || create(:project, group: create(:group)) + new_project.group.add_developer(user) if project.blank? + issue = create(:issue, project: new_project) + create(:todo, project: new_project, target: issue, author: user, user: user) + end end describe 'POST /todos/:id/mark_as_done' do diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index c379fd9e73..d6cd82ac76 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -706,6 +706,32 @@ RSpec.describe 'Git HTTP requests' do end end end + + context 'when token is impersonated' do + context 'when impersonation is off' do + before do + stub_config_setting(impersonation_enabled: false) + end + + it 'responds to uploads with status 401 unauthorized' do + write_access_token = create(:personal_access_token, :impersonation, user: user, scopes: [:write_repository]) + + upload(path, user: user.username, password: write_access_token.token) do |response| + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + + context 'when impersonation is on' do + it 'responds to uploads with status 200' do + write_access_token = create(:personal_access_token, :impersonation, user: user, scopes: [:write_repository]) + + upload(path, user: user.username, password: write_access_token.token) do |response| + expect(response).to have_gitlab_http_status(:ok) + end + end + end + end end end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 9fdce1ae92..ca630def4f 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -1156,6 +1156,73 @@ RSpec.describe Ci::CreatePipelineService do end end + context 'when pipeline is running for a nonexistant-branch' do + let(:gitlab_ci_yaml) { YAML.dump(test: { script: 'test' }) } + + let(:ref_name) { 'refs/heads/nonexistant-branch' } + + let(:pipeline) { execute_service } + + it 'does not create the pipeline' do + expect(pipeline).not_to be_created_successfully + expect(pipeline.errors[:base]).to eq(['Reference not found']) + end + + context 'when there is a tag with that nonexistant-branch' do + # v1.0.0 is on the test repo as a tag + let(:ref_name) { 'refs/heads/v1.0.0' } + + it 'does not create the pipeline' do + expect(pipeline).not_to be_created_successfully + expect(pipeline.errors[:base]).to eq(['Reference not found']) + end + end + end + + context 'when pipeline is running for a branch with the name of both a branch and a tag' do + let(:gitlab_ci_yaml) { YAML.dump(test: { script: 'test' }) } + + # v1.1.0 is on the test repo as branch and tag + let(:ref_name) { 'refs/heads/v1.1.0' } + + let(:pipeline) { execute_service } + + it 'creates the pipeline for the branch' do + expect(pipeline).to be_created_successfully + expect(pipeline.branch?).to be true + expect(pipeline.tag?).to be false + end + end + + context 'when pipeline is running for a tag with the name of both a branch and a tag' do + let(:gitlab_ci_yaml) { YAML.dump(test: { script: 'test' }) } + + # v1.1.0 is on the test repo as branch and tag + let(:ref_name) { 'refs/tags/v1.1.0' } + + let(:pipeline) { execute_service } + + it 'creates the pipeline for the tag' do + expect(pipeline).to be_created_successfully + expect(pipeline.branch?).to be false + expect(pipeline.tag?).to be true + end + end + + context 'when pipeline is running for an ambiguous ref' do + let(:gitlab_ci_yaml) { YAML.dump(test: { script: 'test' }) } + + # v1.1.0 is on the test repo as branch and tag + let(:ref_name) { 'v1.1.0' } + + let(:pipeline) { execute_service } + + it 'does not create the pipeline' do + expect(pipeline).not_to be_created_successfully + expect(pipeline.errors[:base]).to eq(['Ref is ambiguous']) + end + end + context 'when pipeline variables are specified' do let(:variables_attributes) do [{ key: 'first', secret_value: 'world' }, diff --git a/spec/services/git/process_ref_changes_service_spec.rb b/spec/services/git/process_ref_changes_service_spec.rb index 087f4ba372..ac9bac4e6a 100644 --- a/spec/services/git/process_ref_changes_service_spec.rb +++ b/spec/services/git/process_ref_changes_service_spec.rb @@ -109,9 +109,13 @@ RSpec.describe Git::ProcessRefChangesService do .to receive(:commit) .and_return(project.commit) - allow_any_instance_of(Repository) - .to receive(:branch_exists?) - .and_return(true) + if changes_method == :branch_changes + allow_any_instance_of(Repository).to receive(:branch_exists?) { true } + end + + if changes_method == :tag_changes + allow_any_instance_of(Repository).to receive(:tag_exists?) { true } + end end context 'when git_push_create_all_pipelines is disabled' do diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 9c84242d8a..f52e86b3f4 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -224,6 +224,27 @@ RSpec.describe Issues::CreateService do end end + context 'when sentry identifier is given' do + before do + sentry_attributes = { sentry_issue_attributes: { sentry_issue_identifier: 42 } } + opts.merge!(sentry_attributes) + end + + context 'user is a guest' do + before do + project.add_guest(user) + end + + it 'does not assign the sentry error' do + expect(issue.sentry_issue).to eq(nil) + end + end + + it 'assigns the sentry error' do + expect(issue.sentry_issue).to be_kind_of(SentryIssue) + end + end + it 'executes issue hooks when issue is not confidential' do opts = { title: 'Title', description: 'Description', confidential: false } diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 8c97dd95ce..7f31e8466c 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -82,6 +82,31 @@ RSpec.describe Issues::UpdateService, :mailer do expect(issue.milestone).to eq milestone end + context 'when sentry identifier is given' do + before do + sentry_attributes = { sentry_issue_attributes: { sentry_issue_identifier: 42 } } + opts.merge!(sentry_attributes) + end + + it 'assigns the sentry error' do + update_issue(opts) + + expect(issue.sentry_issue).to be_kind_of(SentryIssue) + end + + context 'user is a guest' do + before do + project.add_guest(user) + end + + it 'does not assign the sentry error' do + update_issue(opts) + + expect(issue.sentry_issue).to eq(nil) + end + end + end + context 'when issue type is not incident' do it 'returns default severity' do update_issue(opts) diff --git a/spec/services/todos/allowed_target_filter_service_spec.rb b/spec/services/todos/allowed_target_filter_service_spec.rb new file mode 100644 index 0000000000..707df8e851 --- /dev/null +++ b/spec/services/todos/allowed_target_filter_service_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Todos::AllowedTargetFilterService do + include DesignManagementTestHelpers + + let_it_be(:authorized_group) { create(:group, :private) } + let_it_be(:authorized_project) { create(:project, group: authorized_group) } + let_it_be(:unauthorized_group) { create(:group, :private) } + let_it_be(:unauthorized_project) { create(:project, group: unauthorized_group) } + let_it_be(:user) { create(:user) } + let_it_be(:authorized_issue) { create(:issue, project: authorized_project) } + let_it_be(:authorized_issue_todo) { create(:todo, project: authorized_project, target: authorized_issue, user: user) } + let_it_be(:unauthorized_issue) { create(:issue, project: unauthorized_project) } + let_it_be(:unauthorized_issue_todo) { create(:todo, project: unauthorized_project, target: unauthorized_issue, user: user) } + let_it_be(:authorized_design) { create(:design, issue: authorized_issue) } + let_it_be(:authorized_design_todo) { create(:todo, project: authorized_project, target: authorized_design, user: user) } + let_it_be(:unauthorized_design) { create(:design, issue: unauthorized_issue) } + let_it_be(:unauthorized_design_todo) { create(:todo, project: unauthorized_project, target: unauthorized_design, user: user) } + + # Cannot use let_it_be with MRs + let(:authorized_mr) { create(:merge_request, source_project: authorized_project) } + let(:authorized_mr_todo) { create(:todo, project: authorized_project, user: user, target: authorized_mr) } + let(:unauthorized_mr) { create(:merge_request, source_project: unauthorized_project) } + let(:unauthorized_mr_todo) { create(:todo, project: unauthorized_project, user: user, target: unauthorized_mr) } + + before_all do + authorized_group.add_developer(user) + end + + describe '#execute' do + subject(:execute_service) { described_class.new(all_todos, user).execute } + + let!(:all_todos) { authorized_todos + unauthorized_todos } + + let(:authorized_todos) do + [ + authorized_mr_todo, + authorized_issue_todo, + authorized_design_todo + ] + end + + let(:unauthorized_todos) do + [ + unauthorized_mr_todo, + unauthorized_issue_todo, + unauthorized_design_todo + ] + end + + before do + enable_design_management + end + + it { is_expected.to match_array(authorized_todos) } + end +end From 27478f47b891bbcde91e8e051e03cd9d93169f3a Mon Sep 17 00:00:00 2001 From: Pirate Praveen Date: Wed, 4 Aug 2021 17:16:45 +0530 Subject: [PATCH 2/2] Upload to experimental --- debian/changelog | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/debian/changelog b/debian/changelog index 521b623b3a..edfcf4eb56 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,10 @@ +gitlab (13.12.9+ds1-1) experimental; urgency=medium + + * New upstream security release 13.12.9+ds1 (Fixes: CVE-2021-22237, + CVE-2021-22236, CVE-2021-22239) + + -- Pirate Praveen Wed, 04 Aug 2021 16:31:30 +0530 + gitlab (13.12.8+ds1-1) experimental; urgency=medium * New upstream security release 13.12.8+ds1