Merge branch 'security-docker-blocked-users' into 'master'

Reject blocked users in Gitlab::Auth.find_for_git_client

Closes #65

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

changelogs/unreleased/security-docker-blocked-users.yml

+---
+title: Reject all container registry requests from blocked users
+merge_request:
+author:
+type: security

lib/gitlab/auth.rb

@@ -171,6 +171,8 @@ module Gitlab
 
           if valid_oauth_token?(token)
             user = User.find_by(id: token.resource_owner_id)
+            return unless user.can?(:log_in)
+
             Gitlab::Auth::Result.new(user, nil, :oauth, full_authentication_abilities)
           end
         end
@@ -182,7 +184,7 @@ module Gitlab
 
         token = PersonalAccessTokensFinder.new(state: 'active').find_by_token(password)
 
-        if token && valid_scoped_token?(token, all_available_scopes)
+        if token && valid_scoped_token?(token, all_available_scopes) && token.user.can?(:log_in)
           Gitlab::Auth::Result.new(token.user, nil, :personal_access_token, abilities_for_scopes(token.scopes))
         end
       end
@@ -260,6 +262,8 @@ module Gitlab
         return unless build.project.builds_enabled?
 
         if build.user
+          return unless build.user.can?(:log_in)
+
           # If user is assigned to build, use restricted credentials of user
           Gitlab::Auth::Result.new(build.user, build.project, :build, build_authentication_abilities)
         else

spec/lib/gitlab/auth_spec.rb

@@ -164,6 +164,12 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
 
           expect(subject).to eq(Gitlab::Auth::Result.new(build.user, build.project, :build, described_class.build_authentication_abilities))
         end
+
+        it 'fails with blocked user token' do
+          build.update(user: create(:user, :blocked))
+
+          expect(subject).to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
+        end
       end
 
       (HasStatus::AVAILABLE_STATUSES - ['running']).each do |build_status|
@@ -259,6 +265,15 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
 
         gl_auth.find_for_git_client("oauth2", token_w_api_scope.token, project: nil, ip: 'ip')
       end
+
+      context 'blocked user' do
+        let(:user) { create(:user, :blocked) }
+
+        it 'fails' do
+          expect(gl_auth.find_for_git_client("oauth2", token_w_api_scope.token, project: nil, ip: 'ip'))
+            .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
+        end
+      end
     end
 
     context 'while using personal access tokens as passwords' do
@@ -307,9 +322,35 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
       it 'fails if password is nil' do
         expect_results_with_abilities(nil, nil, false)
       end
+
+      context 'when user is blocked' do
+        let(:user) { create(:user, :blocked) }
+        let(:personal_access_token) { create(:personal_access_token, scopes: ['read_registry'], user: user) }
+
+        before do
+          stub_container_registry_config(enabled: true)
+        end
+
+        it 'fails if user is blocked' do
+          expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip'))
+          .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
+        end
+      end
     end
 
     context 'while using regular user and password' do
+      it 'fails for a blocked user' do
+        user = create(
+          :user,
+          :blocked,
+          username: 'normal_user',
+          password: 'my-secret'
+        )
+
+        expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip'))
+          .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
+      end
+
       it 'goes through lfs authentication' do
         user = create(
           :user,

spec/requests/jwt_controller_spec.rb

@@ -25,6 +25,17 @@ describe JwtController do
   end
 
   context 'when using authenticated request' do
+    shared_examples 'rejecting a blocked user' do
+      context 'with blocked user' do
+        let(:user) { create(:user, :blocked) }
+
+        it 'rejects the request as unauthorized' do
+          expect(response).to have_gitlab_http_status(:unauthorized)
+          expect(response.body).to include('HTTP Basic: Access denied')
+        end
+      end
+    end
+
     context 'using CI token' do
       let(:build) { create(:ci_build, :running) }
       let(:project) { build.project }
@@ -61,6 +72,8 @@ describe JwtController do
           expect(response).to have_gitlab_http_status(:ok)
           expect(service_class).to have_received(:new).with(nil, user, ActionController::Parameters.new(parameters).permit!)
         end
+
+        it_behaves_like 'rejecting a blocked user'
       end
     end
 
@@ -72,6 +85,8 @@ describe JwtController do
 
       it { expect(service_class).to have_received(:new).with(nil, user, ActionController::Parameters.new(parameters).permit!) }
 
+      it_behaves_like 'rejecting a blocked user'
+
       context 'when passing a flat array of scopes' do
         # We use this trick to make rails to generate a query_string:
         # scope=scope1&scope=scope2