diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-08-03 00:26:53 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-08-03 00:26:53 +0300 |
commit | b30f7e36de53f94df4022815d3fbdadc4368a7e3 (patch) | |
tree | 422cc3db247e7d5e9d6dcb9cc40618b863cd64ce /app | |
parent | c8edb9de30c95e9e715a1e31e7667f94fb7f3dec (diff) |
Add latest changes from gitlab-org/security/gitlab@14-1-stable-ee
Diffstat (limited to 'app')
-rw-r--r-- | app/assets/javascripts/behaviors/markdown/render_mermaid.js | 1 | ||||
-rw-r--r-- | app/controllers/dashboard/todos_controller.rb | 2 | ||||
-rw-r--r-- | app/graphql/resolvers/todo_resolver.rb | 2 | ||||
-rw-r--r-- | app/graphql/types/alert_management/alert_type.rb | 8 | ||||
-rw-r--r-- | app/graphql/types/user_interface.rb | 7 | ||||
-rw-r--r-- | app/models/alert_management/alert.rb | 4 | ||||
-rw-r--r-- | app/models/application_record.rb | 8 | ||||
-rw-r--r-- | app/models/commit.rb | 4 | ||||
-rw-r--r-- | app/models/design_management/design.rb | 4 | ||||
-rw-r--r-- | app/models/group.rb | 4 | ||||
-rw-r--r-- | app/models/issue.rb | 38 | ||||
-rw-r--r-- | app/models/note.rb | 10 | ||||
-rw-r--r-- | app/models/project.rb | 4 | ||||
-rw-r--r-- | app/policies/todo_policy.rb | 7 | ||||
-rw-r--r-- | app/services/todos/allowed_target_filter_service.rb | 18 | ||||
-rw-r--r-- | app/views/dashboard/todos/index.html.haml | 8 |
16 files changed, 73 insertions, 56 deletions
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/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/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/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/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/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 |