From 2b73aaa15ad9f651f51f8c71de461da6664a4fbb Mon Sep 17 00:00:00 2001 From: Ali Ibrahim Date: Sun, 14 Aug 2016 13:49:08 -0400 Subject: [PATCH] Allow to add deploy keys with write-access --- CHANGELOG | 1 + .../projects/deploy_keys_controller.rb | 2 +- .../projects/deploy_keys/_form.html.haml | 9 +++++++ .../20160811172945_add_can_push_to_keys.rb | 27 +++++++++++++++++++ db/schema.rb | 21 ++++++++------- lib/gitlab/git_access.rb | 17 ++++++++---- spec/lib/gitlab/git_access_spec.rb | 11 +++++--- 7 files changed, 68 insertions(+), 20 deletions(-) create mode 100644 db/migrate/20160811172945_add_can_push_to_keys.rb diff --git a/CHANGELOG b/CHANGELOG index 837e9e27aba..d3d0f79039f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -126,6 +126,7 @@ v 8.10.6 - Restore "Largest repository" sort option on Admin > Projects page. !5797 - Fix privilege escalation via project export. - Require administrator privileges to perform a project import. + - Allow to add deploy keys with write-access !5807 (Ali Ibrahim) v 8.10.5 - Add a data migration to fix some missing timestamps in the members table. !5670 diff --git a/app/controllers/projects/deploy_keys_controller.rb b/app/controllers/projects/deploy_keys_controller.rb index 529e0aa2d33..77bd883a5bd 100644 --- a/app/controllers/projects/deploy_keys_controller.rb +++ b/app/controllers/projects/deploy_keys_controller.rb @@ -53,6 +53,6 @@ class Projects::DeployKeysController < Projects::ApplicationController end def deploy_key_params - params.require(:deploy_key).permit(:key, :title) + params.require(:deploy_key).permit(:key, :title, :can_push) end end diff --git a/app/views/projects/deploy_keys/_form.html.haml b/app/views/projects/deploy_keys/_form.html.haml index 901605f7ca3..68f472e8886 100644 --- a/app/views/projects/deploy_keys/_form.html.haml +++ b/app/views/projects/deploy_keys/_form.html.haml @@ -10,4 +10,13 @@ %p.light.append-bottom-0 Paste a machine public key here. Read more about how to generate it = link_to "here", help_page_path("ssh/README") + .form-group + .checkbox + = f.label :can_push do + = f.check_box :can_push + %strong Allow write access + .form-group + %p.light.append-bottom-0 + Can this key be used to push to this repository? Deploy keys always have pull access + = f.submit "Add key", class: "btn-create btn" diff --git a/db/migrate/20160811172945_add_can_push_to_keys.rb b/db/migrate/20160811172945_add_can_push_to_keys.rb new file mode 100644 index 00000000000..de7d07ccabb --- /dev/null +++ b/db/migrate/20160811172945_add_can_push_to_keys.rb @@ -0,0 +1,27 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddCanPushToKeys < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + disable_ddl_transaction! + + def change + add_column_with_default(:keys, :can_push, :boolean, default: false, allow_null: false) + end +end diff --git a/db/schema.rb b/db/schema.rb index 445f484a8c7..d8c6922fb1d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160810142633) do +ActiveRecord::Schema.define(version: 20160811172945) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -484,6 +484,7 @@ ActiveRecord::Schema.define(version: 20160810142633) do t.string "type" t.string "fingerprint" t.boolean "public", default: false, null: false + t.boolean "can_push", default: false, null: false end add_index "keys", ["fingerprint"], name: "index_keys_on_fingerprint", unique: true, using: :btree @@ -589,12 +590,12 @@ ActiveRecord::Schema.define(version: 20160810142633) do t.datetime "locked_at" t.integer "updated_by_id" t.string "merge_error" + t.text "merge_params" t.boolean "merge_when_build_succeeds", default: false, null: false t.integer "merge_user_id" t.string "merge_commit_sha" t.datetime "deleted_at" t.string "in_progress_merge_commit_sha" - t.text "merge_params" end add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree @@ -930,8 +931,8 @@ ActiveRecord::Schema.define(version: 20160810142633) do t.string "noteable_type" t.string "title" t.text "description" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false t.boolean "submitted_as_ham", default: false, null: false end @@ -1001,13 +1002,13 @@ ActiveRecord::Schema.define(version: 20160810142633) do add_index "u2f_registrations", ["user_id"], name: "index_u2f_registrations_on_user_id", using: :btree create_table "user_agent_details", force: :cascade do |t| - t.string "user_agent", null: false - t.string "ip_address", null: false - t.integer "subject_id", null: false - t.string "subject_type", null: false + t.string "user_agent", null: false + t.string "ip_address", null: false + t.integer "subject_id", null: false + t.string "subject_type", null: false t.boolean "submitted", default: false, null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false end create_table "users", force: :cascade do |t| diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 1882eb8d050..f208df3cdce 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -50,10 +50,13 @@ module Gitlab end def push_access_check(changes) + unless project.repository.exists? + return build_status_object(false, "A repository for this project does not exist yet.") + end if user user_push_access_check(changes) elsif deploy_key - build_status_object(false, "Deploy keys are not allowed to push code.") + deploy_key_push_access_check(changes) else raise 'Wrong actor' end @@ -72,10 +75,6 @@ module Gitlab return build_status_object(true) end - unless project.repository.exists? - return build_status_object(false, "A repository for this project does not exist yet.") - end - changes_list = Gitlab::ChangesList.new(changes) # Iterate over all changes to find if user allowed all of them to be applied @@ -90,6 +89,14 @@ module Gitlab build_status_object(true) end + def deploy_key_push_access_check(changes) + if actor.can_push? + build_status_object(true) + else + build_status_object(false, "The deploy key does not have write access to the project.") + end + end + def change_access_check(change) Checks::ChangeAccess.new(change, user_access: user_access, project: project).exec end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index f12c9a370f7..d0e73d70e6e 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -284,19 +284,22 @@ describe Gitlab::GitAccess, lib: true do end describe 'deploy key permissions' do - let(:key) { create(:deploy_key) } - let(:actor) { key } - context 'push code' do subject { access.check('git-receive-pack', '_any') } context 'when project is authorized' do + let(:key) { create(:deploy_key, can_push: true) } + let(:actor) { key } + before { key.projects << project } - it { expect(subject).not_to be_allowed } + it { expect(subject).to be_allowed } end context 'when unauthorized' do + let(:key) { create(:deploy_key, can_push: false) } + let(:actor) { key } + context 'to public project' do let(:project) { create(:project, :public) }