diff options
-rw-r--r-- | app/controllers/projects/git_http_client_controller.rb | 7 | ||||
-rw-r--r-- | lib/ci/api/helpers.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/auth.rb | 3 | ||||
-rw-r--r-- | spec/lib/gitlab/git_access_spec.rb | 14 | ||||
-rw-r--r-- | spec/lib/gitlab/git_access_wiki_spec.rb | 9 | ||||
-rw-r--r-- | spec/requests/lfs_http_spec.rb | 10 | ||||
-rw-r--r-- | spec/services/auth/container_registry_authentication_service_spec.rb | 12 |
7 files changed, 39 insertions, 20 deletions
diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb index aabb5b0fe01..c2a298fe37f 100644 --- a/app/controllers/projects/git_http_client_controller.rb +++ b/app/controllers/projects/git_http_client_controller.rb @@ -23,10 +23,12 @@ class Projects::GitHttpClientController < Projects::ApplicationController login, password = user_name_and_password(request) auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip) - if auth_result.type == :ci && download_request? - @ci = true + if auth_result.type == :ci && !download_request? + # Not allowed + auth_result = Gitlab::Auth::Result.new elsif auth_result.type == :oauth && !download_request? # Not allowed + auth_result = Gitlab::Auth::Result.new elsif auth_result.type == :missing_personal_token render_missing_personal_token return # Render above denied access, nothing left to do @@ -35,6 +37,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController end @capabilities = auth_result.capabilities || [] + @ci = auth_result.type == :ci if auth_result.succeeded? return # Allow access diff --git a/lib/ci/api/helpers.rb b/lib/ci/api/helpers.rb index 3dfebc0cac1..23353c62885 100644 --- a/lib/ci/api/helpers.rb +++ b/lib/ci/api/helpers.rb @@ -14,7 +14,7 @@ module Ci end def authenticate_build_token!(build) - forbidden! unless build_token_valid? + forbidden! unless build_token_valid?(build) end def runner_registration_token_valid? @@ -23,7 +23,7 @@ module Ci current_application_settings.runners_registration_token) end - def build_token_valid? + def build_token_valid?(build) token = (params[BUILD_TOKEN_PARAM] || env[BUILD_TOKEN_HEADER]).to_s # We require to also check `runners_token` to maintain compatibility with old version of runners diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 666cf1b3e26..b1427f412b0 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -117,6 +117,7 @@ module Gitlab build = ::Ci::Build.running.find_by_token(password) return unless build + return unless build.project.builds_enabled? if build.user # If user is assigned to build, use restricted credentials of user @@ -127,8 +128,6 @@ module Gitlab end end - private - def build_capabilities [ :read_project, diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index d418b0be0ed..c6fe56aac1c 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -324,7 +324,7 @@ describe Gitlab::GitAccess, lib: true do subject { access.check('git-receive-pack', '_any') } context 'when project is authorized' do - before { key.projects << project } + before { authorize } it { expect(subject).not_to be_allowed } end @@ -353,14 +353,22 @@ describe Gitlab::GitAccess, lib: true do describe 'build capabilities permissions' do let(:capabilities) { build_capabilities } - it_behaves_like 'can not push code' + it_behaves_like 'can not push code' do + def authorize + project.team << [user, :reporter] + end + end end describe 'deploy key permissions' do let(:key) { create(:deploy_key) } let(:actor) { key } - it_behaves_like 'can not push code' + it_behaves_like 'can not push code' do + def authorize + key.projects << project + end + end end private diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb index 4244b807d41..860e701c1a1 100644 --- a/spec/lib/gitlab/git_access_wiki_spec.rb +++ b/spec/lib/gitlab/git_access_wiki_spec.rb @@ -1,9 +1,16 @@ require 'spec_helper' describe Gitlab::GitAccessWiki, lib: true do - let(:access) { Gitlab::GitAccessWiki.new(user, project, 'web') } + let(:access) { Gitlab::GitAccessWiki.new(user, project, 'web', capabilities: capabilities) } let(:project) { create(:project) } let(:user) { create(:user) } + let(:capabilities) do + [ + :read_project, + :download_code, + :push_code + ] + end describe 'push_allowed?' do before do diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 6e551bb65fa..85290ec05c2 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -586,8 +586,8 @@ describe 'Git LFS API and storage' do context 'when CI is authorized' do let(:authorization) { authorize_ci_project } - it 'responds with 401' do - expect(response).to have_http_status(401) + it 'responds with 403' do + expect(response).to have_http_status(403) end end end @@ -614,7 +614,7 @@ describe 'Git LFS API and storage' do let(:authorization) { authorize_ci_project } it 'responds with status 403' do - expect(response).to have_http_status(401) + expect(response).to have_http_status(403) end end end @@ -897,7 +897,9 @@ describe 'Git LFS API and storage' do end def authorize_ci_project - ActionController::HttpAuthentication::Basic.encode_credentials('gitlab-ci-token', project.runners_token) + pipeline = create(:ci_empty_pipeline, project: project) + build = create(:ci_build, :running, pipeline: pipeline) + ActionController::HttpAuthentication::Basic.encode_credentials('gitlab-ci-token', build.token) end def authorize_user diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index c82deb7d423..5f82fee43c6 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -48,12 +48,6 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do 'actions' => actions, }] end - let(:capabilities) do - [ - :build_read_container_image, - :build_create_container_image - ] - end it_behaves_like 'a valid token' it { expect(payload).to include('access' => access) } @@ -203,6 +197,12 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do context 'project authorization' do let(:current_project) { create(:empty_project) } + let(:capabilities) do + [ + :build_read_container_image, + :build_create_container_image + ] + end context 'allow to use scope-less authentication' do it_behaves_like 'a valid token' |