From 55d980905540bae6d02ac104dac5c23e96dd5711 Mon Sep 17 00:00:00 2001 From: Ciro Santilli Date: Tue, 30 Sep 2014 16:39:37 +0200 Subject: [PATCH 01/19] Convert Javascript links to buttons. --- app/views/admin/projects/index.html.haml | 2 +- app/views/explore/groups/index.html.haml | 2 +- app/views/explore/projects/index.html.haml | 2 +- app/views/groups/group_members/_group_member.html.haml | 3 ++- app/views/groups/members.html.haml | 2 +- app/views/projects/branches/index.html.haml | 2 +- app/views/search/_filter.html.haml | 4 ++-- app/views/shared/_issuable_filter.html.haml | 8 ++++---- app/views/shared/_sort_dropdown.html.haml | 2 +- features/steps/groups.rb | 2 +- 10 files changed, 15 insertions(+), 14 deletions(-) diff --git a/app/views/admin/projects/index.html.haml b/app/views/admin/projects/index.html.haml index aa59f38d21..c9271d1dad 100644 --- a/app/views/admin/projects/index.html.haml +++ b/app/views/admin/projects/index.html.haml @@ -44,7 +44,7 @@ Projects (#{@projects.total_count}) .panel-head-actions .dropdown.inline - %a.dropdown-toggle.btn{href: '#', "data-toggle" => "dropdown"} + %button.dropdown-toggle.btn{type: 'button', 'data-toggle' => 'dropdown'} %span.light sort: - if @sort.present? = @sort.humanize diff --git a/app/views/explore/groups/index.html.haml b/app/views/explore/groups/index.html.haml index 9b1d7d0416..c3df924433 100644 --- a/app/views/explore/groups/index.html.haml +++ b/app/views/explore/groups/index.html.haml @@ -8,7 +8,7 @@ .pull-right .dropdown.inline - %a.dropdown-toggle.btn{href: '#', "data-toggle" => "dropdown"} + %button.dropdown-toggle.btn{type: 'button', 'data-toggle' => 'dropdown'} %span.light sort: - if @sort.present? = @sort.humanize diff --git a/app/views/explore/projects/index.html.haml b/app/views/explore/projects/index.html.haml index 02586077d8..0c74517151 100644 --- a/app/views/explore/projects/index.html.haml +++ b/app/views/explore/projects/index.html.haml @@ -8,7 +8,7 @@ .pull-right .dropdown.inline - %a.dropdown-toggle.btn{href: '#', "data-toggle" => "dropdown"} + %button.dropdown-toggle.btn{type: 'button', 'data-toggle' => 'dropdown'} %span.light sort: - if @sort.present? = @sort.humanize diff --git a/app/views/groups/group_members/_group_member.html.haml b/app/views/groups/group_members/_group_member.html.haml index d05016d9c3..21029c3a07 100644 --- a/app/views/groups/group_members/_group_member.html.haml +++ b/app/views/groups/group_members/_group_member.html.haml @@ -14,7 +14,8 @@ %strong= member.human_access - if show_controls - if can?(current_user, :modify, member) - = link_to '#', class: "btn-tiny btn js-toggle-button", title: 'Edit access level' do + = button_tag class: "btn-tiny btn js-toggle-button", + title: 'Edit access level', type: 'button' do %i.fa.fa-pencil-square-o - if can?(current_user, :destroy, member) - if current_user == member.user diff --git a/app/views/groups/members.html.haml b/app/views/groups/members.html.haml index d2ebcdab7e..688c22e962 100644 --- a/app/views/groups/members.html.haml +++ b/app/views/groups/members.html.haml @@ -17,7 +17,7 @@ - if current_user && current_user.can?(:manage_group, @group) .pull-right - = link_to '#', class: 'btn btn-new js-toggle-button' do + = button_tag class: 'btn btn-new js-toggle-button', type: 'button' do Add members %i.fa.fa-chevron-down diff --git a/app/views/projects/branches/index.html.haml b/app/views/projects/branches/index.html.haml index d2aefd815a..02f5fffcd6 100644 --- a/app/views/projects/branches/index.html.haml +++ b/app/views/projects/branches/index.html.haml @@ -8,7 +8,7 @@ New branch   .dropdown.inline - %a.dropdown-toggle.btn{href: '#', "data-toggle" => "dropdown"} + %button.dropdown-toggle.btn{type: 'button', 'data-toggle' => 'dropdown'} %span.light sort: - if @sort.present? = @sort.humanize diff --git a/app/views/search/_filter.html.haml b/app/views/search/_filter.html.haml index eca69ce50b..c635c04fb8 100644 --- a/app/views/search/_filter.html.haml +++ b/app/views/search/_filter.html.haml @@ -1,5 +1,5 @@ .dropdown.inline - %a.dropdown-toggle.btn.btn-small{href: '#', "data-toggle" => "dropdown"} + %button.dropdown-toggle.btn.btn-small{type: 'button', 'data-toggle' => 'dropdown'} %i.fa.fa-tags %span.light Group: - if @group.present? @@ -17,7 +17,7 @@ = group.name .dropdown.inline.prepend-left-10.project-filter - %a.dropdown-toggle.btn.btn-small{href: '#', "data-toggle" => "dropdown"} + %button.dropdown-toggle.btn.btn-small{type: 'button', 'data-toggle' => 'dropdown'} %i.fa.fa-tags %span.light Project: - if @project.present? diff --git a/app/views/shared/_issuable_filter.html.haml b/app/views/shared/_issuable_filter.html.haml index 4f683258fa..0d5fdb120d 100644 --- a/app/views/shared/_issuable_filter.html.haml +++ b/app/views/shared/_issuable_filter.html.haml @@ -15,7 +15,7 @@ All .dropdown.inline.assignee-filter - %a.dropdown-toggle.btn{href: '#', "data-toggle" => "dropdown"} + %button.dropdown-toggle.btn{type: 'button', 'data-toggle' => 'dropdown'} %i.fa.fa-user %span.light assignee: - if @assignee.present? @@ -38,7 +38,7 @@ = user.name .dropdown.inline.prepend-left-10.author-filter - %a.dropdown-toggle.btn{href: '#', "data-toggle" => "dropdown"} + %button.dropdown-toggle.btn{type: 'button', 'data-toggle' => 'dropdown'} %i.fa.fa-user %span.light author: - if @author.present? @@ -61,7 +61,7 @@ = user.name .dropdown.inline.prepend-left-10.milestone-filter - %a.dropdown-toggle.btn{href: '#', "data-toggle" => "dropdown"} + %button.dropdown-toggle.btn{type: 'button', 'data-toggle' => 'dropdown'} %i.fa.fa-clock-o %span.light milestone: - if @milestone.present? @@ -85,7 +85,7 @@ - if @project .dropdown.inline.prepend-left-10.labels-filter - %a.dropdown-toggle.btn{href: '#', "data-toggle" => "dropdown"} + %button.dropdown-toggle.btn{type: 'button', 'data-toggle' => 'dropdown'} %i.fa.fa-tags %span.light label: - if params[:label_name].present? diff --git a/app/views/shared/_sort_dropdown.html.haml b/app/views/shared/_sort_dropdown.html.haml index 93ed9b6733..00c95bf302 100644 --- a/app/views/shared/_sort_dropdown.html.haml +++ b/app/views/shared/_sort_dropdown.html.haml @@ -1,5 +1,5 @@ .dropdown.inline.prepend-left-10 - %a.dropdown-toggle.btn{href: '#', "data-toggle" => "dropdown"} + %button.dropdown-toggle.btn{type: 'button', 'data-toggle' => 'dropdown'} %span.light sort: - if @sort.present? = @sort diff --git a/features/steps/groups.rb b/features/steps/groups.rb index f09d751dba..b752d1edb2 100644 --- a/features/steps/groups.rb +++ b/features/steps/groups.rb @@ -29,7 +29,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps step 'I select user "Mary Jane" from list with role "Reporter"' do user = User.find_by(name: "Mary Jane") || create(:user, name: "Mary Jane") - click_link 'Add members' + click_button 'Add members' within ".users-group-form" do select2(user.id, from: "#user_ids", multiple: true) select "Reporter", from: "access_level" From e363f2e67544e210e92acc06a5af90d91c0aa684 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 24 Feb 2015 06:50:40 +0000 Subject: [PATCH 02/19] Fix merge request URL passed to Webhooks. Previously the symbol "url" in the object_attributes hash would always be nil. --- CHANGELOG | 1 + lib/gitlab/url_builder.rb | 9 +++++++++ spec/lib/gitlab/url_builder_spec.rb | 8 ++++++++ 3 files changed, 18 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index d879ee8572..4a5c27b06b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.9.0 (unreleased) + - Fix merge request URL passed to Webhooks. (Stan Hu) - Move labels/milestones tabs to sidebar - Upgrade Rails gem to version 4.1.9. - Improve error messages for file edit failures diff --git a/lib/gitlab/url_builder.rb b/lib/gitlab/url_builder.rb index e7153cc322..7ab3f090a8 100644 --- a/lib/gitlab/url_builder.rb +++ b/lib/gitlab/url_builder.rb @@ -10,6 +10,8 @@ module Gitlab case @type when :issue issue_url(id) + when :merge_request + merge_request_url(id) end end @@ -22,5 +24,12 @@ module Gitlab project_id: issue.project, host: Gitlab.config.gitlab['url']) end + + def merge_request_url(id) + merge_request = MergeRequest.find(id) + project_merge_request_url(id: merge_request.id, + project_id: merge_request.project, + host: Gitlab.config.gitlab['url']) + end end end diff --git a/spec/lib/gitlab/url_builder_spec.rb b/spec/lib/gitlab/url_builder_spec.rb index 716430340b..518239fab6 100644 --- a/spec/lib/gitlab/url_builder_spec.rb +++ b/spec/lib/gitlab/url_builder_spec.rb @@ -8,4 +8,12 @@ describe Gitlab::UrlBuilder do expect(url).to eq "#{Settings.gitlab['url']}/#{issue.project.path_with_namespace}/issues/#{issue.iid}" end end + + describe 'When asking for an merge request' do + it 'returns the merge request url' do + merge_request = create(:merge_request) + url = Gitlab::UrlBuilder.new(:merge_request).build(merge_request.id) + expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.to_param}/merge_requests/#{merge_request.id}" + end + end end From dd37a10df44bd1771aa8b163fd857628d03842d9 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 23 Feb 2015 23:14:57 +0100 Subject: [PATCH 03/19] Don't leak information about private project existence via Git-over-SSH/HTTP. --- lib/api/internal.rb | 39 +++++++++++++----------- lib/gitlab/backend/grack_auth.rb | 52 +++++++++++++++++--------------- 2 files changed, 50 insertions(+), 41 deletions(-) diff --git a/lib/api/internal.rb b/lib/api/internal.rb index ba3fe619b9..753d0fcbd9 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -16,6 +16,17 @@ module API # post "/allowed" do status 200 + + actor = if params[:key_id] + Key.find_by(id: params[:key_id]) + elsif params[:user_id] + User.find_by(id: params[:user_id]) + end + + unless actor + return Gitlab::GitAccessStatus.new(false, 'No such user or key') + end + project_path = params[:project] # Check for *.wiki repositories. @@ -32,26 +43,20 @@ module API project = Project.find_with_namespace(project_path) - unless project - return Gitlab::GitAccessStatus.new(false, 'No such project') + if project + status = access.check( + actor, + params[:action], + project, + params[:changes] + ) end - actor = if params[:key_id] - Key.find_by(id: params[:key_id]) - elsif params[:user_id] - User.find_by(id: params[:user_id]) - end - - unless actor - return Gitlab::GitAccessStatus.new(false, 'No such user or key') + if project && status && status.allowed? + status + else + Gitlab::GitAccessStatus.new(false, 'No such project') end - - access.check( - actor, - params[:action], - project, - params[:changes] - ) end # diff --git a/lib/gitlab/backend/grack_auth.rb b/lib/gitlab/backend/grack_auth.rb index dc4b945f9d..ee877e099b 100644 --- a/lib/gitlab/backend/grack_auth.rb +++ b/lib/gitlab/backend/grack_auth.rb @@ -10,8 +10,9 @@ module Grack @request = Rack::Request.new(env) @auth = Request.new(env) - # Need this patch due to the rails mount + @gitlab_ci = false + # Need this patch due to the rails mount # Need this if under RELATIVE_URL_ROOT unless Gitlab.config.gitlab.relative_url_root.empty? # If website is mounted using relative_url_root need to remove it first @@ -22,8 +23,12 @@ module Grack @env['SCRIPT_NAME'] = "" - if project - auth! + auth! + + if project && authorized_request? + @app.call(env) + elsif @user.nil? && !@gitlab_ci + unauthorized else render_not_found end @@ -32,35 +37,30 @@ module Grack private def auth! - if @auth.provided? - return bad_request unless @auth.basic? + return unless @auth.provided? - # Authentication with username and password - login, password = @auth.credentials + return bad_request unless @auth.basic? - # Allow authentication for GitLab CI service - # if valid token passed - if gitlab_ci_request?(login, password) - return @app.call(env) - end + # Authentication with username and password + login, password = @auth.credentials - @user = authenticate_user(login, password) - - if @user - Gitlab::ShellEnv.set_env(@user) - @env['REMOTE_USER'] = @auth.username - end + # Allow authentication for GitLab CI service + # if valid token passed + if gitlab_ci_request?(login, password) + @gitlab_ci = true + return end - if authorized_request? - @app.call(env) - else - unauthorized + @user = authenticate_user(login, password) + + if @user + Gitlab::ShellEnv.set_env(@user) + @env['REMOTE_USER'] = @auth.username end end def gitlab_ci_request?(login, password) - if login == "gitlab-ci-token" && project.gitlab_ci? + if login == "gitlab-ci-token" && project && project.gitlab_ci? token = project.gitlab_ci_service.token if token.present? && token == password && git_cmd == 'git-upload-pack' @@ -107,6 +107,8 @@ module Grack end def authorized_request? + return true if @gitlab_ci + case git_cmd when *Gitlab::GitAccess::DOWNLOAD_COMMANDS if user @@ -141,7 +143,9 @@ module Grack end def project - @project ||= project_by_path(@request.path_info) + return @project if defined?(@project) + + @project = project_by_path(@request.path_info) end def project_by_path(path) From b8c9257fb1dc70cd65a14cb7dec62455ea54e394 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 2 Mar 2015 15:05:23 -0800 Subject: [PATCH 04/19] Fix bug where editing a comment with "+1" or "-1" would cause a server error Closes #1151 --- CHANGELOG | 1 + app/controllers/projects/notes_controller.rb | 8 +++++++- app/helpers/notes_helper.rb | 6 +++--- app/views/projects/notes/_edit_form.html.haml | 1 + app/views/projects/notes/_form.html.haml | 2 +- features/project/commits/comments.feature | 6 ++++++ features/project/issues/issues.feature | 9 +++++++++ features/steps/shared/note.rb | 17 +++++++++++++++++ 8 files changed, 45 insertions(+), 5 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 6bd93b8cd4..e26d4ab690 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.9.0 (unreleased) + - Fix bug that caused a server error when editing a comment to "+1" or "-1" (Stan Hu) - Move labels/milestones tabs to sidebar - Upgrade Rails gem to version 4.1.9. - Improve error messages for file edit failures diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 2f1d631c14..868629a0bc 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -3,10 +3,10 @@ class Projects::NotesController < Projects::ApplicationController before_filter :authorize_read_note! before_filter :authorize_write_note!, only: [:create] before_filter :authorize_admin_note!, only: [:update, :destroy] + before_filter :find_current_user_notes, except: [:destroy, :delete_attachment] def index current_fetched_at = Time.now.to_i - @notes = NotesFinder.new.execute(project, current_user, params) notes_json = { notes: [], last_fetched_at: current_fetched_at } @@ -116,4 +116,10 @@ class Projects::NotesController < Projects::ApplicationController :attachment, :line_code, :commit_id ) end + + private + + def find_current_user_notes + @notes = NotesFinder.new.execute(project, current_user, params) + end end diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index 92ecb2abe4..ab44fa6ee4 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -4,9 +4,9 @@ module NotesHelper (@noteable.class.name == note.noteable_type && !note.for_diff_line?) end - def note_target_fields - hidden_field_tag(:target_type, @target_type) + - hidden_field_tag(:target_id, @target_id) + def note_target_fields(note) + hidden_field_tag(:target_type, note.noteable.class.name.underscore) + + hidden_field_tag(:target_id, note.noteable.id) end def link_to_commit_diff_line_note(note) diff --git a/app/views/projects/notes/_edit_form.html.haml b/app/views/projects/notes/_edit_form.html.haml index b51ca79541..acb3991d29 100644 --- a/app/views/projects/notes/_edit_form.html.haml +++ b/app/views/projects/notes/_edit_form.html.haml @@ -1,5 +1,6 @@ .note-edit-form = form_for note, url: namespace_project_note_path(@project.namespace, @project, note), method: :put, remote: true, authenticity_token: true do |f| + = note_target_fields(note) = render layout: 'projects/md_preview', locals: { preview_class: "note-text" } do = render 'projects/zen', f: f, attr: :note, classes: 'note_text js-note-text' diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml index 4476337cb1..be96c30214 100644 --- a/app/views/projects/notes/_form.html.haml +++ b/app/views/projects/notes/_form.html.haml @@ -1,5 +1,5 @@ = form_for [@project.namespace.becomes(Namespace), @project, @note], remote: true, html: { :'data-type' => 'json', multipart: true, id: nil, class: "new_note js-new-note-form common-note-form gfm-form" }, authenticity_token: true do |f| - = note_target_fields + = note_target_fields(@note) = f.hidden_field :commit_id = f.hidden_field :line_code = f.hidden_field :noteable_id diff --git a/features/project/commits/comments.feature b/features/project/commits/comments.feature index afcf0fdbb0..c41075d7ad 100644 --- a/features/project/commits/comments.feature +++ b/features/project/commits/comments.feature @@ -41,3 +41,9 @@ Feature: Project Commits Comments Given I leave a comment like "XML attached" And I delete a comment Then I should not see a comment saying "XML attached" + + @javascript + Scenario: I can edit a comment with +1 + Given I leave a comment like "XML attached" + And I edit the last comment with a +1 + Then I should see +1 in the description diff --git a/features/project/issues/issues.feature b/features/project/issues/issues.feature index 28ea44530f..283979204d 100644 --- a/features/project/issues/issues.feature +++ b/features/project/issues/issues.feature @@ -139,6 +139,15 @@ Feature: Project Issues And I leave a comment with task markdown Then I should not see task checkboxes in the comment + @javascript + Scenario: Issue notes should be editable with +1 + Given project "Shop" has "Tasks-open" open issue with task markdown + When I visit issue page "Tasks-open" + And I leave a comment with a header containing "Comment with a header" + Then The comment with the header should not have an ID + And I edit the last comment with a +1 + Then I should see +1 in the description + # Task status in issues list Scenario: Issues list should display task status diff --git a/features/steps/shared/note.rb b/features/steps/shared/note.rb index 4577305695..583746d447 100644 --- a/features/steps/shared/note.rb +++ b/features/steps/shared/note.rb @@ -135,4 +135,21 @@ module SharedNote 'li.note div.timeline-content input[type="checkbox"]' ) end + + step 'I edit the last comment with a +1' do + find(".note").hover + find('.js-note-edit').click + + within(".current-note-edit-form") do + fill_in 'note[note]', with: '+1 Awesome!' + click_button 'Save Comment' + sleep 0.05 + end + end + + step 'I should see +1 in the description' do + within(".note") do + page.should have_content("+1 Awesome!") + end + end end From 5b2b9a1f1fcfa323ae56d0d0214ff61bb6088321 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 2 Mar 2015 17:28:47 -0800 Subject: [PATCH 05/19] Add brakeman gem --- Gemfile | 1 + Gemfile.lock | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/Gemfile b/Gemfile index 47e6fe95f3..01c02b5c8d 100644 --- a/Gemfile +++ b/Gemfile @@ -199,6 +199,7 @@ gem "virtus" gem 'addressable' group :development do + gem 'brakeman', require: false gem "annotate", "~> 2.6.0.beta2" gem "letter_opener" gem 'quiet_assets', '~> 1.0.1' diff --git a/Gemfile.lock b/Gemfile.lock index 37880c45a2..102d1a2887 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -63,6 +63,16 @@ GEM bootstrap-sass (3.3.3) autoprefixer-rails (>= 5.0.0.1) sass (>= 3.2.19) + brakeman (3.0.1) + erubis (~> 2.6) + fastercsv (~> 1.5) + haml (>= 3.0, < 5.0) + highline (~> 1.6.20) + multi_json (~> 1.2) + ruby2ruby (~> 2.1.1) + ruby_parser (~> 3.5.0) + sass (~> 3.0) + terminal-table (~> 1.4) browser (0.7.2) builder (3.2.2) byebug (3.2.0) @@ -154,6 +164,7 @@ GEM multipart-post (~> 1.2.0) faraday_middleware (0.9.0) faraday (>= 0.7.4, < 0.9) + fastercsv (1.5.5) ffaker (1.22.1) ffi (1.9.3) fog (1.21.0) @@ -258,6 +269,7 @@ GEM haml (>= 3.1, < 5.0) railties (>= 4.0.1) hashie (2.1.2) + highline (1.6.21) hike (1.2.3) hipchat (1.4.0) httparty @@ -496,6 +508,11 @@ GEM rainbow (>= 1.99.1, < 3.0) ruby-progressbar (~> 1.4) ruby-progressbar (1.7.1) + ruby2ruby (2.1.3) + ruby_parser (~> 3.1) + sexp_processor (~> 4.0) + ruby_parser (3.5.0) + sexp_processor (~> 4.1) rubyntlm (0.4.0) rubypants (0.2.0) rugged (0.21.4) @@ -521,6 +538,7 @@ GEM select2-rails (3.5.2) thor (~> 0.14) settingslogic (2.0.9) + sexp_processor (4.4.5) shoulda-matchers (2.7.0) activesupport (>= 3.0.0) sidekiq (3.3.0) @@ -572,6 +590,7 @@ GEM temple (0.6.7) term-ansicolor (1.2.2) tins (~> 0.8) + terminal-table (1.4.5) test_after_commit (0.2.2) therubyracer (0.12.0) libv8 (~> 3.16.14.0) @@ -651,6 +670,7 @@ DEPENDENCIES better_errors binding_of_caller bootstrap-sass (~> 3.0) + brakeman browser byebug cal-heatmap-rails (~> 0.0.1) From cc877c53abbb1a8799b35dddac35b963dd5ecfdd Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 2 Mar 2015 17:41:05 -0800 Subject: [PATCH 06/19] Add rake task for brakeman --- lib/tasks/brakeman.rake | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 lib/tasks/brakeman.rake diff --git a/lib/tasks/brakeman.rake b/lib/tasks/brakeman.rake new file mode 100644 index 0000000000..0a1e76ea82 --- /dev/null +++ b/lib/tasks/brakeman.rake @@ -0,0 +1,9 @@ +desc 'Security check via brakeman' +task :brakeman do + if system("brakeman -w3 -z") + exit 0 + else + puts 'Security check failed' + exit 1 + end +end From 16e899ca8b44a87883464ada507f521d02548fe2 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 2 Mar 2015 18:11:50 -0800 Subject: [PATCH 07/19] Add brakeman rake task and improve code security --- .../projects/imports_controller.rb | 2 +- .../projects/team_members_controller.rb | 8 +--- app/controllers/projects/wikis_controller.rb | 2 +- app/controllers/uploads_controller.rb | 41 ++++++++++++++++--- lib/tasks/brakeman.rake | 2 +- 5 files changed, 40 insertions(+), 15 deletions(-) diff --git a/app/controllers/projects/imports_controller.rb b/app/controllers/projects/imports_controller.rb index e2f957a640..79d9910ce8 100644 --- a/app/controllers/projects/imports_controller.rb +++ b/app/controllers/projects/imports_controller.rb @@ -26,7 +26,7 @@ class Projects::ImportsController < Projects::ApplicationController def show unless @project.import_in_progress? if @project.import_finished? - redirect_to(@project) and return + redirect_to(project_path(@project)) and return else redirect_to new_namespace_project_import_path(@project.namespace, @project) && return diff --git a/app/controllers/projects/team_members_controller.rb b/app/controllers/projects/team_members_controller.rb index 71b0ab7ee8..f8a248ed72 100644 --- a/app/controllers/projects/team_members_controller.rb +++ b/app/controllers/projects/team_members_controller.rb @@ -15,15 +15,9 @@ class Projects::TeamMembersController < Projects::ApplicationController def create users = User.where(id: params[:user_ids].split(',')) - @project.team << [users, params[:access_level]] - if params[:redirect_to] - redirect_to params[:redirect_to] - else - redirect_to namespace_project_team_index_path(@project.namespace, - @project) - end + redirect_to namespace_project_team_index_path(@project.namespace, @project) end def update diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index 69824dca94..3392fbca91 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/app/controllers/projects/wikis_controller.rb @@ -97,7 +97,7 @@ class Projects::WikisController < Projects::ApplicationController @project_wiki.wiki rescue ProjectWiki::CouldNotCreateWikiError => ex flash[:notice] = "Could not create Wiki Repository at this time. Please try again later." - redirect_to @project + redirect_to project_path(@project) return false end diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index b096c3913e..810ac9f34b 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -3,22 +3,53 @@ class UploadsController < ApplicationController before_filter :authorize_access def show - model = params[:model].camelize.constantize.find(params[:id]) - uploader = model.send(params[:mounted_as]) + unless upload_model && upload_mount + return not_found! + end - return not_found! if model.respond_to?(:project) && !can?(current_user, :read_project, model.project) + model = upload_model.find(params[:id]) + uploader = model.send(upload_mount) - return redirect_to uploader.url unless uploader.file_storage? + if model.respond_to?(:project) && !can?(current_user, :read_project, model.project) + return not_found! + end - return not_found! unless uploader.file.exists? + unless uploader.file_storage? + return redirect_to uploader.url + end + + unless uploader.file.exists? + return not_found! + end disposition = uploader.image? ? 'inline' : 'attachment' send_file uploader.file.path, disposition: disposition end + private + def authorize_access unless params[:mounted_as] == 'avatar' authenticate_user! && reject_blocked! end end + + def upload_model + upload_models = { + user: User, + project: Project, + note: Note, + group: Group + } + + upload_models[params[:model].to_sym] + end + + def upload_mount + upload_mounts = %w(avatar attachment file) + + if upload_mounts.include?(params[:mounted_as]) + params[:mounted_as] + end + end end diff --git a/lib/tasks/brakeman.rake b/lib/tasks/brakeman.rake index 0a1e76ea82..abcb5f0ae4 100644 --- a/lib/tasks/brakeman.rake +++ b/lib/tasks/brakeman.rake @@ -1,6 +1,6 @@ desc 'Security check via brakeman' task :brakeman do - if system("brakeman -w3 -z") + if system("brakeman --skip-files lib/backup/repository.rb -w3 -z") exit 0 else puts 'Security check failed' From be165b18d0f3713a888767550ef66917c5a389ab Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 2 Mar 2015 18:22:37 -0800 Subject: [PATCH 08/19] Add brakeman and jasmine --- CHANGELOG | 1 + lib/tasks/test.rake | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 84bdea3097..6a28772097 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,6 +15,7 @@ v 7.9.0 (unreleased) - Fix mass-unassignment of issues (Robert Speicher) - Allow user confirmation to be skipped for new users via API - Add a service to send updates to an Irker gateway (Romain Coltel) + - Add brakeman (security scanner for Ruby on Rails) v 7.8.1 - Fix run of custom post receive hooks diff --git a/lib/tasks/test.rake b/lib/tasks/test.rake index 3ea9290a81..a39d964987 100644 --- a/lib/tasks/test.rake +++ b/lib/tasks/test.rake @@ -9,5 +9,5 @@ unless Rails.env.production? require 'coveralls/rake/task' Coveralls::RakeTask.new desc "GITLAB | Run all tests on CI with simplecov" - task :test_ci => [:rubocop, :spinach, :spec, 'coveralls:push'] + task :test_ci => [:rubocop, :brakeman, 'jasmine:ci', :spinach, :spec, 'coveralls:push'] end From f850cff4174bfe99a6f2ef0da365bf002990ad92 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 2 Mar 2015 18:34:29 -0800 Subject: [PATCH 09/19] Update ci setup documenation --- doc/development/ci_setup.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/doc/development/ci_setup.md b/doc/development/ci_setup.md index f417667754..f9b4886818 100644 --- a/doc/development/ci_setup.md +++ b/doc/development/ci_setup.md @@ -37,7 +37,10 @@ bundle install --deployment --path vendor/bundle (Setup) cp config/gitlab.yml.example config/gitlab.yml (Setup) bundle exec rake db:create (Setup) bundle exec rake spinach (Thread #1) -bundle exec rake spec (Thread #2) +bundle exec rake spec (thread #2) +bundle exec rake rubocop (thread #3) +bundle exec rake brakeman (thread #4) +bundle exec rake jasmine:ci (thread #5) ``` Use rubygems mirror. From 8348e1a9b57b042e97f14d3b4f7682806902efa1 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 2 Mar 2015 18:45:28 -0800 Subject: [PATCH 10/19] Enable ParenthesesAsGroupedExpression rule --- .rubocop.yml | 2 +- lib/api/entities.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index a4b5100819..53ca2ca219 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -877,7 +877,7 @@ Lint/ParenthesesAsGroupedExpression: Checks for method calls with a space before the opening parenthesis. StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#parens-no-spaces' - Enabled: false + Enabled: true Lint/RequireParentheses: Description: >- diff --git a/lib/api/entities.rb b/lib/api/entities.rb index af76f3c439..489be21078 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -143,7 +143,7 @@ module API class ProjectEntity < Grape::Entity expose :id, :iid - expose (:project_id) { |entity| entity.project.id } + expose(:project_id) { |entity| entity.project.id } expose :title, :description expose :state, :created_at, :updated_at end From f438791721360e547f3661a553f491d258013eb8 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 2 Mar 2015 23:06:59 -0800 Subject: [PATCH 11/19] Fix import check for case sensetive namespaces --- app/controllers/import/base_controller.rb | 2 +- app/models/namespace.rb | 5 +++++ spec/models/namespace_spec.rb | 10 ++++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/app/controllers/import/base_controller.rb b/app/controllers/import/base_controller.rb index 4df171dbcf..7dc0cac8d4 100644 --- a/app/controllers/import/base_controller.rb +++ b/app/controllers/import/base_controller.rb @@ -3,7 +3,7 @@ class Import::BaseController < ApplicationController private def get_or_create_namespace - existing_namespace = Namespace.find_by("path = ? OR name = ?", @target_namespace, @target_namespace) + existing_namespace = Namespace.find_by_path_or_name(@target_namespace) if existing_namespace if existing_namespace.owner == current_user diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 2c7ed37626..35280889a8 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -48,6 +48,11 @@ class Namespace < ActiveRecord::Base where('lower(path) = :value', value: path.downcase).first end + # Case insensetive search for namespace by path or name + def self.find_by_path_or_name(path) + find_by("lower(path) = :path OR lower(name) = :path", path: path.downcase) + end + def self.search(query) where("name LIKE :query OR path LIKE :query", query: "%#{query}%") end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 4e268f8d8f..ed6845c82c 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -75,4 +75,14 @@ describe Namespace do expect(namespace.rm_dir).to be_truthy end end + + describe :find_by_path_or_name do + before do + @namespace = create(:namespace, name: 'WoW', path: 'woW') + end + + it { expect(Namespace.find_by_path_or_name('wow')).to eq(@namespace) } + it { expect(Namespace.find_by_path_or_name('WOW')).to eq(@namespace) } + it { expect(Namespace.find_by_path_or_name('unknown')).to eq(nil) } + end end From 6f71f5bb1be3d3c8ea2364a7763c62421de28f91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A8s=20Koetsier?= Date: Tue, 3 Mar 2015 10:28:44 +0100 Subject: [PATCH 12/19] Fixed changelog for MR 8501 --- CHANGELOG | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 6a28772097..4e56fc8e19 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -16,6 +16,7 @@ v 7.9.0 (unreleased) - Allow user confirmation to be skipped for new users via API - Add a service to send updates to an Irker gateway (Romain Coltel) - Add brakeman (security scanner for Ruby on Rails) + - Slack username and channel options v 7.8.1 - Fix run of custom post receive hooks @@ -92,7 +93,6 @@ v 7.8.0 - Improve database performance for GitLab - Add Asana service (Jeremy Benoist) - Improve project web hooks with extra data - - Slack username and channel options v 7.7.2 - Update GitLab Shell to version 2.4.2 that fixes a bug when developers can push to protected branch From 0e11be40c39df66859ae0f3dc265cd903820c153 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 24 Feb 2015 16:05:39 +0100 Subject: [PATCH 13/19] Add tests for GrackAuth. --- spec/lib/gitlab/backend/grack_auth_spec.rb | 146 +++++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 spec/lib/gitlab/backend/grack_auth_spec.rb diff --git a/spec/lib/gitlab/backend/grack_auth_spec.rb b/spec/lib/gitlab/backend/grack_auth_spec.rb new file mode 100644 index 0000000000..768312f002 --- /dev/null +++ b/spec/lib/gitlab/backend/grack_auth_spec.rb @@ -0,0 +1,146 @@ +require "spec_helper" + +describe Grack::Auth do + let(:user) { create(:user) } + let(:project) { create(:project) } + + let(:app) { lambda { |env| [200, {}, "Success!"] } } + let!(:auth) { Grack::Auth.new(app) } + let(:env) { + { + "rack.input" => "", + "REQUEST_METHOD" => "GET", + "QUERY_STRING" => "service=git-upload-pack" + } + } + let(:status) { auth.call(env).first } + + describe "#call" do + context "when the project doesn't exist" do + before do + env["PATH_INFO"] = "doesnt/exist.git" + end + + context "when no authentication is provided" do + it "responds with status 401" do + expect(status).to eq(401) + end + end + + context "when username and password are provided" do + context "when authentication fails" do + before do + env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, "nope") + end + + it "responds with status 401" do + expect(status).to eq(401) + end + end + + context "when authentication succeeds" do + before do + env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password) + end + + it "responds with status 404" do + expect(status).to eq(404) + end + end + end + end + + context "when the project exists" do + before do + env["PATH_INFO"] = project.path_with_namespace + ".git" + end + + context "when the project is public" do + before do + project.update_attribute(:visibility_level, Project::PUBLIC) + end + + it "responds with status 200" do + expect(status).to eq(200) + end + end + + context "when the project is private" do + before do + project.update_attribute(:visibility_level, Project::PRIVATE) + end + + context "when no authentication is provided" do + it "responds with status 401" do + expect(status).to eq(401) + end + end + + context "when username and password are provided" do + context "when authentication fails" do + before do + env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, "nope") + end + + it "responds with status 401" do + expect(status).to eq(401) + end + end + + context "when authentication succeeds" do + before do + env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password) + end + + context "when the user has access to the project" do + before do + project.team << [user, :master] + end + + context "when the user is blocked" do + before do + user.block + project.team << [user, :master] + end + + it "responds with status 404" do + expect(status).to eq(404) + end + end + + context "when the user isn't blocked" do + it "responds with status 200" do + expect(status).to eq(200) + end + end + end + + context "when the user doesn't have access to the project" do + it "responds with status 404" do + expect(status).to eq(404) + end + end + end + end + + context "when a gitlab ci token is provided" do + let(:token) { "123" } + + before do + gitlab_ci_service = project.build_gitlab_ci_service + gitlab_ci_service.active = true + gitlab_ci_service.token = token + gitlab_ci_service.project_url = "http://google.com" + gitlab_ci_service.save + + env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials("gitlab-ci-token", token) + end + + it "responds with status 200" do + expect(status).to eq(200) + end + end + end + end + end +end From d513ca584aaed7ca2a1de2d2fbd2192422f13d81 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 24 Feb 2015 16:48:22 +0100 Subject: [PATCH 14/19] Revert "Merge branch 'go-get-workaround-nginx' of https://github.com/mattes/gitlabhq into mattes-go-get-workaround-nginx" This reverts commit 51349ca3c83c56e072f87253d375316f7164b49a, reversing changes made to b180476bd69bdf99b1727b041116fa8447c0201f. --- app/views/layouts/_head.html.haml | 7 +++++++ lib/support/nginx/gitlab | 10 ---------- lib/support/nginx/gitlab-ssl | 10 ---------- 3 files changed, 7 insertions(+), 20 deletions(-) diff --git a/app/views/layouts/_head.html.haml b/app/views/layouts/_head.html.haml index d12145651a..bece8061fb 100644 --- a/app/views/layouts/_head.html.haml +++ b/app/views/layouts/_head.html.haml @@ -1,5 +1,12 @@ %head %meta{charset: "utf-8"} + + -# Go repository retrieval support + -# Need to be the fist thing in the head + -# Since Go is using an XML parser to process HTML5 + -# https://github.com/gitlabhq/gitlabhq/pull/5958#issuecomment-45397555 + - if controller_name == 'projects' && action_name == 'show' + %meta{name: "go-import", content: "#{@project.web_url_without_protocol} git #{@project.web_url}.git"} %meta{content: "GitLab Community Edition", name: "description"} %title diff --git a/lib/support/nginx/gitlab b/lib/support/nginx/gitlab index fd5b266478..62a4276536 100644 --- a/lib/support/nginx/gitlab +++ b/lib/support/nginx/gitlab @@ -77,16 +77,6 @@ server { proxy_pass http://gitlab; } - ## If ``go get`` detected, return go-import meta tag. - ## This works for public and for private repositories. - ## See also http://golang.org/cmd/go/#hdr-Remote_import_paths - if ($http_user_agent ~* "Go") { - return 200 " - - - "; - } - ## If a file, which is not found in the root folder is requested, ## then the proxy passes the request to the upsteam (gitlab unicorn). location @gitlab { diff --git a/lib/support/nginx/gitlab-ssl b/lib/support/nginx/gitlab-ssl index a9699bac61..2aefc94469 100644 --- a/lib/support/nginx/gitlab-ssl +++ b/lib/support/nginx/gitlab-ssl @@ -123,16 +123,6 @@ server { proxy_pass http://gitlab; } - ## If ``go get`` detected, return go-import meta tag. - ## This works for public and for private repositories. - ## See also http://golang.org/cmd/go/#hdr-Remote_import_paths - if ($http_user_agent ~* "Go") { - return 200 " - - - "; - } - ## If a file, which is not found in the root folder is requested, ## then the proxy passes the request to the upsteam (gitlab unicorn). location @gitlab { From 3702c4ad80614d71fc5ac3ea1af7c3789ec8146d Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 24 Feb 2015 17:02:11 +0100 Subject: [PATCH 15/19] Render go-import meta tag for private repos. --- app/controllers/projects_controller.rb | 11 +++++++++++ app/views/layouts/_head.html.haml | 7 ------- app/views/projects/go_import.html.haml | 5 +++++ spec/controllers/projects_controller_spec.rb | 16 ++++++++++++++++ 4 files changed, 32 insertions(+), 7 deletions(-) create mode 100644 app/views/projects/go_import.html.haml diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 5486a97e51..82b8a1cc13 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -1,4 +1,5 @@ class ProjectsController < ApplicationController + prepend_before_filter :render_go_import, only: [:show] skip_before_filter :authenticate_user!, only: [:show] before_filter :project, except: [:new, :create] before_filter :repository, except: [:new, :create] @@ -184,4 +185,14 @@ class ProjectsController < ApplicationController end end end + + def render_go_import + return unless params["go-get"] == "1" + + @namespace = params[:namespace_id] + @id = params[:project_id] || params[:id] + @id = @id.gsub(/\.git\Z/, "") + + render "go_import", layout: false + end end diff --git a/app/views/layouts/_head.html.haml b/app/views/layouts/_head.html.haml index bece8061fb..d12145651a 100644 --- a/app/views/layouts/_head.html.haml +++ b/app/views/layouts/_head.html.haml @@ -1,12 +1,5 @@ %head %meta{charset: "utf-8"} - - -# Go repository retrieval support - -# Need to be the fist thing in the head - -# Since Go is using an XML parser to process HTML5 - -# https://github.com/gitlabhq/gitlabhq/pull/5958#issuecomment-45397555 - - if controller_name == 'projects' && action_name == 'show' - %meta{name: "go-import", content: "#{@project.web_url_without_protocol} git #{@project.web_url}.git"} %meta{content: "GitLab Community Edition", name: "description"} %title diff --git a/app/views/projects/go_import.html.haml b/app/views/projects/go_import.html.haml new file mode 100644 index 0000000000..87ac75a350 --- /dev/null +++ b/app/views/projects/go_import.html.haml @@ -0,0 +1,5 @@ +!!! 5 +%html + %head + - web_url = [Gitlab.config.gitlab.url, @namespace, @id].join('/') + %meta{name: "go-import", content: "#{web_url.split('://')[1]} git #{web_url}.git"} diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 89bb35de8f..a1b82a3215 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -7,6 +7,22 @@ describe ProjectsController do let(:jpg) { fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') } let(:txt) { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') } + describe "GET show" do + + context "when requested by `go get`" do + render_views + + it "renders the go-import meta tag" do + get :show, "go-get" => "1", namespace_id: "bogus_namespace", id: "bogus_project" + + expect(response.body).to include("name='go-import'") + + content = "localhost/bogus_namespace/bogus_project git http://localhost/bogus_namespace/bogus_project.git" + expect(response.body).to include("content='#{content}'") + end + end + end + describe "POST #toggle_star" do it "toggles star if user is signed in" do sign_in(user) From 3102454a566292d6435fcaae8d21c6c2d26c0341 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 3 Mar 2015 13:52:13 +0100 Subject: [PATCH 16/19] Don't show Unassigned in user select when searching. --- .../javascripts/project_users_select.js.coffee | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/project_users_select.js.coffee b/app/assets/javascripts/project_users_select.js.coffee index 885f0d58a6..e22c7c11f1 100644 --- a/app/assets/javascripts/project_users_select.js.coffee +++ b/app/assets/javascripts/project_users_select.js.coffee @@ -11,14 +11,15 @@ class @ProjectUsersSelect Api.projectUsers project_id, query.term, (users) -> data = { results: users } - nullUser = { - name: 'Unassigned', - avatar: null, - username: 'none', - id: -1 - } + if query.term.length == 0 + nullUser = { + name: 'Unassigned', + avatar: null, + username: 'none', + id: -1 + } - data.results.unshift(nullUser) + data.results.unshift(nullUser) query.callback(data) From 10212c01fd18aa9961e86bd961475068a7596f00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Rosen=C3=B6gger?= Date: Tue, 3 Mar 2015 15:31:05 +0100 Subject: [PATCH 17/19] Count commits in branches as well in the commit calendar --- app/models/repository.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index bbf35f04bb..5b52739df2 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -146,7 +146,7 @@ class Repository end def timestamps_by_user_log(user) - args = %W(git log --author=#{user.email} --since=#{(Date.today - 1.year).to_s} --pretty=format:%cd --date=short) + args = %W(git log --author=#{user.email} --since=#{(Date.today - 1.year).to_s} --branches --pretty=format:%cd --date=short) dates = Gitlab::Popen.popen(args, path_to_repo).first.split("\n") if dates.present? From 3ff71897384b905218960c8562b681645ee21621 Mon Sep 17 00:00:00 2001 From: Sytse Sijbrandij Date: Tue, 3 Mar 2015 10:01:49 -0800 Subject: [PATCH 18/19] Clearly mark it as installation from source. --- doc/install/installation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/install/installation.md b/doc/install/installation.md index 28597fd39d..2b204c7247 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -1,4 +1,4 @@ -# Installation +# Installation from source ## Consider the Omnibus package installation From 2088cee935e47b569f0c79b10dcb2c506b666af3 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 3 Mar 2015 08:01:27 -0800 Subject: [PATCH 19/19] Fix URL builder to use GitlabRoutingHelper --- lib/gitlab/url_builder.rb | 18 +++++++----------- spec/lib/gitlab/url_builder_spec.rb | 2 +- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/lib/gitlab/url_builder.rb b/lib/gitlab/url_builder.rb index 7ab3f090a8..ab7c8ad89f 100644 --- a/lib/gitlab/url_builder.rb +++ b/lib/gitlab/url_builder.rb @@ -1,6 +1,7 @@ module Gitlab class UrlBuilder include Rails.application.routes.url_helpers + include GitlabRoutingHelper def initialize(type) @type = type @@ -9,27 +10,22 @@ module Gitlab def build(id) case @type when :issue - issue_url(id) + build_issue_url(id) when :merge_request - merge_request_url(id) + build_merge_request_url(id) end end private - def issue_url(id) + def build_issue_url(id) issue = Issue.find(id) - namespace_project_issue_url(namespace_id: issue.project.namespace, - id: issue.iid, - project_id: issue.project, - host: Gitlab.config.gitlab['url']) + issue_url(issue, host: Gitlab.config.gitlab['url']) end - def merge_request_url(id) + def build_merge_request_url(id) merge_request = MergeRequest.find(id) - project_merge_request_url(id: merge_request.id, - project_id: merge_request.project, - host: Gitlab.config.gitlab['url']) + merge_request_url(merge_request, host: Gitlab.config.gitlab['url']) end end end diff --git a/spec/lib/gitlab/url_builder_spec.rb b/spec/lib/gitlab/url_builder_spec.rb index 518239fab6..94b2fd5508 100644 --- a/spec/lib/gitlab/url_builder_spec.rb +++ b/spec/lib/gitlab/url_builder_spec.rb @@ -13,7 +13,7 @@ describe Gitlab::UrlBuilder do it 'returns the merge request url' do merge_request = create(:merge_request) url = Gitlab::UrlBuilder.new(:merge_request).build(merge_request.id) - expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.to_param}/merge_requests/#{merge_request.id}" + expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.path_with_namespace}/merge_requests/#{merge_request.iid}" end end end