Remove todos of users without access to targets migration
This commit is contained in:
parent
0d729d5c54
commit
8338f9b89b
4 changed files with 265 additions and 0 deletions
5
changelogs/unreleased/todos-visibility-migration.yml
Normal file
5
changelogs/unreleased/todos-visibility-migration.yml
Normal file
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Remove todos of users without access to targets migration
|
||||
merge_request: 20927
|
||||
author:
|
||||
type: other
|
31
db/migrate/20180717125853_remove_restricted_todos.rb
Normal file
31
db/migrate/20180717125853_remove_restricted_todos.rb
Normal file
|
@ -0,0 +1,31 @@
|
|||
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
|
||||
# for more information on how to write migrations for GitLab.
|
||||
# frozen_string_literal: true
|
||||
|
||||
class RemoveRestrictedTodos < ActiveRecord::Migration
|
||||
DOWNTIME = false
|
||||
disable_ddl_transaction!
|
||||
|
||||
MIGRATION = 'RemoveRestrictedTodos'.freeze
|
||||
BATCH_SIZE = 1000
|
||||
DELAY_INTERVAL = 5.minutes.to_i
|
||||
|
||||
class Project < ActiveRecord::Base
|
||||
include EachBatch
|
||||
|
||||
self.table_name = 'projects'
|
||||
end
|
||||
|
||||
def up
|
||||
Project.where('EXISTS (SELECT 1 FROM todos WHERE todos.project_id = projects.id)')
|
||||
.each_batch(of: BATCH_SIZE) do |batch, index|
|
||||
range = batch.pluck('MIN(id)', 'MAX(id)').first
|
||||
|
||||
BackgroundMigrationWorker.perform_in(index * DELAY_INTERVAL, MIGRATION, range)
|
||||
end
|
||||
end
|
||||
|
||||
def down
|
||||
# nothing to do
|
||||
end
|
||||
end
|
105
lib/gitlab/background_migration/remove_restricted_todos.rb
Normal file
105
lib/gitlab/background_migration/remove_restricted_todos.rb
Normal file
|
@ -0,0 +1,105 @@
|
|||
# frozen_string_literal: true
|
||||
# rubocop:disable Style/Documentation
|
||||
|
||||
module Gitlab
|
||||
module BackgroundMigration
|
||||
class RemoveRestrictedTodos
|
||||
PRIVATE_FEATURE = 10
|
||||
PRIVATE_PROJECT = 0
|
||||
|
||||
class Project < ActiveRecord::Base
|
||||
self.table_name = 'projects'
|
||||
end
|
||||
|
||||
class ProjectAuthorization < ActiveRecord::Base
|
||||
self.table_name = 'project_authorizations'
|
||||
end
|
||||
|
||||
class ProjectFeature < ActiveRecord::Base
|
||||
self.table_name = 'project_features'
|
||||
end
|
||||
|
||||
class Todo < ActiveRecord::Base
|
||||
include EachBatch
|
||||
|
||||
self.table_name = 'todos'
|
||||
end
|
||||
|
||||
class Issue < ActiveRecord::Base
|
||||
include EachBatch
|
||||
|
||||
self.table_name = 'issues'
|
||||
end
|
||||
|
||||
def perform(start_id, stop_id)
|
||||
projects = Project.where('EXISTS (SELECT 1 FROM todos WHERE todos.project_id = projects.id)')
|
||||
.where(id: start_id..stop_id)
|
||||
|
||||
projects.each do |project|
|
||||
remove_confidential_issue_todos(project.id)
|
||||
|
||||
if project.visibility_level == PRIVATE_PROJECT
|
||||
remove_non_members_todos(project.id)
|
||||
else
|
||||
remove_restricted_features_todos(project.id)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def remove_non_members_todos(project_id)
|
||||
Todo.where(project_id: project_id)
|
||||
.where('user_id NOT IN (?)', authorized_users(project_id))
|
||||
.each_batch(of: 5000) do |batch|
|
||||
batch.delete_all
|
||||
end
|
||||
end
|
||||
|
||||
def remove_confidential_issue_todos(project_id)
|
||||
# min access level to access a confidential issue is reporter
|
||||
min_reporters = authorized_users(project_id)
|
||||
.select(:user_id)
|
||||
.where('access_level >= ?', 20)
|
||||
|
||||
confidential_issues = Issue.select(:id, :author_id).where(confidential: true, project_id: project_id)
|
||||
confidential_issues.each_batch(of: 100) do |batch|
|
||||
batch.each do |issue|
|
||||
assigned_users = IssueAssignee.select(:user_id).where(issue_id: issue.id)
|
||||
|
||||
todos = Todo.where(target_type: 'Issue', target_id: issue.id)
|
||||
.where('user_id NOT IN (?)', min_reporters)
|
||||
.where('user_id NOT IN (?)', assigned_users)
|
||||
todos = todos.where('user_id != ?', issue.author_id) if issue.author_id
|
||||
|
||||
todos.delete_all
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def remove_restricted_features_todos(project_id)
|
||||
ProjectFeature.where(project_id: project_id).each do |project_features|
|
||||
target_types = []
|
||||
target_types << 'Issue' if private?(project_features.issues_access_level)
|
||||
target_types << 'MergeRequest' if private?(project_features.merge_requests_access_level)
|
||||
target_types << 'Commit' if private?(project_features.repository_access_level)
|
||||
|
||||
next if target_types.empty?
|
||||
|
||||
Todo.where(project_id: project_id)
|
||||
.where('user_id NOT IN (?)', authorized_users(project_id))
|
||||
.where(target_type: target_types)
|
||||
.delete_all
|
||||
end
|
||||
end
|
||||
|
||||
def private?(feature_level)
|
||||
feature_level == PRIVATE_FEATURE
|
||||
end
|
||||
|
||||
def authorized_users(project_id)
|
||||
ProjectAuthorization.select(:user_id).where(project_id: project_id)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
|
@ -0,0 +1,124 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require 'spec_helper'
|
||||
|
||||
describe Gitlab::BackgroundMigration::RemoveRestrictedTodos, :migration, schema: 20180704204006 do
|
||||
let(:projects) { table(:projects) }
|
||||
let(:users) { table(:users) }
|
||||
let(:todos) { table(:todos) }
|
||||
let(:issues) { table(:issues) }
|
||||
let(:assignees) { table(:issue_assignees) }
|
||||
let(:project_authorizations) { table(:project_authorizations) }
|
||||
let(:project_features) { table(:project_features) }
|
||||
|
||||
let(:todo_params) { { author_id: 1, target_type: 'Issue', action: 1, state: :pending } }
|
||||
|
||||
before do
|
||||
users.create(id: 1, email: 'user@example.com', projects_limit: 10)
|
||||
users.create(id: 2, email: 'reporter@example.com', projects_limit: 10)
|
||||
users.create(id: 3, email: 'guest@example.com', projects_limit: 10)
|
||||
|
||||
projects.create!(id: 1, name: 'project-1', path: 'project-1', visibility_level: 0, namespace_id: 1)
|
||||
projects.create!(id: 2, name: 'project-2', path: 'project-2', visibility_level: 0, namespace_id: 1)
|
||||
|
||||
issues.create(id: 1, project_id: 1)
|
||||
issues.create(id: 2, project_id: 2)
|
||||
|
||||
project_authorizations.create(user_id: 2, project_id: 2, access_level: 20) # reporter
|
||||
project_authorizations.create(user_id: 3, project_id: 2, access_level: 10) # guest
|
||||
|
||||
todos.create(todo_params.merge(user_id: 1, project_id: 1, target_id: 1)) # out of project ids range
|
||||
todos.create(todo_params.merge(user_id: 1, project_id: 2, target_id: 2)) # non member
|
||||
todos.create(todo_params.merge(user_id: 2, project_id: 2, target_id: 2)) # reporter
|
||||
todos.create(todo_params.merge(user_id: 3, project_id: 2, target_id: 2)) # guest
|
||||
end
|
||||
|
||||
subject { described_class.new.perform(2, 5) }
|
||||
|
||||
context 'when a project is private' do
|
||||
it 'removes todos of users without project access' do
|
||||
expect { subject }.to change { Todo.count }.from(4).to(3)
|
||||
end
|
||||
|
||||
context 'with a confidential issue' do
|
||||
it 'removes todos of users without project access and guests for confidential issues' do
|
||||
issues.create(id: 3, project_id: 2, confidential: true)
|
||||
issues.create(id: 4, project_id: 1, confidential: true) # not in the batch
|
||||
todos.create(todo_params.merge(user_id: 3, project_id: 2, target_id: 3))
|
||||
todos.create(todo_params.merge(user_id: 2, project_id: 2, target_id: 3))
|
||||
todos.create(todo_params.merge(user_id: 1, project_id: 1, target_id: 4))
|
||||
|
||||
expect { subject }.to change { Todo.count }.from(7).to(5)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when a project is public' do
|
||||
before do
|
||||
projects.find(2).update_attribute(:visibility_level, 20)
|
||||
end
|
||||
|
||||
context 'when all features have the same visibility as the project, no confidential issues' do
|
||||
it 'does not remove any todos' do
|
||||
expect { subject }.not_to change { Todo.count }
|
||||
end
|
||||
end
|
||||
|
||||
context 'with confidential issues' do
|
||||
before do
|
||||
users.create(id: 4, email: 'author@example.com', projects_limit: 10)
|
||||
users.create(id: 5, email: 'assignee@example.com', projects_limit: 10)
|
||||
issues.create(id: 3, project_id: 2, confidential: true, author_id: 4)
|
||||
assignees.create(user_id: 5, issue_id: 3)
|
||||
|
||||
todos.create(todo_params.merge(user_id: 1, project_id: 2, target_id: 3)) # to be deleted
|
||||
todos.create(todo_params.merge(user_id: 2, project_id: 2, target_id: 3)) # authorized user
|
||||
todos.create(todo_params.merge(user_id: 3, project_id: 2, target_id: 3)) # to be deleted guest
|
||||
todos.create(todo_params.merge(user_id: 4, project_id: 2, target_id: 3)) # conf issue author
|
||||
todos.create(todo_params.merge(user_id: 5, project_id: 2, target_id: 3)) # conf issue assignee
|
||||
end
|
||||
|
||||
it 'removes confidential issue todos for non authorized users' do
|
||||
expect { subject }.to change { Todo.count }.from(9).to(7)
|
||||
end
|
||||
end
|
||||
|
||||
context 'features visibility restrictions' do
|
||||
before do
|
||||
todo_params.merge!(project_id: 2, user_id: 1, target_id: 3)
|
||||
todos.create(todo_params.merge(user_id: 1, target_id: 3, target_type: 'MergeRequest'))
|
||||
todos.create(todo_params.merge(user_id: 1, target_id: 3, target_type: 'Commit'))
|
||||
end
|
||||
|
||||
context 'when issues are restricted to project members' do
|
||||
before do
|
||||
project_features.create(issues_access_level: 10, project_id: 2)
|
||||
end
|
||||
|
||||
it 'removes non members issue todos' do
|
||||
expect { subject }.to change { Todo.count }.from(6).to(5)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when merge requests are restricted to project members' do
|
||||
before do
|
||||
project_features.create(merge_requests_access_level: 10, project_id: 2)
|
||||
end
|
||||
|
||||
it 'removes non members issue todos' do
|
||||
expect { subject }.to change { Todo.count }.from(6).to(5)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when repository and merge requests are restricted to project members' do
|
||||
before do
|
||||
project_features.create(repository_access_level: 10, merge_requests_access_level: 10, project_id: 2)
|
||||
end
|
||||
|
||||
it 'removes non members commit and merge requests todos' do
|
||||
expect { subject }.to change { Todo.count }.from(6).to(4)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in a new issue