From 9b58b8e363fd388635385085c58be3d4637eaa45 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 6 Nov 2017 22:20:44 +0900 Subject: Do not allow jobs to be erased --- lib/api/jobs.rb | 2 +- lib/api/v3/builds.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/api') diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb index 3c1c412ba42..a116ab3c9bd 100644 --- a/lib/api/jobs.rb +++ b/lib/api/jobs.rb @@ -136,7 +136,7 @@ module API authorize_update_builds! build = find_build!(params[:job_id]) - authorize!(:update_build, build) + authorize!(:erase_build, build) return forbidden!('Job is not erasable!') unless build.erasable? build.erase(erased_by: current_user) diff --git a/lib/api/v3/builds.rb b/lib/api/v3/builds.rb index f493fd7c7ec..fa0bef39602 100644 --- a/lib/api/v3/builds.rb +++ b/lib/api/v3/builds.rb @@ -169,7 +169,7 @@ module API authorize_update_builds! build = get_build!(params[:build_id]) - authorize!(:update_build, build) + authorize!(:erase_build, build) return forbidden!('Build is not erasable!') unless build.erasable? build.erase(erased_by: current_user) -- cgit v1.2.1 From c8eb2a914b6f9348ffa16436853964998c115085 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 6 Nov 2017 23:24:25 +0900 Subject: Fix spec. Revert update check. --- lib/api/jobs.rb | 1 + lib/api/v3/builds.rb | 1 + 2 files changed, 2 insertions(+) (limited to 'lib/api') diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb index a116ab3c9bd..6dcbe2ff936 100644 --- a/lib/api/jobs.rb +++ b/lib/api/jobs.rb @@ -136,6 +136,7 @@ module API authorize_update_builds! build = find_build!(params[:job_id]) + authorize!(:update_build, build) authorize!(:erase_build, build) return forbidden!('Job is not erasable!') unless build.erasable? diff --git a/lib/api/v3/builds.rb b/lib/api/v3/builds.rb index fa0bef39602..1c0f9f73c78 100644 --- a/lib/api/v3/builds.rb +++ b/lib/api/v3/builds.rb @@ -169,6 +169,7 @@ module API authorize_update_builds! build = get_build!(params[:build_id]) + authorize!(:update_build, build) authorize!(:erase_build, build) return forbidden!('Build is not erasable!') unless build.erasable? -- cgit v1.2.1 From afef38533727cf32a7be324243a25b4db5eb5498 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 7 Nov 2017 02:47:05 +0900 Subject: Add doc. Fix spec. Add erase_build in protected_ref rule --- lib/api/jobs.rb | 1 - lib/api/v3/builds.rb | 1 - 2 files changed, 2 deletions(-) (limited to 'lib/api') diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb index 6dcbe2ff936..a116ab3c9bd 100644 --- a/lib/api/jobs.rb +++ b/lib/api/jobs.rb @@ -136,7 +136,6 @@ module API authorize_update_builds! build = find_build!(params[:job_id]) - authorize!(:update_build, build) authorize!(:erase_build, build) return forbidden!('Job is not erasable!') unless build.erasable? diff --git a/lib/api/v3/builds.rb b/lib/api/v3/builds.rb index 1c0f9f73c78..fa0bef39602 100644 --- a/lib/api/v3/builds.rb +++ b/lib/api/v3/builds.rb @@ -169,7 +169,6 @@ module API authorize_update_builds! build = get_build!(params[:build_id]) - authorize!(:update_build, build) authorize!(:erase_build, build) return forbidden!('Build is not erasable!') unless build.erasable? -- cgit v1.2.1 From 4d4ddb6004e6f7f56b337a49c6eedaad70d70862 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Tue, 7 Nov 2017 14:00:21 +0000 Subject: Fail when issuable_meta_data is called on an unlimited collection This method can be called with an array, or a relation: 1. Arrays always have a limited amount of values, so that's fine. 2. If the relation does not have a limit value applied, then we will load every single object in that collection, and prevent N+1 queries for the metadata for that. But that's wrong, because we should never call this without an explicit limit set. So we raise in that case, and this commit will see which specs fail. The only failing specs here were the issues API specs, and the specs for IssuableMetadata itself, and both have been addressed. --- lib/api/issues.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'lib/api') diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 0df41dcc903..74dfd9f96de 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -68,7 +68,7 @@ module API desc: 'Return issues for the given scope: `created-by-me`, `assigned-to-me` or `all`' end get do - issues = find_issues + issues = paginate(find_issues) options = { with: Entities::IssueBasic, @@ -76,7 +76,7 @@ module API issuable_metadata: issuable_meta_data(issues, 'Issue') } - present paginate(issues), options + present issues, options end end @@ -95,7 +95,7 @@ module API get ":id/issues" do group = find_group!(params[:id]) - issues = find_issues(group_id: group.id) + issues = paginate(find_issues(group_id: group.id)) options = { with: Entities::IssueBasic, @@ -103,7 +103,7 @@ module API issuable_metadata: issuable_meta_data(issues, 'Issue') } - present paginate(issues), options + present issues, options end end @@ -124,7 +124,7 @@ module API get ":id/issues" do project = find_project!(params[:id]) - issues = find_issues(project_id: project.id) + issues = paginate(find_issues(project_id: project.id)) options = { with: Entities::IssueBasic, @@ -133,7 +133,7 @@ module API issuable_metadata: issuable_meta_data(issues, 'Issue') } - present paginate(issues), options + present issues, options end desc 'Get a single project issue' do -- cgit v1.2.1 From fec48c6e170fb0032cece5d8cc3b06bb45caee57 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 7 Nov 2017 17:11:37 +0100 Subject: Use Commit#notes and Note.for_commit_id when possible to make sure we use all the indexes available to us --- lib/api/commits.rb | 2 +- lib/api/v3/commits.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/api') diff --git a/lib/api/commits.rb b/lib/api/commits.rb index 2685dc27252..2bc4039b019 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -117,7 +117,7 @@ module API commit = user_project.commit(params[:sha]) not_found! 'Commit' unless commit - notes = user_project.notes.where(commit_id: commit.id).order(:created_at) + notes = commit.notes.order(:created_at) present paginate(notes), with: Entities::CommitNote end diff --git a/lib/api/v3/commits.rb b/lib/api/v3/commits.rb index ed206a6def0..be360fbfc0c 100644 --- a/lib/api/v3/commits.rb +++ b/lib/api/v3/commits.rb @@ -106,7 +106,7 @@ module API commit = user_project.commit(params[:sha]) not_found! 'Commit' unless commit - notes = Note.where(commit_id: commit.id).order(:created_at) + notes = commit.notes.order(:created_at) present paginate(notes), with: ::API::Entities::CommitNote end -- cgit v1.2.1 From 12d622eb996b6499e5fbd2be01cca27c08a976fa Mon Sep 17 00:00:00 2001 From: Mark Fletcher Date: Wed, 8 Nov 2017 10:22:24 +0000 Subject: Fix acceptance of username for Mattermost service update via API --- lib/api/services.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'lib/api') diff --git a/lib/api/services.rb b/lib/api/services.rb index 6454e475036..bbcc851d07a 100644 --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -522,6 +522,12 @@ module API name: :webhook, type: String, desc: 'The Mattermost webhook. e.g. http://mattermost_host/hooks/...' + }, + { + required: false, + name: :username, + type: String, + desc: 'The username to use to post the message' } ], 'teamcity' => [ -- cgit v1.2.1 From 20ac30a705f4edd22efd934ecf68b58557f868db Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 1 Nov 2017 09:25:49 +0000 Subject: Merge branch '36099-api-responses-missing-x-content-type-options-header' into '10-1-stable' Include X-Content-Type-Options (XCTO) header into API responses See merge request gitlab/gitlabhq!2211 (cherry picked from commit 6c818e77f2abeef2dd7b17a269611b018701fa79) e087e075 Include X-Content-Type-Options (XCTO) header into API responses --- lib/api/api.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'lib/api') diff --git a/lib/api/api.rb b/lib/api/api.rb index c37e596eb9d..8094597d238 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -61,7 +61,10 @@ module API mount ::API::V3::Variables end - before { header['X-Frame-Options'] = 'SAMEORIGIN' } + before do + header['X-Frame-Options'] = 'SAMEORIGIN' + header['X-Content-Type-Options'] = 'nosniff' + end # The locale is set to the current user's locale when `current_user` is loaded after { Gitlab::I18n.use_default_locale } -- cgit v1.2.1 From 26b6dfaf6f74a8b1e9cc1b05c30879f8a60f86bb Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Thu, 9 Nov 2017 16:07:04 +0000 Subject: Add /groups/:id/subgroups endpoint to API --- lib/api/groups.rb | 66 ++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 22 deletions(-) (limited to 'lib/api') diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 340a7cecf09..bcf2e6dae1d 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -25,24 +25,7 @@ module API optional :statistics, type: Boolean, default: false, desc: 'Include project statistics' end - def present_groups(groups, options = {}) - options = options.reverse_merge( - with: Entities::Group, - current_user: current_user - ) - - groups = groups.with_statistics if options[:statistics] - present paginate(groups), options - end - end - - resource :groups do - include CustomAttributesEndpoints - - desc 'Get a groups list' do - success Entities::Group - end - params do + params :group_list_params do use :statistics_params optional :skip_groups, type: Array[Integer], desc: 'Array of group ids to exclude from list' optional :all_available, type: Boolean, desc: 'Show all group that you have access to' @@ -52,19 +35,47 @@ module API optional :sort, type: String, values: %w[asc desc], default: 'asc', desc: 'Sort by asc (ascending) or desc (descending)' use :pagination end - get do + + def find_groups(params) find_params = { all_available: params[:all_available], - owned: params[:owned], - custom_attributes: params[:custom_attributes] + custom_attributes: params[:custom_attributes], + owned: params[:owned] } + find_params[:parent] = find_group!(params[:id]) if params[:id] groups = GroupsFinder.new(current_user, find_params).execute groups = groups.search(params[:search]) if params[:search].present? groups = groups.where.not(id: params[:skip_groups]) if params[:skip_groups].present? groups = groups.reorder(params[:order_by] => params[:sort]) - present_groups groups, statistics: params[:statistics] && current_user.admin? + groups + end + + def present_groups(params, groups) + options = { + with: Entities::Group, + current_user: current_user, + statistics: params[:statistics] && current_user.admin? + } + + groups = groups.with_statistics if options[:statistics] + present paginate(groups), options + end + end + + resource :groups do + include CustomAttributesEndpoints + + desc 'Get a groups list' do + success Entities::Group + end + params do + use :group_list_params + end + get do + groups = find_groups(params) + present_groups params, groups end desc 'Create a group. Available only for users who can create groups.' do @@ -166,6 +177,17 @@ module API present paginate(projects), with: entity, current_user: current_user end + desc 'Get a list of subgroups in this group.' do + success Entities::Group + end + params do + use :group_list_params + end + get ":id/subgroups" do + groups = find_groups(params) + present_groups params, groups + end + desc 'Transfer a project to the group namespace. Available only for admin.' do success Entities::GroupDetail end -- cgit v1.2.1 From 258bf3e187538bd326491e5d1b25a0511fbd96a1 Mon Sep 17 00:00:00 2001 From: "Lin Jen-Shin (godfat)" Date: Mon, 13 Nov 2017 15:27:30 +0000 Subject: Add Gitlab::Utils::StrongMemoize --- lib/api/api_guard.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'lib/api') diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index b9c7d443f6c..c1c0d344917 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -42,6 +42,8 @@ module API # Helper Methods for Grape Endpoint module HelperMethods + include Gitlab::Utils::StrongMemoize + def find_current_user! user = find_user_from_access_token || find_user_from_warden return unless user @@ -52,9 +54,9 @@ module API end def access_token - return @access_token if defined?(@access_token) - - @access_token = find_oauth_access_token || find_personal_access_token + strong_memoize(:access_token) do + find_oauth_access_token || find_personal_access_token + end end def validate_access_token!(scopes: []) -- cgit v1.2.1 From 1162d89ac49553c579ec4d049e74206893ff6302 Mon Sep 17 00:00:00 2001 From: Travis Miller Date: Mon, 13 Nov 2017 16:05:44 +0000 Subject: Add administrative endpoint to list all pages domains --- lib/api/entities.rb | 20 ++++++++++++++++++-- lib/api/helpers.rb | 9 +++++++++ lib/api/pages_domains.rb | 22 +++++++++++++++++++++- 3 files changed, 48 insertions(+), 3 deletions(-) (limited to 'lib/api') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index a382db92e8d..16ae99b5c6c 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1042,6 +1042,11 @@ module API expose :value end + class PagesDomainCertificateExpiration < Grape::Entity + expose :expired?, as: :expired + expose :expiration + end + class PagesDomainCertificate < Grape::Entity expose :subject expose :expired?, as: :expired @@ -1049,12 +1054,23 @@ module API expose :certificate_text end + class PagesDomainBasic < Grape::Entity + expose :domain + expose :url + expose :certificate, + as: :certificate_expiration, + if: ->(pages_domain, _) { pages_domain.certificate? }, + using: PagesDomainCertificateExpiration do |pages_domain| + pages_domain + end + end + class PagesDomain < Grape::Entity expose :domain expose :url expose :certificate, - if: ->(pages_domain, _) { pages_domain.certificate? }, - using: PagesDomainCertificate do |pages_domain| + if: ->(pages_domain, _) { pages_domain.certificate? }, + using: PagesDomainCertificate do |pages_domain| pages_domain end end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 5f9b94cc89c..3c8960cb1ab 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -155,6 +155,11 @@ module API end end + def authenticated_with_full_private_access! + authenticate! + forbidden! unless current_user.full_private_access? + end + def authenticated_as_admin! authenticate! forbidden! unless current_user.admin? @@ -190,6 +195,10 @@ module API not_found! unless user_project.pages_available? end + def require_pages_config_enabled! + not_found! unless Gitlab.config.pages.enabled + end + def can?(object, action, subject = :global) Ability.allowed?(object, action, subject) end diff --git a/lib/api/pages_domains.rb b/lib/api/pages_domains.rb index 259f3f34068..d7b613a717e 100644 --- a/lib/api/pages_domains.rb +++ b/lib/api/pages_domains.rb @@ -4,7 +4,6 @@ module API before do authenticate! - require_pages_enabled! end after_validation do @@ -29,10 +28,31 @@ module API end end + resource :pages do + before do + require_pages_config_enabled! + authenticated_with_full_private_access! + end + + desc "Get all pages domains" do + success Entities::PagesDomainBasic + end + params do + use :pagination + end + get "domains" do + present paginate(PagesDomain.all), with: Entities::PagesDomainBasic + end + end + params do requires :id, type: String, desc: 'The ID of a project' end resource :projects, requirements: { id: %r{[^/]+} } do + before do + require_pages_enabled! + end + desc 'Get all pages domains' do success Entities::PagesDomain end -- cgit v1.2.1 From 52bfd064627c6a63b82bc81e7ebabac299b25eed Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Tue, 14 Nov 2017 16:14:35 +0100 Subject: Use relative git object paths to construct absolute ones before setting Env --- lib/api/helpers/internal_helpers.rb | 12 ++++++++++++ lib/api/internal.rb | 4 +++- 2 files changed, 15 insertions(+), 1 deletion(-) (limited to 'lib/api') diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb index 4c0db4d42b1..4b3c473b0bb 100644 --- a/lib/api/helpers/internal_helpers.rb +++ b/lib/api/helpers/internal_helpers.rb @@ -36,6 +36,18 @@ module API {} end + def fix_git_env_repository_paths(env, repository_path) + if obj_dir_relative = env['GIT_OBJECT_DIRECTORY_RELATIVE'].presence + env['GIT_OBJECT_DIRECTORY'] = File.join(repository_path, obj_dir_relative) + end + + if alt_obj_dirs_relative = env['GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE'].presence + env['GIT_ALTERNATE_OBJECT_DIRECTORIES'] = alt_obj_dirs_relative.map { |dir| File.join(repository_path, dir) } + end + + env + end + def log_user_activity(actor) commands = Gitlab::GitAccess::DOWNLOAD_COMMANDS diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 6e78ac2c903..451121a4cea 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -19,7 +19,9 @@ module API status 200 # Stores some Git-specific env thread-safely - Gitlab::Git::Env.set(parse_env) + env = parse_env + env = fix_git_env_repository_paths(env, repository_path) if project + Gitlab::Git::Env.set(env) actor = if params[:key_id] -- cgit v1.2.1 From 181cd299f9e06223e8338e93b1c318c671ccb1aa Mon Sep 17 00:00:00 2001 From: Jacopo Date: Tue, 14 Nov 2017 10:02:39 +0100 Subject: Adds Rubocop rule for line break after guard clause Adds a rubocop rule (with autocorrect) to ensure line break after guard clauses. --- lib/api/commits.rb | 2 ++ lib/api/helpers/custom_validators.rb | 1 + lib/api/helpers/runner.rb | 1 + lib/api/runners.rb | 4 ++++ lib/api/snippets.rb | 1 + lib/api/v3/commits.rb | 2 ++ lib/api/v3/runners.rb | 1 + lib/api/v3/snippets.rb | 2 ++ 8 files changed, 14 insertions(+) (limited to 'lib/api') diff --git a/lib/api/commits.rb b/lib/api/commits.rb index 2bc4039b019..38e05074353 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -180,10 +180,12 @@ module API if params[:path] commit.raw_diffs(limits: false).each do |diff| next unless diff.new_path == params[:path] + lines = Gitlab::Diff::Parser.new.parse(diff.diff.each_line) lines.each do |line| next unless line.new_pos == params[:line] && line.type == params[:line_type] + break opts[:line_code] = Gitlab::Git.diff_line_code(diff.new_path, line.new_pos, line.old_pos) end diff --git a/lib/api/helpers/custom_validators.rb b/lib/api/helpers/custom_validators.rb index 0a8f3073a50..dd4f6c41131 100644 --- a/lib/api/helpers/custom_validators.rb +++ b/lib/api/helpers/custom_validators.rb @@ -4,6 +4,7 @@ module API class Absence < Grape::Validations::Base def validate_param!(attr_name, params) return if params.respond_to?(:key?) && !params.key?(attr_name) + raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:absence) end end diff --git a/lib/api/helpers/runner.rb b/lib/api/helpers/runner.rb index 282af32ca94..2cae53dba53 100644 --- a/lib/api/helpers/runner.rb +++ b/lib/api/helpers/runner.rb @@ -14,6 +14,7 @@ module API def get_runner_version_from_params return unless params['info'].present? + attributes_for_keys(%w(name version revision platform architecture), params['info']) end diff --git a/lib/api/runners.rb b/lib/api/runners.rb index d3559ef71be..e816fcdd928 100644 --- a/lib/api/runners.rb +++ b/lib/api/runners.rb @@ -165,17 +165,20 @@ module API def authenticate_show_runner!(runner) return if runner.is_shared || current_user.admin? + forbidden!("No access granted") unless user_can_access_runner?(runner) end def authenticate_update_runner!(runner) return if current_user.admin? + forbidden!("Runner is shared") if runner.is_shared? forbidden!("No access granted") unless user_can_access_runner?(runner) end def authenticate_delete_runner!(runner) return if current_user.admin? + forbidden!("Runner is shared") if runner.is_shared? forbidden!("Runner associated with more than one project") if runner.projects.count > 1 forbidden!("No access granted") unless user_can_access_runner?(runner) @@ -185,6 +188,7 @@ module API forbidden!("Runner is shared") if runner.is_shared? forbidden!("Runner is locked") if runner.locked? return if current_user.admin? + forbidden!("No access granted") unless user_can_access_runner?(runner) end diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb index 00eb7c60f16..c736cc32021 100644 --- a/lib/api/snippets.rb +++ b/lib/api/snippets.rb @@ -95,6 +95,7 @@ module API put ':id' do snippet = snippets_for_current_user.find_by(id: params.delete(:id)) return not_found!('Snippet') unless snippet + authorize! :update_personal_snippet, snippet attrs = declared_params(include_missing: false).merge(request: request, api: true) diff --git a/lib/api/v3/commits.rb b/lib/api/v3/commits.rb index be360fbfc0c..0ef26aa696a 100644 --- a/lib/api/v3/commits.rb +++ b/lib/api/v3/commits.rb @@ -169,10 +169,12 @@ module API if params[:path] commit.raw_diffs(limits: false).each do |diff| next unless diff.new_path == params[:path] + lines = Gitlab::Diff::Parser.new.parse(diff.diff.each_line) lines.each do |line| next unless line.new_pos == params[:line] && line.type == params[:line_type] + break opts[:line_code] = Gitlab::Git.diff_line_code(diff.new_path, line.new_pos, line.old_pos) end diff --git a/lib/api/v3/runners.rb b/lib/api/v3/runners.rb index faa265f3314..c6d9957d452 100644 --- a/lib/api/v3/runners.rb +++ b/lib/api/v3/runners.rb @@ -51,6 +51,7 @@ module API helpers do def authenticate_delete_runner!(runner) return if current_user.admin? + forbidden!("Runner is shared") if runner.is_shared? forbidden!("Runner associated with more than one project") if runner.projects.count > 1 forbidden!("No access granted") unless user_can_access_runner?(runner) diff --git a/lib/api/v3/snippets.rb b/lib/api/v3/snippets.rb index 0762fc02d70..126ec72248e 100644 --- a/lib/api/v3/snippets.rb +++ b/lib/api/v3/snippets.rb @@ -91,6 +91,7 @@ module API put ':id' do snippet = snippets_for_current_user.find_by(id: params.delete(:id)) return not_found!('Snippet') unless snippet + authorize! :update_personal_snippet, snippet attrs = declared_params(include_missing: false) @@ -113,6 +114,7 @@ module API delete ':id' do snippet = snippets_for_current_user.find_by(id: params.delete(:id)) return not_found!('Snippet') unless snippet + authorize! :destroy_personal_snippet, snippet snippet.destroy no_content! -- cgit v1.2.1 From d948e6791300b14d18b95881290ccfcba7928ea0 Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Tue, 7 Nov 2017 10:52:05 +0100 Subject: First refactor --- lib/api/api_guard.rb | 58 +++++++++++++++++----------------------------------- 1 file changed, 19 insertions(+), 39 deletions(-) (limited to 'lib/api') diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index c1c0d344917..0a93e71858e 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -74,43 +74,27 @@ module API private - def find_user_from_access_token - return unless access_token - - validate_access_token! - - access_token.user || raise(UnauthorizedError) - end - - # Check the Rails session for valid authentication details - def find_user_from_warden - warden.try(:authenticate) if verified_request? - end - - def warden - env['warden'] - end - - # Check if the request is GET/HEAD, or if CSRF token is valid. - def verified_request? - Gitlab::RequestForgeryProtection.verified?(env) - end - - def find_oauth_access_token - token = Doorkeeper::OAuth::Token.from_request(doorkeeper_request, *Doorkeeper.configuration.access_token_methods) - return unless token - - # Expiration, revocation and scopes are verified in `find_user_by_access_token` - access_token = OauthAccessToken.by_token(token) - raise UnauthorizedError unless access_token - - access_token.revoke_previous_refresh_token! - access_token + def raise_unauthorized_error! + raise UnauthorizedError end - def find_personal_access_token - token = (params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]).to_s - return unless token.present? + # If token is presented and valid, then it sets @current_user. + # + # If the token does not have sufficient scopes to cover the requred scopes, + # then it raises InsufficientScopeError. + # + # If the token is expired, then it raises ExpiredError. + # + # If the token is revoked, then it raises RevokedError. + # + # If the token is not found (nil), then it returns nil + # + # Arguments: + # + # scopes: (optional) scopes required for this guard. + # Defaults to empty array. + def find_user_by_access_token(access_token) + scopes = scopes_registered_for_endpoint # Expiration, revocation and scopes are verified in `find_user_by_access_token` access_token = PersonalAccessToken.find_by(token: token) @@ -119,10 +103,6 @@ module API access_token end - def doorkeeper_request - @doorkeeper_request ||= ActionDispatch::Request.new(env) - end - # An array of scopes that were registered (using `allow_access_with_scope`) # for the current endpoint class. It also returns scopes registered on # `API::API`, since these are meant to apply to all API routes. -- cgit v1.2.1 From 470b5dc32633cd4ec873e655ac6a70011c835e17 Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Tue, 7 Nov 2017 16:13:00 +0100 Subject: Updated refactor and pushing to see if test fails --- lib/api/api_guard.rb | 2 -- 1 file changed, 2 deletions(-) (limited to 'lib/api') diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 0a93e71858e..66ad2b77f75 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -72,8 +72,6 @@ module API end end - private - def raise_unauthorized_error! raise UnauthorizedError end -- cgit v1.2.1 From 41ebd06ddc837c80ba6ca95c6d5fea2b76cef8d2 Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Tue, 7 Nov 2017 19:17:41 +0100 Subject: Some fixes after rebase --- lib/api/api_guard.rb | 31 +++++++------------------------ 1 file changed, 7 insertions(+), 24 deletions(-) (limited to 'lib/api') diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 66ad2b77f75..9ada2d5ebb1 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -72,33 +72,16 @@ module API end end - def raise_unauthorized_error! - raise UnauthorizedError - end + private - # If token is presented and valid, then it sets @current_user. - # - # If the token does not have sufficient scopes to cover the requred scopes, - # then it raises InsufficientScopeError. - # - # If the token is expired, then it raises ExpiredError. - # - # If the token is revoked, then it raises RevokedError. - # - # If the token is not found (nil), then it returns nil - # - # Arguments: - # - # scopes: (optional) scopes required for this guard. - # Defaults to empty array. - def find_user_by_access_token(access_token) - scopes = scopes_registered_for_endpoint + def handle_return_value!(value, &block) + raise UnauthorizedError unless value - # Expiration, revocation and scopes are verified in `find_user_by_access_token` - access_token = PersonalAccessToken.find_by(token: token) - raise UnauthorizedError unless access_token + block_given? ? yield(value) : value + end - access_token + def private_token + params[PRIVATE_TOKEN_PARAM].presence || env[PRIVATE_TOKEN_HEADER].presence end # An array of scopes that were registered (using `allow_access_with_scope`) -- cgit v1.2.1 From 374179a97042da3a4d5312afcdb0dc90a44634f0 Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Wed, 8 Nov 2017 10:13:22 +0100 Subject: Removing private token --- lib/api/api_guard.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'lib/api') diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 9ada2d5ebb1..9c68830ae34 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -45,6 +45,7 @@ module API include Gitlab::Utils::StrongMemoize def find_current_user! + set_raise_unauthorized_error user = find_user_from_access_token || find_user_from_warden return unless user @@ -74,12 +75,6 @@ module API private - def handle_return_value!(value, &block) - raise UnauthorizedError unless value - - block_given? ? yield(value) : value - end - def private_token params[PRIVATE_TOKEN_PARAM].presence || env[PRIVATE_TOKEN_HEADER].presence end -- cgit v1.2.1 From aecc3eb0809c4436a57f5ecdd88def58e704205d Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Wed, 8 Nov 2017 19:41:07 +0100 Subject: Applied some code review comments --- lib/api/api_guard.rb | 5 ----- 1 file changed, 5 deletions(-) (limited to 'lib/api') diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 9c68830ae34..01e15ffee84 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -45,7 +45,6 @@ module API include Gitlab::Utils::StrongMemoize def find_current_user! - set_raise_unauthorized_error user = find_user_from_access_token || find_user_from_warden return unless user @@ -75,10 +74,6 @@ module API private - def private_token - params[PRIVATE_TOKEN_PARAM].presence || env[PRIVATE_TOKEN_HEADER].presence - end - # An array of scopes that were registered (using `allow_access_with_scope`) # for the current endpoint class. It also returns scopes registered on # `API::API`, since these are meant to apply to all API routes. -- cgit v1.2.1 From 21153a4f47871733f3c0d333a10ffa69ada9a5a9 Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Thu, 9 Nov 2017 19:04:19 +0100 Subject: Homogenising the type of the request handled by UserAuthFinder. Also tests fixed --- lib/api/api_guard.rb | 3 --- 1 file changed, 3 deletions(-) (limited to 'lib/api') diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 01e15ffee84..e2a1a51b300 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -6,9 +6,6 @@ module API module APIGuard extend ActiveSupport::Concern - PRIVATE_TOKEN_HEADER = "HTTP_PRIVATE_TOKEN".freeze - PRIVATE_TOKEN_PARAM = :private_token - included do |base| # OAuth2 Resource Server Authentication use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request| -- cgit v1.2.1 From f1896575237cb92dce5a413bb6b6cc6474cbb19d Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Fri, 10 Nov 2017 11:41:33 +0100 Subject: Added some more comments --- lib/api/api_guard.rb | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'lib/api') diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index e2a1a51b300..0caf2aa25bc 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -139,13 +139,14 @@ module API # Exceptions # - MissingTokenError = Class.new(StandardError) - TokenNotFoundError = Class.new(StandardError) - ExpiredError = Class.new(StandardError) - RevokedError = Class.new(StandardError) - UnauthorizedError = Class.new(StandardError) - - class InsufficientScopeError < StandardError + AuthenticationException = Class.new(StandardError) + MissingTokenError = Class.new(AuthenticationException) + TokenNotFoundError = Class.new(AuthenticationException) + ExpiredError = Class.new(AuthenticationException) + RevokedError = Class.new(AuthenticationException) + UnauthorizedError = Class.new(AuthenticationException) + + class InsufficientScopeError < AuthenticationException attr_reader :scopes def initialize(scopes) @scopes = scopes.map { |s| s.try(:name) || s } -- cgit v1.2.1 From aa84ef1e1af0bac40279e02e4ce889cb660ed9d0 Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Thu, 16 Nov 2017 15:39:30 +0100 Subject: Moving exceptions to UserAuthFinders --- lib/api/api_guard.rb | 35 ++++++++++------------------------- lib/api/helpers.rb | 2 +- 2 files changed, 11 insertions(+), 26 deletions(-) (limited to 'lib/api') diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 0caf2aa25bc..a07015406b1 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -93,8 +93,11 @@ module API private def install_error_responders(base) - error_classes = [MissingTokenError, TokenNotFoundError, - ExpiredError, RevokedError, InsufficientScopeError] + error_classes = [Gitlab::Auth::UserAuthFinders::MissingTokenError, + Gitlab::Auth::UserAuthFinders::TokenNotFoundError, + Gitlab::Auth::UserAuthFinders::ExpiredError, + Gitlab::Auth::UserAuthFinders::RevokedError, + Gitlab::Auth::UserAuthFinders::InsufficientScopeError] base.__send__(:rescue_from, *error_classes, oauth2_bearer_token_error_handler) # rubocop:disable GitlabSecurity/PublicSend end @@ -103,25 +106,25 @@ module API proc do |e| response = case e - when MissingTokenError + when Gitlab::Auth::UserAuthFinders::MissingTokenError Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new - when TokenNotFoundError + when Gitlab::Auth::UserAuthFinders::TokenNotFoundError Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( :invalid_token, "Bad Access Token.") - when ExpiredError + when Gitlab::Auth::UserAuthFinders::ExpiredError Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( :invalid_token, "Token is expired. You can either do re-authorization or token refresh.") - when RevokedError + when Gitlab::Auth::UserAuthFinders::RevokedError Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( :invalid_token, "Token was revoked. You have to re-authorize from the user.") - when InsufficientScopeError + when Gitlab::Auth::UserAuthFinders::InsufficientScopeError # FIXME: ForbiddenError (inherited from Bearer::Forbidden of Rack::Oauth2) # does not include WWW-Authenticate header, which breaks the standard. Rack::OAuth2::Server::Resource::Bearer::Forbidden.new( @@ -134,23 +137,5 @@ module API end end end - - # - # Exceptions - # - - AuthenticationException = Class.new(StandardError) - MissingTokenError = Class.new(AuthenticationException) - TokenNotFoundError = Class.new(AuthenticationException) - ExpiredError = Class.new(AuthenticationException) - RevokedError = Class.new(AuthenticationException) - UnauthorizedError = Class.new(AuthenticationException) - - class InsufficientScopeError < AuthenticationException - attr_reader :scopes - def initialize(scopes) - @scopes = scopes.map { |s| s.try(:name) || s } - end - end end end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 3c8960cb1ab..09e9753b010 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -398,7 +398,7 @@ module API begin @initial_current_user = Gitlab::Auth::UniqueIpsLimiter.limit_user! { find_current_user! } - rescue APIGuard::UnauthorizedError + rescue Gitlab::Auth::UserAuthFinders::UnauthorizedError unauthorized! end end -- cgit v1.2.1 From 1436598e49792b78f5f753477a9d8c097d666b99 Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Thu, 16 Nov 2017 17:03:19 +0100 Subject: Moved Exceptions to Gitlab::Auth --- lib/api/api_guard.rb | 20 ++++++++++---------- lib/api/helpers.rb | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) (limited to 'lib/api') diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index a07015406b1..1953a613f1d 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -93,11 +93,11 @@ module API private def install_error_responders(base) - error_classes = [Gitlab::Auth::UserAuthFinders::MissingTokenError, - Gitlab::Auth::UserAuthFinders::TokenNotFoundError, - Gitlab::Auth::UserAuthFinders::ExpiredError, - Gitlab::Auth::UserAuthFinders::RevokedError, - Gitlab::Auth::UserAuthFinders::InsufficientScopeError] + error_classes = [Gitlab::Auth::MissingTokenError, + Gitlab::Auth::TokenNotFoundError, + Gitlab::Auth::ExpiredError, + Gitlab::Auth::RevokedError, + Gitlab::Auth::InsufficientScopeError] base.__send__(:rescue_from, *error_classes, oauth2_bearer_token_error_handler) # rubocop:disable GitlabSecurity/PublicSend end @@ -106,25 +106,25 @@ module API proc do |e| response = case e - when Gitlab::Auth::UserAuthFinders::MissingTokenError + when Gitlab::Auth::MissingTokenError Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new - when Gitlab::Auth::UserAuthFinders::TokenNotFoundError + when Gitlab::Auth::TokenNotFoundError Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( :invalid_token, "Bad Access Token.") - when Gitlab::Auth::UserAuthFinders::ExpiredError + when Gitlab::Auth::ExpiredError Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( :invalid_token, "Token is expired. You can either do re-authorization or token refresh.") - when Gitlab::Auth::UserAuthFinders::RevokedError + when Gitlab::Auth::RevokedError Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( :invalid_token, "Token was revoked. You have to re-authorize from the user.") - when Gitlab::Auth::UserAuthFinders::InsufficientScopeError + when Gitlab::Auth::InsufficientScopeError # FIXME: ForbiddenError (inherited from Bearer::Forbidden of Rack::Oauth2) # does not include WWW-Authenticate header, which breaks the standard. Rack::OAuth2::Server::Resource::Bearer::Forbidden.new( diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 09e9753b010..b26c61ab8da 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -398,7 +398,7 @@ module API begin @initial_current_user = Gitlab::Auth::UniqueIpsLimiter.limit_user! { find_current_user! } - rescue Gitlab::Auth::UserAuthFinders::UnauthorizedError + rescue Gitlab::Auth::UnauthorizedError unauthorized! end end -- cgit v1.2.1 From 7f0317917a6684189b1637ea73f90d258e8a72b6 Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Fri, 17 Nov 2017 10:09:56 +0100 Subject: Changes after rebase --- lib/api/api_guard.rb | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) (limited to 'lib/api') diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 1953a613f1d..9aeebc34525 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -39,7 +39,7 @@ module API # Helper Methods for Grape Endpoint module HelperMethods - include Gitlab::Utils::StrongMemoize + include Gitlab::Auth::UserAuthFinders def find_current_user! user = find_user_from_access_token || find_user_from_warden @@ -50,25 +50,6 @@ module API user end - def access_token - strong_memoize(:access_token) do - find_oauth_access_token || find_personal_access_token - end - end - - def validate_access_token!(scopes: []) - return unless access_token - - case AccessTokenValidationService.new(access_token, request: request).validate(scopes: scopes) - when AccessTokenValidationService::INSUFFICIENT_SCOPE - raise InsufficientScopeError.new(scopes) - when AccessTokenValidationService::EXPIRED - raise ExpiredError - when AccessTokenValidationService::REVOKED - raise RevokedError - end - end - private # An array of scopes that were registered (using `allow_access_with_scope`) -- cgit v1.2.1 From c900c21eef9235306d7d0da42b07aa2de346e263 Mon Sep 17 00:00:00 2001 From: "micael.bergeron" Date: Tue, 21 Nov 2017 08:31:23 -0500 Subject: add `#with_metadata` scope to remove a N+1 from the notes' API --- lib/api/notes.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/api') diff --git a/lib/api/notes.rb b/lib/api/notes.rb index 0b9ab4eeb05..ceaaeca4046 100644 --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -33,7 +33,7 @@ module API # paginate() only works with a relation. This could lead to a # mismatch between the pagination headers info and the actual notes # array returned, but this is really a edge-case. - paginate(noteable.notes) + paginate(noteable.notes.with_metadata) .reject { |n| n.cross_reference_not_visible_for?(current_user) } present notes, with: Entities::Note else @@ -50,7 +50,7 @@ module API end get ":id/#{noteables_str}/:noteable_id/notes/:note_id" do noteable = find_project_noteable(noteables_str, params[:noteable_id]) - note = noteable.notes.find(params[:note_id]) + note = noteable.notes.with_metadata.find(params[:note_id]) can_read_note = can?(current_user, noteable_read_ability_name(noteable), noteable) && !note.cross_reference_not_visible_for?(current_user) if can_read_note -- cgit v1.2.1 From 0b9e1e16626eff4cd8ae43ce47ec0f965beaf843 Mon Sep 17 00:00:00 2001 From: Daniel Juarez Date: Tue, 21 Nov 2017 15:47:58 +0000 Subject: Skip confirmation user api --- lib/api/users.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib/api') diff --git a/lib/api/users.rb b/lib/api/users.rb index d80b364bd09..0cd89b1bcf8 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -31,7 +31,6 @@ module API optional :location, type: String, desc: 'The location of the user' optional :admin, type: Boolean, desc: 'Flag indicating the user is an administrator' optional :can_create_group, type: Boolean, desc: 'Flag indicating the user can create groups' - optional :skip_confirmation, type: Boolean, default: false, desc: 'Flag indicating the account is confirmed' optional :external, type: Boolean, desc: 'Flag indicating the user is an external user' optional :avatar, type: File, desc: 'Avatar image for user' all_or_none_of :extern_uid, :provider @@ -101,6 +100,7 @@ module API requires :email, type: String, desc: 'The email of the user' optional :password, type: String, desc: 'The password of the new user' optional :reset_password, type: Boolean, desc: 'Flag indicating the user will be sent a password reset token' + optional :skip_confirmation, type: Boolean, desc: 'Flag indicating the account is confirmed' at_least_one_of :password, :reset_password requires :name, type: String, desc: 'The name of the user' requires :username, type: String, desc: 'The username of the user' @@ -134,6 +134,7 @@ module API requires :id, type: Integer, desc: 'The ID of the user' optional :email, type: String, desc: 'The email of the user' optional :password, type: String, desc: 'The password of the new user' + optional :skip_reconfirmation, type: Boolean, desc: 'Flag indicating the account skips the confirmation by email' optional :name, type: String, desc: 'The name of the user' optional :username, type: String, desc: 'The username of the user' use :optional_attributes -- cgit v1.2.1 From 7df1cb528e5d977f5f18b1c82433b3c76220d6db Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Sat, 18 Nov 2017 02:13:45 +0800 Subject: Move identical merged branch check to merged_branch_names --- lib/api/entities.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'lib/api') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 16ae99b5c6c..acfd4a1ef4d 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -242,7 +242,11 @@ module API end expose :merged do |repo_branch, options| - options[:project].repository.merged_to_root_ref?(repo_branch, options[:merged_branch_names]) + if options[:merged_branch_names] + options[:merged_branch_names].include?(repo_branch.name) + else + options[:project].repository.merged_to_root_ref?(repo_branch) + end end expose :protected do |repo_branch, options| -- cgit v1.2.1 From 991bf24ec8890eca248a00deb4f33f309c9ffb83 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Wed, 15 Nov 2017 17:22:18 +0000 Subject: Use latest_merge_request_diff association Compared to the merge_request_diff association: 1. It's simpler to query. The query uses a foreign key to the merge_request_diffs table, so no ordering is necessary. 2. It's faster for preloading. The merge_request_diff association has to load every diff for the MRs in the set, then discard all but the most recent for each. This association means that Rails can just query for N diffs from N MRs. 3. It's more complicated to update. This is a bidirectional foreign key, so we need to update two tables when adding a diff record. This also means we need to handle this as a special case when importing a GitLab project. There is some juggling with this association in the merge request model: * `MergeRequest#latest_merge_request_diff` is _always_ the latest diff. * `MergeRequest#merge_request_diff` reuses `MergeRequest#latest_merge_request_diff` unless: * Arguments are passed. These are typically to force-reload the association. * It doesn't exist. That means we might be trying to implicitly create a diff. This only seems to happen in specs. * The association is already loaded. This is important for the reasons explained in the comment, which I'll reiterate here: if we a) load a non-latest diff, then b) get its `merge_request`, then c) get that MR's `merge_request_diff`, we should get the diff we loaded in c), even though that's not the latest diff. Basically, `MergeRequest#merge_request_diff` is the latest diff in most cases, but not quite all. --- lib/api/merge_requests.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/api') diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 726f09e3669..5b4642a2f57 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -21,7 +21,7 @@ module API return merge_requests if args[:view] == 'simple' merge_requests - .preload(:notes, :author, :assignee, :milestone, :merge_request_diff, :labels, :timelogs) + .preload(:notes, :author, :assignee, :milestone, :latest_merge_request_diff, :labels, :timelogs) end params :merge_requests_params do -- cgit v1.2.1 From fceffe4dca9af5ab3ffaa2e110c5fb4bad254950 Mon Sep 17 00:00:00 2001 From: James Edwards-Jones Date: Wed, 22 Nov 2017 15:16:08 +0000 Subject: Renamed ProtectedBranches::ApiUpdateService to LegacyApiUpdateService --- lib/api/branches.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/api') diff --git a/lib/api/branches.rb b/lib/api/branches.rb index cdef1b546a9..0791a110c39 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -81,9 +81,9 @@ module API service_args = [user_project, current_user, protected_branch_params] protected_branch = if protected_branch - ::ProtectedBranches::ApiUpdateService.new(*service_args).execute(protected_branch) + ::ProtectedBranches::LegacyApiUpdateService.new(*service_args).execute(protected_branch) else - ::ProtectedBranches::ApiCreateService.new(*service_args).execute + ::ProtectedBranches::LegacyApiCreateService.new(*service_args).execute end if protected_branch.valid? -- cgit v1.2.1 From 257fd5713485a05460a9170190100643199a7e48 Mon Sep 17 00:00:00 2001 From: Markus Koller Date: Thu, 23 Nov 2017 13:16:14 +0000 Subject: Allow password authentication to be disabled entirely --- lib/api/entities.rb | 5 ++++- lib/api/settings.rb | 13 +++++++++---- lib/api/v3/entities.rb | 4 ++-- lib/api/v3/settings.rb | 8 +++++--- 4 files changed, 20 insertions(+), 10 deletions(-) (limited to 'lib/api') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 16ae99b5c6c..e45c87e4f4f 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -763,7 +763,10 @@ module API expose(:default_project_visibility) { |setting, _options| Gitlab::VisibilityLevel.string_level(setting.default_project_visibility) } expose(:default_snippet_visibility) { |setting, _options| Gitlab::VisibilityLevel.string_level(setting.default_snippet_visibility) } expose(:default_group_visibility) { |setting, _options| Gitlab::VisibilityLevel.string_level(setting.default_group_visibility) } - expose :password_authentication_enabled, as: :signin_enabled + + # support legacy names, can be removed in v5 + expose :password_authentication_enabled_for_web, as: :password_authentication_enabled + expose :password_authentication_enabled_for_web, as: :signin_enabled end class Release < Grape::Entity diff --git a/lib/api/settings.rb b/lib/api/settings.rb index 851b226e9e5..06373fe5069 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -44,9 +44,11 @@ module API requires :domain_blacklist, type: String, desc: 'Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' end optional :after_sign_up_text, type: String, desc: 'Text shown after sign up' - optional :password_authentication_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled' - optional :signin_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled' - mutually_exclusive :password_authentication_enabled, :signin_enabled + optional :password_authentication_enabled_for_web, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' + optional :password_authentication_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' # support legacy names, can be removed in v5 + optional :signin_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' # support legacy names, can be removed in v5 + mutually_exclusive :password_authentication_enabled_for_web, :password_authentication_enabled, :signin_enabled + optional :password_authentication_enabled_for_git, type: Boolean, desc: 'Flag indicating if password authentication is enabled for Git over HTTP(S)' optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to setup Two-factor authentication' given require_two_factor_authentication: ->(val) { val } do requires :two_factor_grace_period, type: Integer, desc: 'Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication' @@ -135,8 +137,11 @@ module API put "application/settings" do attrs = declared_params(include_missing: false) + # support legacy names, can be removed in v5 if attrs.has_key?(:signin_enabled) - attrs[:password_authentication_enabled] = attrs.delete(:signin_enabled) + attrs[:password_authentication_enabled_for_web] = attrs.delete(:signin_enabled) + elsif attrs.has_key?(:password_authentication_enabled) + attrs[:password_authentication_enabled_for_web] = attrs.delete(:password_authentication_enabled) end if current_settings.update_attributes(attrs) diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb index afdd7b83998..c17b6f45ed8 100644 --- a/lib/api/v3/entities.rb +++ b/lib/api/v3/entities.rb @@ -172,8 +172,8 @@ module API expose :id expose :default_projects_limit expose :signup_enabled - expose :password_authentication_enabled - expose :password_authentication_enabled, as: :signin_enabled + expose :password_authentication_enabled_for_web, as: :password_authentication_enabled + expose :password_authentication_enabled_for_web, as: :signin_enabled expose :gravatar_enabled expose :sign_in_text expose :after_sign_up_text diff --git a/lib/api/v3/settings.rb b/lib/api/v3/settings.rb index 202011cfcbe..9b4ab7630fb 100644 --- a/lib/api/v3/settings.rb +++ b/lib/api/v3/settings.rb @@ -44,8 +44,8 @@ module API requires :domain_blacklist, type: String, desc: 'Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' end optional :after_sign_up_text, type: String, desc: 'Text shown after sign up' - optional :password_authentication_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled' - optional :signin_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled' + optional :password_authentication_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' + optional :signin_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' mutually_exclusive :password_authentication_enabled, :signin_enabled optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to setup Two-factor authentication' given require_two_factor_authentication: ->(val) { val } do @@ -131,7 +131,9 @@ module API attrs = declared_params(include_missing: false) if attrs.has_key?(:signin_enabled) - attrs[:password_authentication_enabled] = attrs.delete(:signin_enabled) + attrs[:password_authentication_enabled_for_web] = attrs.delete(:signin_enabled) + elsif attrs.has_key?(:password_authentication_enabled) + attrs[:password_authentication_enabled_for_web] = attrs.delete(:password_authentication_enabled) end if current_settings.update_attributes(attrs) -- cgit v1.2.1 From 392cc887097bfd0c28b547700b9a67c32cf14e69 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Fri, 17 Nov 2017 02:43:55 +0100 Subject: Add new API endpoint - get a namespace by ID --- lib/api/namespaces.rb | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'lib/api') diff --git a/lib/api/namespaces.rb b/lib/api/namespaces.rb index f1eaff6b0eb..21dc5009d0e 100644 --- a/lib/api/namespaces.rb +++ b/lib/api/namespaces.rb @@ -19,6 +19,30 @@ module API present paginate(namespaces), with: Entities::Namespace, current_user: current_user end + + desc 'Get a namespace by ID' do + success Entities::Namespace + end + params do + requires :id, type: Integer, desc: "Namespace's ID" + end + get ':id' do + namespace = Namespace.find(params[:id]) + authenticate_get_namespace!(namespace) + + present namespace, with: Entities::Namespace, current_user: current_user + end + end + + helpers do + def authenticate_get_namespace!(namespace) + return if current_user.admin? + forbidden!('No access granted') unless user_can_access_namespace?(namespace) + end + + def user_can_access_namespace?(namespace) + namespace.has_owner?(current_user) + end end end end -- cgit v1.2.1 From dfbfd3c7d7d4677ac99a7f8147a673911e8d4e98 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 23 Nov 2017 14:32:16 +0100 Subject: Allow request namespace by ID or path --- lib/api/helpers.rb | 22 ++++++++++++++++++++++ lib/api/namespaces.rb | 18 ++---------------- 2 files changed, 24 insertions(+), 16 deletions(-) (limited to 'lib/api') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index b26c61ab8da..52ac416f9ad 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -50,6 +50,10 @@ module API initial_current_user != current_user end + def user_namespace + @user_namespace ||= find_namespace!(params[:id]) + end + def user_group @group ||= find_group!(params[:id]) end @@ -112,6 +116,24 @@ module API end end + def find_namespace(id) + if id.to_s =~ /^\d+$/ + Namespace.find_by(id: id) + else + Namespace.find_by_full_path(id) + end + end + + def find_namespace!(id) + namespace = find_namespace(id) + + if can?(current_user, :admin_namespace, namespace) + namespace + else + not_found!('Namespace') + end + end + def find_project_label(id) label = available_labels.find_by_id(id) || available_labels.find_by_title(id) label || not_found!('Label') diff --git a/lib/api/namespaces.rb b/lib/api/namespaces.rb index 21dc5009d0e..32b77aedba8 100644 --- a/lib/api/namespaces.rb +++ b/lib/api/namespaces.rb @@ -24,24 +24,10 @@ module API success Entities::Namespace end params do - requires :id, type: Integer, desc: "Namespace's ID" + requires :id, type: String, desc: "Namespace's ID or path" end get ':id' do - namespace = Namespace.find(params[:id]) - authenticate_get_namespace!(namespace) - - present namespace, with: Entities::Namespace, current_user: current_user - end - end - - helpers do - def authenticate_get_namespace!(namespace) - return if current_user.admin? - forbidden!('No access granted') unless user_can_access_namespace?(namespace) - end - - def user_can_access_namespace?(namespace) - namespace.has_owner?(current_user) + present user_namespace, with: Entities::Namespace, current_user: current_user end end end -- cgit v1.2.1 From 97f966c445c0c2191a8017aa981a34737b9adf56 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 23 Nov 2017 15:47:15 +0100 Subject: Introduce :read_namespace access policy for namespace and group --- lib/api/helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/api') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 52ac416f9ad..686bf7a3c2b 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -127,7 +127,7 @@ module API def find_namespace!(id) namespace = find_namespace(id) - if can?(current_user, :admin_namespace, namespace) + if can?(current_user, :read_namespace, namespace) namespace else not_found!('Namespace') -- cgit v1.2.1 From d6dd9d712ac24a095d0b0506731f9415b7c3b5f5 Mon Sep 17 00:00:00 2001 From: James Edwards-Jones Date: Fri, 24 Nov 2017 12:41:36 +0000 Subject: Fix ProtectedBranch access level validations Before an access_level was required in EE even when an it had been set for a user/group. --- lib/api/protected_branches.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/api') diff --git a/lib/api/protected_branches.rb b/lib/api/protected_branches.rb index 15fcb9e8e27..b5021e8a712 100644 --- a/lib/api/protected_branches.rb +++ b/lib/api/protected_branches.rb @@ -40,10 +40,10 @@ module API params do requires :name, type: String, desc: 'The name of the protected branch' optional :push_access_level, type: Integer, default: Gitlab::Access::MASTER, - values: ProtectedBranchAccess::ALLOWED_ACCESS_LEVELS, + values: ProtectedRefAccess::ALLOWED_ACCESS_LEVELS, desc: 'Access levels allowed to push (defaults: `40`, master access level)' optional :merge_access_level, type: Integer, default: Gitlab::Access::MASTER, - values: ProtectedBranchAccess::ALLOWED_ACCESS_LEVELS, + values: ProtectedRefAccess::ALLOWED_ACCESS_LEVELS, desc: 'Access levels allowed to merge (defaults: `40`, master access level)' end post ':id/protected_branches' do -- cgit v1.2.1 From 7fb1bb01bd669cc46514ed17b1b8822a1d962970 Mon Sep 17 00:00:00 2001 From: George Andrinopoulos Date: Sat, 25 Nov 2017 17:04:45 +0200 Subject: Create issue and merge request destroy services --- lib/api/issues.rb | 4 +++- lib/api/merge_requests.rb | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) (limited to 'lib/api') diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 74dfd9f96de..e60e00d7956 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -255,7 +255,9 @@ module API authorize!(:destroy_issue, issue) - destroy_conditionally!(issue) + destroy_conditionally!(issue) do |issue| + Issuable::DestroyService.new(user_project, current_user).execute(issue) + end end desc 'List merge requests closing issue' do diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 5b4642a2f57..d34886fca2e 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -167,7 +167,9 @@ module API authorize!(:destroy_merge_request, merge_request) - destroy_conditionally!(merge_request) + destroy_conditionally!(merge_request) do |merge_request| + Issuable::DestroyService.new(user_project, current_user).execute(merge_request) + end end params do -- cgit v1.2.1 From 623eb68195d51ea50e09970771442d992ff19a4a Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 16 Nov 2017 17:12:33 +0100 Subject: Add new API endpoint - list jobs of a specified runner --- lib/api/runners.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'lib/api') diff --git a/lib/api/runners.rb b/lib/api/runners.rb index e816fcdd928..56b70681852 100644 --- a/lib/api/runners.rb +++ b/lib/api/runners.rb @@ -84,6 +84,18 @@ module API destroy_conditionally!(runner) end + + desc 'List jobs running on a runner' + params do + requires :id, type: Integer, desc: 'The ID of the runner' + use :pagination + end + get ':id/jobs' do + runner = get_runner(params[:id]) + authenticate_list_runners_jobs!(runner) + + present paginate(runner.builds.running), with: Entities::Job + end end params do @@ -192,6 +204,12 @@ module API forbidden!("No access granted") unless user_can_access_runner?(runner) end + def authenticate_list_runners_jobs!(runner) + return if current_user.admin? + + forbidden!("No access granted") unless user_can_access_runner?(runner) + end + def user_can_access_runner?(runner) current_user.ci_authorized_runners.exists?(runner.id) end -- cgit v1.2.1 From 8d3e80692cbeea06dd28a052554f0c262004e18d Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 16 Nov 2017 19:44:14 +0100 Subject: Add information about project --- lib/api/entities.rb | 17 +++++++++++++---- lib/api/runners.rb | 6 ++++-- 2 files changed, 17 insertions(+), 6 deletions(-) (limited to 'lib/api') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 7d5d68c8f14..cea9e9a8028 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -80,16 +80,21 @@ module API expose :group_access, as: :group_access_level end - class BasicProjectDetails < Grape::Entity - expose :id, :description, :default_branch, :tag_list - expose :ssh_url_to_repo, :http_url_to_repo, :web_url + class ProjectIdentity < Grape::Entity + expose :id, :description expose :name, :name_with_namespace expose :path, :path_with_namespace + expose :created_at + end + + class BasicProjectDetails < ProjectIdentity + expose :default_branch, :tag_list + expose :ssh_url_to_repo, :http_url_to_repo, :web_url expose :avatar_url do |project, options| project.avatar_url(only_path: false) end expose :star_count, :forks_count - expose :created_at, :last_activity_at + expose :last_activity_at end class Project < BasicProjectDetails @@ -838,6 +843,10 @@ module API expose :pipeline, with: PipelineBasic end + class JobWithProject < Job + expose :project, with: ProjectIdentity + end + class Trigger < Grape::Entity expose :id expose :token, :description diff --git a/lib/api/runners.rb b/lib/api/runners.rb index 56b70681852..b92a2c36cf3 100644 --- a/lib/api/runners.rb +++ b/lib/api/runners.rb @@ -85,7 +85,9 @@ module API destroy_conditionally!(runner) end - desc 'List jobs running on a runner' + desc 'List jobs running on a runner' do + success Entities::JobWithProject + end params do requires :id, type: Integer, desc: 'The ID of the runner' use :pagination @@ -94,7 +96,7 @@ module API runner = get_runner(params[:id]) authenticate_list_runners_jobs!(runner) - present paginate(runner.builds.running), with: Entities::Job + present paginate(runner.builds.running), with: Entities::JobWithProject end end -- cgit v1.2.1 From b7ed102ea601fb4c6f65c5a982058f8c92883d31 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Tue, 21 Nov 2017 12:37:07 +0100 Subject: Allow filtering by 'status' --- lib/api/runners.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'lib/api') diff --git a/lib/api/runners.rb b/lib/api/runners.rb index b92a2c36cf3..18f9f142580 100644 --- a/lib/api/runners.rb +++ b/lib/api/runners.rb @@ -90,13 +90,20 @@ module API end params do requires :id, type: Integer, desc: 'The ID of the runner' + optional :status, type: String, desc: 'Status of job' use :pagination end get ':id/jobs' do runner = get_runner(params[:id]) authenticate_list_runners_jobs!(runner) - present paginate(runner.builds.running), with: Entities::JobWithProject + jobs = runner.builds + if params[:status] + not_found!('Status') unless Ci::Build::AVAILABLE_STATUSES.include?(params[:status]) + jobs = jobs.where(status: params[:status].to_sym) + end + + present paginate(jobs), with: Entities::JobWithProject end end -- cgit v1.2.1 From 7b643c02c20e7552a35c09adf8d8dca1e8c3a4a3 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Tue, 21 Nov 2017 12:37:18 +0100 Subject: Modify output --- lib/api/entities.rb | 11 +++++++---- lib/api/runners.rb | 4 ++-- 2 files changed, 9 insertions(+), 6 deletions(-) (limited to 'lib/api') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index cea9e9a8028..ce332fe85d2 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -832,18 +832,21 @@ module API expose :id, :sha, :ref, :status end - class Job < Grape::Entity + class JobBasic < Grape::Entity expose :id, :status, :stage, :name, :ref, :tag, :coverage expose :created_at, :started_at, :finished_at expose :duration expose :user, with: User - expose :artifacts_file, using: JobArtifactFile, if: -> (job, opts) { job.artifacts? } expose :commit, with: Commit - expose :runner, with: Runner expose :pipeline, with: PipelineBasic end - class JobWithProject < Job + class Job < JobBasic + expose :artifacts_file, using: JobArtifactFile, if: -> (job, opts) { job.artifacts? } + expose :runner, with: Runner + end + + class JobBasicWithProject < JobBasic expose :project, with: ProjectIdentity end diff --git a/lib/api/runners.rb b/lib/api/runners.rb index 18f9f142580..3ea7340d9be 100644 --- a/lib/api/runners.rb +++ b/lib/api/runners.rb @@ -86,7 +86,7 @@ module API end desc 'List jobs running on a runner' do - success Entities::JobWithProject + success Entities::JobBasicWithProject end params do requires :id, type: Integer, desc: 'The ID of the runner' @@ -103,7 +103,7 @@ module API jobs = jobs.where(status: params[:status].to_sym) end - present paginate(jobs), with: Entities::JobWithProject + present paginate(jobs), with: Entities::JobBasicWithProject end end -- cgit v1.2.1 From 13a902a9a42c0909ad8c6790c040447a9e12211f Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Mon, 27 Nov 2017 22:59:01 +0100 Subject: Refactorize jobs finding logic --- lib/api/runners.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'lib/api') diff --git a/lib/api/runners.rb b/lib/api/runners.rb index 3ea7340d9be..996457c5dfe 100644 --- a/lib/api/runners.rb +++ b/lib/api/runners.rb @@ -90,18 +90,14 @@ module API end params do requires :id, type: Integer, desc: 'The ID of the runner' - optional :status, type: String, desc: 'Status of job' + optional :status, type: String, desc: 'Status of the job', values: Ci::Build::AVAILABLE_STATUSES use :pagination end get ':id/jobs' do runner = get_runner(params[:id]) authenticate_list_runners_jobs!(runner) - jobs = runner.builds - if params[:status] - not_found!('Status') unless Ci::Build::AVAILABLE_STATUSES.include?(params[:status]) - jobs = jobs.where(status: params[:status].to_sym) - end + jobs = RunnerJobsFinder.new(runner, params).execute present paginate(jobs), with: Entities::JobBasicWithProject end -- cgit v1.2.1 From a402056472aa9d8472b9ef92e70128e472b2b030 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Tue, 28 Nov 2017 11:17:57 +0100 Subject: SSHUploadPack over Gitaly is now OptOut - Better gitaly-handling in /api/internal/allowed specs --- lib/api/helpers/internal_helpers.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'lib/api') diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb index 4b3c473b0bb..d6dea4c30e3 100644 --- a/lib/api/helpers/internal_helpers.rb +++ b/lib/api/helpers/internal_helpers.rb @@ -2,8 +2,8 @@ module API module Helpers module InternalHelpers SSH_GITALY_FEATURES = { - 'git-receive-pack' => :ssh_receive_pack, - 'git-upload-pack' => :ssh_upload_pack + 'git-receive-pack' => [:ssh_receive_pack, Gitlab::GitalyClient::MigrationStatus::OPT_IN], + 'git-upload-pack' => [:ssh_upload_pack, Gitlab::GitalyClient::MigrationStatus::OPT_OUT] }.freeze def wiki? @@ -102,8 +102,8 @@ module API # Return the Gitaly Address if it is enabled def gitaly_payload(action) - feature = SSH_GITALY_FEATURES[action] - return unless feature && Gitlab::GitalyClient.feature_enabled?(feature) + feature, status = SSH_GITALY_FEATURES[action] + return unless feature && Gitlab::GitalyClient.feature_enabled?(feature, status: status) { repository: repository.gitaly_repository, -- cgit v1.2.1 From 64e5f996fa01d2e9c8462cfbb647997531eaf6c7 Mon Sep 17 00:00:00 2001 From: Andrew Newdigate Date: Wed, 29 Nov 2017 09:12:12 +0000 Subject: Add timeouts for Gitaly calls --- lib/api/settings.rb | 3 +++ 1 file changed, 3 insertions(+) (limited to 'lib/api') diff --git a/lib/api/settings.rb b/lib/api/settings.rb index 06373fe5069..cee4d309816 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -123,6 +123,9 @@ module API end optional :terminal_max_session_time, type: Integer, desc: 'Maximum time for web terminal websocket connection (in seconds). Set to 0 for unlimited time.' optional :polling_interval_multiplier, type: BigDecimal, desc: 'Interval multiplier used by endpoints that perform polling. Set to 0 to disable polling.' + optional :gitaly_timeout_default, type: Integer, desc: 'Default Gitaly timeout, in seconds. Set to 0 to disable timeouts.' + optional :gitaly_timeout_medium, type: Integer, desc: 'Medium Gitaly timeout, in seconds. Set to 0 to disable timeouts.' + optional :gitaly_timeout_fast, type: Integer, desc: 'Gitaly fast operation timeout, in seconds. Set to 0 to disable timeouts.' ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type| optional :"#{type}_key_restriction", -- cgit v1.2.1 From 57d9121127eb9745ea196bbd8596ffa03afdee68 Mon Sep 17 00:00:00 2001 From: haseeb Date: Wed, 29 Nov 2017 16:22:22 +0000 Subject: support ordering of project notes in notes api --- lib/api/notes.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'lib/api') diff --git a/lib/api/notes.rb b/lib/api/notes.rb index ceaaeca4046..3588dc85c9e 100644 --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -18,6 +18,10 @@ module API end params do requires :noteable_id, type: Integer, desc: 'The ID of the noteable' + optional :order_by, type: String, values: %w[created_at updated_at], default: 'created_at', + desc: 'Return notes ordered by `created_at` or `updated_at` fields.' + optional :sort, type: String, values: %w[asc desc], default: 'desc', + desc: 'Return notes sorted in `asc` or `desc` order.' use :pagination end get ":id/#{noteables_str}/:noteable_id/notes" do @@ -29,11 +33,12 @@ module API # at the DB query level (which we cannot in that case), the current # page can have less elements than :per_page even if # there's more than one page. + raw_notes = noteable.notes.with_metadata.reorder(params[:order_by] => params[:sort]) notes = # paginate() only works with a relation. This could lead to a # mismatch between the pagination headers info and the actual notes # array returned, but this is really a edge-case. - paginate(noteable.notes.with_metadata) + paginate(raw_notes) .reject { |n| n.cross_reference_not_visible_for?(current_user) } present notes, with: Entities::Note else -- cgit v1.2.1 From 88e3ce30ae97ac8eb4b25381cfbe7772819cce0c Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 19 Nov 2017 06:35:25 -0800 Subject: Optimize API /groups/:id/projects by preloading associations Closes #40308 --- lib/api/groups.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'lib/api') diff --git a/lib/api/groups.rb b/lib/api/groups.rb index bcf2e6dae1d..7e9a5502949 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -172,6 +172,7 @@ module API get ":id/projects" do group = find_group!(params[:id]) projects = GroupProjectsFinder.new(group: group, current_user: current_user, params: project_finder_params).execute + projects = projects.preload(:fork_network, :forked_project_link, :project_feature, :project_group_links, :tags, :taggings, :group, :namespace) projects = reorder_projects(projects) entity = params[:simple] ? Entities::BasicProjectDetails : Entities::Project present paginate(projects), with: entity, current_user: current_user -- cgit v1.2.1 From 39d293abd2ec135c53448552d482d17f9726701c Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 19 Nov 2017 15:25:48 -0800 Subject: Omit the `all` call after Project#project_group_links to avoid unnecessary loads --- lib/api/entities.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/api') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index ce332fe85d2..ca7708e366e 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -156,7 +156,7 @@ module API expose :public_builds, as: :public_jobs expose :ci_config_path expose :shared_with_groups do |project, options| - SharedGroup.represent(project.project_group_links.all, options) + SharedGroup.represent(project.project_group_links, options) end expose :only_allow_merge_if_pipeline_succeeds expose :request_access_enabled -- cgit v1.2.1 From 02cd1702b27ddb15f15f7efeb5ce60412492e41d Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 19 Nov 2017 16:02:46 -0800 Subject: Only serialize namespace essentials in group's projects API response --- lib/api/entities.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'lib/api') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index ca7708e366e..5b9b0afed0f 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -146,7 +146,7 @@ module API expose :shared_runners_enabled expose :lfs_enabled?, as: :lfs_enabled expose :creator_id - expose :namespace, using: 'API::Entities::Namespace' + expose :namespace, using: 'API::Entities::NamespaceBasic' expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? } expose :import_status expose :import_error, if: lambda { |_project, options| options[:user_can_admin_project] } @@ -618,9 +618,11 @@ module API expose :created_at end - class Namespace < Grape::Entity + class NamespaceBasic < Grape::Entity expose :id, :name, :path, :kind, :full_path, :parent_id + end + class Namespace < NamespaceBasic expose :members_count_with_descendants, if: -> (namespace, opts) { expose_members_count_with_descendants?(namespace, opts) } do |namespace, _| namespace.users_with_descendants.count end -- cgit v1.2.1 From 0dfb8801012de19be006026a01d7db0f04a2d3b3 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 19 Nov 2017 16:46:40 -0800 Subject: Preload project route to avoid N+1 query --- lib/api/groups.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/api') diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 7e9a5502949..df2d97bceda 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -172,7 +172,7 @@ module API get ":id/projects" do group = find_group!(params[:id]) projects = GroupProjectsFinder.new(group: group, current_user: current_user, params: project_finder_params).execute - projects = projects.preload(:fork_network, :forked_project_link, :project_feature, :project_group_links, :tags, :taggings, :group, :namespace) + projects = projects.preload(:fork_network, :forked_project_link, :project_feature, :project_group_links, :tags, :taggings, :group, :namespace, :route) projects = reorder_projects(projects) entity = params[:simple] ? Entities::BasicProjectDetails : Entities::Project present paginate(projects), with: entity, current_user: current_user -- cgit v1.2.1 From a2babf32fe2b57850bd678919947d53e5127f6fe Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Fri, 24 Nov 2017 13:20:52 +0100 Subject: More preloading improvement and added batch count services --- lib/api/groups.rb | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) (limited to 'lib/api') diff --git a/lib/api/groups.rb b/lib/api/groups.rb index df2d97bceda..746dcc9fc91 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -52,6 +52,24 @@ module API groups end + def find_group_projects(params) + group = find_group!(params[:id]) + projects = GroupProjectsFinder.new(group: group, current_user: current_user, params: project_finder_params).execute + projects = projects.preload(:project_feature, :route, :group) + .preload(namespace: [:route, :owner], + tags: :taggings, + project_group_links: :group, + fork_network: :root_project, + forked_project_link: :forked_from_project, + forked_from_project: [:route, :forks, namespace: :route, tags: :taggings]) + projects = reorder_projects(projects) + paginated_projects = paginate(projects) + projects_with_fork = paginated_projects + paginated_projects.map(&:forked_from_project).compact + ::Projects::BatchForksCountService.new(projects_with_fork).refresh_cache + ::Projects::BatchOpenIssuesCountService.new(paginated_projects).refresh_cache + paginated_projects + end + def present_groups(params, groups) options = { with: Entities::Group, @@ -170,12 +188,9 @@ module API use :pagination end get ":id/projects" do - group = find_group!(params[:id]) - projects = GroupProjectsFinder.new(group: group, current_user: current_user, params: project_finder_params).execute - projects = projects.preload(:fork_network, :forked_project_link, :project_feature, :project_group_links, :tags, :taggings, :group, :namespace, :route) - projects = reorder_projects(projects) + projects = find_group_projects(params) entity = params[:simple] ? Entities::BasicProjectDetails : Entities::Project - present paginate(projects), with: entity, current_user: current_user + present projects, with: entity, current_user: current_user end desc 'Get a list of subgroups in this group.' do -- cgit v1.2.1 From 7c7877b54dfb0a07bf128102226e338463654431 Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Fri, 24 Nov 2017 13:32:01 +0100 Subject: Small renaming --- lib/api/groups.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'lib/api') diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 746dcc9fc91..05443329a32 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -63,11 +63,11 @@ module API forked_project_link: :forked_from_project, forked_from_project: [:route, :forks, namespace: :route, tags: :taggings]) projects = reorder_projects(projects) - paginated_projects = paginate(projects) - projects_with_fork = paginated_projects + paginated_projects.map(&:forked_from_project).compact + projects = paginate(projects) + projects_with_fork = projects + projects.map(&:forked_from_project).compact ::Projects::BatchForksCountService.new(projects_with_fork).refresh_cache - ::Projects::BatchOpenIssuesCountService.new(paginated_projects).refresh_cache - paginated_projects + ::Projects::BatchOpenIssuesCountService.new(projects).refresh_cache + projects end def present_groups(params, groups) -- cgit v1.2.1 From c85f9c5b1d320f7a6f75e3d08bbafd2fb20d3f58 Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Fri, 24 Nov 2017 17:37:02 +0100 Subject: Code review comments applied --- lib/api/groups.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/api') diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 05443329a32..83c7898150a 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -64,8 +64,8 @@ module API forked_from_project: [:route, :forks, namespace: :route, tags: :taggings]) projects = reorder_projects(projects) projects = paginate(projects) - projects_with_fork = projects + projects.map(&:forked_from_project).compact - ::Projects::BatchForksCountService.new(projects_with_fork).refresh_cache + projects_with_forks = projects + projects.map(&:forked_from_project).compact + ::Projects::BatchForksCountService.new(projects_with_forks).refresh_cache ::Projects::BatchOpenIssuesCountService.new(projects).refresh_cache projects end -- cgit v1.2.1 From 58c5b463ff75618a557d067c16f49ef581cda85c Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Mon, 27 Nov 2017 12:24:33 +0100 Subject: Refactored /projects and /user/:user_id/projects endpoints --- lib/api/entities.rb | 48 ++++++++++++++++++++++++++++-------- lib/api/groups.rb | 16 +++--------- lib/api/projects.rb | 4 +-- lib/api/projects_relation_builder.rb | 34 +++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 25 deletions(-) create mode 100644 lib/api/projects_relation_builder.rb (limited to 'lib/api') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 5b9b0afed0f..8f41b930c0a 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -94,7 +94,13 @@ module API project.avatar_url(only_path: false) end expose :star_count, :forks_count - expose :last_activity_at + expose :created_at, :last_activity_at + + def self.preload_relation(projects_relation) + projects_relation.preload(:project_feature, :route) + .preload(namespace: [:route, :owner], + tags: :taggings) + end end class Project < BasicProjectDetails @@ -128,6 +134,18 @@ module API expose :members do |project| expose_url(api_v4_projects_members_path(id: project.id)) end + + def self.preload_relation(projects_relation) + super(projects_relation).preload(:group) + .preload(project_group_links: :group, + fork_network: :root_project, + forked_project_link: :forked_from_project, + forked_from_project: [:route, :forks, namespace: :route, tags: :taggings]) + end + + def self.forks_counting_projects(projects_relation) + projects_relation + projects_relation.map(&:forked_from_project).compact + end end expose :archived?, as: :archived @@ -635,9 +653,16 @@ module API class MemberAccess < Grape::Entity expose :access_level expose :notification_level do |member, options| - if member.notification_setting - ::NotificationSetting.levels[member.notification_setting.level] - end + notification = member_notification_setting(member) + ::NotificationSetting.levels[notification.level] if notification + end + + private + + def member_notification_setting(member) + member.user.notification_settings.select do |notification| + notification.source_id == member.source_id && notification.source_type == member.source_type + end.last end end @@ -680,18 +705,21 @@ module API expose :permissions do expose :project_access, using: Entities::ProjectAccess do |project, options| if options.key?(:project_members) - (options[:project_members] || []).find { |member| member.source_id == project.id } - else - project.project_members.find_by(user_id: options[:current_user].id) + (options[:project_members] || []).select { |member| member.source_id == project.id }.last + elsif project.project_members.any? + # This is not the bet option to search in a CollectionProxy, but if + # we use find_by we will perform another query, even if the association + # is loaded + project.project_members.select { |project_member| project_member.user_id == options[:current_user].id }.last end end expose :group_access, using: Entities::GroupAccess do |project, options| if project.group if options.key?(:group_members) - (options[:group_members] || []).find { |member| member.source_id == project.namespace_id } - else - project.group.group_members.find_by(user_id: options[:current_user].id) + (options[:group_members] || []).select { |member| member.source_id == project.namespace_id }.last + elsif project.group.group_members.any? + project.group.group_members.select { |group_member| group_member.user_id == options[:current_user].id }.last end end end diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 83c7898150a..b81f07a1770 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -55,19 +55,8 @@ module API def find_group_projects(params) group = find_group!(params[:id]) projects = GroupProjectsFinder.new(group: group, current_user: current_user, params: project_finder_params).execute - projects = projects.preload(:project_feature, :route, :group) - .preload(namespace: [:route, :owner], - tags: :taggings, - project_group_links: :group, - fork_network: :root_project, - forked_project_link: :forked_from_project, - forked_from_project: [:route, :forks, namespace: :route, tags: :taggings]) projects = reorder_projects(projects) - projects = paginate(projects) - projects_with_forks = projects + projects.map(&:forked_from_project).compact - ::Projects::BatchForksCountService.new(projects_with_forks).refresh_cache - ::Projects::BatchOpenIssuesCountService.new(projects).refresh_cache - projects + paginate(projects) end def present_groups(params, groups) @@ -190,7 +179,8 @@ module API get ":id/projects" do projects = find_group_projects(params) entity = params[:simple] ? Entities::BasicProjectDetails : Entities::Project - present projects, with: entity, current_user: current_user + + present entity.prepare_relation(projects), with: entity, current_user: current_user end desc 'Get a list of subgroups in this group.' do diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 4cd7e714aa2..12506203429 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -79,9 +79,9 @@ module API projects = projects.with_statistics if params[:statistics] projects = projects.with_issues_enabled if params[:with_issues_enabled] projects = projects.with_merge_requests_enabled if params[:with_merge_requests_enabled] + projects = paginate(projects) if current_user - projects = projects.includes(:route, :taggings, namespace: :route) project_members = current_user.project_members group_members = current_user.group_members end @@ -95,7 +95,7 @@ module API ) options[:with] = Entities::BasicProjectDetails if params[:simple] - present paginate(projects), options + present options[:with].prepare_relation(projects), options end end diff --git a/lib/api/projects_relation_builder.rb b/lib/api/projects_relation_builder.rb new file mode 100644 index 00000000000..57df6157225 --- /dev/null +++ b/lib/api/projects_relation_builder.rb @@ -0,0 +1,34 @@ +module API + module ProjectsRelationBuilder + extend ActiveSupport::Concern + + module ClassMethods + def prepare_relation(relation) + relation = preload_relation(relation) + execute_batch_counting(relation) + relation + end + + def preload_relation(relation) + raise NotImplementedError, 'self.preload_relation method must be defined and return a relation' + end + + def forks_counting_projects(projects_relation) + projects_relation + end + + def batch_forks_counting(projects_relation) + ::Projects::BatchForksCountService.new(forks_counting_projects(projects_relation)).refresh_cache + end + + def batch_open_issues_counting(projects_relation) + ::Projects::BatchOpenIssuesCountService.new(projects_relation).refresh_cache + end + + def execute_batch_counting(projects_relation) + batch_forks_counting(projects_relation) + batch_open_issues_counting(projects_relation) + end + end + end +end -- cgit v1.2.1 From 194f7bca9a286942bcd764c836180964e33a1e92 Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Thu, 30 Nov 2017 12:52:38 +0100 Subject: Comments from code review applied. Also switched forked_from_project and ForkedProjectLinks to ForkNetworkMember --- lib/api/entities.rb | 75 ++++++++++++++++++++---------------- lib/api/projects.rb | 10 ++--- lib/api/projects_relation_builder.rb | 10 ++--- 3 files changed, 52 insertions(+), 43 deletions(-) (limited to 'lib/api') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 8f41b930c0a..dada353a314 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -96,7 +96,7 @@ module API expose :star_count, :forks_count expose :created_at, :last_activity_at - def self.preload_relation(projects_relation) + def self.preload_relation(projects_relation, options = {}) projects_relation.preload(:project_feature, :route) .preload(namespace: [:route, :owner], tags: :taggings) @@ -134,18 +134,6 @@ module API expose :members do |project| expose_url(api_v4_projects_members_path(id: project.id)) end - - def self.preload_relation(projects_relation) - super(projects_relation).preload(:group) - .preload(project_group_links: :group, - fork_network: :root_project, - forked_project_link: :forked_from_project, - forked_from_project: [:route, :forks, namespace: :route, tags: :taggings]) - end - - def self.forks_counting_projects(projects_relation) - projects_relation + projects_relation.map(&:forked_from_project).compact - end end expose :archived?, as: :archived @@ -165,7 +153,9 @@ module API expose :lfs_enabled?, as: :lfs_enabled expose :creator_id expose :namespace, using: 'API::Entities::NamespaceBasic' - expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? } + expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? } do |project, options| + project.fork_network_member.forked_from_project + end expose :import_status expose :import_error, if: lambda { |_project, options| options[:user_can_admin_project] } @@ -182,6 +172,20 @@ module API expose :printing_merge_request_link_enabled expose :statistics, using: 'API::Entities::ProjectStatistics', if: :statistics + + def self.preload_relation(projects_relation, options = {}) + relation = super(projects_relation).preload(:group) + .preload(project_group_links: :group, + fork_network: :root_project, + fork_network_member: [forked_from_project: [:route, namespace: :route, tags: :taggings]]) + + # Remove this preload once forked_project_links and forked_from_project models have been removed + relation.preload(forked_project_link: :forked_from_project) + end + + def self.forks_counting_projects(projects_relation) + projects_relation + projects_relation.map(&:fork_network_member).compact.map(&:forked_from_project).compact + end end class ProjectStatistics < Grape::Entity @@ -653,16 +657,10 @@ module API class MemberAccess < Grape::Entity expose :access_level expose :notification_level do |member, options| - notification = member_notification_setting(member) - ::NotificationSetting.levels[notification.level] if notification - end - - private - - def member_notification_setting(member) - member.user.notification_settings.select do |notification| - notification.source_id == member.source_id && notification.source_type == member.source_type - end.last + # binding.pry if member.id == 5 + if member.notification_setting + ::NotificationSetting.levels[member.notification_setting.level] + end end end @@ -705,25 +703,36 @@ module API expose :permissions do expose :project_access, using: Entities::ProjectAccess do |project, options| if options.key?(:project_members) - (options[:project_members] || []).select { |member| member.source_id == project.id }.last - elsif project.project_members.any? - # This is not the bet option to search in a CollectionProxy, but if - # we use find_by we will perform another query, even if the association - # is loaded - project.project_members.select { |project_member| project_member.user_id == options[:current_user].id }.last + (options[:project_members] || []).find { |member| member.source_id == project.id } + else + project.project_member(options[:current_user]) end end expose :group_access, using: Entities::GroupAccess do |project, options| if project.group if options.key?(:group_members) - (options[:group_members] || []).select { |member| member.source_id == project.namespace_id }.last - elsif project.group.group_members.any? - project.group.group_members.select { |group_member| group_member.user_id == options[:current_user].id }.last + (options[:group_members] || []).find { |member| member.source_id == project.namespace_id } + else + project.group.group_member(options[:current_user]) end end end end + + def self.preload_relation(projects_relation, options = {}) + relation = super(projects_relation, options) + + unless options.key?(:group_members) + relation = relation.preload(group: [group_members: [:source, user: [notification_settings: :source]]]) + end + + unless options.key?(:project_members) + relation = relation.preload(project_members: [:source, user: [notification_settings: :source]]) + end + + relation + end end class LabelBasic < Grape::Entity diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 12506203429..9e92ff1417e 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -82,20 +82,20 @@ module API projects = paginate(projects) if current_user - project_members = current_user.project_members - group_members = current_user.group_members + project_members = current_user.project_members.preload(:source, user: [notification_settings: :source]) + group_members = current_user.group_members.preload(:source, user: [notification_settings: :source]) end options = options.reverse_merge( with: current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails, statistics: params[:statistics], - project_members: project_members, - group_members: group_members, + project_members: nil, + group_members: nil, current_user: current_user ) options[:with] = Entities::BasicProjectDetails if params[:simple] - present options[:with].prepare_relation(projects), options + present options[:with].prepare_relation(projects, options), options end end diff --git a/lib/api/projects_relation_builder.rb b/lib/api/projects_relation_builder.rb index 57df6157225..2e042d51c05 100644 --- a/lib/api/projects_relation_builder.rb +++ b/lib/api/projects_relation_builder.rb @@ -3,13 +3,13 @@ module API extend ActiveSupport::Concern module ClassMethods - def prepare_relation(relation) - relation = preload_relation(relation) - execute_batch_counting(relation) - relation + def prepare_relation(projects_relation, options = {}) + projects_relation = preload_relation(projects_relation, options) + execute_batch_counting(projects_relation) + projects_relation end - def preload_relation(relation) + def preload_relation(projects_relation, options = {}) raise NotImplementedError, 'self.preload_relation method must be defined and return a relation' end -- cgit v1.2.1 From 966f83f9653aa774ee82be65dbdae0c6e4f297da Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Thu, 30 Nov 2017 13:13:00 +0100 Subject: Undoing debugging change --- lib/api/projects.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/api') diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 9e92ff1417e..14a4fc6f025 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -89,8 +89,8 @@ module API options = options.reverse_merge( with: current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails, statistics: params[:statistics], - project_members: nil, - group_members: nil, + project_members: project_members, + group_members: group_members, current_user: current_user ) options[:with] = Entities::BasicProjectDetails if params[:simple] -- cgit v1.2.1 From fa6b0a36bd315a4777bb8b3229b89808b95a9489 Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Thu, 30 Nov 2017 14:01:56 +0100 Subject: Changes after rebase --- lib/api/entities.rb | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) (limited to 'lib/api') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index dada353a314..e0eb2ecfb73 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -87,14 +87,25 @@ module API expose :created_at end - class BasicProjectDetails < ProjectIdentity - expose :default_branch, :tag_list + class BasicProjectDetails < Grape::Entity + include ::API::ProjectsRelationBuilder + + expose :default_branch + + # Avoids an N+1 query: https://github.com/mbleigh/acts-as-taggable-on/issues/91#issuecomment-168273770 + expose :tag_list do |project| + # project.tags.order(:name).pluck(:name) is the most suitable option + # to avoid loading all the ActiveRecord objects but, if we use it here + # it override the preloaded associations and makes a query + # (fixed in https://github.com/rails/rails/pull/25976). + project.tags.map(&:name).sort + end expose :ssh_url_to_repo, :http_url_to_repo, :web_url expose :avatar_url do |project, options| project.avatar_url(only_path: false) end expose :star_count, :forks_count - expose :created_at, :last_activity_at + expose :last_activity_at def self.preload_relation(projects_relation, options = {}) projects_relation.preload(:project_feature, :route) -- cgit v1.2.1 From c7e7f4444c791eb0ad409310394954578aa232c6 Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Thu, 30 Nov 2017 16:20:13 +0100 Subject: Removing blank line --- lib/api/entities.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'lib/api') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index e0eb2ecfb73..910ac4f1814 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -91,7 +91,6 @@ module API include ::API::ProjectsRelationBuilder expose :default_branch - # Avoids an N+1 query: https://github.com/mbleigh/acts-as-taggable-on/issues/91#issuecomment-168273770 expose :tag_list do |project| # project.tags.order(:name).pluck(:name) is the most suitable option -- cgit v1.2.1 From c0c0926accdc3c49fc2a75a0eec2f96a8a5ad15c Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Thu, 30 Nov 2017 18:24:31 +0100 Subject: Removed binding.pry --- lib/api/entities.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'lib/api') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 910ac4f1814..248e234580f 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -667,7 +667,6 @@ module API class MemberAccess < Grape::Entity expose :access_level expose :notification_level do |member, options| - # binding.pry if member.id == 5 if member.notification_setting ::NotificationSetting.levels[member.notification_setting.level] end -- cgit v1.2.1 From fe95de88551bd3c8d22591764d948205f9fbc10e Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Thu, 30 Nov 2017 19:09:25 +0100 Subject: Fixed BasicProjectDetail parent --- lib/api/entities.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/api') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 248e234580f..d224f468c18 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -87,7 +87,7 @@ module API expose :created_at end - class BasicProjectDetails < Grape::Entity + class BasicProjectDetails < ProjectIdentity include ::API::ProjectsRelationBuilder expose :default_branch -- cgit v1.2.1 From 3527d1ff2bc06ba38e820b300e49f817d2833379 Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Thu, 30 Nov 2017 22:17:17 +0100 Subject: Undoing the change to ForkNetworkMember --- lib/api/entities.rb | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) (limited to 'lib/api') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index d224f468c18..7cec8da013d 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -163,9 +163,7 @@ module API expose :lfs_enabled?, as: :lfs_enabled expose :creator_id expose :namespace, using: 'API::Entities::NamespaceBasic' - expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? } do |project, options| - project.fork_network_member.forked_from_project - end + expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? } expose :import_status expose :import_error, if: lambda { |_project, options| options[:user_can_admin_project] } @@ -184,17 +182,15 @@ module API expose :statistics, using: 'API::Entities::ProjectStatistics', if: :statistics def self.preload_relation(projects_relation, options = {}) - relation = super(projects_relation).preload(:group) - .preload(project_group_links: :group, - fork_network: :root_project, - fork_network_member: [forked_from_project: [:route, namespace: :route, tags: :taggings]]) - - # Remove this preload once forked_project_links and forked_from_project models have been removed - relation.preload(forked_project_link: :forked_from_project) + super(projects_relation).preload(:group) + .preload(project_group_links: :group, + fork_network: :root_project, + forked_project_link: :forked_from_project, + forked_from_project: [:route, :forks, namespace: :route, tags: :taggings]) end def self.forks_counting_projects(projects_relation) - projects_relation + projects_relation.map(&:fork_network_member).compact.map(&:forked_from_project).compact + projects_relation + projects_relation.map(&:forked_from_project).compact end end -- cgit v1.2.1 From 979056e964827a9d6efc979843ac567a3dd5cdfd Mon Sep 17 00:00:00 2001 From: Francisco Lopez Date: Fri, 1 Dec 2017 19:21:04 +0100 Subject: Not forcing to redefine preload_relation --- lib/api/projects_relation_builder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/api') diff --git a/lib/api/projects_relation_builder.rb b/lib/api/projects_relation_builder.rb index 2e042d51c05..6482fd94ab8 100644 --- a/lib/api/projects_relation_builder.rb +++ b/lib/api/projects_relation_builder.rb @@ -10,7 +10,7 @@ module API end def preload_relation(projects_relation, options = {}) - raise NotImplementedError, 'self.preload_relation method must be defined and return a relation' + projects_relation end def forks_counting_projects(projects_relation) -- cgit v1.2.1 From 25df666156279e5b392b429519b4f4ba01eefaac Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Tue, 19 Sep 2017 09:14:06 +0200 Subject: Create Ci::Artifacts To allow jobs/builds to have multiple artifacts, and to start seperating concerns from Ci::Build a new model is created: Ci::Artifact. Changes include the updating of the ArtifactUploader to adapt to a slightly different interface. The uploader expects to be initialized with a `Ci::Build`. Futher a migration with the minimal fields, the needed foreign keys and an index. Last, the way this works is by prepending a module to Ci::Build so we can basically override behaviour but if needed use `super` to get the original behaviour. --- lib/api/entities.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/api') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index ce332fe85d2..9eb2c98c436 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1036,7 +1036,7 @@ module API expose :steps, using: Step expose :image, using: Image expose :services, using: Service - expose :artifacts, using: Artifacts + expose :artifacts_options, as: :artifacts, using: Artifacts expose :cache, using: Cache expose :credentials, using: Credentials expose :dependencies, using: Dependency -- cgit v1.2.1 From 61864a5a5bb523953589c9398a431c4369fbfc76 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 21 Sep 2017 10:34:12 +0200 Subject: Rename Artifact to JobArtifact, split metadata out Two things at ones, as there was no clean way to seperate the commit and give me feedback from the tests. But the model Artifact is now JobArtifact, and the table does not have a type anymore, but the metadata is now its own model: Ci::JobArtifactMetadata. --- lib/api/entities.rb | 8 ++------ lib/api/runner.rb | 7 ++++--- 2 files changed, 6 insertions(+), 9 deletions(-) (limited to 'lib/api') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 9eb2c98c436..0964bd98fbb 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1006,13 +1006,9 @@ module API expose :type, :url, :username, :password end - class ArtifactFile < Grape::Entity - expose :filename, :size - end - class Dependency < Grape::Entity expose :id, :name, :token - expose :artifacts_file, using: ArtifactFile, if: ->(job, _) { job.artifacts? } + expose :artifacts_file, using: JobArtifactFile, if: ->(job, _) { job.artifacts? } end class Response < Grape::Entity @@ -1036,7 +1032,7 @@ module API expose :steps, using: Step expose :image, using: Image expose :services, using: Service - expose :artifacts_options, as: :artifacts, using: Artifacts + expose :artifacts, using: Artifacts expose :cache, using: Cache expose :credentials, using: Credentials expose :dependencies, using: Dependency diff --git a/lib/api/runner.rb b/lib/api/runner.rb index a3987c560dd..8de0868c063 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -215,15 +215,16 @@ module API job = authenticate_job! forbidden!('Job is not running!') unless job.running? - artifacts_upload_path = ArtifactUploader.artifacts_upload_path + artifacts_upload_path = JobArtifactUploader.artifacts_upload_path artifacts = uploaded_file(:file, artifacts_upload_path) metadata = uploaded_file(:metadata, artifacts_upload_path) bad_request!('Missing artifacts file!') unless artifacts file_to_large! unless artifacts.size < max_artifacts_size - job.artifacts_file = artifacts - job.artifacts_metadata = metadata + job.job_artifacts.build(project: job.project, file_type: :archive, file: artifacts) + job.job_artifacts.build(project: job.project, file_type: :metadata, file: metadata) + job.artifacts_expire_in = params['expire_in'] || Gitlab::CurrentSettings.current_application_settings.default_artifacts_expire_in -- cgit v1.2.1 From 0464c25f602dc2e4d147ca2ac714d49bf96ddcf2 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 3 Dec 2017 12:02:11 +0100 Subject: Store expire_at in ci_job_artifacts --- lib/api/runner.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'lib/api') diff --git a/lib/api/runner.rb b/lib/api/runner.rb index 8de0868c063..80feb629d54 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -222,12 +222,13 @@ module API bad_request!('Missing artifacts file!') unless artifacts file_to_large! unless artifacts.size < max_artifacts_size - job.job_artifacts.build(project: job.project, file_type: :archive, file: artifacts) - job.job_artifacts.build(project: job.project, file_type: :metadata, file: metadata) - - job.artifacts_expire_in = params['expire_in'] || + expire_in = params['expire_in'] || Gitlab::CurrentSettings.current_application_settings.default_artifacts_expire_in + job.build_job_artifacts_archive(project: job.project, file_type: :archive, file: artifacts, expire_in: expire_in) + job.build_job_artifacts_metadata(project: job.project, file_type: :metadata, file: metadata, expire_in: expire_in) if metadata + job.artifacts_expire_in = expire_in + if job.save present job, with: Entities::JobRequest::Response else -- cgit v1.2.1 From 7c2b7296d4e623ba4eaa19cf03405ee7f2ae1ca9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Javier=20L=C3=B3pez?= Date: Mon, 4 Dec 2017 09:49:53 +0000 Subject: Added default order to UserFinder --- lib/api/helpers/pagination.rb | 10 ++++++++++ lib/api/users.rb | 2 ++ 2 files changed, 12 insertions(+) (limited to 'lib/api') diff --git a/lib/api/helpers/pagination.rb b/lib/api/helpers/pagination.rb index 95108292aac..bb70370ba77 100644 --- a/lib/api/helpers/pagination.rb +++ b/lib/api/helpers/pagination.rb @@ -2,6 +2,8 @@ module API module Helpers module Pagination def paginate(relation) + relation = add_default_order(relation) + relation.page(params[:page]).per(params[:per_page]).tap do |data| add_pagination_headers(data) end @@ -45,6 +47,14 @@ module API # Ensure there is in total at least 1 page [paginated_data.total_pages, 1].max end + + def add_default_order(relation) + if relation.is_a?(ActiveRecord::Relation) && relation.order_values.empty? + relation = relation.order(:id) + end + + relation + end end end end diff --git a/lib/api/users.rb b/lib/api/users.rb index 0cd89b1bcf8..e5de31ad51b 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -76,6 +76,8 @@ module API forbidden!("Not authorized to access /api/v4/users") unless authorized entity = current_user&.admin? ? Entities::UserWithAdmin : Entities::UserBasic + users = users.preload(:identities, :u2f_registrations) if entity == Entities::UserWithAdmin + present paginate(users), with: entity end -- cgit v1.2.1 From c9871e84e4b605922689aee5957d206516dca8e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Javier=20L=C3=B3pez?= Date: Thu, 7 Dec 2017 08:44:55 +0000 Subject: The API isn't using the appropriate services for managing forks --- lib/api/projects.rb | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) (limited to 'lib/api') diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 14a4fc6f025..fa222bf2b1c 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -367,15 +367,16 @@ module API post ":id/fork/:forked_from_id" do authenticated_as_admin! - forked_from_project = find_project!(params[:forked_from_id]) - not_found!("Source Project") unless forked_from_project + fork_from_project = find_project!(params[:forked_from_id]) - if user_project.forked_from_project.nil? - user_project.create_forked_project_link(forked_to_project_id: user_project.id, forked_from_project_id: forked_from_project.id) + not_found!("Source Project") unless fork_from_project - ::Projects::ForksCountService.new(forked_from_project).refresh_cache + result = ::Projects::ForkService.new(fork_from_project, current_user).execute(user_project) + + if result + present user_project.reload, with: Entities::Project else - render_api_error!("Project already forked", 409) + render_api_error!("Project already forked", 409) if user_project.forked? end end @@ -383,11 +384,11 @@ module API delete ":id/fork" do authorize! :remove_fork_project, user_project - if user_project.forked? - destroy_conditionally!(user_project.forked_project_link) - else - not_modified! + result = destroy_conditionally!(user_project) do + ::Projects::UnlinkForkService.new(user_project, current_user).execute end + + result ? status(204) : not_modified! end desc 'Share the project with a group' do -- cgit v1.2.1 From 1d47ae1365e259406233764885891923bebc555c Mon Sep 17 00:00:00 2001 From: James Edwards-Jones Date: Wed, 22 Nov 2017 17:08:47 +0000 Subject: CE backport of ProtectedBranches API changes In EE we now allow individual users/groups to be set on POST, which required some refactoring. See https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/3516 --- lib/api/protected_branches.rb | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'lib/api') diff --git a/lib/api/protected_branches.rb b/lib/api/protected_branches.rb index b5021e8a712..614822509f0 100644 --- a/lib/api/protected_branches.rb +++ b/lib/api/protected_branches.rb @@ -39,10 +39,10 @@ module API end params do requires :name, type: String, desc: 'The name of the protected branch' - optional :push_access_level, type: Integer, default: Gitlab::Access::MASTER, + optional :push_access_level, type: Integer, values: ProtectedRefAccess::ALLOWED_ACCESS_LEVELS, desc: 'Access levels allowed to push (defaults: `40`, master access level)' - optional :merge_access_level, type: Integer, default: Gitlab::Access::MASTER, + optional :merge_access_level, type: Integer, values: ProtectedRefAccess::ALLOWED_ACCESS_LEVELS, desc: 'Access levels allowed to merge (defaults: `40`, master access level)' end @@ -52,15 +52,13 @@ module API conflict!("Protected branch '#{params[:name]}' already exists") end - protected_branch_params = { - name: params[:name], - push_access_levels_attributes: [{ access_level: params[:push_access_level] }], - merge_access_levels_attributes: [{ access_level: params[:merge_access_level] }] - } + # Replace with `declared(params)` after updating to grape v1.0.2 + # See https://github.com/ruby-grape/grape/pull/1710 + # and https://gitlab.com/gitlab-org/gitlab-ce/issues/40843 + declared_params = params.slice("name", "push_access_level", "merge_access_level", "allowed_to_push", "allowed_to_merge") - service_args = [user_project, current_user, protected_branch_params] - - protected_branch = ::ProtectedBranches::CreateService.new(*service_args).execute + api_service = ::ProtectedBranches::ApiService.new(user_project, current_user, declared_params) + protected_branch = api_service.create if protected_branch.persisted? present protected_branch, with: Entities::ProtectedBranch, project: user_project -- cgit v1.2.1 From e6ac6734c2f636d3d063718a95ba1169e299b51f Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 4 Dec 2017 20:18:45 -0600 Subject: Use relative _path helper URLs in the GitLab UI Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/40825 --- lib/api/entities.rb | 3 +++ 1 file changed, 3 insertions(+) (limited to 'lib/api') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 62ee20bf7de..d96e7f2770f 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -16,10 +16,13 @@ module API class UserBasic < UserSafe expose :state + expose :avatar_url do |user, options| user.avatar_url(only_path: false) end + expose :avatar_path, if: ->(user, options) { options.fetch(:only_path, false) && user.avatar_path } + expose :web_url do |user, options| Gitlab::Routing.url_helpers.user_url(user) end -- cgit v1.2.1 From f1ae1e39ce6b7578c5697c977bc3b52b119301ab Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Mon, 13 Nov 2017 16:52:07 +0100 Subject: Move the circuitbreaker check out in a separate process Moving the check out of the general requests, makes sure we don't have any slowdown in the regular requests. To keep the process performing this checks small, the check is still performed inside a unicorn. But that is called from a process running on the same server. Because the checks are now done outside normal request, we can have a simpler failure strategy: The check is now performed in the background every `circuitbreaker_check_interval`. Failures are logged in redis. The failures are reset when the check succeeds. Per check we will try `circuitbreaker_access_retries` times within `circuitbreaker_storage_timeout` seconds. When the number of failures exceeds `circuitbreaker_failure_count_threshold`, we will block access to the storage. After `failure_reset_time` of no checks, we will clear the stored failures. This could happen when the process that performs the checks is not running. --- lib/api/circuit_breakers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/api') diff --git a/lib/api/circuit_breakers.rb b/lib/api/circuit_breakers.rb index 118883f5ea5..598c76f6168 100644 --- a/lib/api/circuit_breakers.rb +++ b/lib/api/circuit_breakers.rb @@ -41,7 +41,7 @@ module API detail 'This feature was introduced in GitLab 9.5' end delete do - Gitlab::Git::Storage::CircuitBreaker.reset_all! + Gitlab::Git::Storage::FailureInfo.reset_all! end end end -- cgit v1.2.1