Commit cda8de63 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'file_locks' into 'master'

File Lock

Fixes https://gitlab.com/gitlab-org/gitlab-ee/issues/497

- [x] Data layer(tables, models, migrations)
- [x] Checks on git push
- [x] Author should be able to push if one owns the lock
- [x] Path matcher with tokenizer and memoization
- [x] Show lock status icons by files and folders
- [x] UI: mockup buttons for locking files, folders
- [x] UI: Locking files
- [x] UI: Locking folders
- [x] Permissions check everywhere
- [x] Add page to list all locked files/dirs to get overview in case people forget to unlock
- [x] Refactor `lock_file_button` and `lock_file_link`
- [x] Testing Git LFS
- [x] Get rid of code duplication in pre-receive hook
- [x] Test case: I want to unlock file if I'm a developer and I'm not an author of lock
- [x] Make it EE option
- [x] Specs and spinach
- [x] TODOs in the code

Can be moved to the next iteration:
- [ ] Duplicate lock checks in the service since pre-receive hook does not work for UI (according to Douwe)
- [ ] timeago in the lock icon tooltip. It's not working out of the box.
- [ ] Ajax load bar on click "Lock/Unlock" in the file tree
- [ ] Nested locking UI. If we look at file and it's locked because of parent we should show it in tooltip. 


**Screenshots**

![joxi_screenshot_1466188707474](/uploads/623e65c9a246b07c9786fd8babdb2dd8/joxi_screenshot_1466188707474.png)![joxi_screenshot_1466188735083](/uploads/4588eff352ee544de5bb5727d501a2cc/joxi_screenshot_1466188735083.png)![joxi_screenshot_1466188758950](/uploads/dee7b342c7825a79ed1629535eced5ed/joxi_screenshot_1466188758950.png)


Related info:
Douwe wrote:
```For a next iteration, we need to think about nested locking more. If user A has locked lib/, user B shouldn't be able to lock lib/foo.rb since that falls under lib/. Similarly, if user A has locked lib/foo.rb, user B shouldn't be able to lock lib/, because part of that directory is locked by someone else already.```

