Commit 65c5cdb6 authored by Douwe Maan's avatar Douwe Maan Committed by Oswaldo Ferreira

Merge branch 'sh-migrate-can-push-to-deploy-keys-projects-10-3' into 'security-10-3'

[10.3] Migrate `can_push` column from `keys` to `deploy_keys_project`

See merge request gitlab/gitlabhq!2276

(cherry picked from commit f6ca52d31bac350a23938e0aebf717c767b4710c)

1f2bd3c0 Backport to 10.3
parent 523050b6
<script>
import actionBtn from './action_btn.vue';
import tooltip from '../../vue_shared/directives/tooltip';
export default {
props: {
......@@ -16,6 +17,9 @@
required: true,
},
},
directives: {
tooltip,
},
components: {
actionBtn,
},
......@@ -31,6 +35,9 @@
isEnabled(id) {
return this.store.findEnabledKey(id) !== undefined;
},
tooltipTitle(project) {
return project.can_push ? 'Write access allowed' : 'Read access only';
},
},
};
</script>
......@@ -51,20 +58,22 @@
<div class="description">
{{ deployKey.fingerprint }}
</div>
<div
v-if="deployKey.can_push"
class="write-access-allowed"
>
Write access allowed
</div>
</div>
<div class="deploy-key-content prepend-left-default deploy-key-projects">
<a
v-for="project in deployKey.projects"
v-for="deployKeysProject in deployKey.deploy_keys_projects"
class="label deploy-project-label"
:href="project.full_path"
:href="deployKeysProject.project.full_path"
:title="tooltipTitle(deployKeysProject)"
v-tooltip
>
{{ project.full_name }}
{{ deployKeysProject.project.full_name }}
<i
v-if="!deployKeysProject.can_push"
aria-hidden="true"
class="fa fa-lock"
>
</i>
</a>
</div>
<div class="deploy-key-content">
......
......@@ -50,10 +50,10 @@ class Admin::DeployKeysController < Admin::ApplicationController
end
def create_params
params.require(:deploy_key).permit(:key, :title, :can_push)
params.require(:deploy_key).permit(:key, :title)
end
def update_params
params.require(:deploy_key).permit(:title, :can_push)
params.require(:deploy_key).permit(:title)
end
end
......@@ -24,7 +24,7 @@ class Projects::DeployKeysController < Projects::ApplicationController
def create
@key = DeployKeys::CreateService.new(current_user, create_params).execute
unless @key.valid? && @project.deploy_keys << @key
unless @key.valid?
flash[:alert] = @key.errors.full_messages.join(', ').html_safe
end
redirect_to_repository_settings(@project)
......@@ -70,11 +70,14 @@ class Projects::DeployKeysController < Projects::ApplicationController
end
def create_params
params.require(:deploy_key).permit(:key, :title, :can_push)
create_params = params.require(:deploy_key)
.permit(:key, :title, deploy_keys_projects_attributes: [:can_push])
create_params.dig(:deploy_keys_projects_attributes, '0')&.merge!(project_id: @project.id)
create_params
end
def update_params
params.require(:deploy_key).permit(:title, :can_push)
params.require(:deploy_key).permit(:title, deploy_keys_projects_attributes: [:id, :can_push])
end
def authorize_update_deploy_key!
......
class DeployKey < Key
has_many :deploy_keys_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
include IgnorableColumn
has_many :deploy_keys_projects, inverse_of: :deploy_key, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :projects, through: :deploy_keys_projects
scope :in_projects, ->(projects) { joins(:deploy_keys_projects).where('deploy_keys_projects.project_id in (?)', projects) }
scope :are_public, -> { where(public: true) }
ignore_column :can_push
accepts_nested_attributes_for :deploy_keys_projects
def private?
!public?
end
......@@ -22,10 +28,18 @@ class DeployKey < Key
end
def has_access_to?(project)
projects.include?(project)
deploy_keys_project_for(project).present?
end
def can_push_to?(project)
can_push? && has_access_to?(project)
!!deploy_keys_project_for(project)&.can_push?
end
def deploy_keys_project_for(project)
deploy_keys_projects.find_by(project: project)
end
def projects_with_write_access
Project.preload(:route).where(id: deploy_keys_projects.with_write_access.select(:project_id))
end
end
class DeployKeysProject < ActiveRecord::Base
belongs_to :project
belongs_to :deploy_key
belongs_to :deploy_key, inverse_of: :deploy_keys_projects
validates :deploy_key_id, presence: true
scope :without_project_deleted, -> { joins(:project).where(projects: { pending_delete: false }) }
scope :in_project, ->(project) { where(project: project) }
scope :with_write_access, -> { where(can_push: true) }
accepts_nested_attributes_for :deploy_key
validates :deploy_key, presence: true
validates :deploy_key_id, uniqueness: { scope: [:project_id], message: "already exists in project" }
validates :project_id, presence: true
......
......@@ -7,7 +7,7 @@ module Projects
delegate :size, to: :available_public_keys, prefix: true
def new_key
@key ||= DeployKey.new
@key ||= DeployKey.new.tap { |dk| dk.deploy_keys_projects.build }
end
def enabled_keys
......
......@@ -3,19 +3,20 @@ class DeployKeyEntity < Grape::Entity
expose :user_id
expose :title
expose :fingerprint
expose :can_push
expose :destroyed_when_orphaned?, as: :destroyed_when_orphaned
expose :almost_orphaned?, as: :almost_orphaned
expose :created_at
expose :updated_at
expose :projects, using: ProjectEntity do |deploy_key|
deploy_key.projects.without_deleted.select { |project| options[:user].can?(:read_project, project) }
expose :deploy_keys_projects, using: DeployKeysProjectEntity do |deploy_key|
deploy_key.deploy_keys_projects
.without_project_deleted
.select { |deploy_key_project| Ability.allowed?(options[:user], :read_project, deploy_key_project.project) }
end
expose :can_edit
private
def can_edit
options[:user].can?(:update_deploy_key, object)
Ability.allowed?(options[:user], :update_deploy_key, object)
end
end
class DeployKeysProjectEntity < Grape::Entity
expose :can_push
expose :project, using: ProjectEntity
end
......@@ -12,7 +12,7 @@
%tr
%th.col-sm-2 Title
%th.col-sm-4 Fingerprint
%th.col-sm-2 Write access allowed
%th.col-sm-2 Projects with write access
%th.col-sm-2 Added at
%th.col-sm-2
%tbody
......@@ -23,10 +23,8 @@
%td
%code.key-fingerprint= deploy_key.fingerprint
%td
- if deploy_key.can_push?
Yes
- else
No
- deploy_key.projects_with_write_access.each do |project|
= link_to project.full_name, admin_project_path(project), class: 'label deploy-project-label'
%td
%span.cgray
added #{time_ago_with_tooltip(deploy_key.created_at)}
......
......@@ -10,13 +10,15 @@
%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 Write access allowed
.form-group
%p.light.append-bottom-0
Allow this key to push to repository as well? (Default only allows pull access.)
= f.fields_for :deploy_keys_projects do |deploy_keys_project_form|
.form-group
.checkbox
= deploy_keys_project_form.label :can_push do
= deploy_keys_project_form.check_box :can_push
%strong Write access allowed
.form-group
%p.light.append-bottom-0
Allow this key to push to repository as well? (Default only allows pull access.)
= f.submit "Add key", class: "btn-create btn"
- form = local_assigns.fetch(:form)
- deploy_key = local_assigns.fetch(:deploy_key)
- deploy_keys_project = deploy_key.deploy_keys_project_for(@project)
= form_errors(deploy_key)
......@@ -20,11 +21,13 @@
.col-sm-10
= form.text_field :fingerprint, class: 'form-control', readonly: 'readonly'
.form-group
.control-label
.col-sm-10
= form.label :can_push do
= form.check_box :can_push
%strong Write access allowed
%p.light.append-bottom-0
Allow this key to push to repository as well? (Default only allows pull access.)
- if deploy_keys_project.present?
= form.fields_for :deploy_keys_projects, deploy_keys_project do |deploy_keys_project_form|
.form-group
.control-label
.col-sm-10
= deploy_keys_project_form.label :can_push do
= deploy_keys_project_form.check_box :can_push
%strong Write access allowed
%p.light.append-bottom-0
Allow this key to push to repository as well? (Default only allows pull access.)
---
title: Fix writable shared deploy keys
merge_request:
author:
type: security
class AddCanPushToDeployKeysProjects < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
disable_ddl_transaction!
def up
add_column_with_default :deploy_keys_projects, :can_push, :boolean, default: false, allow_null: false
end
def down
remove_column :deploy_keys_projects, :can_push
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class PopulateCanPushFromDeployKeysProjects < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
disable_ddl_transaction!
class DeploysKeyProject < ActiveRecord::Base
include EachBatch
self.table_name = 'deploy_keys_projects'
end
def up
DeploysKeyProject.each_batch(of: 10_000) do |batch|
start_id, end_id = batch.pluck('MIN(id), MAX(id)').first
execute <<-EOF
UPDATE deploy_keys_projects
SET can_push = keys.can_push
FROM keys
WHERE deploy_key_id = keys.id
AND deploy_keys_projects.id BETWEEN #{start_id} AND #{end_id}
EOF
end
end
def down
DeploysKeyProject.each_batch(of: 10_000) do |batch|
start_id, end_id = batch.pluck('MIN(id), MAX(id)').first
execute <<-EOF
UPDATE keys
SET can_push = deploy_keys_projects.can_push
FROM deploy_keys_projects
WHERE deploy_keys_projects.deploy_key_id = keys.id
AND deploy_keys_projects.id BETWEEN #{start_id} AND #{end_id}
EOF
end
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class PostPopulateCanPushFromDeployKeysProjects < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
class DeploysKeyProject < ActiveRecord::Base
include EachBatch
self.table_name = 'deploy_keys_projects'
end
def up
DeploysKeyProject.each_batch(of: 10_000) do |batch|
start_id, end_id = batch.pluck('MIN(id), MAX(id)').first
execute <<-EOF
UPDATE deploy_keys_projects
SET can_push = keys.can_push
FROM keys
WHERE deploy_key_id = keys.id
AND deploy_keys_projects.id BETWEEN #{start_id} AND #{end_id}
EOF
end
end
def down
DeploysKeyProject.each_batch(of: 10_000) do |batch|
start_id, end_id = batch.pluck('MIN(id), MAX(id)').first
execute <<-EOF
UPDATE keys
SET can_push = deploy_keys_projects.can_push
FROM deploy_keys_projects
WHERE deploy_keys_projects.deploy_key_id = keys.id
AND deploy_keys_projects.id BETWEEN #{start_id} AND #{end_id}
EOF
end
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class RemoveCanPushFromKeys < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
remove_column :keys, :can_push
end
def down
add_column_with_default :keys, :can_push, :boolean, default: false, allow_null: false
end
end
......@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20171206221519) do
ActiveRecord::Schema.define(version: 20171215121259) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
......@@ -618,6 +618,7 @@ ActiveRecord::Schema.define(version: 20171206221519) do
t.integer "project_id", null: false
t.datetime "created_at"
t.datetime "updated_at"
t.boolean "can_push", default: false, null: false
end
add_index "deploy_keys_projects", ["project_id"], name: "index_deploy_keys_projects_on_project_id", using: :btree
......@@ -892,7 +893,6 @@ ActiveRecord::Schema.define(version: 20171206221519) do
t.string "type"
t.string "fingerprint"
t.boolean "public", default: false, null: false
t.boolean "can_push", default: false, null: false
t.datetime "last_used_at"
end
......
......@@ -19,15 +19,13 @@ Example response:
{
"id": 1,
"title": "Public key",
"key": "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=",
"can_push": false,
"key": "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=",
"created_at": "2013-10-02T10:12:29Z"
},
{
"id": 3,
"title": "Another Public key",
"key": "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=",
"can_push": true,
"key": "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=",
"created_at": "2013-10-02T11:12:29Z"
}
]
......@@ -57,15 +55,15 @@ Example response:
"id": 1,
"title": "Public key",
"key": "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=",
"can_push": false,
"created_at": "2013-10-02T10:12:29Z"
"created_at": "2013-10-02T10:12:29Z",
"can_push": false
},
{
"id": 3,
"title": "Another Public key",
"key": "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=",
"can_push": false,
"created_at": "2013-10-02T11:12:29Z"
"created_at": "2013-10-02T11:12:29Z",
"can_push": false
}
]
```
......@@ -96,8 +94,8 @@ Example response:
"id": 1,
"title": "Public key",
"key": "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=",
"can_push": false,
"created_at": "2013-10-02T10:12:29Z"
"created_at": "2013-10-02T10:12:29Z",
"can_push": false
}
```
......@@ -135,6 +133,36 @@ Example response:
}
```
## Update deploy key
Updates a deploy key for a project.
```
PUT /projects/:id/deploy_keys/:key_id
```
| Attribute | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
| `title` | string | no | New deploy key's title |
| `can_push` | boolean | no | Can deploy key push to the project's repository |
```bash
curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --header "Content-Type: application/json" --data '{"title": "New deploy key", "can_push": true}' "https://gitlab.example.com/api/v4/projects/5/deploy_keys/11"
```
Example response:
```json
{
"id": 11,
"title": "New deploy key",
"key": "ssh-rsa AAAA...",
"created_at": "2015-08-29T12:44:31.550Z",
"can_push": true
}
```
## Delete deploy key
Removes a deploy key from the project. If the deploy key is used only for this project, it will be deleted from the system.
......
......@@ -4,6 +4,16 @@ module API
before { authenticate! }
helpers do
def add_deploy_keys_project(project, attrs = {})
project.deploy_keys_projects.create(attrs)
end
def find_by_deploy_key(project, key_id)
project.deploy_keys_projects.find_by!(deploy_key: key_id)
end
end
desc 'Return all deploy keys'
params do
use :pagination
......@@ -21,28 +31,31 @@ module API
before { authorize_admin_project }
desc "Get a specific project's deploy keys" do
success Entities::SSHKey
success Entities::DeployKeysProject
end
params do
use :pagination
end
get ":id/deploy_keys" do
present paginate(user_project.deploy_keys), with: Entities::SSHKey
keys = user_project.deploy_keys_projects.preload(:deploy_key)
present paginate(keys), with: Entities::DeployKeysProject
end
desc 'Get single deploy key' do
success Entities::SSHKey
success Entities::DeployKeysProject
end
params do
requires :key_id, type: Integer, desc: 'The ID of the deploy key'
end
get ":id/deploy_keys/:key_id" do
key = user_project.deploy_keys.find params[:key_id]
present key, with: Entities::SSHKey
key = find_by_deploy_key(user_project, params[:key_id])
present key, with: Entities::DeployKeysProject
end
desc 'Add new deploy key to currently authenticated user' do
success Entities::SSHKey
success Entities::DeployKeysProject
end
params do
requires :key, type: String, desc: 'The new deploy key'
......@@ -53,24 +66,31 @@ module API
params[:key].strip!
# Check for an existing key joined to this project
key = user_project.deploy_keys.find_by(key: params[:key])
key = user_project.deploy_keys_projects
.joins(:deploy_key)
.find_by(keys: { key: params[:key] })
if key
present key, with: Entities::SSHKey
present key, with: Entities::DeployKeysProject
break
end
# Check for available deploy keys in other projects
key = current_user.accessible_deploy_keys.find_by(key: params[:key])
if key
user_project.deploy_keys << key
present key, with: Entities::SSHKey
added_key = add_deploy_keys_project(user_project, deploy_key: key, can_push: !!params[:can_push])
present added_key, with: Entities::DeployKeysProject
break
end
# Create a new deploy key
key = DeployKey.new(declared_params(include_missing: false))
if key.valid? && user_project.deploy_keys << key
present key, with: Entities::SSHKey
key_attributes = { can_push: !!params[:can_push],
deploy_key_attributes: declared_params.except(:can_push) }
key = add_deploy_keys_project(user_project, key_attributes)
if key.valid?
present key, with: Entities::DeployKeysProject
else
render_validation_error!(key)
end
......@@ -86,14 +106,20 @@ module API
at_least_one_of :title, :can_push
end
put ":id/deploy_keys/:key_id" do
key = DeployKey.find(params.delete(:key_id))
deploy_keys_project = find_by_deploy_key(user_project, params[:key_id])
authorize!(:update_deploy_key, key)
authorize!(:update_deploy_key, deploy_keys_project.deploy_key)
if key.update_attributes(declared_params(include_missing: false))
present key, with: Entities::SSHKey
can_push = params[:can_push].nil? ? deploy_keys_project.can_push : params[:can_push]
title = params[:title] || deploy_keys_project.deploy_key.title
result = deploy_keys_project.update_attributes(can_push: can_push,
deploy_key_attributes: { id: params[:key_id],
title: title })
if result
present deploy_keys_project, with: Entities::DeployKeysProject
else
render_validation_error!(key)
render_validation_error!(deploy_keys_project)
end
end
......@@ -122,7 +148,7 @@ module API
requires :key_id, type: Integer, desc: 'The ID of the deploy key'
end
delete ":id/deploy_keys/:key_id" do
key = user_project.deploy_keys_projects.find_by(deploy_key_id: params[:key_id])
key = user_project.deploy_keys.find(params[:key_id])
not_found!('Deploy Key') unless key
destroy_conditionally!(key)
......
......@@ -554,13 +554,18 @@ module API
end
class SSHKey < Grape::Entity
expose :id, :title, :key, :created_at, :can_push
expose :id, :title, :key, :created_at
end
class SSHKeyWithUser < SSHKey
expose :user, using: Entities::UserPublic
end
class DeployKeysProject < Grape::Entity
expose :deploy_key, merge: true, using: Entities::SSHKey
expose :can_push
end
class GPGKey < Grape::Entity
expose :id, :key, :created_at
end
......
......@@ -3,6 +3,16 @@ module API
class DeployKeys < Grape::API
before { authenticate! }
helpers do
def add_deploy_keys_project(project, attrs = {})
project.deploy_keys_projects.create(attrs)
end
def find_by_deploy_key(project, key_id)
project.deploy_keys_projects.find_by!(deploy_key: key_id)
end
end
get "deploy_keys" do
authenticated_as_admin!
......@@ -18,25 +28,28 @@ module API
%w(keys deploy_keys).each do |path|
desc "Get a specific project's deploy keys" do
success ::API::Entities::SSHKey
success ::API::Entities::DeployKeysProject
end
get ":id/#{path}" do
present user_project.deploy_keys, with: ::API::Entities::SSHKey
keys = user_project.deploy_keys_projects.preload(:deploy_key)
present keys, with: ::API::Entities::DeployKeysProject
end
desc 'Get single deploy key' do
success ::API::Entities::SSHKey
success ::API::Entities::DeployKeysProject
end
params do
requires :key_id, type: Integer, desc: 'The ID of the deploy key'
end
get ":id/#{path}/:key_id" do
key = user_project.deploy_keys.find params[:key_id]
present key, with: ::API::Entities::SSHKey
key = find_by_deploy_key(user_project, params[:key_id])
present key, with: ::API::Entities::DeployKeysProject
end
desc 'Add new deploy key to currently authenticated user' do
success ::API::Entities::SSHKey
success ::API::Entities::DeployKeysProject
end
params do
requires :key, type: String, desc: 'The new deploy key'
......@@ -47,24 +60,31 @@ module API
params[:key].strip!