Merge branch '22743-deploy-token-write-registry' into 'master'

Add write_registry scope to deploy tokens

See merge request gitlab-org/gitlab!28958

app/controllers/groups/settings/ci_cd_controller.rb

@@ -114,7 +114,7 @@ module Groups
       end
 
       def deploy_token_params
-        params.require(:deploy_token).permit(:name, :expires_at, :read_repository, :read_registry, :username)
+        params.require(:deploy_token).permit(:name, :expires_at, :read_repository, :read_registry, :write_registry, :username)
       end
     end
   end

app/controllers/projects/settings/ci_cd_controller.rb

@@ -93,7 +93,7 @@ module Projects
       end
 
       def deploy_token_params
-        params.require(:deploy_token).permit(:name, :expires_at, :read_repository, :read_registry, :username)
+        params.require(:deploy_token).permit(:name, :expires_at, :read_repository, :read_registry, :write_registry, :username)
       end
 
       def run_autodevops_pipeline(service)

app/models/deploy_token.rb

@@ -7,7 +7,7 @@ class DeployToken < ApplicationRecord
   include Gitlab::Utils::StrongMemoize
   add_authentication_token_field :token, encrypted: :optional
 
-  AVAILABLE_SCOPES = %i(read_repository read_registry).freeze
+  AVAILABLE_SCOPES = %i(read_repository read_registry write_registry).freeze
   GITLAB_DEPLOY_TOKEN_NAME = 'gitlab-deploy-token'
 
   default_value_for(:expires_at) { Forever.date }
@@ -105,7 +105,7 @@ class DeployToken < ApplicationRecord
   end
 
   def ensure_at_least_one_scope
-    errors.add(:base, _("Scopes can't be blank")) unless read_repository || read_registry
+    errors.add(:base, _("Scopes can't be blank")) unless read_repository || read_registry || write_registry
   end
 
   def default_username

app/services/auth/container_registry_authentication_service.rb

@@ -135,7 +135,7 @@ module Auth
       when 'pull'
         build_can_pull?(requested_project) || user_can_pull?(requested_project) || deploy_token_can_pull?(requested_project)
       when 'push'
-        build_can_push?(requested_project) || user_can_push?(requested_project)
+        build_can_push?(requested_project) || user_can_push?(requested_project) || deploy_token_can_push?(requested_project)
       when 'delete'
         build_can_delete?(requested_project) || user_can_admin?(requested_project)
       when '*'
@@ -185,6 +185,13 @@ module Auth
         current_user.read_registry?
     end
 
+    def deploy_token_can_push?(requested_project)
+      has_authentication_ability?(:create_container_image) &&
+        current_user.is_a?(DeployToken) &&
+        current_user.has_access_to?(requested_project) &&
+        current_user.write_registry?
+    end
+
     ##
     # We still support legacy pipeline triggers which do not have associated
     # actor. New permissions model and new triggers are always associated with

app/views/projects/settings/ci_cd/show.html.haml

@@ -4,7 +4,7 @@
 
 - expanded = expanded_by_default?
 - general_expanded = @project.errors.empty? ? expanded : true
-- deploy_token_description = s_('DeployTokens|Deploy tokens allow read-only access to your repository and registry images.')
+- deploy_token_description = s_('DeployTokens|Deploy tokens allow access to your repository and registry images.')
 
 %section.settings#js-general-pipeline-settings.no-animate{ class: ('expanded' if general_expanded) }
   .settings-header

app/views/shared/deploy_tokens/_form.html.haml

@@ -30,5 +30,10 @@
         = label_tag ("deploy_token_read_registry"), 'read_registry', class: 'label-bold form-check-label'
         .text-secondary= s_('DeployTokens|Allows read-only access to the registry images')
 
+      %fieldset.form-group.form-check
+        = f.check_box :write_registry, class: 'form-check-input'
+        = label_tag ("deploy_token_write_registry"), 'write_registry', class: 'label-bold form-check-label'
+        .text-secondary= s_('DeployTokens|Allows write access to the registry images')
+
   .prepend-top-default
     = f.submit s_('DeployTokens|Create deploy token'), class: 'btn btn-success qa-create-deploy-token'

changelogs/unreleased/22743-deploy-token-write-registry.yml

+---
+title: Add write_registry scope to deploy tokens for container registry push access
+merge_request: 28958
+author:
+type: added

db/migrate/20200406192059_add_write_registry_to_deploy_tokens.rb

+# frozen_string_literal: true
+
+class AddWriteRegistryToDeployTokens < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(:deploy_tokens, :write_registry, :boolean, default: false, allow_null: false)
+  end
+
+  def down
+    remove_column(:deploy_tokens, :write_registry)
+  end
+end

db/structure.sql

@@ -1997,7 +1997,8 @@ CREATE TABLE public.deploy_tokens (
     token character varying,
     username character varying,
     token_encrypted character varying(255),
-    deploy_token_type smallint DEFAULT 2 NOT NULL
+    deploy_token_type smallint DEFAULT 2 NOT NULL,
+    write_registry boolean DEFAULT false NOT NULL
 );
 
 CREATE SEQUENCE public.deploy_tokens_id_seq
@@ -13087,6 +13088,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200403185127
 20200403185422
 20200406135648
+20200406192059
 20200407094005
 20200407094923
 20200408110856

doc/api/deploy_tokens.md

@@ -92,7 +92,7 @@ POST /projects/:id/deploy_tokens
 | `name`            | string    | yes | New deploy token's name |
 | `expires_at`      | datetime  | no  | Expiration date for the deploy token. Does not expire if no value is provided. |
 | `username`        | string    | no  | Username for deploy token. Default is `gitlab+deploy-token-{n}` |
