Create read_registry scope with JWT auth

This is the first commit doing mainly 3 things:
1. create a new scope and allow users to use it
2. Have the JWTController respond correctly on this
3. Updates documentation to suggest usage of PATs

There is one gotcha, there will be no support for impersonation tokens, as this
seems not needed.

Fixes gitlab-org/gitlab-ce#19219

10 files changed, 93 insertions, 33 deletions

diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb
index 1c01be06451..2648b901f01 100644
--- a/app/controllers/jwt_controller.rb
+++ b/app/controllers/jwt_controller.rb
@@ -20,13 +20,15 @@ class JwtController < ApplicationController
   private
 
   def authenticate_project_or_user
-    @authentication_result = Gitlab::Auth::Result.new(nil, nil, :none, Gitlab::Auth.read_authentication_abilities)
+    @authentication_result = Gitlab::Auth::Result.new(nil, nil, :none, Gitlab::Auth.read_api_abilities)
 
     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 e8b000ddad6..0aee696bed3 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 6a2ca7fb428..b2eca9ef809 100644
--- a/doc/user/project/container_registry.md
+++ b/doc/user/project/container_registry.md
@@ -106,12 +106,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
 
@@ -257,4 +259,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..f461d0f97f1 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)
@@ -103,15 +107,16 @@ module Gitlab
 
         raise Gitlab::Auth::MissingPersonalTokenError if user.two_factor_enabled?
 
-        Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities)
+        Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_api_abilities)
       end
 
       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)
+            Gitlab::Auth::Result.new(user, nil, :oauth, full_api_abilities)
           end
         end
       end
@@ -121,17 +126,26 @@ 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, scopes: 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)
       end
 
-      def valid_api_token?(token)
-        AccessTokenValidationService.new(token).include_any_scope?(['api'])
+      def valid_scoped_token?(token, scopes: %w[api])
+        AccessTokenValidationService.new(token).include_any_scope?(scopes)
+      end
+
+      def abilities_for_scope(scopes)
+        abilities = Set.new
+
+        abilities.merge(full_api_abilities) if scopes.include?("api")
+        abilities << :read_container_image if scopes.include?("read_registry")
+
+        abilities.to_a
       end
 
       def lfs_token_check(login, password)
@@ -150,9 +164,9 @@ module Gitlab
 
         authentication_abilities =
           if token_handler.user?
-            full_authentication_abilities
+            full_api_abilities
           else
-            read_authentication_abilities
+            read_api_abilities
           end
 
         if Devise.secure_compare(token_handler.token, password)
@@ -188,7 +202,7 @@ module Gitlab
         ]
       end
 
-      def read_authentication_abilities
+      def read_api_abilities
         [
           :read_project,
           :download_code,
@@ -196,8 +210,8 @@ module Gitlab
         ]
       end
 
-      def full_authentication_abilities
-        read_authentication_abilities + [
+      def full_api_abilities
+        read_api_abilities + [
           :push_code,
           :create_container_image
         ]
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/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 50bc3ef1b7c..6574e6d0087 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -143,6 +143,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'])
 
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..8ddae9f6b89 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
@@ -89,7 +102,7 @@ describe JwtController do
     end
 
     it 'allows read access' do
-      expect(service).to receive(:execute).with(authentication_abilities: Gitlab::Auth.read_authentication_abilities)
+      expect(service).to receive(:execute).with(authentication_abilities: Gitlab::Auth.read_api_abilities)
 
       get '/jwt/auth', parameters
     end