Merge branch 'zj-read-registry-pat' into 'master'

Allow pulling container images using personal access tokens

Closes #19219

See merge request !11845

11 files changed, 104 insertions, 36 deletions

diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb
index 1c01be06451..c585d26df77 100644
--- a/app/controllers/jwt_controller.rb
+++ b/app/controllers/jwt_controller.rb
@@ -25,8 +25,10 @@ class JwtController < ApplicationController
     authenticate_with_http_basic do |login, password|
       @authentication_result = Gitlab::Auth.find_for_git_client(login, password, project: nil, ip: request.ip)
 
-      render_unauthorized unless @authentication_result.success? &&
-          (@authentication_result.actor.nil? || @authentication_result.actor.is_a?(User))
+      if @authentication_result.failed? ||
+          (@authentication_result.actor.present? && !@authentication_result.actor.is_a?(User))
+        render_unauthorized
+      end
     end
   rescue Gitlab::Auth::MissingPersonalTokenError
     render_missing_personal_token
diff --git a/app/controllers/profiles/personal_access_tokens_controller.rb b/app/controllers/profiles/personal_access_tokens_controller.rb
index 0abe7ea3c9b..f748d191ef4 100644
--- a/app/controllers/profiles/personal_access_tokens_controller.rb
+++ b/app/controllers/profiles/personal_access_tokens_controller.rb
@@ -38,7 +38,7 @@ class Profiles::PersonalAccessTokensController < Profiles::ApplicationController
   end
 
   def set_index_vars
-    @scopes = Gitlab::Auth::API_SCOPES
+    @scopes = Gitlab::Auth::AVAILABLE_SCOPES
 
     @personal_access_token = finder.build
     @inactive_personal_access_tokens = finder(state: 'inactive').execute
diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb
index ae9f71e7747..6e13f9b2089 100644
--- a/app/models/personal_access_token.rb
+++ b/app/models/personal_access_token.rb
@@ -15,11 +15,10 @@ class PersonalAccessToken < ActiveRecord::Base
   scope :without_impersonation, -> { where(impersonation: false) }
 
   validates :scopes, presence: true
-  validate :validate_api_scopes
+  validate :validate_scopes
 
   def revoke!
-    self.revoked = true
-    self.save
+    update!(revoked: true)
   end
 
   def active?
@@ -28,9 +27,9 @@ class PersonalAccessToken < ActiveRecord::Base
 
   protected
 
-  def validate_api_scopes
-    unless scopes.all? { |scope| Gitlab::Auth::API_SCOPES.include?(scope.to_sym) }
-      errors.add :scopes, "can only contain API scopes"
+  def validate_scopes
+    unless scopes.all? { |scope| Gitlab::Auth::AVAILABLE_SCOPES.include?(scope.to_sym) }
+      errors.add :scopes, "can only contain available scopes"
     end
   end
 end