-| `scopes`   | array of strings | yes | Indicates the deploy token scopes. Must be at least one of `read_repository` or `read_registry`. |
+| `scopes`   | array of strings | yes | Indicates the deploy token scopes. Must be at least one of `read_repository`, `read_registry`, or `write_registry`. |
 
 ```shell
 curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" --header "Content-Type: application/json" --data '{"name": "My deploy token", "expires_at": "2021-01-01", "username": "custom-user", "scopes": ["read_repository"]}' "https://gitlab.example.com/api/v4/projects/5/deploy_tokens/"
@@ -193,7 +193,7 @@ POST /groups/:id/deploy_tokens
 | `name`            | string    | yes | New deploy token's name |
 | `expires_at`      | datetime  | no  | Expiration date for the deploy token. Does not expire if no value is provided. |
 | `username`        | string    | no  | Username for deploy token. Default is `gitlab+deploy-token-{n}` |
-| `scopes`   | array of strings | yes | Indicates the deploy token scopes. Must be at least one of `read_repository` or `read_registry`. |
+| `scopes`   | array of strings | yes | Indicates the deploy token scopes. Must be at least one of `read_repository`, `read_registry`, or `write_registry`. |
 
 Example request:
 

doc/user/project/deploy_tokens/index.md

@@ -2,8 +2,9 @@
 
 > - [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/17894) in GitLab 10.7.
 > - [Moved](https://gitlab.com/gitlab-org/gitlab/issues/199370) from **Settings > Repository** in GitLab 12.9.
+> - [Added `write_registry` scope](https://gitlab.com/gitlab-org/gitlab/-/issues/22743) in GitLab 12.10.
 
-Deploy tokens allow you to download (`git clone`) or read the container registry images of a project without having a user and a password.
+Deploy tokens allow you to download (`git clone`) or push and pull the container registry images of a project without having a user and a password.
 
 Deploy tokens can be managed by [maintainers only](../../permissions.md).
 
@@ -44,6 +45,7 @@ the following table.
 | ----- | ----------- |
 | `read_repository` | Allows read-access to the repository through `git clone` |
 | `read_registry` | Allows read-access to [container registry](../../packages/container_registry/index.md) images if a project is private and authorization is required. |
+| `write_registry` | Allows write-access (push) to [container registry](../../packages/container_registry/index.md). |
 
 ## Deploy token custom username
 
@@ -83,6 +85,21 @@ docker login -u <username> -p <deploy_token> registry.example.com
 Just replace `<username>` and `<deploy_token>` with the proper values. Then you can simply
 pull images from your Container Registry.
 
+### Push Container Registry images
+
+To push the container registry images, you'll need to:
+
+1. Create a Deploy Token with `write_registry` as a scope.
+1. Take note of your `username` and `token`.
+1. Log in to GitLab’s Container Registry using the deploy token:
+
+   ```shell
+   docker login -u <username> -p <deploy_token> registry.example.com
+   ```
+
+Just replace `<username>` and `<deploy_token>` with the proper values. Then you can simply
+push images to your Container Registry.
+
 ### Group Deploy Token
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/21765) in GitLab 12.9.
@@ -107,7 +124,7 @@ There's a special case when it comes to Deploy Tokens. If a user creates one
 named `gitlab-deploy-token`, the username and token of the Deploy Token will be
 automatically exposed to the CI/CD jobs as environment variables: `CI_DEPLOY_USER` and
 `CI_DEPLOY_PASSWORD`, respectively. With the GitLab Deploy Token, the
-`read_registry` scope is implied.
+`read_registry` and `write_registry` scopes are implied.
 
 After you create the token, you can login to the Container Registry using
 those variables:

lib/api/deploy_tokens.rb

@@ -10,6 +10,7 @@ module API
 
         result_hash = {}
         result_hash[:read_registry] = scopes.include?('read_registry')
+        result_hash[:write_registry] = scopes.include?('write_registry')
         result_hash[:read_repository] = scopes.include?('read_repository')
         result_hash
       end
@@ -54,7 +55,7 @@ module API
       params do
         requires :name, type: String, desc: "New deploy token's name"
         requires :scopes, type: Array[String], values: ::DeployToken::AVAILABLE_SCOPES.map(&:to_s),
-          desc: 'Indicates the deploy token scopes. Must be at least one of "read_repository" or "read_registry".'
+          desc: 'Indicates the deploy token scopes. Must be at least one of "read_repository", "read_registry", or "write_registry".'
         optional :expires_at, type: DateTime, desc: 'Expiration date for the deploy token. Does not expire if no value is provided.'
         optional :username, type: String, desc: 'Username for deploy token. Default is `gitlab+deploy-token-{n}`'
       end
@@ -117,7 +118,7 @@ module API
       params do
         requires :name, type: String, desc: 'The name of the deploy token'
         requires :scopes, type: Array[String], values: ::DeployToken::AVAILABLE_SCOPES.map(&:to_s),
-          desc: 'Indicates the deploy token scopes. Must be at least one of "read_repository" or "read_registry".'
+          desc: 'Indicates the deploy token scopes. Must be at least one of "read_repository", "read_registry", or "write_registry".'
         optional :expires_at, type: DateTime, desc: 'Expiration date for the deploy token. Does not expire if no value is provided.'
         optional :username, type: String, desc: 'Username for deploy token. Default is `gitlab+deploy-token-{n}`'
       end

lib/gitlab/auth.rb

@@ -12,7 +12,7 @@ module Gitlab
     REPOSITORY_SCOPES = [:read_repository, :write_repository].freeze
 
     # Scopes used for GitLab Docker Registry access
-    REGISTRY_SCOPES = [:read_registry].freeze
+    REGISTRY_SCOPES = [:read_registry, :write_registry].freeze
 
     # Scopes used for GitLab as admin
     ADMIN_SCOPES = [:sudo].freeze
@@ -200,6 +200,7 @@ module Gitlab
           api: full_authentication_abilities,
           read_api: read_only_authentication_abilities,
           read_registry: [:read_container_image],
+          write_registry: [:create_container_image],
           read_repository: [:download_code],
           write_repository: [:download_code, :push_code]
         }

locale/gitlab.pot

@@ -6835,6 +6835,9 @@ msgstr ""
 msgid "DeployTokens|Allows read-only access to the repository"
 msgstr ""
 
+msgid "DeployTokens|Allows write access to the registry images"
+msgstr ""
+
 msgid "DeployTokens|Copy deploy token"
 msgstr ""
 
@@ -6853,7 +6856,7 @@ msgstr ""
 msgid "DeployTokens|Deploy Tokens"
 msgstr ""
 
-msgid "DeployTokens|Deploy tokens allow read-only access to your repository and registry images."
+msgid "DeployTokens|Deploy tokens allow access to your repository and registry images."
 msgstr ""
 
 msgid "DeployTokens|Expires"

spec/factories/deploy_tokens.rb

@@ -7,6 +7,7 @@ FactoryBot.define do
     sequence(:name) { |n| "PDT #{n}" }
     read_repository { true }
     read_registry { true }
+    write_registry { false }
     revoked { false }
     expires_at { 5.days.from_now }
     deploy_token_type { DeployToken.deploy_token_types[:project_type] }

spec/lib/gitlab/auth_spec.rb

@@ -30,7 +30,7 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
     it 'optional_scopes contains all non-default scopes' do
       stub_container_registry_config(enabled: true)
 
-      expect(subject.optional_scopes).to eq %i[read_user read_api read_repository write_repository read_registry sudo openid profile email]
+      expect(subject.optional_scopes).to eq %i[read_user read_api read_repository write_repository read_registry write_registry sudo openid profile email]
     end
   end
 
@@ -38,21 +38,21 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
     it 'contains all non-default scopes' do
       stub_container_registry_config(enabled: true)
 
-      expect(subject.all_available_scopes).to eq %i[api read_user read_api read_repository write_repository read_registry sudo]
+      expect(subject.all_available_scopes).to eq %i[api read_user read_api read_repository write_repository read_registry write_registry sudo]
     end
 
     it 'contains for non-admin user all non-default scopes without ADMIN access' do
       stub_container_registry_config(enabled: true)
       user = create(:user, admin: false)
 
-      expect(subject.available_scopes_for(user)).to eq %i[api read_user read_api read_repository write_repository read_registry]
+      expect(subject.available_scopes_for(user)).to eq %i[api read_user read_api read_repository write_repository read_registry write_registry]
     end
 
     it 'contains for admin user all non-default scopes with ADMIN access' do
       stub_container_registry_config(enabled: true)
       user = create(:user, admin: true)
 
-      expect(subject.available_scopes_for(user)).to eq %i[api read_user read_api read_repository write_repository read_registry sudo]
+      expect(subject.available_scopes_for(user)).to eq %i[api read_user read_api read_repository write_repository read_registry write_registry sudo]
     end
 
     context 'registry_scopes' do
@@ -72,7 +72,7 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
         end
 
         it 'contains all registry related scopes' do
-          expect(subject.registry_scopes).to eq %i[read_registry]
+          expect(subject.registry_scopes).to eq %i[read_registry write_registry]
         end
       end
     end
@@ -401,6 +401,49 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
     context 'while using deploy tokens' do
       let(:auth_failure) { Gitlab::Auth::Result.new(nil, nil) }
 
+      shared_examples 'registry token scope' do
+        it 'fails when login is not valid' do
+          expect(gl_auth.find_for_git_client('random_login', deploy_token.token, project: project, ip: 'ip'))
+            .to eq(auth_failure)
+        end
+
+        it 'fails when token is not valid' do
+          expect(gl_auth.find_for_git_client(login, '123123', project: project, ip: 'ip'))
+            .to eq(auth_failure)
+        end
+
+        it 'fails if token is nil' do
+          expect(gl_auth.find_for_git_client(login, nil, project: nil, ip: 'ip'))
+            .to eq(auth_failure)
+        end
+
+        it 'fails if token is not related to project' do
+          expect(gl_auth.find_for_git_client(login, 'abcdef', project: nil, ip: 'ip'))
+            .to eq(auth_failure)
+        end
+
+        it 'fails if token has been revoked' do
+          deploy_token.revoke!
+
+          expect(deploy_token.revoked?).to be_truthy
+          expect(gl_auth.find_for_git_client('deploy-token', deploy_token.token, project: nil, ip: 'ip'))
+            .to eq(auth_failure)
+        end
+      end
+
+      shared_examples 'deploy token with disabled registry' do
+        context 'when registry disabled' do
+          before do
+            stub_container_registry_config(enabled: false)
+          end
+
+          it 'fails when login and token are valid' do
+            expect(gl_auth.find_for_git_client(login, deploy_token.token, project: nil, ip: 'ip'))
+              .to eq(auth_failure)
+          end
+        end
+      end
+
       context 'when deploy token and user have the same username' do
         let(:username) { 'normal_user' }
         let(:user) { create(:user, username: username, password: 'my-secret') }
@@ -425,34 +468,33 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
         context 'and belong to the same project' do
           let!(:read_registry) { create(:deploy_token, username: 'deployer', read_repository: false, projects: [project]) }
           let!(:read_repository) { create(:deploy_token, username: read_registry.username, read_registry: false, projects: [project]) }
+          let(:auth_success) { Gitlab::Auth::Result.new(read_repository, project, :deploy_token, [:download_code]) }
 
           it 'succeeds for the right token' do
-            auth_success = Gitlab::Auth::Result.new(read_repository, project, :deploy_token, [:download_code])
-
             expect(gl_auth.find_for_git_client('deployer', read_repository.token, project: project, ip: 'ip'))
               .to eq(auth_success)
           end
 
           it 'fails for the wrong token' do
             expect(gl_auth.find_for_git_client('deployer', read_registry.token, project: project, ip: 'ip'))
-              .to eq(auth_failure)
+              .not_to eq(auth_success)
           end
         end
 
         context 'and belong to different projects' do
+          let_it_be(:other_project) { create(:project) }
           let!(:read_registry) { create(:deploy_token, username: 'deployer', read_repository: false, projects: [project]) }
-          let!(:read_repository) { create(:deploy_token, username: read_registry.username, read_registry: false, projects: [project]) }
+          let!(:read_repository) { create(:deploy_token, username: read_registry.username, read_registry: false, projects: [other_project]) }
+          let(:auth_success) { Gitlab::Auth::Result.new(read_repository, other_project, :deploy_token, [:download_code]) }
 
           it 'succeeds for the right token' do
-            auth_success = Gitlab::Auth::Result.new(read_repository, project, :deploy_token, [:download_code])
-
-            expect(gl_auth.find_for_git_client('deployer', read_repository.token, project: project, ip: 'ip'))
+            expect(gl_auth.find_for_git_client('deployer', read_repository.token, project: other_project, ip: 'ip'))
               .to eq(auth_success)
           end
 
           it 'fails for the wrong token' do
-            expect(gl_auth.find_for_git_client('deployer', read_registry.token, project: project, ip: 'ip'))
-              .to eq(auth_failure)
+            expect(gl_auth.find_for_git_client('deployer', read_registry.token, project: other_project, ip: 'ip'))
+              .not_to eq(auth_success)
           end
         end
       end
@@ -542,45 +584,32 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
               .to eq(auth_success)
           end
 
-          it 'fails when login is not valid' do
-            expect(gl_auth.find_for_git_client('random_login', deploy_token.token, project: project, ip: 'ip'))
-              .to eq(auth_failure)
-          end
+          it_behaves_like 'registry token scope'
+        end
 
-          it 'fails when token is not valid' do
-            expect(gl_auth.find_for_git_client(login, '123123', project: project, ip: 'ip'))
-              .to eq(auth_failure)
-          end
+        it_behaves_like 'deploy token with disabled registry'
+      end
 
-          it 'fails if token is nil' do
-            expect(gl_auth.find_for_git_client(login, nil, project: nil, ip: 'ip'))
-              .to eq(auth_failure)
-          end
+      context 'when the deploy token has write_registry as a scope' do
+        let_it_be(:deploy_token) { create(:deploy_token, write_registry: true, read_repository: false, read_registry: false, projects: [project]) }
+        let_it_be(:login) { deploy_token.username }
 
-          it 'fails if token is not related to project' do
-            expect(gl_auth.find_for_git_client(login, 'abcdef', project: nil, ip: 'ip'))
-              .to eq(auth_failure)
+        context 'when registry enabled' do
+          before do
+            stub_container_registry_config(enabled: true)
           end
 
-          it 'fails if token has been revoked' do
-            deploy_token.revoke!
-
-            expect(deploy_token.revoked?).to be_truthy
-            expect(gl_auth.find_for_git_client('deploy-token', deploy_token.token, project: nil, ip: 'ip'))
-              .to eq(auth_failure)
-          end
-        end
+          it 'succeeds when login and a project token are valid' do
+            auth_success = Gitlab::Auth::Result.new(deploy_token, project, :deploy_token, [:create_container_image])
 
-        context 'when registry disabled' do
-          before do
-            stub_container_registry_config(enabled: false)
+            expect(gl_auth.find_for_git_client(login, deploy_token.token, project: project, ip: 'ip'))
+              .to eq(auth_success)
           end
 
-          it 'fails when login and token are valid' do
-            expect(gl_auth.find_for_git_client(login, deploy_token.token, project: nil, ip: 'ip'))
-              .to eq(auth_failure)
-          end
+          it_behaves_like 'registry token scope'
         end
+
+        it_behaves_like 'deploy token with disabled registry'
       end
     end
   end

spec/models/deploy_token_spec.rb

@@ -62,7 +62,7 @@ describe DeployToken do
 
     context 'with no scopes' do
       it 'is invalid' do
-        deploy_token = build(:deploy_token, read_repository: false, read_registry: false)
+        deploy_token = build(:deploy_token, read_repository: false, read_registry: false, write_registry: false)
 
         expect(deploy_token).not_to be_valid
         expect(deploy_token.errors[:base].first).to eq("Scopes can't be blank")
@@ -79,7 +79,7 @@ describe DeployToken do
 
     context 'with only one scope' do
       it 'returns scopes assigned to DeployToken' do
-        deploy_token = create(:deploy_token, read_registry: false)
+        deploy_token = create(:deploy_token, read_registry: false, write_registry: false)
         expect(deploy_token.scopes).to eq([:read_repository])
       end
     end

spec/services/auth/container_registry_authentication_service_spec.rb

@@ -766,8 +766,8 @@ describe Auth::ContainerRegistryAuthenticationService do
       { scopes: ["repository:#{project.full_path}:pull"] }
     end
 
-    context 'when deploy token has read_registry as a scope' do
-      let(:current_user) { create(:deploy_token, projects: [project]) }
+    context 'when deploy token has read and write registry as scopes' do
+      let(:current_user) { create(:deploy_token, write_registry: true, projects: [project]) }
 
       shared_examples 'able to login' do
         context 'registry provides read_container_image authentication_abilities' do
@@ -790,7 +790,7 @@ describe Auth::ContainerRegistryAuthenticationService do
             { scopes: ["repository:#{project.full_path}:push"] }
           end
 
-          it_behaves_like 'an inaccessible'
+          it_behaves_like 'a pushable'
         end
 
         it_behaves_like 'able to login'
@@ -808,7 +808,7 @@ describe Auth::ContainerRegistryAuthenticationService do
             { scopes: ["repository:#{project.full_path}:push"] }
           end
 
-          it_behaves_like 'an inaccessible'
+          it_behaves_like 'a pushable'
         end
 
         it_behaves_like 'able to login'
@@ -826,7 +826,7 @@ describe Auth::ContainerRegistryAuthenticationService do
             { scopes: ["repository:#{project.full_path}:push"] }
           end
 
-          it_behaves_like 'an inaccessible'
+          it_behaves_like 'a pushable'
         end
 
         it_behaves_like 'able to login'

spec/support/services/deploy_token_shared_examples.rb

@@ -46,7 +46,7 @@ RSpec.shared_examples 'a deploy token creation service' do
     end
 
     context 'when the deploy token is invalid' do
-      let(:deploy_token_params) { attributes_for(:deploy_token, read_repository: false, read_registry: false) }
+      let(:deploy_token_params) { attributes_for(:deploy_token, read_repository: false, read_registry: false, write_registry: false) }
 
       it 'does not create a new DeployToken' do
         expect { subject }.not_to change { DeployToken.count }