Commit b9ae6b69 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Merge branch '37403-npm-install-fails-with-ci_job_token-in-npmrc' into 'master'

Allow npm install endpoints to use job tokens

See merge request gitlab-org/gitlab!20868
parents 63534f6e a400e4bd
---
title: Allow NPM package downloads with CI_JOB_TOKEN
merge_request: 20868
author:
type: added
...@@ -28,6 +28,7 @@ module API ...@@ -28,6 +28,7 @@ module API
params do params do
requires :package_name, type: String, desc: 'Package name' requires :package_name, type: String, desc: 'Package name'
end end
route_setting :authentication, job_token_allowed: true
get 'packages/npm/*package_name', format: false, requirements: NPM_ENDPOINT_REQUIREMENTS do get 'packages/npm/*package_name', format: false, requirements: NPM_ENDPOINT_REQUIREMENTS do
package_name = params[:package_name] package_name = params[:package_name]
...@@ -58,6 +59,7 @@ module API ...@@ -58,6 +59,7 @@ module API
requires :package_name, type: String, desc: 'Package name' requires :package_name, type: String, desc: 'Package name'
requires :file_name, type: String, desc: 'Package file name' requires :file_name, type: String, desc: 'Package file name'
end end
route_setting :authentication, job_token_allowed: true
get ':id/packages/npm/*package_name/-/*file_name', format: false do get ':id/packages/npm/*package_name/-/*file_name', format: false do
authorize_read_package!(user_project) authorize_read_package!(user_project)
......
# frozen_string_literal: true # frozen_string_literal: true
require 'spec_helper' require 'spec_helper'
describe API::NpmPackages do describe API::NpmPackages do
...@@ -6,6 +7,7 @@ describe API::NpmPackages do ...@@ -6,6 +7,7 @@ describe API::NpmPackages do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project, :public, namespace: group) } let(:project) { create(:project, :public, namespace: group) }
let(:token) { create(:oauth_access_token, scopes: 'api', resource_owner: user) } let(:token) { create(:oauth_access_token, scopes: 'api', resource_owner: user) }
let(:job) { create(:ci_build, user: user) }
before do before do
project.add_developer(user) project.add_developer(user)
...@@ -19,6 +21,12 @@ describe API::NpmPackages do ...@@ -19,6 +21,12 @@ describe API::NpmPackages do
expect_a_valid_package_response expect_a_valid_package_response
end end
it 'returns the package info with job token' do
get_package_with_job_token(package)
expect_a_valid_package_response
end
it 'denies request without oauth token' do it 'denies request without oauth token' do
get_package(package) get_package(package)
...@@ -87,32 +95,53 @@ describe API::NpmPackages do ...@@ -87,32 +95,53 @@ describe API::NpmPackages do
def get_package_with_token(package, params = {}) def get_package_with_token(package, params = {})
get_package(package, params.merge(access_token: token.token)) get_package(package, params.merge(access_token: token.token))
end end
def get_package_with_job_token(package, params = {})
get_package(package, params.merge(job_token: job.token))
end
end end
describe 'GET /api/v4/projects/:id/packages/npm/*package_name/-/*file_name' do describe 'GET /api/v4/projects/:id/packages/npm/*package_name/-/*file_name' do
let(:package) { create(:npm_package, project: project) } let(:package) { create(:npm_package, project: project) }
let(:package_file) { package.package_files.first } let(:package_file) { package.package_files.first }
context 'a public project' do shared_examples 'a package file that requires auth' do
it 'returns the file' do it 'returns the file with an access token' do
get_file_with_token(package_file) get_file_with_token(package_file)
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
expect(response.content_type.to_s).to eq('application/octet-stream') expect(response.content_type.to_s).to eq('application/octet-stream')
end end
it 'returns the file with a job token' do
get_file_with_job_token(package_file)
expect(response).to have_gitlab_http_status(200)
expect(response.content_type.to_s).to eq('application/octet-stream')
end end
context 'private project' do it 'denies download with no token' do
before do get_file(package_file)
project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
expect(response).to have_gitlab_http_status(404)
end
end end
it 'returns the file' do context 'a public project' do
get_file_with_token(package_file) it 'returns the file with no token needed' do
get_file(package_file)
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
expect(response.content_type.to_s).to eq('application/octet-stream') expect(response.content_type.to_s).to eq('application/octet-stream')
end end
end
context 'private project' do
before do
project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
end
it_behaves_like 'a package file that requires auth'
it 'denies download when not enough permissions' do it 'denies download when not enough permissions' do
project.add_guest(user) project.add_guest(user)
...@@ -121,12 +150,14 @@ describe API::NpmPackages do ...@@ -121,12 +150,14 @@ describe API::NpmPackages do
expect(response).to have_gitlab_http_status(403) expect(response).to have_gitlab_http_status(403)
end end
end
it 'denies download when no private token' do context 'internal project' do
get_file(package_file) before do
project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
expect(response).to have_gitlab_http_status(404)
end end
it_behaves_like 'a package file that requires auth'
end end
it 'rejects request if feature is not in the license' do it 'rejects request if feature is not in the license' do
...@@ -145,6 +176,10 @@ describe API::NpmPackages do ...@@ -145,6 +176,10 @@ describe API::NpmPackages do
def get_file_with_token(package_file, params = {}) def get_file_with_token(package_file, params = {})
get_file(package_file, params.merge(access_token: token.token)) get_file(package_file, params.merge(access_token: token.token))
end end
def get_file_with_job_token(package_file, params = {})
get_file(package_file, params.merge(job_token: job.token))
end
end end
describe 'PUT /api/v4/projects/:id/packages/npm/:package_name' do describe 'PUT /api/v4/projects/:id/packages/npm/:package_name' do
...@@ -179,13 +214,21 @@ describe API::NpmPackages do ...@@ -179,13 +214,21 @@ describe API::NpmPackages do
let(:package_name) { "@#{group.path}/my_package_name" } let(:package_name) { "@#{group.path}/my_package_name" }
let(:params) { upload_params(package_name) } let(:params) { upload_params(package_name) }
it 'creates npm package with file' do it 'creates npm package with file with access token' do
expect { upload_package_with_token(package_name, params) } expect { upload_package_with_token(package_name, params) }
.to change { project.packages.count }.by(1) .to change { project.packages.count }.by(1)
.and change { Packages::PackageFile.count }.by(1) .and change { Packages::PackageFile.count }.by(1)
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
end end
it 'creates npm package with file with job token' do
expect { upload_package_with_job_token(package_name, params) }
.to change { project.packages.count }.by(1)
.and change { Packages::PackageFile.count }.by(1)
expect(response).to have_gitlab_http_status(200)
end
end end
context 'package creation fails' do context 'package creation fails' do
...@@ -210,6 +253,10 @@ describe API::NpmPackages do ...@@ -210,6 +253,10 @@ describe API::NpmPackages do
upload_package(package_name, params.merge(access_token: token.token)) upload_package(package_name, params.merge(access_token: token.token))
end end
def upload_package_with_job_token(package_name, params = {})
upload_package(package_name, params.merge(job_token: job.token))
end
def upload_params(package_name) def upload_params(package_name)
JSON.parse( JSON.parse(
fixture_file('npm/payload.json', dir: 'ee') fixture_file('npm/payload.json', dir: 'ee')
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment