diff options
| author | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2021-08-03 19:05:59 +0000 |
|---|---|---|
| committer | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2021-08-03 19:05:59 +0000 |
| commit | a75a9a2364e2c397ac2c114129516c1cc9b1ecea (patch) | |
| tree | 998c34c313877741aa4748cc87891d0aa2bf4bce | |
| parent | c8edb9de30c95e9e715a1e31e7667f94fb7f3dec (diff) | |
| parent | 8c67b4991465e92dcd6752cc42e54a9c5cd9f0f2 (diff) | |
| download | gitlab-ce-a75a9a2364e2c397ac2c114129516c1cc9b1ecea.tar.gz | |
Merge remote-tracking branch 'dev/14-1-stable' into 14-1-stable
83 files changed, 1122 insertions, 331 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b798d147b1..50c0e9a883c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,30 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 14.1.2 (2021-08-03) + +### Security (19 changes) + +- [Add project member validation for domain limitation](gitlab-org/security/gitlab@d17016dde463811c81a22c07aeab817ff7b5757c) ([merge request](gitlab-org/security/gitlab!1564)) +- [Hide project-level CI/CD Analytics for Guests](gitlab-org/security/gitlab@ce3b41daadd795e906b5bbbec424a494c491a1d4) ([merge request](gitlab-org/security/gitlab!1600)) +- [Only allow invite to be accepted by user with matching email](gitlab-org/security/gitlab@9d9e439c6a923fa4791a056e599c7b7e76de59a1) ([merge request](gitlab-org/security/gitlab!1632)) +- [Add html escaping for default branch name](gitlab-org/security/gitlab@549101007452bd43d866d314b1c787120cfcb36a) ([merge request](gitlab-org/security/gitlab!1630)) +- [Configure OmniAuth to use GitLab AppLogger](gitlab-org/security/gitlab@0b234f0058bbaa0415ab43182761757c332764d1) ([merge request](gitlab-org/security/gitlab!1615)) +- [Add permissions check to pipelines#show action](gitlab-org/security/gitlab@6901d52d5265d126419e78848344ae9a886ee1a7) ([merge request](gitlab-org/security/gitlab!1612)) +- [Prevent impersonation in gitlab-shell SSH certs](gitlab-org/security/gitlab@82a878ba276c6500af5aa3d951819240535127de) ([merge request](gitlab-org/security/gitlab!1609)) +- [Fix Protected Environment Accesses Cleanup](gitlab-org/security/gitlab@0c954547dbdee6a47fc755eebef0882852080579) ([merge request](gitlab-org/security/gitlab!1606)) **GitLab Enterprise Edition** +- [Use oauth_app id instead of uid](gitlab-org/security/gitlab@9c49cbbbc730eb16ef109c1f1fc1b167768d5dd3) ([merge request](gitlab-org/security/gitlab!1603)) **GitLab Enterprise Edition** +- [Block impersonation token use if it is not permitted](gitlab-org/security/gitlab@1a73b228549dfe1fe98f44a8cee8e3ebcc36d841) ([merge request](gitlab-org/security/gitlab!1583)) +- [Fix XSS in Mermaid Markdown rendering](gitlab-org/security/gitlab@6bff57b10739c42d177371dbf44143d92de1e595) ([merge request](gitlab-org/security/gitlab!1488)) +- [Do not show email address in error message](gitlab-org/security/gitlab@fdee78b193d9744253c7b7d671247cc50175c643) ([merge request](gitlab-org/security/gitlab!1596)) **GitLab Enterprise Edition** +- [Updates oauth to 0.5.6](gitlab-org/security/gitlab@bfa3de880659b0156cf8c2a7085b1705596380a4) ([merge request](gitlab-org/security/gitlab!1592)) +- [Fix tag ref detection for pipelines](gitlab-org/security/gitlab@87a03ffd263ad153a911e14512cb7776b98a435d) ([merge request](gitlab-org/security/gitlab!1591)) +- [Disallow non-members to set issue metadata on issue create](gitlab-org/security/gitlab@abe9d660ce3314c1540ec20b3a0640e623c56ecc) ([merge request](gitlab-org/security/gitlab!1586)) +- [Prevent guests from linking issues with errors](gitlab-org/security/gitlab@4a74667407b725176c4722e86bba3f942ffc9487) ([merge request](gitlab-org/security/gitlab!1587)) +- [Filter todos whose target users no longer have access to](gitlab-org/security/gitlab@a05dd90c43ae84bb37956217d9cf4effd1edae50) ([merge request](gitlab-org/security/gitlab!1556)) +- [Remove impersonation token from api response for non-admin user](gitlab-org/security/gitlab@928eaf1b82d45fbfa0d82b6515d192453b944ab9) ([merge request](gitlab-org/security/gitlab!1565)) +- [Restrict access to instance-level security features for reporters](gitlab-org/security/gitlab@d4097341cede050e0066fa2a5445cbf51a1cc1bd) ([merge request](gitlab-org/security/gitlab!1561)) **GitLab Enterprise Edition** + ## 14.1.1 (2021-07-28) ### Added (1 change) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index e199ade4b1d..73c5170b317 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -14.1.1
\ No newline at end of file +14.1.2
\ No newline at end of file diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index dc18eab7e57..bf4ba6520ea 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -13.19.0 +13.19.1 diff --git a/Gemfile.lock b/Gemfile.lock index e3bb0584149..190435b796f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -805,7 +805,7 @@ GEM nenv (~> 0.1) shellany (~> 0.0) numerizer (0.2.0) - oauth (0.5.4) + oauth (0.5.6) oauth2 (1.4.7) faraday (>= 0.8, < 2.0) jwt (>= 1.0, < 3.0) @@ -1 +1 @@ -14.1.1
\ No newline at end of file +14.1.2
\ 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 293fe9f4133..3f878949f9b 100644 --- a/app/assets/javascripts/behaviors/markdown/render_mermaid.js +++ b/app/assets/javascripts/behaviors/markdown/render_mermaid.js @@ -66,6 +66,7 @@ export function initMermaid(mermaid) { useMaxWidth: true, htmlLabels: true, }, + 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 c3166d5dd82..eb279298baf 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 25ac0af9731..21bbb4d0c98 100644 --- a/app/controllers/dashboard/todos_controller.rb +++ b/app/controllers/dashboard/todos_controller.rb @@ -16,6 +16,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 e6aae144da6..3c81b698546 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 b4196878c4f..ba7c86434e0 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 29ab9402f5b..79d01b9bf2e 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 8966285fccc..af48ceefd6f 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 0ff0775ca86..eb0d999554f 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 e5abc033155..7d61b296eae 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 679406e68d7..d0e4163dcdb 100644 --- a/app/models/alert_management/alert.rb +++ b/app/models/alert_management/alert.rb @@ -266,6 +266,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 a93348a3b27..527b67712ee 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -86,4 +86,12 @@ 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 diff --git a/app/models/commit.rb b/app/models/commit.rb index a1ed5eb9ab9..8e7f526c512 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -550,6 +550,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 e2d10cc7e78..79f5a63bcb6 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 eefb8d3d16a..1e7308499a0 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -713,10 +713,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 00fcba5298a..d91d72e1fba 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -537,6 +537,25 @@ class Issue < ApplicationRecord self.update_column(:upvotes_count, self.upvotes) end + # Returns `true` if the given User can read the current Issue. + # + # This method duplicates the same check of issue_policy.rb + # for performance reasons, check commit: 002ad215818450d2cbbc5fa065850a953dc7ada8 + # Make sure to sync this method with issue_policy.rb + def readable_by?(user) + if user.can_read_all_resources? + true + elsif project.owner == user + true + elsif confidential? && !assignee_or_author?(user) + project.team.member?(user, Gitlab::Access::REPORTER) + else + project.public? || + project.internal? && !user.external? || + project.team.member?(user) + end + end + private def spammable_attribute_changed? @@ -562,25 +581,6 @@ class Issue < ApplicationRecord 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 - # for performance reasons, check commit: 002ad215818450d2cbbc5fa065850a953dc7ada8 - # Make sure to sync this method with issue_policy.rb - def readable_by?(user) - if user.can_read_all_resources? - true - elsif project.owner == user - true - elsif confidential? && !assignee_or_author?(user) - project.team.member?(user, Gitlab::Access::REPORTER) - else - project.public? || - project.internal? && !user.external? || - project.team.member?(user) - end - 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 ed341e58436..2ad6df85e5f 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -384,12 +384,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 @@ -406,10 +400,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 9e6e29aadda..c5522737b87 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1485,10 +1485,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/issue_policy.rb b/app/policies/issue_policy.rb index e58179e320d..053243e2296 100644 --- a/app/policies/issue_policy.rb +++ b/app/policies/issue_policy.rb @@ -44,7 +44,18 @@ class IssuePolicy < IssuablePolicy enable :update_subscription end - rule { ~persisted & can?(:guest_access) }.policy do + # admin can set metadata on new issues + rule { ~persisted & admin }.policy do + enable :set_issue_metadata + end + + # support bot needs to be able to set metadata on new issues when service desk is enabled + rule { ~persisted & support_bot & can?(:guest_access) }.policy do + enable :set_issue_metadata + end + + # guest members need to be able to set issue metadata per https://gitlab.com/gitlab-org/gitlab/-/issues/300100 + rule { ~persisted & is_project_member & can?(:guest_access) }.policy do enable :set_issue_metadata end diff --git a/app/policies/personal_access_token_policy.rb b/app/policies/personal_access_token_policy.rb index 1e5404b7822..31c973f575b 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 85547834a2e..6f5bbf8c021 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -288,6 +288,7 @@ class ProjectPolicy < BasePolicy enable :read_confidential_issues enable :read_package enable :read_product_analytics + enable :read_ci_cd_analytics end # We define `:public_user_access` separately because there are cases in gitlab-ee @@ -487,6 +488,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 @@ -562,6 +564,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 @@ -673,6 +676,7 @@ class ProjectPolicy < BasePolicy rule { support_bot & ~service_desk_enabled }.policy do prevent :create_note prevent :read_project + prevent :guest_access end rule { project_bot }.enable :project_bot_access diff --git a/app/policies/todo_policy.rb b/app/policies/todo_policy.rb index d01a046c343..6237fbc50fa 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 bf66a33a7b2..5e0a86fdeee 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -48,6 +48,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 00000000000..dfed616710b --- /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 b7b712e078d..f4b1a2853f1 100644 --- a/app/views/admin/users/_head.html.haml +++ b/app/views/admin/users/_head.html.haml @@ -42,6 +42,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 ca10861115b..58f817bf63b 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 ae13ef831dd..3622fc46983 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]) + + = 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: '<strong>'.html_safe, link_to_source: link_to_source, strong_close: '</strong>'.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" -- if !current_user_matches_invite? +- 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/app/views/projects/empty.html.haml b/app/views/projects/empty.html.haml index 0fda74a3be5..70d86e79282 100644 --- a/app/views/projects/empty.html.haml +++ b/app/views/projects/empty.html.haml @@ -44,26 +44,26 @@ :preserve git clone #{ content_tag(:span, default_url_to_repo, class: 'js-clone')} cd #{h @project.path} - git switch -c #{default_branch_name} + git switch -c #{h default_branch_name} touch README.md git add README.md git commit -m "add README" - if @project.can_current_user_push_to_default_branch? %span>< - git push -u origin #{ default_branch_name } + git push -u origin #{h default_branch_name } %fieldset %h5= _('Push an existing folder') %pre.bg-light :preserve cd existing_folder - git init --initial-branch=#{default_branch_name} + git init --initial-branch=#{h default_branch_name} git remote add origin #{ content_tag(:span, default_url_to_repo, class: 'js-clone')} git add . git commit -m "Initial commit" - if @project.can_current_user_push_to_default_branch? %span>< - git push -u origin #{ default_branch_name } + git push -u origin #{h default_branch_name } %fieldset %h5= _('Push an existing Git repository') diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 85984772d05..478a5828809 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 2c60938b75a..56590bb9a8f 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 a001313a11f..e0e5ca615ac 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 8cab2f65726..13e78e72175 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -193,7 +193,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 c3c1728602c..7564d0c3ed5 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/<tag-name>) + # + # 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/<branch-name>) + # + # 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 00000000000..77f3b1ac71a --- /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 cdd4da31803..00000000000 --- 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 9f366a19e90..29fd0609596 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 8e1502bce6f..7e0f6f0b2c5 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -15211,6 +15211,9 @@ msgstr "" msgid "Go full screen" msgstr "" +msgid "Go to %{source_name}" +msgstr "" + msgid "Go to commits" msgstr "" @@ -16328,9 +16331,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 "" @@ -22420,9 +22420,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 "" @@ -30225,6 +30222,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 "" @@ -33471,6 +33471,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 "" @@ -37281,6 +37284,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 "" @@ -37602,7 +37608,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." @@ -38195,9 +38201,6 @@ msgstr "" msgid "archived:" msgstr "" -msgid "as %{role}." -msgstr "" - msgid "assign yourself" msgstr "" @@ -38666,14 +38669,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 "" @@ -39691,9 +39694,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 0d9cde88eca..fd7631edbbb 100644 --- a/spec/controllers/invites_controller_spec.rb +++ b/spec/controllers/invites_controller_spec.rb @@ -25,9 +25,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' } } @@ -59,34 +114,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 @@ -213,6 +240,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 2379ff9fd98..65a563fac7c 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) - end + context 'when the project is public' do + render_views - before do - create_build_with_artifacts('build', 0, 'job1') - create_build_with_artifacts('build', 0, 'job2') + 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 - it 'avoids N+1 database queries', :request_store do - get_pipeline_html + context 'when the project is private' do + let(:project) { create(:project, :private, :repository) } + let(:pipeline) { create(:ci_pipeline, project: project) } - control_count = ActiveRecord::QueryRecorder.new { get_pipeline_html }.count - expect(response).to have_gitlab_http_status(:ok) + it 'returns `not_found` when the user does not have access' do + sign_in(create(:user)) - create_build_with_artifacts('build', 0, 'job3') + get_pipeline_html - 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 ee64e71f176..7466150addf 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 4c43948201c..b0aafdda59a 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 b1464af4194..53209db3107 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 0bc6cc9c017..7345bfa19e2 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 93602033d73..b7e1004aef5 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 c4994838d26..e080c7ffb3f 100644 --- a/spec/features/markdown/mermaid_spec.rb +++ b/spec/features/markdown/mermaid_spec.rb @@ -260,8 +260,6 @@ RSpec.describe 'Mermaid rendering', :js do description *= 51 - project = create(:project, :public) - wiki_page = build(:wiki_page, { container: project, content: description }) wiki_page.create message: 'mermaid test commit' # rubocop:disable Rails/SaveBang wiki_page = project.wiki.find_page(wiki_page.slug) @@ -277,6 +275,27 @@ RSpec.describe 'Mermaid rendering', :js do expect(page).not_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["<iframe></iframe>"] + ``` + 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 0958e1d1891..ce2083b397a 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -365,9 +365,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 7ad7034eb6d..78bbad394a0 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 b5f2ff5d044..9723ac8af42 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 22fb1bba7a8..954bb3db668 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 c0367f7d42e..ccc861baae5 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 ac14852b365..0760935a2fe 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 d529d4a96e1..1d708b17076 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 6019318a401..7771289abe6 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 900dfec38e2..2e73043e309 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/email/handler/service_desk_handler_spec.rb b/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb index e8470657181..2ef3b324db8 100644 --- a/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb @@ -138,6 +138,25 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do expect(issue.assignees).to be_empty expect(issue.milestone).to be_nil end + + context 'when issues are set to private' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'applies quick action commands present on templates' do + file_content = %(Text from service_desk2 template \n/label ~#{label.title} \n/milestone %"#{milestone.name}") + set_template_file('service_desk2', file_content) + + receiver.execute + + issue = Issue.last + expect(issue.description).to include('Text from service_desk2 template') + expect(issue.label_ids).to include(label.id) + expect(issue.author_id).to eq(User.support_bot.id) + expect(issue.milestone).to eq(milestone) + end + end end end 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 f65b247d5d7..00000000000 --- 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 ed94b81520e..9d5f029fff5 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 2731eadecc0..11652d9841b 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 00000000000..6e1b39cc438 --- /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 00000000000..ee3bbf186b9 --- /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 00000000000..55fa4f7c33d --- /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/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb index bc09191f6ec..8ff936d5a35 100644 --- a/spec/policies/issue_policy_spec.rb +++ b/spec/policies/issue_policy_spec.rb @@ -11,13 +11,37 @@ RSpec.describe IssuePolicy do let(:reporter) { create(:user) } let(:group) { create(:group, :public) } let(:reporter_from_group_link) { create(:user) } + let(:non_member) { create(:user) } + let(:support_bot) { User.support_bot } def permissions(user, issue) described_class.new(user, issue) end + shared_examples 'support bot with service desk enabled' do + before do + allow(::Gitlab::IncomingEmail).to receive(:enabled?) { true } + allow(::Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true } + + project.update!(service_desk_enabled: true) + end + + it 'allows support_bot to read issues, create and set metadata on new issues' do + expect(permissions(support_bot, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(support_bot, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(support_bot, new_issue)).to be_allowed(:create_issue, :set_issue_metadata) + end + end + + shared_examples 'support bot with service desk disabled' do + it 'allows support_bot to read issues, create and set metadata on new issues' do + expect(permissions(support_bot, issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(support_bot, issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(support_bot, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata) + end + end + context 'a private project' do - let(:non_member) { create(:user) } let(:project) { create(:project, :private) } let(:issue) { create(:issue, project: project, assignees: [assignee], author: author) } let(:issue_no_assignee) { create(:issue, project: project) } @@ -34,12 +58,6 @@ RSpec.describe IssuePolicy do create(:project_group_link, group: group, project: project) end - it 'does not allow non-members to read issues' do - expect(permissions(non_member, issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) - expect(permissions(non_member, issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) - expect(permissions(non_member, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata) - end - it 'allows guests to read issues' do expect(permissions(guest, issue)).to be_allowed(:read_issue, :read_issue_iid) expect(permissions(guest, issue)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata) @@ -82,6 +100,15 @@ RSpec.describe IssuePolicy do expect(permissions(assignee, new_issue)).to be_allowed(:create_issue, :set_issue_metadata) end + it 'does not allow non-members to read, update or create issues' do + expect(permissions(non_member, issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(non_member, issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(non_member, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata) + end + + it_behaves_like 'support bot with service desk disabled' + it_behaves_like 'support bot with service desk enabled' + context 'with confidential issues' do let(:confidential_issue) { create(:issue, :confidential, project: project, assignees: [assignee], author: author) } let(:confidential_issue_no_assignee) { create(:issue, :confidential, project: project) } @@ -196,7 +223,8 @@ RSpec.describe IssuePolicy do expect(permissions(author, issue_locked)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) expect(permissions(author, issue_locked)).to be_disallowed(:admin_issue, :reopen_issue, :set_issue_metadata) - expect(permissions(author, new_issue)).to be_allowed(:create_issue, :set_issue_metadata) + expect(permissions(author, new_issue)).to be_allowed(:create_issue) + expect(permissions(author, new_issue)).to be_disallowed(:set_issue_metadata) end it 'allows issue assignees to read, reopen and update their issues' do @@ -208,14 +236,44 @@ RSpec.describe IssuePolicy do expect(permissions(assignee, issue_locked)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) expect(permissions(assignee, issue_locked)).to be_disallowed(:admin_issue, :reopen_issue, :set_issue_metadata) + end - expect(permissions(author, new_issue)).to be_allowed(:create_issue, :set_issue_metadata) + it 'allows non-members to read and create issues' do + expect(permissions(non_member, issue)).to be_allowed(:read_issue, :read_issue_iid) + expect(permissions(non_member, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid) + expect(permissions(non_member, new_issue)).to be_allowed(:create_issue) + end + + it 'allows non-members to read issues' do + expect(permissions(non_member, issue)).to be_allowed(:read_issue, :read_issue_iid) + expect(permissions(non_member, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid) + end + + it 'does not allow non-members to update, admin or set metadata' do + expect(permissions(non_member, issue)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(non_member, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(non_member, new_issue)).to be_disallowed(:set_issue_metadata) end + it 'allows support_bot to read issues' do + # support_bot is still allowed read access in public projects through :public_access permission, + # see project_policy public_access rules policy (rule { can?(:public_access) }.policy {...}) + expect(permissions(support_bot, issue)).to be_allowed(:read_issue, :read_issue_iid) + expect(permissions(support_bot, issue)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata) + + expect(permissions(support_bot, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid) + expect(permissions(support_bot, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata) + + expect(permissions(support_bot, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata) + end + + it_behaves_like 'support bot with service desk enabled' + context 'when issues are private' do before do project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) end + let(:issue) { create(:issue, project: project, author: author) } let(:visitor) { create(:user) } let(:admin) { create(:user, :admin) } @@ -258,6 +316,15 @@ RSpec.describe IssuePolicy do expect(permissions(admin, issue)).to be_disallowed(:create_note) end end + + it 'does not allow non-members to update or create issues' do + expect(permissions(non_member, issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(non_member, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(non_member, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata) + end + + it_behaves_like 'support bot with service desk disabled' + it_behaves_like 'support bot with service desk enabled' end context 'with confidential issues' do diff --git a/spec/policies/personal_access_token_policy_spec.rb b/spec/policies/personal_access_token_policy_spec.rb index b5e8d40b133..e146133429b 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 59123c3695a..051a4420e73 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -480,8 +480,8 @@ RSpec.describe ProjectPolicy do let(:current_user) { User.support_bot } context 'with service desk disabled' do - it { expect_allowed(:guest_access) } - it { expect_disallowed(:create_note, :read_project) } + it { expect_allowed(:public_access) } + it { expect_disallowed(:guest_access, :create_note, :read_project) } end context 'with service desk enabled' do @@ -1131,12 +1131,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 @@ -1145,6 +1153,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 @@ -1153,6 +1171,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 @@ -1162,9 +1181,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 @@ -1173,18 +1202,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 @@ -1193,6 +1233,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 b4876baa504..16435b21666 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 00000000000..018f497e7e5 --- /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 e298de0df01..981b10a7467 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 8f92105dc9c..9ac98db91e2 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 8a9a0b9e845..7f5ea71c760 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 e71a232ff7c..70e3cc7f5cd 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) } @@ -30,6 +32,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 a58c7fc69fc..d995191c97e 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 ccc5f322ff9..0ff2c46e693 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 00de1ef5964..d31f571e636 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) } + + 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) - 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) + expect { get api('/todos', john_doe) }.not_to exceed_query_limit(control1).with_threshold(4) + control2 = ActiveRecord::QueryRecorder.new { get api('/todos', john_doe) } - 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) + 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 3fb683ea0fa..34f8a479719 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 64e8c6ac2df..56bfeda3bff 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 087f4ba372b..ac9bac4e6ad 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 b073ffd291f..0e2b3b957a5 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -226,6 +226,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 + + it 'does not assign the sentry error' do + expect(issue.sentry_issue).to eq(nil) + end + + context 'user is reporter or above' do + before do + project.add_reporter(user) + end + + it 'assigns the sentry error' do + expect(issue.sentry_issue).to be_kind_of(SentryIssue) + end + 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 70c3c2a0f5d..1e922401028 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 before 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 00000000000..707df8e8514 --- /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 |