Commit 5281e722 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'rd-1830-integrate-git-lfs-file-locking-with-gitlab-file-locking' into 'master'

Implement LFS locking API

Closes #1830

See merge request gitlab-org/gitlab-ee!4091
parents 149cf8a5 2eb2486c
...@@ -10,6 +10,8 @@ ...@@ -10,6 +10,8 @@
module LfsRequest module LfsRequest
extend ActiveSupport::Concern extend ActiveSupport::Concern
CONTENT_TYPE = 'application/vnd.git-lfs+json'.freeze
included do included do
prepend EE::LfsRequest prepend EE::LfsRequest
before_action :require_lfs_enabled! before_action :require_lfs_enabled!
...@@ -51,7 +53,7 @@ module LfsRequest ...@@ -51,7 +53,7 @@ module LfsRequest
message: 'Access forbidden. Check your access level.', message: 'Access forbidden. Check your access level.',
documentation_url: help_url documentation_url: help_url
}, },
content_type: "application/vnd.git-lfs+json", content_type: CONTENT_TYPE,
status: 403 status: 403
) )
end end
...@@ -62,7 +64,7 @@ module LfsRequest ...@@ -62,7 +64,7 @@ module LfsRequest
message: 'Not found.', message: 'Not found.',
documentation_url: help_url documentation_url: help_url
}, },
content_type: "application/vnd.git-lfs+json", content_type: CONTENT_TYPE,
status: 404 status: 404
) )
end end
......
...@@ -103,7 +103,7 @@ class Projects::LfsApiController < Projects::GitHttpClientController ...@@ -103,7 +103,7 @@ class Projects::LfsApiController < Projects::GitHttpClientController
json: { json: {
message: lfs_read_only_message message: lfs_read_only_message
}, },
content_type: 'application/vnd.git-lfs+json', content_type: LfsRequest::CONTENT_TYPE,
status: 403 status: 403
) )
end 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
...@@ -184,6 +184,7 @@ class Project < ActiveRecord::Base ...@@ -184,6 +184,7 @@ class Project < ActiveRecord::Base
has_many :releases has_many :releases
has_many :lfs_objects_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :lfs_objects_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :lfs_objects, through: :lfs_objects_projects has_many :lfs_objects, through: :lfs_objects_projects
has_many :lfs_file_locks
has_many :project_group_links has_many :project_group_links
has_many :invited_groups, through: :project_group_links, source: :group has_many :invited_groups, through: :project_group_links, source: :group
has_many :pages_domains has_many :pages_domains
......
...@@ -171,6 +171,13 @@ class Repository ...@@ -171,6 +171,13 @@ class Repository
commits commits
end 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 # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/384
def find_commits_by_message(query, ref = nil, path = nil, limit = 1000, offset = 0) def find_commits_by_message(query, ref = nil, path = nil, limit = 1000, offset = 0)
unless exists? && has_visible_content? && query.present? 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
prepend EE::Lfs::LockFileService
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
prepend EE::Lfs::UnlockFileService
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: Integrate current File Locking feature with LFS File Locking
merge_request: 4091
author:
type: added
---
title: Backport of LFS File Locking API
merge_request: 16935
author:
type: added
...@@ -14,4 +14,4 @@ Mime::Type.register "video/webm", :webm ...@@ -14,4 +14,4 @@ Mime::Type.register "video/webm", :webm
Mime::Type.register "video/ogg", :ogv Mime::Type.register "video/ogg", :ogv
Mime::Type.unregister :json 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', ...@@ -16,6 +16,13 @@ scope(path: '*namespace_id/:project_id',
get '/*oid', action: :deprecated get '/*oid', action: :deprecated
end 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 # GitLab LFS object storage
scope(path: 'gitlab-lfs/objects/*oid', controller: :lfs_storage, constraints: { oid: /[a-f0-9]{64}/ }) do scope(path: 'gitlab-lfs/objects/*oid', controller: :lfs_storage, constraints: { oid: /[a-f0-9]{64}/ }) do
get '/', action: :download 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
...@@ -1302,6 +1302,16 @@ ActiveRecord::Schema.define(version: 20180204200836) do ...@@ -1302,6 +1302,16 @@ ActiveRecord::Schema.define(version: 20180204200836) do
t.string "filter" t.string "filter"
end end
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| create_table "lfs_objects", force: :cascade do |t|
t.string "oid", null: false t.string "oid", null: false
t.integer "size", limit: 8, null: false t.integer "size", limit: 8, null: false
...@@ -2552,6 +2562,8 @@ ActiveRecord::Schema.define(version: 20180204200836) do ...@@ -2552,6 +2562,8 @@ ActiveRecord::Schema.define(version: 20180204200836) do
add_foreign_key "label_priorities", "projects", on_delete: :cascade add_foreign_key "label_priorities", "projects", on_delete: :cascade
add_foreign_key "labels", "namespaces", column: "group_id", 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 "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", "boards", name: "fk_0d3f677137", on_delete: :cascade
add_foreign_key "lists", "labels", name: "fk_7a5553d60f", 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 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`: ...@@ -83,6 +83,72 @@ that are on the remote repository, eg. from branch `master`:
git lfs fetch 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 ## Troubleshooting
### error: Repository or object not found ### error: Repository or object not found
......
...@@ -17,7 +17,7 @@ module EE ...@@ -17,7 +17,7 @@ module EE
message: ::Gitlab::RepositorySizeError.new(project).push_error(@exceeded_limit), # rubocop:disable Gitlab/ModuleWithInstanceVariables message: ::Gitlab::RepositorySizeError.new(project).push_error(@exceeded_limit), # rubocop:disable Gitlab/ModuleWithInstanceVariables
documentation_url: help_url documentation_url: help_url
}, },
content_type: "application/vnd.git-lfs+json", content_type: ::LfsRequest::CONTENT_TYPE,
status: 406 status: 406
) )
end end
......
class Projects::PathLocksController < Projects::ApplicationController class Projects::PathLocksController < Projects::ApplicationController
include PathLocksHelper include PathLocksHelper
include ExtractsPath
# Authorize # Authorize
before_action :require_non_empty_project before_action :require_non_empty_project
before_action :authorize_push_code!, only: [:toggle] before_action :authorize_push_code!, only: [:toggle]
before_action :check_license before_action :check_license
before_action :assign_ref_vars, only: :toggle
before_action :lfs_blob_ids, only: :toggle
def index def index
@path_locks = @project.path_locks.page(params[:page]) @path_locks = @project.path_locks.page(params[:page])
...@@ -15,9 +18,9 @@ class Projects::PathLocksController < Projects::ApplicationController ...@@ -15,9 +18,9 @@ class Projects::PathLocksController < Projects::ApplicationController
path_lock = @project.path_locks.find_by(path: params[:path]) path_lock = @project.path_locks.find_by(path: params[:path])
if path_lock if path_lock
PathLocks::UnlockService.new(project, current_user).execute(path_lock) unlock_file(path_lock)
else else
PathLocks::LockService.new(project, current_user).execute(params[:path]) lock_file
end end
head :ok head :ok
...@@ -50,4 +53,35 @@ class Projects::PathLocksController < Projects::ApplicationController ...@@ -50,4 +53,35 @@ class Projects::PathLocksController < Projects::ApplicationController
redirect_to admin_license_path redirect_to admin_license_path
end end
end end
def lock_file
path_lock = PathLocks::LockService.new(project, current_user).execute(params[:path])
if path_lock.persisted? && sync_with_lfs?
Lfs::LockFileService.new(project, current_user, path: params[:path]).execute
end
end
def unlock_file(path_lock)
PathLocks::UnlockService.new(project, current_user).execute(path_lock)
if sync_with_lfs?
Lfs::UnlockFileService.new(project, current_user, path: path_lock.path, force: true).execute
end
end
# Override get_id from ExtractsPath in this case is just the root of the default branch.
def get_id
@ref ||= project.repository.root_ref
end
def lfs_file?
blob = project.repository.blob_at_branch(get_id, params[:path])
@lfs_blob_ids.include?(blob.id)
end
def sync_with_lfs?
project.lfs_enabled? && lfs_file?
end
end end
...@@ -22,13 +22,6 @@ module EE ...@@ -22,13 +22,6 @@ module EE
expire_content_cache expire_content_cache
end 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
def squash(user, merge_request) def squash(user, merge_request)
raw.squash(user, merge_request.id, branch: merge_request.target_branch, raw.squash(user, merge_request.id, branch: merge_request.target_branch,
start_sha: merge_request.diff_start_sha, start_sha: merge_request.diff_start_sha,
......
module EE
module Lfs
module LockFileService
def execute
result = super
if (result[:status] == :success) && project.feature_available?(:file_locks)
PathLocks::LockService.new(project, current_user).execute(result[:lock].path)
end
result
end
end
end
end
module EE
module Lfs
module UnlockFileService
def execute
result = super
if (result[:status] == :success) && project.feature_available?(:file_locks)
if path_lock = project.path_locks.find_by(path: result[:lock].path)
PathLocks::UnlockService.new(project, current_user).execute(path_lock)
end
end
result
end
end
end
end
...@@ -17,35 +17,40 @@ module EE ...@@ -17,35 +17,40 @@ module EE
def exec def exec
return true if skip_authorization return true if skip_authorization
super super(skip_commits_check: true)
push_rule_check push_rule_check
# Check of commits should happen as the last step
# given they're expensive in terms of performance
commits_check
true true
end end
private private
def push_rule
project.push_rule
end
def push_rule_check def push_rule_check
return unless newrev && oldrev && project.feature_available?(:push_rules) return unless newrev && oldrev && project.feature_available?(:push_rules)
push_rule = project.push_rule
if tag_name if tag_name
push_rule_tag_check(push_rule) push_rule_tag_check
else else
push_rule_branch_check(push_rule) push_rule_branch_check
end end
end end
def push_rule_tag_check(push_rule) def push_rule_tag_check
if tag_deletion_denied_by_push_rule?(push_rule) if tag_deletion_denied_by_push_rule?
raise ::Gitlab::GitAccess::UnauthorizedError, 'You cannot delete a tag' raise ::Gitlab::GitAccess::UnauthorizedError, 'You cannot delete a tag'
end end
end end
def push_rule_branch_check(push_rule) def push_rule_branch_check
unless branch_name_allowed_by_push_rule?(push_rule) unless branch_name_allowed_by_push_rule?
message = ERROR_MESSAGES[:push_rule_branch_name] % { branch_name_regex: push_rule.branch_name_regex } message = ERROR_MESSAGES[:push_rule_branch_name] % { branch_name_regex: push_rule.branch_name_regex }
raise ::Gitlab::GitAccess::UnauthorizedError.new(message) raise ::Gitlab::GitAccess::UnauthorizedError.new(message)
end end
...@@ -54,51 +59,44 @@ module EE ...@@ -54,51 +59,44 @@ module EE
# if newrev is blank, the branch was deleted # if newrev is blank, the branch was deleted
return if deletion? || !(commit_validation || validate_path_locks?) return if deletion? || !(commit_validation || validate_path_locks?)
# n+1: https://gitlab.com/gitlab-org/gitlab-ee/issues/3593 commits.each do |commit|
::Gitlab::GitalyClient.allow_n_plus_1_calls do push_rule_commit_check(commit)
commits.each do |commit|
push_rule_commit_check(commit, push_rule)
end
end end
rescue ::PushRule::MatchError => e rescue ::PushRule::MatchError => e
raise ::Gitlab::GitAccess::UnauthorizedError, e.message raise ::Gitlab::GitAccess::UnauthorizedError, e.message
end end
def branch_name_allowed_by_push_rule?(push_rule) def branch_name_allowed_by_push_rule?
return true if skip_branch_name_push_rule?(push_rule) return true if skip_branch_name_push_rule?
push_rule.branch_name_allowed?(branch_name) push_rule.branch_name_allowed?(branch_name)
end end
def skip_branch_name_push_rule?(push_rule) def skip_branch_name_push_rule?
push_rule.nil? || push_rule.nil? ||
deletion? || deletion? ||
branch_name.blank? || branch_name.blank? ||
branch_name == project.default_branch branch_name == project.default_branch
end end
def tag_deletion_denied_by_push_rule?(push_rule) def tag_deletion_denied_by_push_rule?
push_rule.try(:deny_delete_tag) && push_rule.try(:deny_delete_tag) &&
!updated_from_web? && !updated_from_web? &&
deletion? && deletion? &&
tag_exists? tag_exists?
end end
def push_rule_commit_check(commit, push_rule) def push_rule_commit_check(commit)
if push_rule.try(:commit_validation?) if push_rule.try(:commit_validation?)
error = check_commit(commit, push_rule) error = check_commit(commit)
raise ::Gitlab::GitAccess::UnauthorizedError, error if error raise ::Gitlab::GitAccess::UnauthorizedError, error if error
end end
if error = check_commit_diff(commit, push_rule)
raise ::Gitlab::GitAccess::UnauthorizedError, error
end
end end
# If commit does not pass push rule validation the whole push should be rejected. # If commit does not pass push rule validation the whole push should be rejected.
# This method should return nil if no error found or a string if error. # This method should return nil if no error found or a string if error.
# In case of errors - all other checks will be canceled and push will be rejected. # In case of errors - all other checks will be canceled and push will be rejected.
def check_commit(commit, push_rule) def check_commit(commit)
unless push_rule.commit_message_allowed?(commit.safe_message) unless push_rule.commit_message_allowed?(commit.safe_message)
return "Commit message does not follow the pattern '#{push_rule.commit_message_regex}'" return "Commit message does not follow the pattern '#{push_rule.commit_message_regex}'"
end end
...@@ -111,7 +109,7 @@ module EE ...@@ -111,7 +109,7 @@ module EE
return "Author's email '#{commit.author_email}' does not follow the pattern '#{push_rule.author_email_regex}'" return "Author's email '#{commit.author_email}' does not follow the pattern '#{push_rule.author_email_regex}'"
end end
committer_error_message = committer_check(commit, push_rule) committer_error_message = committer_check(commit)
return committer_error_message if committer_error_message return committer_error_message if committer_error_message
if !updated_from_web? && !push_rule.commit_signature_allowed?(commit) if !updated_from_web? && !push_rule.commit_signature_allowed?(commit)
...@@ -134,7 +132,7 @@ module EE ...@@ -134,7 +132,7 @@ module EE
nil nil
end end
def committer_check(commit, push_rule) def committer_check(commit)
unless push_rule.committer_allowed?(commit.committer_email, user_access.user) unless push_rule.committer_allowed?(commit.committer_email, user_access.user)
committer_is_current_user = commit.committer == user_access.user committer_is_current_user = commit.committer == user_access.user
...@@ -146,38 +144,22 @@ module EE ...@@ -146,38 +144,22 @@ module EE
end end
end end
def check_commit_diff(commit, push_rule) override :validations_for_commit
validations = validations_for_commit(commit, push_rule) def validations_for_commit(commit)
validations = super
return if validations.empty?
commit.raw_deltas.each do |diff|
validations.each do |validation|
if error = validation.call(diff)
return error
end
end
end
nil validations.push(path_locks_validation) if validate_path_locks?
validations.concat(push_rule_commit_validations(commit))
end end
def validations_for_commit(commit, push_rule) def push_rule_commit_validations(commit)
validations = base_validations return [] unless push_rule
return validations unless push_rule
validations << file_name_validation(push_rule)
if push_rule.max_file_size > 0 [file_name_validation].tap do |validations|
validations << file_size_validation(commit, push_rule.max_file_size) if push_rule.max_file_size > 0
validations << file_size_validation(commit, push_rule.max_file_size)
end
end end
validations
end
def base_validations
validate_path_locks? ? [path_locks_validation] : []
end end
def validate_path_locks? def validate_path_locks?
...@@ -200,12 +182,16 @@ module EE ...@@ -200,12 +182,16 @@ module EE
end end
end end
def file_name_validation(push_rule) def file_name_validation
lambda do |diff| lambda do |diff|
if (diff.renamed_file || diff.new_file) && blacklisted_regex = push_rule.filename_blacklisted?(diff.new_path) begin
return nil unless blacklisted_regex.present? if (diff.renamed_file || diff.new_file) && blacklisted_regex = push_rule.filename_blacklisted?(diff.new_path)
return nil unless blacklisted_regex.present?
"File name #{diff.new_path} was blacklisted by the pattern #{blacklisted_regex}." "File name #{diff.new_path} was blacklisted by the pattern #{blacklisted_regex}."
end
rescue ::PushRule::MatchError => e
raise ::Gitlab::GitAccess::UnauthorizedError, e.message
end end
end end
end end
...@@ -220,10 +206,6 @@ module EE ...@@ -220,10 +206,6 @@ module EE
end end
end end
end end
def commits
project.repository.new_commits(newrev)
end
end end
end end
end end
......
...@@ -33,13 +33,14 @@ module Gitlab ...@@ -33,13 +33,14 @@ module Gitlab
@protocol = protocol @protocol = protocol
end end
def exec def exec(skip_commits_check: false)
return true if skip_authorization return true if skip_authorization
push_checks push_checks
branch_checks branch_checks
tag_checks tag_checks
lfs_objects_exist_check lfs_objects_exist_check
commits_check unless skip_commits_check
true true
end end
...@@ -119,6 +120,24 @@ module Gitlab ...@@ -119,6 +120,24 @@ module Gitlab
end end
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 private
def updated_from_web? def updated_from_web?
...@@ -152,6 +171,14 @@ module Gitlab ...@@ -152,6 +171,14 @@ module Gitlab
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:lfs_objects_missing] raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:lfs_objects_missing]
end end
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 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: ...@@ -27,6 +27,8 @@ project_tree:
- :releases - :releases
- project_members: - project_members:
- :user - :user
- lfs_file_locks:
- :user
- merge_requests: - merge_requests:
- notes: - notes:
- :author - :author
......
require 'rails_helper'
describe Projects::PathLocksController, type: :request do
let(:project) { create(:project, :repository) }
let(:user) { project.owner }
let(:viewer) { user }
before do
login_as(viewer)
end
describe 'POST #toggle' do
context 'when locking a file' do
context 'when LFS is enabled' do
before do
allow_any_instance_of(Projects::PathLocksController).to receive(:sync_with_lfs?).and_return(true)
end
it 'locks the file' do
expect { toggle_lock('README.md') }.to change { PathLock.count }.to(1)
expect(response).to have_gitlab_http_status(200)
end
it "locks the file in LFS" do
expect { toggle_lock('README.md') }.to change { LfsFileLock.count }.to(1)
end
end
context 'when LFS is not enabled' do
it 'locks the file' do
expect { toggle_lock('README.md') }.to change { PathLock.count }.to(1)
expect(response).to have_gitlab_http_status(200)
end
it "doesn't lock the file in LFS" do
expect { toggle_lock('README.md') }.not_to change { LfsFileLock.count }
end
end
end
context 'when unlocking a file' do
context 'when LFS is enabled' do
before do
allow_any_instance_of(Projects::PathLocksController).to receive(:sync_with_lfs?).and_return(true)
toggle_lock('README.md')
end
it 'unlocks the file' do
expect { toggle_lock('README.md') }.to change { PathLock.count }.to(0)
expect(response).to have_gitlab_http_status(200)
end
it "unlocks the file in LFS" do
expect { toggle_lock('README.md') }.to change { LfsFileLock.count }.to(0)
end
end
end
context 'when LFS is not enabled' do
before do
toggle_lock('README.md')
end
it 'unlocks the file' do
expect { toggle_lock('README.md') }.to change { PathLock.count }.to(0)
expect(response).to have_gitlab_http_status(200)
end
end
end
def toggle_lock(path)
post toggle_project_path_locks_path(project), path: path
end
end
require 'spec_helper'
describe EE::Repository do
let(:project) { create(:project, :repository) }
let(:repository) { project.repository }
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,
newrev: 'aaaabbbbccccddddeeeeffffgggghhhhiiiijjjj').and_return(new_refs)
commits = repository.new_commits('aaaabbbbccccddddeeeeffffgggghhhhiiiijjjj')
expect(commits).to eq([
repository.commit('c1acaa58bbcbc3eafe538cb8274ba387047b69f8'),
repository.commit('5937ac0a7beb003549fc5fd26fc247adbce4a52e')
])
end
end
end
FactoryBot.define do
factory :lfs_file_lock do
user
project
path 'README.md'
end
end
...@@ -208,5 +208,44 @@ describe Gitlab::Checks::ChangeAccess do ...@@ -208,5 +208,44 @@ describe Gitlab::Checks::ChangeAccess do
end end
end end
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
end end
...@@ -305,6 +305,7 @@ project: ...@@ -305,6 +305,7 @@ project:
- fork_network_member - fork_network_member
- fork_network - fork_network
- custom_attributes - custom_attributes
- lfs_file_locks
award_emoji: award_emoji:
- awardable - awardable
- user - user
...@@ -322,3 +323,5 @@ issue_assignees: ...@@ -322,3 +323,5 @@ issue_assignees:
epic_issues: epic_issues:
- issue - issue
- epic - epic
lfs_file_locks:
- user
...@@ -550,3 +550,9 @@ ProjectCustomAttribute: ...@@ -550,3 +550,9 @@ ProjectCustomAttribute:
- project_id - project_id
- key - key
- value - 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
...@@ -81,6 +81,7 @@ describe Project do ...@@ -81,6 +81,7 @@ describe Project do
it { is_expected.to have_many(:members_and_requesters) } it { is_expected.to have_many(:members_and_requesters) }
it { is_expected.to have_many(:clusters) } it { is_expected.to have_many(:clusters) }
it { is_expected.to have_many(:custom_attributes).class_name('ProjectCustomAttribute') } it { is_expected.to have_many(:custom_attributes).class_name('ProjectCustomAttribute') }
it { is_expected.to have_many(:lfs_file_locks) }
context 'after initialized' do context 'after initialized' do
it "has a project_feature" do it "has a project_feature" do
......
...@@ -262,6 +262,28 @@ describe Repository do ...@@ -262,6 +262,28 @@ describe Repository do
end end
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,
newrev: 'aaaabbbbccccddddeeeeffffgggghhhhiiiijjjj').and_return(new_refs)
commits = repository.new_commits('aaaabbbbccccddddeeeeffffgggghhhhiiiijjjj')
expect(commits).to eq([
repository.commit('c1acaa58bbcbc3eafe538cb8274ba387047b69f8'),
repository.commit('5937ac0a7beb003549fc5fd26fc247adbce4a52e')
])
end
end
describe '#commits_by' do describe '#commits_by' do
set(:project) { create(:project, :repository) } set(:project) { create(:project, :repository) }
......
...@@ -1302,7 +1302,7 @@ describe 'Git LFS API and storage' do ...@@ -1302,7 +1302,7 @@ describe 'Git LFS API and storage' do
end end
def post_lfs_json(url, body = nil, headers = nil) def post_lfs_json(url, body = nil, headers = nil)
post(url, body.try(:to_json), (headers || {}).merge('Content-Type' => 'application/vnd.git-lfs+json')) post(url, body.try(:to_json), (headers || {}).merge('Content-Type' => LfsRequest::CONTENT_TYPE))
end end
def json_response def json_response
......
require 'spec_helper'
describe 'Git LFS File Locking API' do
include WorkhorseHelpers
let(:project) { create(:project) }
let(:master) { create(:user) }
let(:developer) { create(:user) }
let(:guest) { create(:user) }
let(:path) { 'README.md' }
let(:headers) do
{
'Authorization' => authorization
}.compact
end
shared_examples 'unauthorized request' do
context 'when user is not authorized' do
let(:authorization) { authorize_user(guest) }
it 'returns a forbidden 403 response' do
post_lfs_json url, body, headers
expect(response).to have_gitlab_http_status(403)
end
end
end
before do
allow(Gitlab.config.lfs).to receive(:enabled).and_return(true)
project.add_developer(master)
project.add_developer(developer)
project.add_guest(guest)
end
describe 'Create File Lock endpoint' do
let(:url) { "#{project.http_url_to_repo}/info/lfs/locks" }
let(:authorization) { authorize_user(developer) }
let(:body) { { path: path } }
include_examples 'unauthorized request'
context 'with an existent lock' do
before do
lock_file('README.md', developer)
end
it 'return an error message' do
post_lfs_json url, body, headers
expect(response).to have_gitlab_http_status(409)
expect(json_response.keys).to match_array(%w(lock message documentation_url))
expect(json_response['message']).to match(/already locked/)
end
it 'returns the existen lock' do
post_lfs_json url, body, headers
expect(json_response['lock']['path']).to eq('README.md')
end
end
context 'without an existent lock' do
it 'creates the lock' do
post_lfs_json url, body, headers
expect(response).to have_gitlab_http_status(201)
expect(json_response['lock'].keys).to match_array(%w(id path locked_at owner))
end
end
end
describe 'Listing File Locks endpoint' do
let(:url) { "#{project.http_url_to_repo}/info/lfs/locks" }
let(:authorization) { authorize_user(developer) }
include_examples 'unauthorized request'
it 'returns the list of locked files' do
lock_file('README.md', developer)
lock_file('README', developer)
do_get url, nil, headers
expect(response).to have_gitlab_http_status(200)
expect(json_response['locks'].size).to eq(2)
expect(json_response['locks'].first.keys).to match_array(%w(id path locked_at owner))
end
end
describe 'List File Locks for verification endpoint' do
let(:url) { "#{project.http_url_to_repo}/info/lfs/locks/verify" }
let(:authorization) { authorize_user(developer) }
include_examples 'unauthorized request'
it 'returns the list of locked files grouped by owner' do
lock_file('README.md', master)
lock_file('README', developer)
post_lfs_json url, nil, headers
expect(response).to have_gitlab_http_status(200)
expect(json_response['ours'].size).to eq(1)
expect(json_response['ours'].first['path']).to eq('README')
expect(json_response['theirs'].size).to eq(1)
expect(json_response['theirs'].first['path']).to eq('README.md')
end
end
describe 'Delete File Lock endpoint' do
let!(:lock) { lock_file('README.md', developer) }
let(:url) { "#{project.http_url_to_repo}/info/lfs/locks/#{lock[:id]}/unlock" }
let(:authorization) { authorize_user(developer) }
include_examples 'unauthorized request'
context 'with an existent lock' do
it 'deletes the lock' do
post_lfs_json url, nil, headers
expect(response).to have_gitlab_http_status(200)
end
it 'returns the deleted lock' do
post_lfs_json url, nil, headers
expect(json_response['lock'].keys).to match_array(%w(id path locked_at owner))
end
end
end
def lock_file(path, author)
result = Lfs::LockFileService.new(project, author, { path: path }).execute
result[:lock]
end
def authorize_user(user)
ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password)
end
def post_lfs_json(url, body = nil, headers = nil)
post(url, body.try(:to_json), (headers || {}).merge('Content-Type' => LfsRequest::CONTENT_TYPE))
end
def do_get(url, params = nil, headers = nil)
get(url, (params || {}), (headers || {}).merge('Content-Type' => LfsRequest::CONTENT_TYPE))
end
def json_response
@json_response ||= JSON.parse(response.body)
end
end
require 'spec_helper'
describe LfsFileLockEntity do
let(:user) { create(:user) }
let(:resource) { create(:lfs_file_lock, user: user) }
let(:request) { double('request', current_user: user) }
subject { described_class.new(resource, request: request).as_json }
it 'exposes basic attrs of the lock' do
expect(subject).to include(:id, :path, :locked_at)
end
it 'exposes the owner info' do
expect(subject).to include(:owner)
expect(subject[:owner][:name]).to eq(user.name)
end
end
require 'spec_helper'
describe Lfs::LockFileService do
let(:project) { create(:project) }
let(:current_user) { create(:user) }
subject { described_class.new(project, current_user, params) }
describe '#execute' do
let(:params) { { path: 'README.md' } }
context 'when not authorized' do
it "doesn't succeed" do
result = subject.execute
expect(result[:status]).to eq(:error)
expect(result[:http_status]).to eq(403)
expect(result[:message]).to eq('You have no permissions')
end
end
context 'when authorized' do
before do
project.add_developer(current_user)
end
context 'with an existent lock' do
let!(:lock) { create(:lfs_file_lock, project: project) }
it "doesn't succeed" do
expect(subject.execute[:status]).to eq(:error)
end
it "doesn't create the Lock" do
expect do
subject.execute
end.not_to change { LfsFileLock.count }
end
end
context 'without an existent lock' do
it "succeeds" do
expect(subject.execute[:status]).to eq(:success)
end
it "creates the Lock" do
expect do
subject.execute
end.to change { LfsFileLock.count }.by(1)
end
end
context 'when an error is raised' do
it "doesn't succeed" do
allow_any_instance_of(described_class).to receive(:create_lock!).and_raise(StandardError)
expect(subject.execute[:status]).to eq(:error)
end
end
context 'when File Locking is available' do
before do
stub_licensed_features(file_locks: true)
end
it 'creates the Path Lock' do
expect { subject.execute }.to change { PathLock.count }.to(1)
end
end
context 'when File Locking is not available' do
before do
stub_licensed_features(file_locks: false)
end
it 'creates the Path Lock' do
expect { subject.execute }.not_to change { PathLock.count }
end
end
end
end
end
require 'spec_helper'
describe Lfs::LocksFinderService do
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:params) { {} }
subject { described_class.new(project, user, params) }
shared_examples 'no results' do
it 'returns an empty list' do
result = subject.execute
expect(result[:status]).to eq(:success)
expect(result[:locks]).to be_blank
end
end
describe '#execute' do
let!(:lock_1) { create(:lfs_file_lock, project: project) }
let!(:lock_2) { create(:lfs_file_lock, project: project, path: 'README') }
context 'find by id' do
context 'with results' do
let(:params) do
{ id: lock_1.id }
end
it 'returns the record' do
result = subject.execute
expect(result[:status]).to eq(:success)
expect(result[:locks].size).to eq(1)
expect(result[:locks].first).to eq(lock_1)
end
end
context 'without results' do
let(:params) do
{ id: 123 }
end
include_examples 'no results'
end
end
context 'find by path' do
context 'with results' do
let(:params) do
{ path: lock_1.path }
end
it 'returns the record' do
result = subject.execute
expect(result[:status]).to eq(:success)
expect(result[:locks].size).to eq(1)
expect(result[:locks].first).to eq(lock_1)
end
end
context 'without results' do
let(:params) do
{ path: 'not-found' }
end
include_examples 'no results'
end
end
context 'find all' do
context 'with results' do
it 'returns all the records' do
result = subject.execute
expect(result[:status]).to eq(:success)
expect(result[:locks].size).to eq(2)
end
end
context 'without results' do
before do
LfsFileLock.delete_all
end
include_examples 'no results'
end
end
context 'when an error is raised' do
it "doesn't succeed" do
allow_any_instance_of(described_class).to receive(:find_locks).and_raise(StandardError)
result = subject.execute
expect(result[:status]).to eq(:error)
expect(result[:locks]).to be_blank
end
end
end
end
require 'spec_helper'
describe Lfs::UnlockFileService do
let(:project) { create(:project) }
let(:current_user) { create(:user) }
let(:lock_author) { create(:user) }
let!(:lock) { create(:lfs_file_lock, user: lock_author, project: project) }
let(:params) { {} }
subject { described_class.new(project, current_user, params) }
describe '#execute' do
context 'when not authorized' do
it "doesn't succeed" do
result = subject.execute
expect(result[:status]).to eq(:error)
expect(result[:http_status]).to eq(403)
expect(result[:message]).to eq('You have no permissions')
end
end
context 'when authorized' do
before do
project.add_developer(current_user)
end
context 'when lock does not exists' do
let(:params) { { id: 123 } }
it "doesn't succeed" do
result = subject.execute
expect(result[:status]).to eq(:error)
expect(result[:http_status]).to eq(404)
end
end
context 'when unlocked by the author' do
let(:current_user) { lock_author }
let(:params) { { id: lock.id } }
it "succeeds" do
result = subject.execute
expect(result[:status]).to eq(:success)
expect(result[:lock]).to be_present
end
end
context 'when unlocked by a different user' do
let(:current_user) { create(:user) }
let(:params) { { id: lock.id } }
it "doesn't succeed" do
result = subject.execute
expect(result[:status]).to eq(:error)
expect(result[:message]).to match(/is locked by GitLab User #{lock_author.id}/)
expect(result[:http_status]).to eq(403)
end
end
context 'when forced' do
let(:developer) { create(:user) }
let(:master) { create(:user) }
before do
project.add_developer(developer)
project.add_master(master)
end
context 'by a regular user' do
let(:current_user) { developer }
let(:params) do
{ id: lock.id,
force: true }
end
it "doesn't succeed" do
result = subject.execute
expect(result[:status]).to eq(:error)
expect(result[:message]).to match(/You must have master access/)
expect(result[:http_status]).to eq(403)
end
end
context 'by a master user' do
let(:current_user) { master }
let(:params) do
{ id: lock.id,
force: true }
end
it "succeeds" do
result = subject.execute
expect(result[:status]).to eq(:success)
expect(result[:lock]).to be_present
end
end
end
describe 'File Locking integraction' do
let(:params) { { id: lock.id } }
let(:current_user) { lock_author }
before do
project.add_developer(lock_author)
project.path_locks.create(path: lock.path, user: lock_author)
end
context 'when File Locking is available' do
it 'deletes the Path Lock' do
expect { subject.execute }.to change { PathLock.count }.to(0)
end
end
context 'when File Locking is not available' do
before do
stub_licensed_features(file_locks: false)
end
# For some reason RSpec is reseting the mock and
# License.feature_available?(:file_locks) returns true when the spec runs.
xit 'does not delete the Path Lock' do
expect { subject.execute }.not_to change { PathLock.count }
end
end
end
end
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