diff --git a/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js b/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js index eec4db41b0a..7143cb50ea6 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js +++ b/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js @@ -1,4 +1,5 @@ -import { objectToQueryString, spriteIcon } from '~/lib/utils/common_utils'; +import { spriteIcon } from '~/lib/utils/common_utils'; +import { objectToQuery } from '~/lib/utils/url_utility'; import FilteredSearchContainer from './container'; import VisualTokenValue from './visual_token_value'; @@ -327,7 +328,7 @@ export default class FilteredSearchVisualTokens { return endpoint; } - const queryString = objectToQueryString(JSON.parse(endpointQueryParams)); + const queryString = objectToQuery(JSON.parse(endpointQueryParams)); return `${endpoint}?${queryString}`; } diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index bedc7c80299..291ef4defd3 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -371,17 +371,6 @@ export const parseIntPagination = (paginationInformation) => ({ previousPage: parseInt(paginationInformation['X-PREV-PAGE'], 10), }); -/** - * Converts object with key-value pairs - * into query-param string - * - * @param {Object} params - */ -export const objectToQueryString = (params = {}) => - Object.keys(params) - .map((param) => `${param}=${params[param]}`) - .join('&'); - export const buildUrlWithCurrentLocation = (param) => { if (param) return `${window.location.pathname}${param}`; diff --git a/app/assets/javascripts/lib/utils/url_utility.js b/app/assets/javascripts/lib/utils/url_utility.js index 9d9364987d8..dd10653b20c 100644 --- a/app/assets/javascripts/lib/utils/url_utility.js +++ b/app/assets/javascripts/lib/utils/url_utility.js @@ -501,15 +501,15 @@ export function queryToObject(query, { gatherArrays = false, legacySpacesDecode /** * Convert search query object back into a search query * - * @param {Object} obj that needs to be converted + * @param {Object?} params that needs to be converted * @returns {String} * * ex: {one: 1, two: 2} into "one=1&two=2" * */ -export function objectToQuery(obj) { - return Object.keys(obj) - .map((k) => `${encodeURIComponent(k)}=${encodeURIComponent(obj[k])}`) +export function objectToQuery(params = {}) { + return Object.keys(params) + .map((k) => `${encodeURIComponent(k)}=${encodeURIComponent(params[k])}`) .join('&'); } diff --git a/app/assets/javascripts/packages/details/components/app.vue b/app/assets/javascripts/packages/details/components/app.vue index 55ffe10a608..59da32e6666 100644 --- a/app/assets/javascripts/packages/details/components/app.vue +++ b/app/assets/javascripts/packages/details/components/app.vue @@ -11,8 +11,8 @@ import { GlSprintf, } from '@gitlab/ui'; import { mapActions, mapState } from 'vuex'; -import { objectToQueryString } from '~/lib/utils/common_utils'; import { numberToHumanSize } from '~/lib/utils/number_utils'; +import { objectToQuery } from '~/lib/utils/url_utility'; import { s__, __ } from '~/locale'; import Tracking from '~/tracking'; import PackageListRow from '../../shared/components/package_list_row.vue'; @@ -114,7 +114,7 @@ export default { !this.groupListUrl || document.referrer.includes(this.projectName) ? this.projectListUrl : this.groupListUrl; // to avoid security issue url are supplied from backend - const modalQuery = objectToQueryString({ [SHOW_DELETE_SUCCESS_ALERT]: true }); + const modalQuery = objectToQuery({ [SHOW_DELETE_SUCCESS_ALERT]: true }); window.location.replace(`${returnTo}?${modalQuery}`); }, handleFileDelete(file) { diff --git a/app/models/deploy_key.rb b/app/models/deploy_key.rb index 25e3b9fe4f0..8f5a713af3f 100644 --- a/app/models/deploy_key.rb +++ b/app/models/deploy_key.rb @@ -3,6 +3,7 @@ class DeployKey < Key include FromUnion include IgnorableColumns + include PolicyActor has_many :deploy_keys_projects, inverse_of: :deploy_key, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :projects, through: :deploy_keys_projects diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index e93c60c3710..3cb4644a60d 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -69,6 +69,16 @@ class ProjectPolicy < BasePolicy project.merge_requests_allowing_push_to_user(user).any? end + desc "Deploy key with read access" + condition(:download_code_deploy_key) do + user.is_a?(DeployKey) && user.has_access_to?(project) + end + + desc "Deploy key with write access" + condition(:push_code_deploy_key) do + user.is_a?(DeployKey) && user.can_push_to?(project) + end + desc "Deploy token with read_package_registry scope" condition(:read_package_registry_deploy_token) do user.is_a?(DeployToken) && user.has_access_to?(project) && user.read_package_registry @@ -616,6 +626,14 @@ class ProjectPolicy < BasePolicy prevent :move_design end + rule { download_code_deploy_key }.policy do + enable :download_code + end + + rule { push_code_deploy_key }.policy do + enable :push_code + end + rule { read_package_registry_deploy_token }.policy do enable :read_package enable :read_project diff --git a/spec/frontend/lib/utils/common_utils_spec.js b/spec/frontend/lib/utils/common_utils_spec.js index 370b01d6e36..130cbf9624a 100644 --- a/spec/frontend/lib/utils/common_utils_spec.js +++ b/spec/frontend/lib/utils/common_utils_spec.js @@ -139,21 +139,6 @@ describe('common_utils', () => { }); }); - describe('objectToQueryString', () => { - it('returns empty string when `param` is undefined, null or empty string', () => { - expect(commonUtils.objectToQueryString()).toBe(''); - expect(commonUtils.objectToQueryString('')).toBe(''); - }); - - it('returns query string with values of `params`', () => { - const singleQueryParams = { foo: true }; - const multipleQueryParams = { foo: true, bar: true }; - - expect(commonUtils.objectToQueryString(singleQueryParams)).toBe('foo=true'); - expect(commonUtils.objectToQueryString(multipleQueryParams)).toBe('foo=true&bar=true'); - }); - }); - describe('buildUrlWithCurrentLocation', () => { it('should build an url with current location and given parameters', () => { expect(commonUtils.buildUrlWithCurrentLocation()).toEqual(window.location.pathname); diff --git a/spec/frontend/lib/utils/url_utility_spec.js b/spec/frontend/lib/utils/url_utility_spec.js index 1f7a2832c34..97ae50d0623 100644 --- a/spec/frontend/lib/utils/url_utility_spec.js +++ b/spec/frontend/lib/utils/url_utility_spec.js @@ -718,6 +718,19 @@ describe('URL utility', () => { expect(urlUtils.objectToQuery(searchQueryObject)).toEqual('one=1&two=2'); }); + + it('returns empty string when `params` is undefined, null or empty string', () => { + expect(urlUtils.objectToQuery()).toBe(''); + expect(urlUtils.objectToQuery('')).toBe(''); + }); + + it('returns query string with values of `params`', () => { + const singleQueryParams = { foo: true }; + const multipleQueryParams = { foo: true, bar: true }; + + expect(urlUtils.objectToQuery(singleQueryParams)).toBe('foo=true'); + expect(urlUtils.objectToQuery(multipleQueryParams)).toBe('foo=true&bar=true'); + }); }); describe('cleanLeadingSeparator', () => { diff --git a/spec/models/deploy_key_spec.rb b/spec/models/deploy_key_spec.rb index d4ccaa6a10e..fa78527e366 100644 --- a/spec/models/deploy_key_spec.rb +++ b/spec/models/deploy_key_spec.rb @@ -93,4 +93,46 @@ RSpec.describe DeployKey, :mailer do end end end + + describe 'PolicyActor methods' do + let_it_be(:user) { create(:user) } + let_it_be(:deploy_key) { create(:deploy_key, user: user) } + let_it_be(:project) { create(:project, creator: user, namespace: user.namespace) } + + let(:methods) { PolicyActor.instance_methods } + + subject { deploy_key } + + it 'responds to all PolicyActor methods' do + methods.each do |method| + expect(subject.respond_to?(method)).to be true + end + end + + describe '#can?' do + it { expect(user.can?(:read_project, project)).to be true } + + context 'when a read deploy key is enabled in the project' do + let!(:deploy_keys_project) { create(:deploy_keys_project, project: project, deploy_key: deploy_key) } + + it { expect(subject.can?(:read_project, project)).to be false } + it { expect(subject.can?(:download_code, project)).to be true } + it { expect(subject.can?(:push_code, project)).to be false } + end + + context 'when a write deploy key is enabled in the project' do + let!(:deploy_keys_project) { create(:deploy_keys_project, :write_access, project: project, deploy_key: deploy_key) } + + it { expect(subject.can?(:read_project, project)).to be false } + it { expect(subject.can?(:download_code, project)).to be true } + it { expect(subject.can?(:push_code, project)).to be true } + end + + context 'when the deploy key is not enabled in the project' do + it { expect(subject.can?(:read_project, project)).to be false } + it { expect(subject.can?(:download_code, project)).to be false } + it { expect(subject.can?(:push_code, project)).to be false } + end + end + end end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 27df461af1c..d3e68141813 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -795,6 +795,37 @@ RSpec.describe ProjectPolicy do end end + context 'deploy key access' do + context 'private project' do + let(:project) { private_project } + let!(:deploy_key) { create(:deploy_key, user: owner) } + + subject { described_class.new(deploy_key, project) } + + context 'when a read deploy key is enabled in the project' do + let!(:deploy_keys_project) { create(:deploy_keys_project, project: project, deploy_key: deploy_key) } + + it { is_expected.to be_allowed(:download_code) } + it { is_expected.to be_disallowed(:push_code) } + it { is_expected.to be_disallowed(:read_project) } + end + + context 'when a write deploy key is enabled in the project' do + let!(:deploy_keys_project) { create(:deploy_keys_project, :write_access, project: project, deploy_key: deploy_key) } + + it { is_expected.to be_allowed(:download_code) } + it { is_expected.to be_allowed(:push_code) } + it { is_expected.to be_disallowed(:read_project) } + end + + context 'when the deploy key is not enabled in the project' do + it { is_expected.to be_disallowed(:download_code) } + it { is_expected.to be_disallowed(:push_code) } + it { is_expected.to be_disallowed(:read_project) } + end + end + end + context 'deploy token access' do let!(:project_deploy_token) do create(:project_deploy_token, project: project, deploy_token: deploy_token)