Conan Token uses PAT rather than ID in payload

Merge branch 'security-296866-conan-token-update-14-10' into '14-10-stable-ee'

See merge request gitlab-org/security/gitlab!2412

Changelog: security

lib/api/helpers/packages/conan/api_helpers.rb

@@ -153,7 +153,7 @@ module API
           def token
             strong_memoize(:token) do
               token = nil
-              token = ::Gitlab::ConanToken.from_personal_access_token(access_token) if access_token
+              token = ::Gitlab::ConanToken.from_personal_access_token(find_personal_access_token.user_id, access_token_from_request) if find_personal_access_token
               token = ::Gitlab::ConanToken.from_deploy_token(deploy_token_from_request) if deploy_token_from_request
               token = ::Gitlab::ConanToken.from_job(find_job_from_token) if find_job_from_token
               token
@@ -224,9 +224,27 @@ module API
             forbidden!
           end
 
+          # We override this method from auth_finders because we need to
+          # extract the token from the Conan JWT which is specific to the Conan API
           def find_personal_access_token
-            find_personal_access_token_from_conan_jwt ||
-              find_personal_access_token_from_http_basic_auth
+            strong_memoize(:find_personal_access_token) do
+              PersonalAccessToken.find_by_token(access_token_from_request)
+            end
+          end
+
+          def access_token_from_request
+            strong_memoize(:access_token_from_request) do
+              find_personal_access_token_from_conan_jwt ||
+                find_password_from_basic_auth
+            end
+          end
+
+          def find_password_from_basic_auth
+            return unless route_authentication_setting[:basic_auth_personal_access_token]
+            return unless has_basic_credentials?(current_request)
+
+            _username, password = user_name_and_password(current_request)
+            password
           end
 
           def find_user_from_job_token
@@ -256,7 +274,7 @@ module API
 
             return unless token
 
-            PersonalAccessToken.find_by_id_and_user_id(token.access_token_id, token.user_id)
+            token.access_token_id
           end
 
           def find_deploy_token_from_conan_jwt

lib/gitlab/conan_token.rb

@@ -13,8 +13,8 @@ module Gitlab
     attr_reader :access_token_id, :user_id
 
     class << self
-      def from_personal_access_token(access_token)
-        new(access_token_id: access_token.id, user_id: access_token.user_id)
+      def from_personal_access_token(user_id, token)
+        new(access_token_id: token, user_id: user_id)
       end
 
       def from_job(job)

spec/lib/gitlab/conan_token_spec.rb

@@ -25,13 +25,17 @@ RSpec.describe Gitlab::ConanToken do
   end
 
   describe '.from_personal_access_token' do
-    it 'sets access token id and user id' do
-      access_token = double(id: 123, user_id: 456)
+    it 'sets access token and user id and does not use the token id' do
+      personal_access_token = double(id: 999, token: 123, user_id: 456)
 
-      token = described_class.from_personal_access_token(access_token)
+      token = described_class.from_personal_access_token(
+        personal_access_token.user_id,
+        personal_access_token.token
+      )
 
-      expect(token.access_token_id).to eq(123)
-      expect(token.user_id).to eq(456)
+      expect(token.access_token_id).not_to eq(personal_access_token.id)
+      expect(token.access_token_id).to eq(personal_access_token.token)
+      expect(token.user_id).to eq(personal_access_token.user_id)
     end
   end
 

spec/support/helpers/packages_manager_api_spec_helper.rb

@@ -3,7 +3,7 @@
 module PackagesManagerApiSpecHelpers
   def build_jwt(personal_access_token, secret: jwt_secret, user_id: nil)
     JSONWebToken::HMACToken.new(secret).tap do |jwt|
-      jwt['access_token'] = personal_access_token.id
+      jwt['access_token'] = personal_access_token.token
       jwt['user_id'] = user_id || personal_access_token.user_id
     end
   end

spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb

@@ -62,15 +62,8 @@ RSpec.shared_examples 'conan authenticate endpoint' do
     end
   end
 
-  it 'responds with 401 Unauthorized when an invalid access token ID is provided' do
-    jwt = build_jwt(double(id: 12345), user_id: personal_access_token.user_id)
-    get api(url), headers: build_token_auth_header(jwt.encoded)
-
-    expect(response).to have_gitlab_http_status(:unauthorized)
-  end
-
-  it 'responds with 401 Unauthorized when invalid user is provided' do
-    jwt = build_jwt(personal_access_token, user_id: 12345)
+  it 'responds with 401 Unauthorized when an invalid access token is provided' do
+    jwt = build_jwt(double(token: 12345), user_id: user.id)
     get api(url), headers: build_token_auth_header(jwt.encoded)
 
     expect(response).to have_gitlab_http_status(:unauthorized)
@@ -102,7 +95,7 @@ RSpec.shared_examples 'conan authenticate endpoint' do
 
         payload = JSONWebToken::HMACToken.decode(
           response.body, jwt_secret).first
-        expect(payload['access_token']).to eq(personal_access_token.id)
+        expect(payload['access_token']).to eq(personal_access_token.token)
         expect(payload['user_id']).to eq(personal_access_token.user_id)
 
         duration = payload['exp'] - payload['iat']