diff --git a/changelogs/unreleased/zj-read-registry-pat.yml b/changelogs/unreleased/zj-read-registry-pat.yml
new file mode 100644
index 00000000000..d36159bbdf5
--- /dev/null
+++ b/changelogs/unreleased/zj-read-registry-pat.yml
@@ -0,0 +1,4 @@
+---
+title: Allow pulling of container images using personal access tokens
+merge_request: 11845
+author:
diff --git a/doc/user/project/container_registry.md b/doc/user/project/container_registry.md
index 3cbb0b5196d..10c281448a3 100644
--- a/doc/user/project/container_registry.md
+++ b/doc/user/project/container_registry.md
@@ -104,12 +104,14 @@ Make sure that your GitLab Runner is configured to allow building Docker images
 following the [Using Docker Build](../../ci/docker/using_docker_build.md)
 and [Using the GitLab Container Registry documentation](../../ci/docker/using_docker_build.md#using-the-gitlab-container-registry).
 
-## Limitations
+## Using with private projects
 
-In order to use a container image from your private project as an `image:` in
-your `.gitlab-ci.yml`, you have to follow the
-[Using a private Docker Registry][private-docker]
-documentation. This workflow will be simplified in the future.
+If a project is private, credentials will need to be provided for authorization.
+The preferred way to do this, is by using personal access tokens, which can be
+created under `/profile/personal_access_tokens`. The minimal scope needed is:
+`read_registry`.
+
+This feature was introduced in GitLab 9.3.
 
 ## Troubleshooting the GitLab Container Registry
 
@@ -255,4 +257,3 @@ Once the right permissions were set, the error will go away.
 
 [ce-4040]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4040
 [docker-docs]: https://docs.docker.com/engine/userguide/intro/
-[private-docker]: https://docs.gitlab.com/runner/configuration/advanced-configuration.html#using-a-private-container-registry
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 099c45dcfb7..da07ba2f2a3 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -2,6 +2,8 @@ module Gitlab
   module Auth
     MissingPersonalTokenError = Class.new(StandardError)
 
+    REGISTRY_SCOPES = [:read_registry].freeze
+
     # Scopes used for GitLab API access
     API_SCOPES = [:api, :read_user].freeze
 
@@ -11,8 +13,10 @@ module Gitlab
     # Default scopes for OAuth applications that don't define their own
     DEFAULT_SCOPES = [:api].freeze
 
+    AVAILABLE_SCOPES = (API_SCOPES + REGISTRY_SCOPES).freeze
+
     # Other available scopes
-    OPTIONAL_SCOPES = (API_SCOPES + OPENID_SCOPES - DEFAULT_SCOPES).freeze
+    OPTIONAL_SCOPES = (AVAILABLE_SCOPES + OPENID_SCOPES - DEFAULT_SCOPES).freeze
 
     class << self
       def find_for_git_client(login, password, project:, ip:)
@@ -26,8 +30,8 @@ module Gitlab
           build_access_token_check(login, password) ||
           lfs_token_check(login, password) ||
           oauth_access_token_check(login, password) ||
-          user_with_password_for_git(login, password) ||
           personal_access_token_check(password) ||
+          user_with_password_for_git(login, password) ||
           Gitlab::Auth::Result.new
 
         rate_limit!(ip, success: result.success?, login: login)
@@ -109,6 +113,7 @@ module Gitlab
       def oauth_access_token_check(login, password)
         if login == "oauth2" && password.present?
           token = Doorkeeper::AccessToken.by_token(password)
+
           if valid_oauth_token?(token)
             user = User.find_by(id: token.resource_owner_id)
             Gitlab::Auth::Result.new(user, nil, :oauth, full_authentication_abilities)
@@ -121,17 +126,23 @@ module Gitlab
 
         token = PersonalAccessTokensFinder.new(state: 'active').find_by(token: password)
 
-        if token && valid_api_token?(token)
-          Gitlab::Auth::Result.new(token.user, nil, :personal_token, full_authentication_abilities)
+        if token && valid_scoped_token?(token, AVAILABLE_SCOPES.map(&:to_s))
+          Gitlab::Auth::Result.new(token.user, nil, :personal_token, abilities_for_scope(token.scopes))
         end
       end
 
       def valid_oauth_token?(token)
-        token && token.accessible? && valid_api_token?(token)
+        token && token.accessible? && valid_scoped_token?(token, ["api"])
       end
 
-      def valid_api_token?(token)
-        AccessTokenValidationService.new(token).include_any_scope?(['api'])
+      def valid_scoped_token?(token, scopes)
+        AccessTokenValidationService.new(token).include_any_scope?(scopes)
+      end
+
+      def abilities_for_scope(scopes)
+        scopes.map do |scope|
+          self.public_send(:"#{scope}_scope_authentication_abilities")
+        end.flatten.uniq
       end
 
       def lfs_token_check(login, password)
@@ -202,6 +213,16 @@ module Gitlab
           :create_container_image
         ]
       end
+      alias_method :api_scope_authentication_abilities, :full_authentication_abilities
+
+      def read_registry_scope_authentication_abilities
+        [:read_container_image]
+      end
+
+      # The currently used auth method doesn't allow any actions for this scope
+      def read_user_scope_authentication_abilities
+        []
+      end
     end
   end
 end
diff --git a/lib/gitlab/auth/result.rb b/lib/gitlab/auth/result.rb
index 39b86c61a18..75451cf8aa9 100644
--- a/lib/gitlab/auth/result.rb
+++ b/lib/gitlab/auth/result.rb
@@ -15,6 +15,10 @@ module Gitlab
       def success?
         actor.present? || type == :ci
       end
+
+      def failed?
+        !success?
+      end
     end
   end
 end
diff --git a/spec/features/profiles/personal_access_tokens_spec.rb b/spec/features/profiles/personal_access_tokens_spec.rb
index 27a20e78a43..7e2e685df26 100644
--- a/spec/features/profiles/personal_access_tokens_spec.rb
+++ b/spec/features/profiles/personal_access_tokens_spec.rb
@@ -17,6 +17,7 @@ describe 'Profile > Personal Access Tokens', feature: true, js: true do
 
   def disallow_personal_access_token_saves!
     allow_any_instance_of(PersonalAccessToken).to receive(:save).and_return(false)
+
     errors = ActiveModel::Errors.new(PersonalAccessToken.new).tap { |e| e.add(:name, "cannot be nil") }
     allow_any_instance_of(PersonalAccessToken).to receive(:errors).and_return(errors)
   end
@@ -91,8 +92,11 @@ describe 'Profile > Personal Access Tokens', feature: true, js: true do
 
     context "when revocation fails" do
       it "displays an error message" do
-        disallow_personal_access_token_saves!
         visit profile_personal_access_tokens_path
+        allow_any_instance_of(PersonalAccessToken).to receive(:update!).and_return(false)
+
+        errors = ActiveModel::Errors.new(PersonalAccessToken.new).tap { |e| e.add(:name, "cannot be nil") }
+        allow_any_instance_of(PersonalAccessToken).to receive(:errors).and_return(errors)
 
         click_on "Revoke"
         expect(active_personal_access_tokens).to have_text(personal_access_token.name)
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 50bc3ef1b7c..d6006eab0c9 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -17,7 +17,11 @@ describe Gitlab::Auth, lib: true do
     end
 
     it 'OPTIONAL_SCOPES contains all non-default scopes' do
-      expect(subject::OPTIONAL_SCOPES).to eq [:read_user, :openid]
+      expect(subject::OPTIONAL_SCOPES).to eq %i[read_user read_registry openid]
+    end
+
+    it 'REGISTRY_SCOPES contains all registry related scopes' do
+      expect(subject::REGISTRY_SCOPES).to eq %i[read_registry]
     end
   end
 
@@ -143,6 +147,13 @@ describe Gitlab::Auth, lib: true do
         expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(personal_access_token.user, nil, :personal_token, full_authentication_abilities))
       end
 
+      it 'succeeds for personal access tokens with the `read_registry` scope' do
+        personal_access_token = create(:personal_access_token, scopes: ['read_registry'])
+
+        expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '')
+        expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(personal_access_token.user, nil, :personal_token, [:read_container_image]))
+      end
+
       it 'succeeds if it is an impersonation token' do
         impersonation_token = create(:personal_access_token, :impersonation, scopes: ['api'])
 
@@ -150,18 +161,11 @@ describe Gitlab::Auth, lib: true do
         expect(gl_auth.find_for_git_client('', impersonation_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(impersonation_token.user, nil, :personal_token, full_authentication_abilities))
       end
 
-      it 'fails for personal access tokens with other scopes' do
+      it 'limits abilities based on scope' do
         personal_access_token = create(:personal_access_token, scopes: ['read_user'])
 
-        expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '')
-        expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil))
-      end
-
-      it 'fails for impersonation token with other scopes' do
-        impersonation_token = create(:personal_access_token, scopes: ['read_user'])
-
-        expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '')
-        expect(gl_auth.find_for_git_client('', impersonation_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil))
+        expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '')
+        expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(personal_access_token.user, nil, :personal_token, []))
       end
 
       it 'fails if password is nil' do
diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb
index 823623d96fa..fa781195608 100644
--- a/spec/models/personal_access_token_spec.rb
+++ b/spec/models/personal_access_token_spec.rb
@@ -35,6 +35,16 @@ describe PersonalAccessToken, models: true do
     end
   end
 
+  describe 'revoke!' do
+    let(:active_personal_access_token) { create(:personal_access_token) }
+
+    it 'revokes the token' do
+      active_personal_access_token.revoke!
+
+      expect(active_personal_access_token.revoked?).to be true
+    end
+  end
+
   context "validations" do
     let(:personal_access_token) { build(:personal_access_token) }
 
@@ -51,11 +61,17 @@ describe PersonalAccessToken, models: true do
       expect(personal_access_token).to be_valid
     end
 
-    it "rejects creating a token with non-API scopes" do
+    it "allows creating a token with read_registry scope" do
+      personal_access_token.scopes = [:read_registry]
+
+      expect(personal_access_token).to be_valid
+    end
+
+    it "rejects creating a token with unavailable scopes" do
       personal_access_token.scopes = [:openid, :api]
 
       expect(personal_access_token).not_to be_valid
-      expect(personal_access_token.errors[:scopes].first).to eq "can only contain API scopes"
+      expect(personal_access_token.errors[:scopes].first).to eq "can only contain available scopes"
     end
   end
 end
diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb
index a3e7844b2f3..e056353fa6f 100644
--- a/spec/requests/jwt_controller_spec.rb
+++ b/spec/requests/jwt_controller_spec.rb
@@ -41,6 +41,19 @@ describe JwtController do
 
         it { expect(response).to have_http_status(401) }
       end
+
+      context 'using personal access tokens' do
+        let(:user) { create(:user) }
+        let(:pat) { create(:personal_access_token, user: user, scopes: ['read_registry']) }
+        let(:headers) { { authorization: credentials('personal_access_token', pat.token) } }
+
+        subject! { get '/jwt/auth', parameters, headers }
+
+        it 'authenticates correctly' do
+          expect(response).to have_http_status(200)
+          expect(service_class).to have_received(:new).with(nil, user, parameters)
+        end
+      end
     end
 
     context 'using User login' do