From 510092c83ad6b35dc4cc0ac1f15c643952f519b5 Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Thu, 10 Nov 2016 16:32:59 +0100 Subject: Use #to_h to convert params to a hash --- lib/api/helpers.rb | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 3c9d7b1aaef..6998b6dc039 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -23,6 +23,11 @@ module API warden.try(:authenticate) if %w[GET HEAD].include?(env['REQUEST_METHOD']) end + def declared_params(options = {}) + options = { include_parent_namespaces: false }.merge(options) + declared(params, options).to_h.symbolize_keys + end + def find_user_by_private_token token = private_token return nil unless token.present? -- cgit v1.2.1 From ef3be00a0297dfa31002616df6ee49a0e2132cb7 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Wed, 16 Nov 2016 12:46:07 +0100 Subject: Defer saving project services to the database if there are no user changes --- lib/api/helpers.rb | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 3c9d7b1aaef..b4e0457f773 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -81,25 +81,10 @@ module API end def project_service - @project_service ||= begin - underscored_service = params[:service_slug].underscore - - if Service.available_services_names.include?(underscored_service) - user_project.build_missing_services - - service_method = "#{underscored_service}_service" - - send_service(service_method) - end - end - + @project_service ||= user_project.find_or_initialize_service(params[:service_slug].underscore) @project_service || not_found!("Service") end - def send_service(service_method) - user_project.send(service_method) - end - def service_attributes @service_attributes ||= project_service.fields.inject([]) do |arr, hash| arr << hash[:name].to_sym -- cgit v1.2.1 From 0d04724fa1cd670124b8ad9a3860bfa476c50f99 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Fri, 18 Nov 2016 10:00:40 +0100 Subject: More coverage on service level --- lib/api/helpers.rb | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 84cc9200d1b..d6526ec4fdc 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -90,6 +90,15 @@ module API @project_service || not_found!("Service") end + def service_by_slug(project, slug) + underscored_service = slug.underscore + + not_found!('Service') unless Service.available_services_names.include?(underscored_service) + service_method = "#{underscored_service}_service" + + service = project.public_send(service_method) + end + def service_attributes @service_attributes ||= project_service.fields.inject([]) do |arr, hash| arr << hash[:name].to_sym -- cgit v1.2.1 From f749fb7fe0574d07eeb38561b9af62754e518281 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Fri, 18 Nov 2016 11:38:54 +0100 Subject: Improve style, add more tests --- lib/api/helpers.rb | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index d6526ec4fdc..2c593dbb4ea 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -85,20 +85,11 @@ module API end end - def project_service - @project_service ||= user_project.find_or_initialize_service(params[:service_slug].underscore) + def project_service(project = user_project) + @project_service ||= project.find_or_initialize_service(params[:service_slug].underscore) @project_service || not_found!("Service") end - def service_by_slug(project, slug) - underscored_service = slug.underscore - - not_found!('Service') unless Service.available_services_names.include?(underscored_service) - service_method = "#{underscored_service}_service" - - service = project.public_send(service_method) - end - def service_attributes @service_attributes ||= project_service.fields.inject([]) do |arr, hash| arr << hash[:name].to_sym -- cgit v1.2.1 From 4d2e7894efa36cc1b5de9432e25fcf22b6cf1d59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 23 Nov 2016 19:18:19 +0100 Subject: Make API::Helpers find a project with only one query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- lib/api/helpers.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 2c593dbb4ea..60067758e95 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -76,7 +76,12 @@ module API end def find_project(id) - project = Project.find_with_namespace(id) || Project.find_by(id: id) + project = + if id =~ /^\d+$/ + Project.find_by(id: id) + else + Project.find_with_namespace(id) + end if can?(current_user, :read_project, project) project -- cgit v1.2.1 From 4f5ed812325845f263fc9b566651c1179b5c24bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 24 Nov 2016 14:40:35 +0100 Subject: API: Introduce `#find_project!` which also check access permission MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- lib/api/helpers.rb | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 60067758e95..42f4c2ccf9d 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -68,7 +68,7 @@ module API end def user_project - @project ||= find_project(params[:id]) + @project ||= find_project!(params[:id]) end def available_labels @@ -76,12 +76,15 @@ module API end def find_project(id) - project = - if id =~ /^\d+$/ - Project.find_by(id: id) - else - Project.find_with_namespace(id) - end + if id =~ /^\d+$/ + Project.find_by(id: id) + else + Project.find_with_namespace(id) + end + end + + def find_project!(id) + project = find_project(id) if can?(current_user, :read_project, project) project -- cgit v1.2.1 From 81ba3f9177fcfd76f6b3b715c572ce4920398345 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 24 Nov 2016 16:58:32 +0100 Subject: API: Introduce `#find_group!` which also check access permission MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- lib/api/helpers.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 42f4c2ccf9d..0d3ddb89dc3 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -105,7 +105,15 @@ module API end def find_group(id) - group = Group.find_by(path: id) || Group.find_by(id: id) + if id =~ /^\d+$/ + Group.find_by(id: id) + else + Group.find_by(path: id) + end + end + + def find_group!(id) + group = find_group(id) if can?(current_user, :read_group, group) group -- cgit v1.2.1 From 7c607a55ab339293b0e67eeb33439d5407e22aad Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Wed, 9 Nov 2016 15:51:27 +0100 Subject: Grapify the projects API --- lib/api/helpers.rb | 26 ++------------------------ 1 file changed, 2 insertions(+), 24 deletions(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 0d3ddb89dc3..2fd50143e91 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -324,11 +324,6 @@ module API # Projects helpers def filter_projects(projects) - # If the archived parameter is passed, limit results accordingly - if params[:archived].present? - projects = projects.where(archived: to_boolean(params[:archived])) - end - if params[:search].present? projects = projects.search(params[:search]) end @@ -337,25 +332,8 @@ module API projects = projects.search_by_visibility(params[:visibility]) end - projects.reorder(project_order_by => project_sort) - end - - def project_order_by - order_fields = %w(id name path created_at updated_at last_activity_at) - - if order_fields.include?(params['order_by']) - params['order_by'] - else - 'created_at' - end - end - - def project_sort - if params["sort"] == 'asc' - :asc - else - :desc - end + projects = projects.where(archived: params[:archived]) + projects.reorder(params[:order_by] => params[:sort]) end # file helpers -- cgit v1.2.1 From 3d7704ae5f62446b8b399c796c64d1f527666376 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 10 Nov 2016 10:23:44 +0000 Subject: Merge branch 'zj-fix-label-creation-non-members' into 'security' Fix label creation non members Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/23416 See merge request !2006 --- lib/api/helpers.rb | 14 -------------- 1 file changed, 14 deletions(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 0d3ddb89dc3..79a83496eee 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -198,20 +198,6 @@ module API ActionController::Parameters.new(attrs).permit! end - # Helper method for validating all labels against its names - def validate_label_params(params) - errors = {} - - params[:labels].to_s.split(',').each do |label_name| - label = available_labels.find_or_initialize_by(title: label_name.strip) - next if label.valid? - - errors[label.title] = label.errors - end - - errors - end - # Checks the occurrences of datetime attributes, each attribute if present in the params hash must be in ISO 8601 # format (YYYY-MM-DDTHH:MM:SSZ) or a Bad Request error is invoked. # -- cgit v1.2.1 From 3bf34face4cacf07ca973705c261369b1f596626 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 22 Nov 2016 10:25:04 +0000 Subject: Merge branch 'jej-use-issuable-finder-instead-of-access-check' into 'security' Replace issue access checks with use of IssuableFinder Split from !2024 to partially solve https://gitlab.com/gitlab-org/gitlab-ce/issues/23867 ## Which fixes are in this MR? :warning: - Potentially untested :bomb: - No test coverage :traffic_light: - Test coverage of some sort exists (a test failed when error raised) :vertical_traffic_light: - Test coverage of return value (a test failed when nil used) :white_check_mark: - Permissions check tested ### Issue lookup with access check Using `visible_to_user` likely makes these security issues too. See [Code smells](#code-smells). - [x] :vertical_traffic_light: app/finders/notes_finder.rb:15 [`visible_to_user`] - [x] :traffic_light: app/views/layouts/nav/_project.html.haml:73 [`visible_to_user`] [`.count`] - [x] :white_check_mark: app/services/merge_requests/build_service.rb:84 [`issue.try(:confidential?)`] - [x] :white_check_mark: lib/api/issues.rb:112 [`visible_to_user`] - CHANGELOG: Prevented API returning issues set to 'Only team members' to everyone - [x] :white_check_mark: lib/api/helpers.rb:126 [`can?(current_user, :read_issue, issue)`] Maybe here too? - [x] :white_check_mark: lib/gitlab/search_results.rb:53 [`visible_to_user`] ### Previous discussions - [ ] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#b2ff264eddf9819d7693c14ae213d941494fe2b3_128_126 - [ ] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#7b6375270d22f880bdcb085e47b519b426a5c6c7_87_87 See merge request !2031 --- lib/api/helpers.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 79a83496eee..34d9c3c6932 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -128,9 +128,7 @@ module API end def find_project_issue(id) - issue = user_project.issues.find(id) - not_found! unless can?(current_user, :read_issue, issue) - issue + IssuesFinder.new(current_user, project_id: user_project.id).find(id) end def paginate(relation) -- cgit v1.2.1 From d757247247ea6015d560eacd29ec7be564e332bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 30 Nov 2016 15:48:19 +0100 Subject: Allow public access to some Project API endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- lib/api/helpers.rb | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index cbafa952ef6..7f94ede7940 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -141,6 +141,10 @@ module API unauthorized! unless current_user end + def authenticate_non_get! + authenticate! unless %w[GET HEAD].include?(route.route_method) + end + def authenticate_by_gitlab_shell_token! input = params['secret_token'].try(:chomp) unless Devise.secure_compare(secret_token, input) @@ -149,6 +153,7 @@ module API end def authenticated_as_admin! + authenticate! forbidden! unless current_user.is_admin? end -- cgit v1.2.1 From 3e7818e93a69d2f628c864e40b00ab0871bff3dc Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Mon, 7 Nov 2016 15:15:14 +0100 Subject: Grapify the issues API --- lib/api/helpers.rb | 16 ---------------- 1 file changed, 16 deletions(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 7f94ede7940..898ca470a30 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -217,22 +217,6 @@ module API end end - def issuable_order_by - if params["order_by"] == 'updated_at' - 'updated_at' - else - 'created_at' - end - end - - def issuable_sort - if params["sort"] == 'asc' - :asc - else - :desc - end - end - def filter_by_iid(items, iid) items.where(iid: iid) end -- cgit v1.2.1 From 7870614fb94ce3eb1531e77dc824f4832a808571 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 7 Dec 2016 13:15:01 +0100 Subject: Extract API pagination code to a separete module --- lib/api/helpers.rb | 39 +-------------------------------------- 1 file changed, 1 insertion(+), 38 deletions(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 7f94ede7940..164aea613e4 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -1,6 +1,7 @@ module API module Helpers include Gitlab::Utils + include Helpers::Pagination PRIVATE_TOKEN_HEADER = "HTTP_PRIVATE_TOKEN" PRIVATE_TOKEN_PARAM = :private_token @@ -131,12 +132,6 @@ module API IssuesFinder.new(current_user, project_id: user_project.id).find(id) end - def paginate(relation) - relation.page(params[:page]).per(params[:per_page].to_i).tap do |data| - add_pagination_headers(data) - end - end - def authenticate! unauthorized! unless current_user end @@ -367,38 +362,6 @@ module API private - def add_pagination_headers(paginated_data) - header 'X-Total', paginated_data.total_count.to_s - header 'X-Total-Pages', paginated_data.total_pages.to_s - header 'X-Per-Page', paginated_data.limit_value.to_s - header 'X-Page', paginated_data.current_page.to_s - header 'X-Next-Page', paginated_data.next_page.to_s - header 'X-Prev-Page', paginated_data.prev_page.to_s - header 'Link', pagination_links(paginated_data) - end - - def pagination_links(paginated_data) - request_url = request.url.split('?').first - request_params = params.clone - request_params[:per_page] = paginated_data.limit_value - - links = [] - - request_params[:page] = paginated_data.current_page - 1 - links << %(<#{request_url}?#{request_params.to_query}>; rel="prev") unless paginated_data.first_page? - - request_params[:page] = paginated_data.current_page + 1 - links << %(<#{request_url}?#{request_params.to_query}>; rel="next") unless paginated_data.last_page? - - request_params[:page] = 1 - links << %(<#{request_url}?#{request_params.to_query}>; rel="first") - - request_params[:page] = paginated_data.total_pages - links << %(<#{request_url}?#{request_params.to_query}>; rel="last") - - links.join(', ') - end - def secret_token Gitlab::Shell.secret_token end -- cgit v1.2.1 From 3ed96afc47c481db4f8c0a6581602abaee920808 Mon Sep 17 00:00:00 2001 From: tiagonbotelho Date: Mon, 21 Nov 2016 12:59:37 +0000 Subject: adds impersonator variable and makes sudo usage overall more clear --- lib/api/helpers.rb | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 7f94ede7940..6a47efc74f0 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -44,11 +44,14 @@ module API return nil end - identifier = sudo_identifier() + identifier = sudo_identifier - # If the sudo is the current user do nothing - if identifier && !(@current_user.id == identifier || @current_user.username == identifier) + if identifier + # We check for private_token because we cannot allow PAT to be used forbidden!('Must be admin to use sudo') unless @current_user.is_admin? + forbidden!('Private token must be specified in order to use sudo') unless private_token_used? + + @impersonator = @current_user @current_user = User.by_username_or_id(identifier) not_found!("No user id or username for: #{identifier}") if @current_user.nil? end @@ -399,6 +402,10 @@ module API links.join(', ') end + def private_token_used? + private_token == @current_user.private_token + end + def secret_token Gitlab::Shell.secret_token end -- cgit v1.2.1 From 83232be0e14cc8b35bf74532203a6e4371c15e70 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 31 Oct 2016 13:00:53 +0200 Subject: Add nested groups support on data level * add parent_id field to namespaces table to store relation with nested groups * create routes table to keep information about full path of every group and project * project/group lookup by full path from routes table Signed-off-by: Dmitriy Zaporozhets --- lib/api/helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 7f94ede7940..96ccde760db 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -108,7 +108,7 @@ module API if id =~ /^\d+$/ Group.find_by(id: id) else - Group.find_by(path: id) + Group.find_by_full_path(id) end end -- cgit v1.2.1 From 593c912151eaef865d31fc2a8307ef6d337c2349 Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Mon, 5 Dec 2016 15:40:53 +0100 Subject: Grapify the service API --- lib/api/helpers.rb | 11 ----------- 1 file changed, 11 deletions(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 7f94ede7940..16ac40b7142 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -93,17 +93,6 @@ module API end end - def project_service(project = user_project) - @project_service ||= project.find_or_initialize_service(params[:service_slug].underscore) - @project_service || not_found!("Service") - end - - def service_attributes - @service_attributes ||= project_service.fields.inject([]) do |arr, hash| - arr << hash[:name].to_sym - end - end - def find_group(id) if id =~ /^\d+$/ Group.find_by(id: id) -- cgit v1.2.1 From 2f45d3bcf0f28d4cd4124b4c9722edc1d3085201 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 9 Dec 2016 18:48:20 +0100 Subject: API: Memoize the current_user so that the sudo can work properly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The issue was arising when `#current_user` was called a second time after a user was impersonated: the `User#is_admin?` check would be performed on it and it would fail. Signed-off-by: Rémy Coutable --- lib/api/helpers.rb | 131 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 78 insertions(+), 53 deletions(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 8b0f8deadfa..2041f0dac6b 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -7,67 +7,23 @@ module API SUDO_HEADER = "HTTP_SUDO" SUDO_PARAM = :sudo - def private_token - params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER] - end - - def warden - env['warden'] - end - - # Check the Rails session for valid authentication details - # - # Until CSRF protection is added to the API, disallow this method for - # state-changing endpoints - def find_user_from_warden - warden.try(:authenticate) if %w[GET HEAD].include?(env['REQUEST_METHOD']) - end - def declared_params(options = {}) options = { include_parent_namespaces: false }.merge(options) declared(params, options).to_h.symbolize_keys end - def find_user_by_private_token - token = private_token - return nil unless token.present? - - User.find_by_authentication_token(token) || User.find_by_personal_access_token(token) - end - def current_user - @current_user ||= find_user_by_private_token - @current_user ||= doorkeeper_guard - @current_user ||= find_user_from_warden - - unless @current_user && Gitlab::UserAccess.new(@current_user).allowed? - return nil - end - - identifier = sudo_identifier + return @current_user if defined?(@current_user) - if identifier - # We check for private_token because we cannot allow PAT to be used - forbidden!('Must be admin to use sudo') unless @current_user.is_admin? - forbidden!('Private token must be specified in order to use sudo') unless private_token_used? + @current_user = initial_current_user - @impersonator = @current_user - @current_user = User.by_username_or_id(identifier) - not_found!("No user id or username for: #{identifier}") if @current_user.nil? - end + sudo! @current_user end - def sudo_identifier - identifier ||= params[SUDO_PARAM] || env[SUDO_HEADER] - - # Regex for integers - if !!(identifier =~ /\A[0-9]+\z/) - identifier.to_i - else - identifier - end + def sudo? + initial_current_user != current_user end def user_project @@ -354,6 +310,79 @@ module API private + def private_token + params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER] + end + + def warden + env['warden'] + end + + # Check the Rails session for valid authentication details + # + # Until CSRF protection is added to the API, disallow this method for + # state-changing endpoints + def find_user_from_warden + warden.try(:authenticate) if %w[GET HEAD].include?(env['REQUEST_METHOD']) + end + + def find_user_by_private_token + token = private_token + return nil unless token.present? + + User.find_by_authentication_token(token) || User.find_by_personal_access_token(token) + end + + def initial_current_user + return @initial_current_user if defined?(@initial_current_user) + + @initial_current_user ||= find_user_by_private_token + @initial_current_user ||= doorkeeper_guard + @initial_current_user ||= find_user_from_warden + + unless @initial_current_user && Gitlab::UserAccess.new(@initial_current_user).allowed? + @initial_current_user = nil + end + + @initial_current_user + end + + def sudo! + return unless sudo_identifier + return unless initial_current_user.is_a?(User) + + unless initial_current_user.is_admin? + forbidden!('Must be admin to use sudo') + end + + # Only private tokens should be used for the SUDO feature + unless private_token == initial_current_user.private_token + forbidden!('Private token must be specified in order to use sudo') + end + + sudoed_user = User.by_username_or_id(sudo_identifier) + + if sudoed_user + @current_user = sudoed_user + else + not_found!("No user id or username for: #{sudo_identifier}") + end + end + + def sudo_identifier + return @sudo_identifier if defined?(@sudo_identifier) + + identifier ||= params[SUDO_PARAM] || env[SUDO_HEADER] + + # Regex for integers + @sudo_identifier = + if !!(identifier =~ /\A[0-9]+\z/) + identifier.to_i + else + identifier + end + end + def add_pagination_headers(paginated_data) header 'X-Total', paginated_data.total_count.to_s header 'X-Total-Pages', paginated_data.total_pages.to_s @@ -386,10 +415,6 @@ module API links.join(', ') end - def private_token_used? - private_token == @current_user.private_token - end - def secret_token Gitlab::Shell.secret_token end -- cgit v1.2.1 From d95b709a66a5597dced25a2b9df9a1e24fc6d49a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 13 Dec 2016 15:53:00 +0100 Subject: Be smarter when finding a sudoed user in API::Helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- lib/api/helpers.rb | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 2041f0dac6b..8260fcab80a 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -34,6 +34,14 @@ module API @available_labels ||= LabelsFinder.new(current_user, project_id: user_project.id).execute end + def find_user(id) + if id =~ /^\d+$/ + User.find_by(id: id) + else + User.find_by(username: id) + end + end + def find_project(id) if id =~ /^\d+$/ Project.find_by(id: id) @@ -349,7 +357,7 @@ module API def sudo! return unless sudo_identifier - return unless initial_current_user.is_a?(User) + return unless initial_current_user unless initial_current_user.is_admin? forbidden!('Must be admin to use sudo') @@ -360,7 +368,7 @@ module API forbidden!('Private token must be specified in order to use sudo') end - sudoed_user = User.by_username_or_id(sudo_identifier) + sudoed_user = find_user(sudo_identifier) if sudoed_user @current_user = sudoed_user @@ -370,17 +378,7 @@ module API end def sudo_identifier - return @sudo_identifier if defined?(@sudo_identifier) - - identifier ||= params[SUDO_PARAM] || env[SUDO_HEADER] - - # Regex for integers - @sudo_identifier = - if !!(identifier =~ /\A[0-9]+\z/) - identifier.to_i - else - identifier - end + @sudo_identifier ||= params[SUDO_PARAM] || env[SUDO_HEADER] end def add_pagination_headers(paginated_data) -- cgit v1.2.1 From 7fa06ed55d18af4d055041eb27d38fecf9b5548f Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Tue, 22 Nov 2016 14:34:23 +0530 Subject: Calls to the API are checked for scope. - Move the `Oauth2::AccessTokenValidationService` class to `AccessTokenValidationService`, since it is now being used for personal access token validation as well. - Each API endpoint declares the scopes it accepts (if any). Currently, the top level API module declares the `api` scope, and the `Users` API module declares the `read_user` scope (for GET requests). - Move the `find_user_by_private_token` from the API `Helpers` module to the `APIGuard` module, to avoid littering `Helpers` with more auth-related methods to support `find_user_by_private_token` --- lib/api/helpers.rb | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 746849ef4c0..4be659fc20b 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -2,8 +2,6 @@ module API module Helpers include Gitlab::Utils - PRIVATE_TOKEN_HEADER = "HTTP_PRIVATE_TOKEN" - PRIVATE_TOKEN_PARAM = :private_token SUDO_HEADER = "HTTP_SUDO" SUDO_PARAM = :sudo @@ -308,7 +306,7 @@ module API private def private_token - params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER] + params[APIGuard::PRIVATE_TOKEN_PARAM] || env[APIGuard::PRIVATE_TOKEN_HEADER] end def warden @@ -323,18 +321,11 @@ module API warden.try(:authenticate) if %w[GET HEAD].include?(env['REQUEST_METHOD']) end - def find_user_by_private_token - token = private_token - return nil unless token.present? - - User.find_by_authentication_token(token) || User.find_by_personal_access_token(token) - end - def initial_current_user return @initial_current_user if defined?(@initial_current_user) - @initial_current_user ||= find_user_by_private_token - @initial_current_user ||= doorkeeper_guard + @initial_current_user ||= find_user_by_private_token(scopes: @scopes) + @initial_current_user ||= doorkeeper_guard(scopes: @scopes) @initial_current_user ||= find_user_from_warden unless @initial_current_user && Gitlab::UserAccess.new(@initial_current_user).allowed? -- cgit v1.2.1 From 468b47d47c004f7a7a2eef7dace2a0b808df9175 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 21 Dec 2016 15:19:00 +0100 Subject: Remove redundant pagination helpers after a bad merge [ci skip] --- lib/api/helpers.rb | 32 -------------------------------- 1 file changed, 32 deletions(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 15ac29b2625..106694e13df 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -356,38 +356,6 @@ module API @sudo_identifier ||= params[SUDO_PARAM] || env[SUDO_HEADER] end - def add_pagination_headers(paginated_data) - header 'X-Total', paginated_data.total_count.to_s - header 'X-Total-Pages', paginated_data.total_pages.to_s - header 'X-Per-Page', paginated_data.limit_value.to_s - header 'X-Page', paginated_data.current_page.to_s - header 'X-Next-Page', paginated_data.next_page.to_s - header 'X-Prev-Page', paginated_data.prev_page.to_s - header 'Link', pagination_links(paginated_data) - end - - def pagination_links(paginated_data) - request_url = request.url.split('?').first - request_params = params.clone - request_params[:per_page] = paginated_data.limit_value - - links = [] - - request_params[:page] = paginated_data.current_page - 1 - links << %(<#{request_url}?#{request_params.to_query}>; rel="prev") unless paginated_data.first_page? - - request_params[:page] = paginated_data.current_page + 1 - links << %(<#{request_url}?#{request_params.to_query}>; rel="next") unless paginated_data.last_page? - - request_params[:page] = 1 - links << %(<#{request_url}?#{request_params.to_query}>; rel="first") - - request_params[:page] = paginated_data.total_pages - links << %(<#{request_url}?#{request_params.to_query}>; rel="last") - - links.join(', ') - end - def secret_token Gitlab::Shell.secret_token end -- cgit v1.2.1 From 3ef4f74b1acc9399db320b53dffc592542de0126 Mon Sep 17 00:00:00 2001 From: Markus Koller Date: Tue, 22 Nov 2016 17:58:10 +0100 Subject: Add more storage statistics This adds counters for build artifacts and LFS objects, and moves the preexisting repository_size and commit_count from the projects table into a new project_statistics table. The counters are displayed in the administration area for projects and groups, and also available through the API for admins (on */all) and normal users (on */owned) The statistics are updated through ProjectCacheWorker, which can now do more granular updates with the new :statistics argument. --- lib/api/helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 4be659fc20b..fe00c83bff3 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -248,7 +248,7 @@ module API rack_response({ 'message' => '500 Internal Server Error' }.to_json, 500) end - # Projects helpers + # project helpers def filter_projects(projects) if params[:search].present? -- cgit v1.2.1 From 1b109c99a4802e2cc6598d19248013c0dc152d7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 23 Dec 2016 17:03:25 +0100 Subject: Fix a Grape deprecation, use `#request_method` instead of `#route_method` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- lib/api/helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 4be659fc20b..151d7400549 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -96,7 +96,7 @@ module API end def authenticate_non_get! - authenticate! unless %w[GET HEAD].include?(route.route_method) + authenticate! unless %w[GET HEAD].include?(route.request_method) end def authenticate_by_gitlab_shell_token! -- cgit v1.2.1 From 8c9a4ed373f4b517aeae669e64023dc52c8d704a Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 4 Jan 2017 17:46:56 +0800 Subject: WIP: Add tests and make sure that headers are set * We realized that headers were not set whenever we give 204 because `render_api_error!` doesn't preserve the headers. * We also realized that `update_runner_info` would be called in POST /builds/register every time therefore runner is updated every time, ticking the queue, making this last_update didn't work very well, and the test would be failing due to that. --- lib/api/helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 746849ef4c0..3324001c468 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -229,7 +229,7 @@ module API end def render_api_error!(message, status) - error!({ 'message' => message }, status) + error!({ 'message' => message }, status, header) end def handle_api_exception(exception) -- cgit v1.2.1 From 1cccb378316d82ffdf4063b5aa3ecb5146d8fd79 Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Mon, 16 Jan 2017 00:38:25 +0100 Subject: Fix a API deprecation warning --- lib/api/helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 20b5bc1502a..eb2d370c68e 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -294,7 +294,7 @@ module API header['X-Sendfile'] = path body else - file FileStreamer.new(path) + path end end -- cgit v1.2.1 From 0f3c9355c1b57a56b4027df4deb78a2520596b15 Mon Sep 17 00:00:00 2001 From: Ruben Davila Date: Wed, 18 Jan 2017 10:48:16 -0600 Subject: Add some API endpoints for time tracking. New endpoints are: POST :project_id/(issues|merge_requests)/(:issue_id|:merge_request_id)/time_estimate" POST :project_id/(issues|merge_requests)/(:issue_id|:merge_request_id)/reset_time_estimate" POST :project_id/(issues|merge_requests)/(:issue_id|:merge_request_id)/add_spent_time" POST :project_id/(issues|merge_requests)/(:issue_id|:merge_request_id)/reset_spent_time" GET :project_id/(issues|merge_requests)/(:issue_id|:merge_request_id)/time_stats" --- lib/api/helpers.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 20b5bc1502a..0fcb352000a 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -86,6 +86,10 @@ module API IssuesFinder.new(current_user, project_id: user_project.id).find(id) end + def find_project_merge_request(id) + MergeRequestsFinder.new(current_user, project_id: user_project.id).find(id) + end + def authenticate! unauthorized! unless current_user end -- cgit v1.2.1 From 31af6be076635e6c6936564558bb977740d39eb0 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 19 Jan 2017 23:31:03 +0100 Subject: Fix specs --- lib/api/helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 49c5f0652ab..6b81fbf294e 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -226,7 +226,7 @@ module API end def render_api_error!(message, status) - error!({ 'message' => message }, status, header) + error!({ 'message' => message }, status) end def handle_api_exception(exception) -- cgit v1.2.1 From d8e440c8e05472b44a8e6cc59aecb6c53ba5a1d7 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 19 Jan 2017 23:41:12 +0100 Subject: Fix specs --- lib/api/helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 6b81fbf294e..49c5f0652ab 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -226,7 +226,7 @@ module API end def render_api_error!(message, status) - error!({ 'message' => message }, status) + error!({ 'message' => message }, status, header) end def handle_api_exception(exception) -- cgit v1.2.1 From 3a5df1d8fc518900d8e33a6be8a2243e399c754a Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 3 Jan 2017 18:03:13 +0000 Subject: Merge branch 'fix-api-mr-permissions' into 'security' Ensure that only privileged users can access merge requests in the API See merge request !2053 --- lib/api/helpers.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 49c5f0652ab..a1d7b323f4f 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -90,6 +90,12 @@ module API MergeRequestsFinder.new(current_user, project_id: user_project.id).find(id) end + def find_merge_request_with_access(id, access_level = :read_merge_request) + merge_request = user_project.merge_requests.find(id) + authorize! access_level, merge_request + merge_request + end + def authenticate! unauthorized! unless current_user end -- cgit v1.2.1