See merge request !440
parents 7b612762 22ff4421
......@@ -11,6 +11,7 @@ v 8.9.0 (unreleased)
- Disable mirror flag for projects without import_url
- Show flash notice when Git Hooks are updated successfully
- [Elastic] Project members with guest role can't access confidential issues
- Ability to lock file or folder in the repository
v 8.8.5
- Make sure OAuth routes that we generate for Geo matches with the ones in Rails routes !444
......
......@@ -39,6 +39,9 @@
e.preventDefault()
e.stopImmediatePropagation()
return false
gl.utils.capitalize = (str) ->
return str[0].toUpperCase() + str.slice(1);
jQuery.timefor = (time, suffix, expiredLabel) ->
......
class @PathLocks
@init: (url, path) ->
$('.path-lock').on 'click', ->
$lockBtn = $(this)
currentState = $lockBtn.data('state')
toggleAction = if currentState is 'lock' then 'unlock' else 'lock'
$.post url, {
path: path
}, ->
$lockBtn.text(gl.utils.capitalize(toggleAction))
$lockBtn.data('state', toggleAction)
class Projects::PathLocksController < Projects::ApplicationController
include PathLocksHelper
# Authorize
before_action :require_non_empty_project
before_action :authorize_push_code!, only: [:toggle]
before_action :check_license
def index
@path_locks = @project.path_locks.page(params[:page])
end
def toggle
path_lock = @project.path_locks.find_by(path: params[:path])
if path_lock
PathLocks::UnlockService.new(project, current_user).execute(path_lock)
else
PathLocks::LockService.new(project, current_user).execute(params[:path])
end
head :ok
rescue PathLocks::UnlockService::AccessDenied, PathLocks::LockService::AccessDenied
return access_denied!
end
def destroy
path_lock = @project.path_locks.find(params[:id])
begin
PathLocks::UnlockService.new(project, current_user).execute(path_lock)
rescue PathLocks::UnlockService::AccessDenied
return access_denied!
end
respond_to do |format|
format.html do
redirect_to namespace_project_locks_path(@project.namespace, @project)
end
format.js
end
end
private
def check_license
unless license_allows_file_locks?
flash[:alert] = 'You need a different license to enable FileLocks feature'
redirect_to admin_license_path
end
end
end
class Projects::RefsController < Projects::ApplicationController
include ExtractsPath
include TreeHelper
include PathLocksHelper
before_action :require_non_empty_project
before_action :validate_ref_id
......@@ -57,16 +58,22 @@ class Projects::RefsController < Projects::ApplicationController
contents.push(*tree.blobs)
contents.push(*tree.submodules)
show_path_locks = license_allows_file_locks? && @project.path_locks.any?
@logs = contents[@offset, @limit].to_a.map do |content|
file = @path ? File.join(@path, content.name) : content.name
last_commit = @repo.last_commit_for_path(@commit.id, file)
path_lock_info = show_path_locks && @project.path_lock_info(file, exact_match: true)
{
file_name: content.name,
commit: last_commit
commit: last_commit,
path_lock_info: path_lock_info
}
end
offset = (@offset + @limit)
offset = @offset + @limit
if contents.size > offset
@more_log_url = logs_file_namespace_project_ref_path(@project.namespace, @project, @ref, @path || '', offset: offset)
end
......
......@@ -29,7 +29,7 @@ module BlobHelper
if !on_top_of_branch?(project, ref)
button_tag "Edit", class: "btn disabled has-tooltip btn-file-option", title: "You can only edit files when you are on a branch", data: { container: 'body' }
elsif can_edit_blob?(blob, project, ref)
link_to "Edit", edit_path, class: 'btn btn-file-option'
link_to "Edit", edit_path, class: 'btn btn-sm'
elsif can?(current_user, :fork_project, project)
continue_params = {
to: edit_path,
......
module PathLocksHelper
def can_unlock?(path_lock, current_user = @current_user)
can?(current_user, :admin_locks, path_lock) || path_lock.user == current_user
end
def license_allows_file_locks?
::License.current && ::License.current.add_on?('GitLab_FileLocks')
end
end
......@@ -123,4 +123,20 @@ module TreeHelper
return tree.name
end
end
def lock_file_link(project = @project, path = @path, html_options: {})
return unless license_allows_file_locks?
return if path.blank?
path_lock = project.path_lock_info(path, exact_match: true)
# Check permissions to unlock
return if path_lock && !can_unlock?(path_lock)
# Check permissions to lock
return if !path_lock && !can?(current_user, :push_code, project)
label = path_lock ? 'Unlock' : 'Lock'
html_options = html_options.merge(data: { state: label.downcase })
link_to label, '#', html_options
end
end
......@@ -300,7 +300,8 @@ class Ability
:admin_pages,
:admin_pipeline,
:admin_environment,
:admin_deployment
:admin_deployment,
:admin_locks
]
end
......
class PathLock < ActiveRecord::Base
belongs_to :project
belongs_to :user
validates :project, presence: true
validates :user, presence: true
validates :path, presence: true, uniqueness: { scope: [:user, :project] }
end
......@@ -132,6 +132,7 @@ class Project < ActiveRecord::Base
has_many :remote_mirrors, dependent: :destroy
has_many :environments, dependent: :destroy
has_many :deployments, dependent: :destroy
has_many :path_locks, dependent: :destroy
accepts_nested_attributes_for :variables, allow_destroy: true
accepts_nested_attributes_for :remote_mirrors,
......@@ -1192,6 +1193,11 @@ class Project < ActiveRecord::Base
Dir.exist?(public_pages_path)
end
def path_lock_info(path, exact_match: false)
@path_lock_finder ||= Gitlab::PathLocksFinder.new(self)
@path_lock_finder.get_lock_info(path, exact_match: exact_match)
end
def schedule_delete!(user_id, params)
# Queue this task for after the commit, so once we mark pending_delete it will run
run_after_commit { ProjectDestroyWorker.perform_async(id, user_id, params) }
......
......@@ -88,6 +88,7 @@ class User < ActiveRecord::Base
has_many :todos, dependent: :destroy
has_many :notification_settings, dependent: :destroy
has_many :award_emoji, as: :awardable, dependent: :destroy
has_many :path_locks, dependent: :destroy
#
# Validations
......
module PathLocks
class LockService < BaseService
AccessDenied = Class.new(StandardError)
include PathLocksHelper
def execute(path)
raise AccessDenied, 'You have no permissions' unless can?(current_user, :push_code, project)
project.path_locks.create(path: path, user: current_user)
end
end
end
module PathLocks
class UnlockService < BaseService
AccessDenied = Class.new(StandardError)
include PathLocksHelper
def execute(path_lock)
raise AccessDenied, 'You have no permissions' unless can_unlock?(path_lock)
path_lock.destroy
end
end
end
......@@ -16,6 +16,14 @@
- if current_user
.btn-group{ role: "group" }
= lock_file_link(html_options: {class: 'btn btn-sm path-lock'})
= edit_blob_link
= replace_blob_link
= delete_blob_link
- if license_allows_file_locks?
:javascript
PathLocks.init(
'#{toggle_namespace_project_path_locks_path(@project.namespace, @project)}',
'#{@path}'
);
......@@ -25,4 +25,9 @@
= nav_link(controller: [:tags, :releases]) do
= link_to namespace_project_tags_path(@project.namespace, @project) do
Tags
- if license_allows_file_locks?
= nav_link(controller: [:path_locks]) do
= link_to namespace_project_path_locks_path(@project.namespace, @project) do
Locked Files
.fade-right
%li
%div
%span.item-title
= icon('lock')
= path_lock.path
.controls
- if can_unlock?(path_lock)
= link_to namespace_project_path_lock_path(@project.namespace, @project, path_lock), class: 'btn btn-grouped btn-xs btn-remove remove-row has-tooltip', title: "Unlock", method: :delete, data: { confirm: "Are you sure you want to unlock #{path_lock.path}?", container: 'body' }, remote: true do
= icon("trash-o")
locked by #{path_lock.user.name} #{time_ago_with_tooltip(path_lock.created_at)}
- unless @project.path_locks.any?
$('.locks').load(document.URL + ' .nothing-here-block').hide().fadeIn(1000)
- page_title "File Locks"
= render "projects/commits/head"
%div{ class: (container_class) }
.top-area
.nav-text
Locks give the ability to lock specific file or folder.
.locks
- if @path_locks.any?
%ul.content-list
= render @path_locks
= paginate @path_locks, theme: 'gitlab'
- else
.nothing-here-block
Repository has no locks.
- @logs.each do |content_data|
- file_name = content_data[:file_name]
- commit = content_data[:commit]
- path_lock_info = content_data[:path_lock_info]
- next unless commit
:plain
......@@ -8,6 +9,14 @@
row.find("td.tree_time_ago").html('#{escape_javascript time_ago_with_tooltip(commit.committed_date)}');
row.find("td.tree_commit").html('#{escape_javascript render("projects/tree/tree_commit_column", commit: commit)}');
- if path_lock_info
:plain
var label = $("<span>")
.attr('title', 'Locked by #{escape_javascript path_lock_info.user.name }')
.attr('data-toggle', 'tooltip')
.addClass('fa fa-lock prepend-left-5');
row.find('td.tree-item-file-name').append(label);
- if @more_log_url
:plain
if($('#tree-slider').length) {
......
......@@ -15,6 +15,7 @@
= link_to @commit.short_id, namespace_project_commit_path(@project.namespace, @project, @commit), class: "monospace"
&ndash;
= truncate(@commit.title, length: 50)
= lock_file_link(html_options: {class: 'pull-right prepend-left-10 path-lock'})
= link_to 'History', namespace_project_commits_path(@project.namespace, @project, @id), class: 'pull-right'
- if @path.present?
......@@ -33,6 +34,14 @@
= render 'projects/blob/upload', title: 'Upload New File', placeholder: 'Upload new file', button_title: 'Upload file', form_path: namespace_project_create_blob_path(@project.namespace, @project, @id), method: :post
= render 'projects/blob/new_dir'
- if license_allows_file_locks?
:javascript
PathLocks.init(
'#{toggle_namespace_project_path_locks_path(@project.namespace, @project)}',
'#{@path}'
);
:javascript
// Load last commit log for each file in tree
$('#tree-slider').waitForImages(function() {
......
......@@ -746,6 +746,11 @@ Rails.application.routes.draw do
resources :tags, only: [:index, :show, :new, :create, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex } do
resource :release, only: [:edit, :update]
end
resources :path_locks, only: [:index, :destroy] do
collection do
post :toggle
end
end
resources :protected_branches, only: [:index, :create, :update, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex }
resources :variables, only: [:index, :show, :update, :create, :destroy]
......
class CreatePathLocksTable < ActiveRecord::Migration
def change
create_table :path_locks do |t|
t.string :path, null: false, index: true
t.references :project, index: true, foreign_key: true
t.references :user, index: true, foreign_key: true
t.timestamps null: false
end
end
end
......@@ -863,6 +863,18 @@ ActiveRecord::Schema.define(version: 20160616084004) do
add_index "pages_domains", ["domain"], name: "index_pages_domains_on_domain", unique: true, using: :btree
create_table "path_locks", force: :cascade do |t|
t.string "path", null: false
t.integer "project_id"
t.integer "user_id"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
end
add_index "path_locks", ["path"], name: "index_path_locks_on_path", using: :btree
add_index "path_locks", ["project_id"], name: "index_path_locks_on_project_id", using: :btree
add_index "path_locks", ["user_id"], name: "index_path_locks_on_user_id", using: :btree
create_table "project_group_links", force: :cascade do |t|
t.integer "project_id", null: false
t.integer "group_id", null: false
......@@ -1233,6 +1245,8 @@ ActiveRecord::Schema.define(version: 20160616084004) do
add_index "web_hooks", ["created_at", "id"], name: "index_web_hooks_on_created_at_and_id", using: :btree
add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree
add_foreign_key "path_locks", "projects"
add_foreign_key "path_locks", "users"
add_foreign_key "remote_mirrors", "projects"
add_foreign_key "u2f_registrations", "users"
end
module Gitlab
class GitAccess
include PathLocksHelper
DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive }
PUSH_COMMANDS = %w{ git-receive-pack }
GIT_ANNEX_COMMANDS = %w{ git-annex-shell }
......@@ -97,7 +99,6 @@ module Gitlab
end
def push_access_check(changes)
if Gitlab::Geo.secondary?
return build_status_object(false, "You can't push code on a secondary GitLab Geo node.")
end
......@@ -186,13 +187,47 @@ module Gitlab
# Return build_status_object(true) if all git hook checks passed successfully
# or build_status_object(false) if any hook fails
git_hook_check(user, project, ref, oldrev, newrev)
result = git_hook_check(user, project, ref, oldrev, newrev)
if result.status && license_allows_file_locks?
result = path_locks_check(user, project, ref, oldrev, newrev)
end
result
end
def forced_push?(oldrev, newrev)
Gitlab::ForcePushCheck.force_push?(project, oldrev, newrev)
end
def path_locks_check(user, project, ref, oldrev, newrev)
unless project.path_locks.any? && newrev && oldrev
return build_status_object(true)
end
# locks protect default branch only
if project.default_branch != branch_name(ref)
return build_status_object(true)
end
commits(newrev, oldrev, project).each do |commit|
next if commit_from_annex_sync?(commit.safe_message)
commit.diffs.each do |diff|
path = diff.new_path || diff.old_path
lock_info = project.path_lock_info(path)
if lock_info && lock_info.user != user
return build_status_object(false, "The path '#{lock_info.path}' is locked by #{lock_info.user.name}")
end
end
end
build_status_object(true)
end
def git_hook_check(user, project, ref, oldrev, newrev)
unless project.git_hook && newrev && oldrev
return build_status_object(true)
......
# The database stores locked paths as following:
# 'app/models/project.rb' or 'lib/gitlab'
# To determine that 'lib/gitlab/some_class.rb' is locked we need to generate
# tokens for every requested paths and check every token whether it exist in path locks table or not.
# So for 'lib/gitlab/some_class.rb' path we would need to search next paths:
# 'lib', 'lib/gitlab' and 'lib/gitlab/some_class.rb'
# This class also implements a memoization for common paths like 'lib' 'lib/gitlab', 'app', etc.
class Gitlab::PathLocksFinder
def initialize(project)
@project = project
@non_locked_paths = []
end
def get_lock_info(path, exact_match: false)
if exact_match
return find_lock(path)
else
tokenize(path).each do |token|
if lock = find_lock(token)
return lock
end
end
false
end
end
private
# This returns hierarchy tokens for path
# app/models/project.rb => ['app', 'app/models', 'app/models/project.rb']
def tokenize(path)
segments = path.split("/")
tokens = []
begin
tokens << segments.join("/")
segments.pop
end until segments.empty?
tokens
end
def find_lock(token)
if @non_locked_paths.include?(token)
return false
end
lock = @project.path_locks.find_by(path: token)
unless lock
@non_locked_paths << token
end
lock
end
end
FactoryGirl.define do
factory :path_lock do
project
user { build :user }
sequence(:path) { |n| "app/model#{n}" }
end
end
require 'spec_helper'
feature 'Path Locks', feature: true, js: true do
let(:user) { create(:user) }
let(:project) { create(:project, namespace: user.namespace) }
let(:project_tree_path) { namespace_project_tree_path(project.namespace, project, project.repository.root_ref) }
before do
allow_any_instance_of(PathLocksHelper).to receive(:license_allows_file_locks?).and_return(true)
project.team << [user, :master]
login_with(user)
visit project_tree_path
end
scenario 'Locking folders' do
within '.tree-content-holder' do
click_link "encoding"
click_link "Lock"
visit project_tree_path
expect(page).to have_selector('.fa-lock')
end
end
scenario 'Locking files' do
page_tree = find('.tree-content-holder')
within page_tree do
click_link "VERSION"
end
within '.file-actions' do
click_link "Lock"
end
visit project_tree_path
within page_tree do
expect(page).to have_selector('.fa-lock')
end
end
scenario 'Managing of lock list' do
create :path_lock, path: 'encoding', user: user, project: project
click_link "Locked Files"
within '.locks' do
expect(page).to have_content('encoding')
find('.btn-remove').click
expect(page).not_to have_content('encoding')
end
end
end
......@@ -25,4 +25,24 @@ describe TreeHelper do
end
end
end
describe '#lock_file_link' do
let(:path_lock) { create :path_lock }
let(:path) { path_lock.path }
let(:user) { path_lock.user }
let(:project) { path_lock.project }
it "renders unlock link" do
allow(helper).to receive(:can?).and_return(true)
allow(helper).to receive(:license_allows_file_locks?).and_return(true)
expect(helper.lock_file_link(project, path)).to match('Unlock')
end
it "renders lock link" do
allow(helper).to receive(:can?).and_return(true)
allow(helper).to receive(:current_user).and_return(user)
allow(helper).to receive(:license_allows_file_locks?).and_return(true)
expect(helper.lock_file_link(project, 'app/controller')).to match('Lock')
end
end
end
require 'spec_helper'
describe Gitlab::PathLocksFinder, lib: true do
let(:project) { create :empty_project }
let(:user) { create :user }
let(:finder) { Gitlab::PathLocksFinder.new(project) }
it "returns correct lock information" do
lock1 = create :path_lock, project: project, path: 'app'
lock2 = create :path_lock, project: project, path: 'lib/gitlab/repo.rb'
expect(finder.get_lock_info('app')).to eq(lock1)
expect(finder.get_lock_info('app/models/project.rb')).to eq(lock1)
expect(finder.get_lock_info('lib')).to be_falsey
expect(finder.get_lock_info('lib/gitlab/repo.rb')).to eq(lock2)
end
end
require 'spec_helper'
describe PathLock, models: true do
let(:path_lock) { create(:path_lock) }
it { is_expected.to belong_to(:project) }
it { is_expected.to belong_to(:user) }
it { is_expected.to validate_presence_of(:user) }
it { is_expected.to validate_presence_of(:project) }
it { is_expected.to validate_presence_of(:path) }
it { is_expected.to validate_uniqueness_of(:path).scoped_to(:project_id, :user_id) }
end
......@@ -32,6 +32,7 @@ describe Project, models: true do
it { is_expected.to have_many(:environments).dependent(:destroy) }
it { is_expected.to have_many(:deployments).dependent(:destroy) }
it { is_expected.to have_many(:todos).dependent(:destroy) }
it { is_expected.to have_many(:path_locks).dependent(:destroy) }
end
describe 'modules' do
......@@ -1079,4 +1080,19 @@ describe Project, models: true do
end
end
end
describe '#path_lock_info' do
let(:project) { create :empty_project }
let(:path_lock) { create :path_lock, project: project }
let(:path) { path_lock.path }
it 'returns path_lock' do
expect(project.path_lock_info(path)).to eq(path_lock)
end
it 'returns nil' do
expect(project.path_lock_info('app/controllers')).to be_falsey
end
end
end
......@@ -31,6 +31,7 @@ describe User, models: true do
it { is_expected.to have_many(:spam_logs).dependent(:destroy) }
it { is_expected.to have_many(:todos).dependent(:destroy) }
it { is_expected.to have_many(:award_emoji).dependent(:destroy) }
it { is_expected.to have_many(:path_locks).dependent(:destroy) }
end
describe 'validations' do
......
require 'spec_helper'
describe PathLocks::LockService, services: true do
let(:current_user) { create(:user) }
let(:project) { create(:empty_project) }
let(:path) { 'app/models' }
it 'locks path' do
allow_any_instance_of(described_class).to receive(:can?).and_return(true)
described_class.new(project, current_user).execute(path)
expect(project.path_locks.find_by(path: path)).to be_truthy
end
it 'raises exception if user has no permissions' do
expect do
described_class.new(project, current_user).execute(path)
end.to raise_exception(PathLocks::LockService::AccessDenied)
end
end
require 'spec_helper'
describe PathLocks::UnlockService, services: true do
let(:path_lock) { create :path_lock }
let(:current_user) { path_lock.user }
let(:project) { path_lock.project }
let(:path) { path_lock.path }
it 'unlocks path' do
allow_any_instance_of(described_class).to receive(:can?).and_return(true)
described_class.new(project, current_user).execute(path_lock)
expect(project.path_locks.find_by(path: path)).to be_falsey
end
it 'raises exception if user has no permissions' do
user = create :user
expect do
described_class.new(project, user).execute(path_lock)
end.to raise_exception(PathLocks::UnlockService::AccessDenied)
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment