Commit bed94832 authored by Rubén Dávila's avatar Rubén Dávila

Backport of LFS File Locking API

parent ead97c55
Pipeline #17212737 failed with stages
in 23 minutes and 18 seconds
......@@ -10,6 +10,8 @@
module LfsRequest
extend ActiveSupport::Concern
CONTENT_TYPE = 'application/vnd.git-lfs+json'.freeze
included do
before_action :require_lfs_enabled!
before_action :lfs_check_access!
......@@ -50,7 +52,7 @@ module LfsRequest
message: 'Access forbidden. Check your access level.',
documentation_url: help_url
},
content_type: "application/vnd.git-lfs+json",
content_type: CONTENT_TYPE,
status: 403
)
end
......@@ -61,7 +63,7 @@ module LfsRequest
message: 'Not found.',
documentation_url: help_url
},
content_type: "application/vnd.git-lfs+json",
content_type: CONTENT_TYPE,
status: 404
)
end
......
......@@ -98,7 +98,7 @@ class Projects::LfsApiController < Projects::GitHttpClientController
json: {
message: lfs_read_only_message
},
content_type: 'application/vnd.git-lfs+json',
content_type: LfsRequest::CONTENT_TYPE,
status: 403
)
end
......
class Projects::LfsLocksApiController < Projects::GitHttpClientController
include LfsRequest
def create
@result = Lfs::LockFileService.new(project, user, params).execute
render_json(@result[:lock])
end
def unlock
@result = Lfs::UnlockFileService.new(project, user, params).execute
render_json(@result[:lock])
end
def index
@result = Lfs::LocksFinderService.new(project, user, params).execute
render_json(@result[:locks])
end
def verify
@result = Lfs::LocksFinderService.new(project, user, {}).execute
ours, theirs = split_by_owner(@result[:locks])
render_json({ ours: ours, theirs: theirs }, false)
end
private
def render_json(data, process = true)
render json: build_payload(data, process),
content_type: LfsRequest::CONTENT_TYPE,
status: @result[:http_status]
end
def build_payload(data, process)
data = LfsFileLockSerializer.new.represent(data) if process
return data if @result[:status] == :success
# When the locking failed due to an existent Lock, the existent record
# is returned in `@result[:lock]`
error_payload(@result[:message], @result[:lock] ? data : {})
end
def error_payload(message, custom_attrs = {})
custom_attrs.merge({
message: message,
documentation_url: help_url
})
end
def split_by_owner(locks)
groups = locks.partition { |lock| lock.user_id == user.id }
groups.map! do |records|
LfsFileLockSerializer.new.represent(records, root: false)
end
end
def download_request?
params[:action] == 'index'
end
def upload_request?
%w(create unlock verify).include?(params[:action])
end
end
class LfsFileLock < ActiveRecord::Base
belongs_to :project
belongs_to :user
validates :project_id, :user_id, :path, presence: true
def can_be_unlocked_by?(current_user, forced = false)
return true if current_user.id == user_id
forced && current_user.can?(:admin_project, project)
end
end
......@@ -179,6 +179,7 @@ class Project < ActiveRecord::Base
has_many :releases
has_many :lfs_objects_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :lfs_objects, through: :lfs_objects_projects
has_many :lfs_file_locks
has_many :project_group_links
has_many :invited_groups, through: :project_group_links, source: :group
has_many :pages_domains
......
......@@ -164,6 +164,13 @@ class Repository
commits
end
# Returns a list of commits that are not present in any reference
def new_commits(newrev)
refs = ::Gitlab::Git::RevList.new(raw, newrev: newrev).new_refs
refs.map { |sha| commit(sha.strip) }
end
# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/384
def find_commits_by_message(query, ref = nil, path = nil, limit = 1000, offset = 0)
unless exists? && has_visible_content? && query.present?
......
class LfsFileLockEntity < Grape::Entity
root 'locks', 'lock'
expose :path
expose(:id) { |entity| entity.id.to_s }
expose(:created_at, as: :locked_at) { |entity| entity.created_at.to_s(:iso8601) }
expose :owner do
expose(:name) { |entity| entity.user&.name }
end
end
class LfsFileLockSerializer < BaseSerializer
entity LfsFileLockEntity
end
module Lfs
class LockFileService < BaseService
def execute
unless can?(current_user, :push_code, project)
raise Gitlab::GitAccess::UnauthorizedError, 'You have no permissions'
end
create_lock!
rescue ActiveRecord::RecordNotUnique
error('already locked', 409, current_lock)
rescue Gitlab::GitAccess::UnauthorizedError => ex
error(ex.message, 403)
rescue => ex
error(ex.message, 500)
end
private
def current_lock
project.lfs_file_locks.find_by(path: params[:path])
end
def create_lock!
lock = project.lfs_file_locks.create!(user: current_user,
path: params[:path])
success(http_status: 201, lock: lock)
end
def error(message, http_status, lock = nil)
{
status: :error,
message: message,
http_status: http_status,
lock: lock
}
end
end
end
module Lfs
class LocksFinderService < BaseService
def execute
success(locks: find_locks)
rescue => ex
error(ex.message, 500)
end
private
def find_locks
options = params.slice(:id, :path).compact.symbolize_keys
project.lfs_file_locks.where(options)
end
end
end
module Lfs
class UnlockFileService < BaseService
def execute
unless can?(current_user, :push_code, project)
raise Gitlab::GitAccess::UnauthorizedError, 'You have no permissions'
end
unlock_file
rescue Gitlab::GitAccess::UnauthorizedError => ex
error(ex.message, 403)
rescue ActiveRecord::RecordNotFound
error('Lock not found', 404)
rescue => ex
error(ex.message, 500)
end
private
def unlock_file
forced = params[:force] == true
if lock.can_be_unlocked_by?(current_user, forced)
lock.destroy!
success(lock: lock, http_status: :ok)
elsif forced
error('You must have master access to force delete a lock', 403)
else
error("#{lock.path} is locked by GitLab User #{lock.user_id}", 403)
end
end
def lock
return @lock if defined?(@lock)
@lock = if params[:id].present?
project.lfs_file_locks.find(params[:id])
elsif params[:path].present?
project.lfs_file_locks.find_by!(path: params[:path])
end
end
end
end
---
title: Backport of LFS File Locking API
merge_request: 16935
author:
type: added
......@@ -14,4 +14,4 @@ Mime::Type.register "video/webm", :webm
Mime::Type.register "video/ogg", :ogv
Mime::Type.unregister :json
Mime::Type.register 'application/json', :json, %w(application/vnd.git-lfs+json application/json)
Mime::Type.register 'application/json', :json, [LfsRequest::CONTENT_TYPE, 'application/json']
......@@ -16,6 +16,13 @@ scope(path: '*namespace_id/:project_id',
get '/*oid', action: :deprecated
end
scope(path: 'info/lfs') do
resources :lfs_locks, controller: :lfs_locks_api, path: 'locks' do
post :unlock, on: :member
post :verify, on: :collection
end
end
# GitLab LFS object storage
scope(path: 'gitlab-lfs/objects/*oid', controller: :lfs_storage, constraints: { oid: /[a-f0-9]{64}/ }) do
get '/', action: :download
......
class CreateLfsFileLocks < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
create_table :lfs_file_locks do |t|
t.references :project, null: false, foreign_key: { on_delete: :cascade }
t.references :user, null: false, index: true, foreign_key: { on_delete: :cascade }
t.datetime :created_at, null: false
t.string :path, limit: 511
end
add_index :lfs_file_locks, [:project_id, :path], unique: true
end
def down
if foreign_keys_for(:lfs_file_locks, :project_id).any?
remove_foreign_key :lfs_file_locks, column: :project_id
end
if index_exists?(:lfs_file_locks, [:project_id, :path])
remove_concurrent_index :lfs_file_locks, [:project_id, :path]
end
drop_table :lfs_file_locks
end
end
......@@ -947,6 +947,16 @@ ActiveRecord::Schema.define(version: 20180206200543) do
add_index "labels", ["title"], name: "index_labels_on_title", using: :btree
add_index "labels", ["type", "project_id"], name: "index_labels_on_type_and_project_id", using: :btree
create_table "lfs_file_locks", force: :cascade do |t|
t.integer "project_id", null: false
t.integer "user_id", null: false
t.datetime "created_at", null: false
t.string "path", limit: 511
end
add_index "lfs_file_locks", ["project_id", "path"], name: "index_lfs_file_locks_on_project_id_and_path", unique: true, using: :btree
add_index "lfs_file_locks", ["user_id"], name: "index_lfs_file_locks_on_user_id", using: :btree
create_table "lfs_objects", force: :cascade do |t|
t.string "oid", null: false
t.integer "size", limit: 8, null: false
......@@ -1998,6 +2008,8 @@ ActiveRecord::Schema.define(version: 20180206200543) do
add_foreign_key "label_priorities", "projects", on_delete: :cascade
add_foreign_key "labels", "namespaces", column: "group_id", on_delete: :cascade
add_foreign_key "labels", "projects", name: "fk_7de4989a69", on_delete: :cascade
add_foreign_key "lfs_file_locks", "projects", on_delete: :cascade
add_foreign_key "lfs_file_locks", "users", on_delete: :cascade
add_foreign_key "lists", "boards", name: "fk_0d3f677137", on_delete: :cascade
add_foreign_key "lists", "labels", name: "fk_7a5553d60f", on_delete: :cascade
add_foreign_key "members", "users", name: "fk_2e88fb7ce9", on_delete: :cascade
......
......@@ -83,6 +83,72 @@ that are on the remote repository, eg. from branch `master`:
git lfs fetch master
```
## File Locking
The first thing to do before using File Locking is to tell Git LFS which
kind of files are lockable. The following command will store PNG files
in LFS and flag them as lockable:
```bash
git lfs track "*.png" --lockable
```
After executing the above command a file named `.gitattributes` will be
created or updated with the following content:
```bash
*.png filter=lfs diff=lfs merge=lfs -text lockable
```
You can also register a file type as lockable without using LFS
(In order to be able to lock/unlock a file you need a remote server that implements the LFS File Locking API),
in order to do that you can edit the `.gitattributes` file manually:
```bash
*.pdf lockable
```
After a file type has been registered as lockable, Git LFS will make
them readonly on the file system automatically. This means you will
need to lock the file before editing it.
### Managing Locked Files
Once you're ready to edit your file you need to lock it first:
```bash
git lfs lock images/banner.png
Locked images/banner.png
```
This will register the file as locked in your name on the server:
```bash
git lfs locks
images/banner.png joe ID:123
```
Once you have pushed your changes, you can unlock the file so others can
also edit it:
```bash
git lfs unlock images/banner.png
```
You can also unlock by id:
```bash
git lfs unlock --id=123
```
If for some reason you need to unlock a file that was not locked by you,
you can use the `--force` flag as long as you have a `master` access on
the project:
```bash
git lfs unlock --id=123 --force
```
## Troubleshooting
### error: Repository or object not found
......
......@@ -31,13 +31,14 @@ module Gitlab
@protocol = protocol
end
def exec
def exec(skip_commits_check: false)
return true if skip_authorization
push_checks
branch_checks
tag_checks
lfs_objects_exist_check
commits_check unless skip_commits_check
true
end
......@@ -117,6 +118,24 @@ module Gitlab
end
end
def commits_check
return if deletion? || newrev.nil?
# n+1: https://gitlab.com/gitlab-org/gitlab-ee/issues/3593
::Gitlab::GitalyClient.allow_n_plus_1_calls do
commits.each do |commit|
commit_check.validate(commit, validations_for_commit(commit))
end
end
commit_check.validate_file_paths
end
# Method overwritten in EE to inject custom validations
def validations_for_commit(_)
[]
end
private
def updated_from_web?
......@@ -150,6 +169,14 @@ module Gitlab
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:lfs_objects_missing]
end
end
def commit_check
@commit_check ||= Gitlab::Checks::CommitCheck.new(project, user_access.user, newrev, oldrev)
end
def commits
project.repository.new_commits(newrev)
end
end
end
end
module Gitlab
module Checks
class CommitCheck
include Gitlab::Utils::StrongMemoize
attr_reader :project, :user, :newrev, :oldrev
def initialize(project, user, newrev, oldrev)
@project = project
@user = user
@newrev = user
@oldrev = user
@file_paths = []
end
def validate(commit, validations)
return if validations.empty? && path_validations.empty?
commit.raw_deltas.each do |diff|
@file_paths << (diff.new_path || diff.old_path)
validations.each do |validation|
if error = validation.call(diff)
raise ::Gitlab::GitAccess::UnauthorizedError, error
end
end
end
end
def validate_file_paths
path_validations.each do |validation|
if error = validation.call(@file_paths)
raise ::Gitlab::GitAccess::UnauthorizedError, error
end
end
end
private
def validate_lfs_file_locks?
strong_memoize(:validate_lfs_file_locks) do
project.lfs_enabled? && project.lfs_file_locks.any? && newrev && oldrev
end
end
def lfs_file_locks_validation
lambda do |paths|
lfs_lock = project.lfs_file_locks.where(path: paths).where.not(user_id: user.id).first
if lfs_lock
return "The path '#{lfs_lock.path}' is locked in Git LFS by #{lfs_lock.user.name}"
end
end
end
def path_validations
validate_lfs_file_locks? ? [lfs_file_locks_validation] : []
end
end
end
end
......@@ -27,6 +27,8 @@ project_tree:
- :releases
- project_members:
- :user
- lfs_file_locks:
- :user
- merge_requests:
- notes:
- :author
......
FactoryBot.define do
factory :lfs_file_lock do
user
project
path 'README.md'
end
end
......@@ -177,5 +177,44 @@ describe Gitlab::Checks::ChangeAccess do
expect { subject.exec }.not_to raise_error
end
end
context 'LFS file lock check' do
let(:owner) { create(:user) }
let!(:lock) { create(:lfs_file_lock, user: owner, project: project, path: 'README') }
before do
allow(project.repository).to receive(:new_commits).and_return(
project.repository.commits_between('be93687618e4b132087f430a4d8fc3a609c9b77c', '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51')
)
end
context 'with LFS not enabled' do
it 'skips the validation' do
expect_any_instance_of(described_class).not_to receive(:lfs_file_locks_validation)
subject.exec
end
end
context 'with LFS enabled' do
before do
allow(project).to receive(:lfs_enabled?).and_return(true)
end
context 'when change is sent by a different user' do
it 'raises an error if the user is not allowed to update the file' do
expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "The path 'README' is locked in Git LFS by #{lock.user.name}")
end
end
context 'when change is sent by the author od the lock' do
let(:user) { owner }
it "doesn't raise any error" do
expect { subject.exec }.not_to raise_error
end
end
end
end
end
end
......@@ -276,6 +276,7 @@ project:
- fork_network_member
- fork_network
- custom_attributes
- lfs_file_locks
award_emoji:
- awardable
- user
......@@ -290,3 +291,5 @@ push_event_payload:
issue_assignees:
- issue
- assignee
lfs_file_locks:
- user
......@@ -530,3 +530,9 @@ ProjectCustomAttribute:
- project_id
- key
- value
LfsFileLock:
- id
- path
- user_id
- project_id
- created_at
require 'rails_helper'
describe LfsFileLock do
set(:lfs_file_lock) { create(:lfs_file_lock) }
subject { lfs_file_lock }
it { is_expected.to belong_to(:project) }
it { is_expected.to belong_to(:user) }
it { is_expected.to validate_presence_of(:project_id) }
it { is_expected.to validate_presence_of(:user_id) }
it { is_expected.to validate_presence_of(:path) }
describe '#can_be_unlocked_by?' do
let(:developer) { create(:user) }
let(:master) { create(:user) }
before do
project = lfs_file_lock.project
project.add_developer(developer)
project.add_master(master)
end
context "when it's forced" do
it 'can be unlocked by the author' do
user = lfs_file_lock.user
expect(lfs_file_lock.can_be_unlocked_by?(user, true)).to eq(true)
end
it 'can be unlocked by a master' do
expect(lfs_file_lock.can_be_unlocked_by?(master, true)).to eq(true)
end
it "can't be unlocked by other user" do
expect(lfs_file_lock.can_be_unlocked_by?(developer, true)).to eq(false)
end
end
context "when it isn't forced" do
it 'can be unlocked by the author' do
user = lfs_file_lock.user
expect(lfs_file_lock.can_be_unlocked_by?(user)).to eq(true)
end
it "can't be unlocked by a master" do
expect(lfs_file_lock.can_be_unlocked_by?(master)).to eq(false)
end
it "can't be unlocked by other user" do
expect(lfs_file_lock.can_be_unlocked_by?(developer)).to eq(false)
end
end
end
end
......@@ -80,6 +80,7 @@ describe Project do
it { is_expected.to have_many(:members_and_requesters) }
it { is_expected.to have_many(:clusters) }
it { is_expected.to have_many(:custom_attributes).class_name('ProjectCustomAttribute') }
it { is_expected.to have_many(:lfs_file_locks) }
context 'after initialized' do
it "has a project_feature" do
......
......@@ -262,6 +262,28 @@ describe Repository do
end
end
describe '#new_commits' do
let(:new_refs) do
double(:git_rev_list, new_refs: %w[
c1acaa58bbcbc3eafe538cb8274ba387047b69f8
5937ac0a7beb003549fc5fd26fc247adbce4a52e
])
end
it 'delegates to Gitlab::Git::RevList' do
expect(Gitlab::Git::RevList).to receive(:new).with(
repository.raw,