From fd51f19c978023160ad759676a0363c12aea3fc8 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Thu, 22 Sep 2016 13:56:43 +0100 Subject: API: disable rails session auth for non-GET/HEAD requests --- lib/api/helpers.rb | 5 ++++- spec/requests/api/api_helpers_spec.rb | 39 +++++++++++++++++++++++++++++------ 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 714d4ea3dc6..8b8c4eb4d46 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -21,8 +21,11 @@ module API 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 ? warden.authenticate : nil + warden.try(:authenticate) if request.get? || request.head? end def find_user_by_private_token diff --git a/spec/requests/api/api_helpers_spec.rb b/spec/requests/api/api_helpers_spec.rb index e66faeed705..0f41f8dc7f1 100644 --- a/spec/requests/api/api_helpers_spec.rb +++ b/spec/requests/api/api_helpers_spec.rb @@ -10,7 +10,8 @@ describe API::Helpers, api: true do let(:key) { create(:key, user: user) } let(:params) { {} } - let(:env) { {} } + let(:env) { { 'REQUEST_METHOD' => 'GET' } } + let(:request) { Rack::Request.new(env) } def set_env(token_usr, identifier) clear_env @@ -52,17 +53,43 @@ describe API::Helpers, api: true do describe ".current_user" do subject { current_user } - describe "when authenticating via Warden" do + describe "Warden authentication" do before { doorkeeper_guard_returns false } - context "fails" do - it { is_expected.to be_nil } + context "with invalid credentials" do + context "GET request" do + before { env['REQUEST_METHOD'] = 'GET' } + it { is_expected.to be_nil } + end end - context "succeeds" do + context "with valid credentials" do before { warden_authenticate_returns user } - it { is_expected.to eq(user) } + context "GET request" do + before { env['REQUEST_METHOD'] = 'GET' } + it { is_expected.to eq(user) } + end + + context "HEAD request" do + before { env['REQUEST_METHOD'] = 'HEAD' } + it { is_expected.to eq(user) } + end + + context "PUT request" do + before { env['REQUEST_METHOD'] = 'PUT' } + it { is_expected.to be_nil } + end + + context "POST request" do + before { env['REQUEST_METHOD'] = 'POST' } + it { is_expected.to be_nil } + end + + context "DELETE request" do + before { env['REQUEST_METHOD'] = 'DELETE' } + it { is_expected.to be_nil } + end end end -- cgit v1.2.1 From af5e54f9ce4f491ccf605c7c74c137785da743a4 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Thu, 22 Sep 2016 17:07:57 +0100 Subject: Convert label creation from API to controller endpoint --- app/assets/javascripts/api.js | 9 +++++---- .../javascripts/boards/components/new_list_dropdown.js.es6 | 3 +-- app/assets/javascripts/create_label.js.es6 | 9 +++++---- app/assets/javascripts/labels_select.js | 7 ++++--- app/controllers/projects/labels_controller.rb | 10 ++++++++-- app/views/shared/issuable/_filter.html.haml | 2 +- app/views/shared/issuable/_label_dropdown.html.haml | 2 +- app/views/shared/issuable/_sidebar.html.haml | 2 +- 8 files changed, 26 insertions(+), 18 deletions(-) diff --git a/app/assets/javascripts/api.js b/app/assets/javascripts/api.js index 1cd2302111e..7e5e9fa9ae5 100644 --- a/app/assets/javascripts/api.js +++ b/app/assets/javascripts/api.js @@ -5,7 +5,7 @@ namespacesPath: "/api/:version/namespaces.json", groupProjectsPath: "/api/:version/groups/:id/projects.json", projectsPath: "/api/:version/projects.json?simple=true", - labelsPath: "/api/:version/projects/:id/labels", + labelsPath: "/:namespace_path/:project_path/labels", licensePath: "/api/:version/licenses/:key", gitignorePath: "/api/:version/gitignores/:key", gitlabCiYmlPath: "/api/:version/gitlab_ci_ymls/:key", @@ -65,13 +65,14 @@ return callback(projects); }); }, - newLabel: function(project_id, data, callback) { + newLabel: function(namespace_path, project_path, data, callback) { var url = Api.buildUrl(Api.labelsPath) - .replace(':id', project_id); + .replace(':namespace_path', namespace_path) + .replace(':project_path', project_path); return $.ajax({ url: url, type: "POST", - data: data, + data: {'label': data}, dataType: "json" }).done(function(label) { return callback(label); diff --git a/app/assets/javascripts/boards/components/new_list_dropdown.js.es6 b/app/assets/javascripts/boards/components/new_list_dropdown.js.es6 index 1a4d8157970..6ccd83e2d84 100644 --- a/app/assets/javascripts/boards/components/new_list_dropdown.js.es6 +++ b/app/assets/javascripts/boards/components/new_list_dropdown.js.es6 @@ -3,8 +3,7 @@ $(() => { $('.js-new-board-list').each(function () { const $this = $(this); - - new gl.CreateLabelDropdown($this.closest('.dropdown').find('.dropdown-new-label'), $this.data('project-id')); + new gl.CreateLabelDropdown($this.closest('.dropdown').find('.dropdown-new-label'), $this.data('namespace-path'), $this.data('project-path')); $this.glDropdown({ data(term, callback) { diff --git a/app/assets/javascripts/create_label.js.es6 b/app/assets/javascripts/create_label.js.es6 index 46d1c3f00c1..c5f8c29242d 100644 --- a/app/assets/javascripts/create_label.js.es6 +++ b/app/assets/javascripts/create_label.js.es6 @@ -1,8 +1,9 @@ (function (w) { class CreateLabelDropdown { - constructor ($el, projectId) { + constructor ($el, namespacePath, projectPath) { this.$el = $el; - this.projectId = projectId; + this.namespacePath = namespacePath; + this.projectPath = projectPath; this.$dropdownBack = $('.dropdown-menu-back', this.$el.closest('.dropdown')); this.$cancelButton = $('.js-cancel-label-btn', this.$el); this.$newLabelField = $('#new_label_name', this.$el); @@ -91,8 +92,8 @@ e.preventDefault(); e.stopPropagation(); - Api.newLabel(this.projectId, { - name: this.$newLabelField.val(), + Api.newLabel(this.namespacePath, this.projectPath, { + title: this.$newLabelField.val(), color: this.$newColorField.val() }, (label) => { this.$newLabelCreateButton.enable(); diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js index 3f15a117ca8..51a84066185 100644 --- a/app/assets/javascripts/labels_select.js +++ b/app/assets/javascripts/labels_select.js @@ -4,9 +4,10 @@ var _this; _this = this; $('.js-label-select').each(function(i, dropdown) { - var $block, $colorPreview, $dropdown, $form, $loading, $selectbox, $sidebarCollapsedValue, $value, abilityName, defaultLabel, enableLabelCreateButton, issueURLSplit, issueUpdateURL, labelHTMLTemplate, labelNoneHTMLTemplate, labelUrl, projectId, saveLabelData, selectedLabel, showAny, showNo, $sidebarLabelTooltip; + var $block, $colorPreview, $dropdown, $form, $loading, $selectbox, $sidebarCollapsedValue, $value, abilityName, defaultLabel, enableLabelCreateButton, issueURLSplit, issueUpdateURL, labelHTMLTemplate, labelNoneHTMLTemplate, labelUrl, namespacePath, projectPath, saveLabelData, selectedLabel, showAny, showNo, $sidebarLabelTooltip; $dropdown = $(dropdown); - projectId = $dropdown.data('project-id'); + namespacePath = $dropdown.data('namespace-path'); + projectPath = $dropdown.data('project-path'); labelUrl = $dropdown.data('labels'); issueUpdateURL = $dropdown.data('issueUpdate'); selectedLabel = $dropdown.data('selected'); @@ -35,7 +36,7 @@ $sidebarLabelTooltip.tooltip(); if ($dropdown.closest('.dropdown').find('.dropdown-new-label').length) { - new gl.CreateLabelDropdown($dropdown.closest('.dropdown').find('.dropdown-new-label'), projectId); + new gl.CreateLabelDropdown($dropdown.closest('.dropdown').find('.dropdown-new-label'), namespacePath, projectPath); } saveLabelData = function() { diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb index 28fa4a5b141..a6626df4826 100644 --- a/app/controllers/projects/labels_controller.rb +++ b/app/controllers/projects/labels_controller.rb @@ -30,9 +30,15 @@ class Projects::LabelsController < Projects::ApplicationController @label = @project.labels.create(label_params) if @label.valid? - redirect_to namespace_project_labels_path(@project.namespace, @project) + respond_to do |format| + format.html { redirect_to namespace_project_labels_path(@project.namespace, @project) } + format.json { render json: @label } + end else - render 'new' + respond_to do |format| + format.html { render 'new' } + format.json { render json: { message: @label.errors.messages }, status: 400 } + end end end diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index cf26197f7d7..f17096968f6 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -37,7 +37,7 @@ %input.pull-left.form-control{ type: "search", placeholder: "Filter by name...", "v-model" => "filters.search", "debounce" => "250" } - if can?(current_user, :admin_list, @project) .dropdown.pull-right - %button.btn.btn-create.js-new-board-list{ type: "button", data: { toggle: "dropdown", labels: labels_filter_path, project_id: @project.try(:id) } } + %button.btn.btn-create.js-new-board-list{ type: "button", data: { toggle: "dropdown", labels: labels_filter_path, namespace_path: @project.try(:namespace).try(:path), project_path: @project.try(:path) } } Create new list .dropdown-menu.dropdown-menu-paging.dropdown-menu-align-right.dropdown-menu-issues-board-new.dropdown-menu-selectable = render partial: "shared/issuable/label_page_default", locals: { show_footer: true, show_create: true, show_boards_content: true, title: "Create a new list" } diff --git a/app/views/shared/issuable/_label_dropdown.html.haml b/app/views/shared/issuable/_label_dropdown.html.haml index d34d28f6736..ebe441c931e 100644 --- a/app/views/shared/issuable/_label_dropdown.html.haml +++ b/app/views/shared/issuable/_label_dropdown.html.haml @@ -4,7 +4,7 @@ - show_footer = local_assigns.fetch(:show_footer, true) - data_options = local_assigns.fetch(:data_options, {}) - classes = local_assigns.fetch(:classes, []) -- dropdown_data = {toggle: 'dropdown', field_name: 'label_name[]', show_no: "true", show_any: "true", selected: params[:label_name], project_id: @project.try(:id), labels: labels_filter_path, default_label: "Label"} +- dropdown_data = {toggle: 'dropdown', field_name: 'label_name[]', show_no: "true", show_any: "true", selected: params[:label_name], namespace_path: @project.try(:namespace).try(:path), project_path: @project.try(:path), labels: labels_filter_path, default_label: "Label"} - dropdown_data.merge!(data_options) - classes << 'js-extra-options' if extra_options - classes << 'js-filter-submit' if filter_submit diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index b13daaf43c9..0464ebddc11 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -128,7 +128,7 @@ - issuable.labels_array.each do |label| = hidden_field_tag "#{issuable.to_ability_name}[label_names][]", label.id, id: nil .dropdown - %button.dropdown-menu-toggle.js-label-select.js-multiselect{type: "button", data: {toggle: "dropdown", field_name: "#{issuable.to_ability_name}[label_names][]", ability_name: issuable.to_ability_name, show_no: "true", show_any: "true", project_id: (@project.id if @project), issue_update: issuable_json_path(issuable), labels: (namespace_project_labels_path(@project.namespace, @project, :json) if @project)}} + %button.dropdown-menu-toggle.js-label-select.js-multiselect{type: "button", data: {toggle: "dropdown", field_name: "#{issuable.to_ability_name}[label_names][]", ability_name: issuable.to_ability_name, show_no: "true", show_any: "true", namespace_path: @project.try(:namespace).try(:path), project_path: @project.try(:path), issue_update: issuable_json_path(issuable), labels: (namespace_project_labels_path(@project.namespace, @project, :json) if @project)}} %span.dropdown-toggle-text Label = icon('chevron-down') -- cgit v1.2.1 From 528b988aea44cc1016ee5a3c09ce0d383114e395 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 26 Sep 2016 11:43:55 +0200 Subject: Escape HTML nodes in builds commands in ci linter --- app/views/ci/lints/_create.html.haml | 3 +-- spec/views/ci/lints/show.html.haml_spec.rb | 35 ++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 spec/views/ci/lints/show.html.haml_spec.rb diff --git a/app/views/ci/lints/_create.html.haml b/app/views/ci/lints/_create.html.haml index f7875e68b7e..1545c00af45 100644 --- a/app/views/ci/lints/_create.html.haml +++ b/app/views/ci/lints/_create.html.haml @@ -16,8 +16,7 @@ %tr %td #{stage.capitalize} Job - #{build[:name]} %td - %pre - = simple_format build[:commands] + %pre= build[:commands] %br %b Tag list: diff --git a/spec/views/ci/lints/show.html.haml_spec.rb b/spec/views/ci/lints/show.html.haml_spec.rb new file mode 100644 index 00000000000..3a65a86cd88 --- /dev/null +++ b/spec/views/ci/lints/show.html.haml_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' + +describe 'ci/lints/show' do + include Devise::TestHelpers + + before do + assign(:status, true) + assign(:stages, %w[test]) + assign(:builds, builds) + end + + context 'when builds attrbiutes contain HTML nodes' do + let(:builds) do + [ { name: 'rspec', stage: 'test', commands: '

rspec

' } ] + end + + it 'does not render HTML elements' do + render + + expect(rendered).not_to have_css('h1', text: 'rspec') + end + end + + context 'when builds attributes do not contain HTML nodes' do + let(:builds) do + [ { name: 'rspec', stage: 'test', commands: 'rspec' } ] + end + + it 'shows configuration in the table' do + render + + expect(rendered).to have_css('td pre', text: 'rspec') + end + end +end -- cgit v1.2.1 From 3ed80a0176a0c8155ff6f84a8f3e70718babd8ce Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Mon, 19 Sep 2016 19:28:41 +0100 Subject: Enforce the fork_project permission in Projects::CreateService Projects::ForkService delegates to this service almost entirely, but needed one small change so it would propagate create errors correctly. CreateService#execute needs significant refactoring; it is now right at the complexity limit set by Rubocop. I avoided doing so in this commit to keep the diff as small as possible. Several tests depend on the insecure behaviour of ForkService, so fi them up at the same time. --- app/services/projects/create_service.rb | 12 ++++++++++++ app/services/projects/fork_service.rb | 2 ++ features/steps/project/fork.rb | 1 + spec/controllers/users_controller_spec.rb | 4 ++-- spec/helpers/projects_helper_spec.rb | 2 +- spec/models/forked_project_link_spec.rb | 1 + spec/requests/api/fork_spec.rb | 2 +- spec/services/projects/fork_service_spec.rb | 25 +++++++++++++++++++++++-- spec/services/system_note_service_spec.rb | 2 +- 9 files changed, 44 insertions(+), 7 deletions(-) diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index be749ba4a1c..696fe3efe8f 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -15,6 +15,11 @@ module Projects return @project end + unless allowed_fork?(forked_from_project_id) + @project.errors.add(:forked_from_project_id, 'is forbidden') + return @project + end + # Set project name from path if @project.name.present? && @project.path.present? # if both name and path set - everything is ok @@ -71,6 +76,13 @@ module Projects @project.errors.add(:namespace, "is not valid") end + def allowed_fork?(source_project_id) + return true if source_project_id.nil? + + source_project = Project.find_by(id: source_project_id) + current_user.can?(:fork_project, source_project) + end + def allowed_namespace?(user, namespace_id) namespace = Namespace.find_by(id: namespace_id) current_user.can?(:create_projects, namespace) diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb index a2de4dccece..a2b23ea6171 100644 --- a/app/services/projects/fork_service.rb +++ b/app/services/projects/fork_service.rb @@ -16,6 +16,8 @@ module Projects end new_project = CreateService.new(current_user, new_params).execute + return new_project unless new_project.persisted? + builds_access_level = @project.project_feature.builds_access_level new_project.project_feature.update_attributes(builds_access_level: builds_access_level) diff --git a/features/steps/project/fork.rb b/features/steps/project/fork.rb index 8abeb5ee242..70dbd030003 100644 --- a/features/steps/project/fork.rb +++ b/features/steps/project/fork.rb @@ -70,6 +70,7 @@ class Spinach::Features::ProjectFork < Spinach::FeatureSteps step 'There is an existent fork of the "Shop" project' do user = create(:user, name: 'Mike') + @project.team << [user, :reporter] @forked_project = Projects::ForkService.new(@project, user).execute end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 54a2d3d9460..19a8b1fe524 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -73,8 +73,8 @@ describe UsersController do end context 'forked project' do - let!(:project) { create(:project) } - let!(:forked_project) { Projects::ForkService.new(project, user).execute } + let(:project) { create(:project) } + let(:forked_project) { Projects::ForkService.new(project, user).execute } before do sign_in(user) diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 70032e7df94..bcd53440cb4 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -11,7 +11,7 @@ describe ProjectsHelper do describe "can_change_visibility_level?" do let(:project) { create(:project) } - let(:user) { create(:user) } + let(:user) { create(:project_member, :reporter, user: create(:user), project: project).user } let(:fork_project) { Projects::ForkService.new(project, user).execute } it "returns false if there are no appropriate permissions" do diff --git a/spec/models/forked_project_link_spec.rb b/spec/models/forked_project_link_spec.rb index 9c81d159cdf..1863581f57b 100644 --- a/spec/models/forked_project_link_spec.rb +++ b/spec/models/forked_project_link_spec.rb @@ -6,6 +6,7 @@ describe ForkedProjectLink, "add link on fork" do let(:user) { create(:user, namespace: namespace) } before do + create(:project_member, :reporter, user: user, project: project_from) @project_to = fork_project(project_from, user) end diff --git a/spec/requests/api/fork_spec.rb b/spec/requests/api/fork_spec.rb index 34f84f78952..e38d5745d44 100644 --- a/spec/requests/api/fork_spec.rb +++ b/spec/requests/api/fork_spec.rb @@ -18,7 +18,7 @@ describe API::API, api: true do end let(:project_user2) do - create(:project_member, :guest, user: user2, project: project) + create(:project_member, :reporter, user: user2, project: project) end describe 'POST /projects/fork/:id' do diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index ef2036c78b1..64d15c0523c 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -12,12 +12,26 @@ describe Projects::ForkService, services: true do description: 'wow such project') @to_namespace = create(:namespace) @to_user = create(:user, namespace: @to_namespace) + @from_project.add_user(@to_user, :developer) end context 'fork project' do + context 'when forker is a guest' do + before do + @guest = create(:user) + @from_project.add_user(@guest, :guest) + end + subject { fork_project(@from_project, @guest) } + + it { is_expected.not_to be_persisted } + it { expect(subject.errors[:forked_from_project_id]).to eq(['is forbidden']) } + end + describe "successfully creates project in the user namespace" do let(:to_project) { fork_project(@from_project, @to_user) } + it { expect(to_project).to be_persisted } + it { expect(to_project.errors).to be_empty } it { expect(to_project.owner).to eq(@to_user) } it { expect(to_project.namespace).to eq(@to_user.namespace) } it { expect(to_project.star_count).to be_zero } @@ -29,7 +43,9 @@ describe Projects::ForkService, services: true do it "fails due to validation, not transaction failure" do @existing_project = create(:project, creator_id: @to_user.id, name: @from_project.name, namespace: @to_namespace) @to_project = fork_project(@from_project, @to_user) - expect(@existing_project.persisted?).to be_truthy + expect(@existing_project).to be_persisted + + expect(@to_project).not_to be_persisted expect(@to_project.errors[:name]).to eq(['has already been taken']) expect(@to_project.errors[:path]).to eq(['has already been taken']) end @@ -81,18 +97,23 @@ describe Projects::ForkService, services: true do @group = create(:group) @group.add_user(@group_owner, GroupMember::OWNER) @group.add_user(@developer, GroupMember::DEVELOPER) + @project.add_user(@developer, :developer) + @project.add_user(@group_owner, :developer) @opts = { namespace: @group } end context 'fork project for group' do it 'group owner successfully forks project into the group' do to_project = fork_project(@project, @group_owner, @opts) + + expect(to_project).to be_persisted + expect(to_project.errors).to be_empty expect(to_project.owner).to eq(@group) expect(to_project.namespace).to eq(@group) expect(to_project.name).to eq(@project.name) expect(to_project.path).to eq(@project.path) expect(to_project.description).to eq(@project.description) - expect(to_project.star_count).to be_zero + expect(to_project.star_count).to be_zero end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 3d854a959f3..a3f16e2d945 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -445,7 +445,7 @@ describe SystemNoteService, services: true do end context 'commit with cross-reference from fork' do - let(:author2) { create(:user) } + let(:author2) { create(:project_member, :reporter, user: create(:user), project: project).user } let(:forked_project) { Projects::ForkService.new(project, author2).execute } let(:commit2) { forked_project.commit } -- cgit v1.2.1 From 619dd1cfcf661bc1c0ce64010504cee6623c0b8a Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Tue, 27 Sep 2016 13:34:40 +0100 Subject: Add a CHANGELOG entry --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 61e3ed748c9..64918b89264 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,7 @@ v 8.13.0 (unreleased) v 8.12.2 (unreleased) - Fix Import/Export not recognising correctly the imported services. + - Respect the fork_project permission when forking projects v 8.12.1 - Fix a memory leak in HTML::Pipeline::SanitizationFilter::WHITELIST -- cgit v1.2.1 From 958d9f11e80633f7120a782900fe1f78b3dbebea Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 29 Sep 2016 17:17:22 +0200 Subject: fix export project file permissions issue --- CHANGELOG | 3 +++ lib/gitlab/import_export/command_line_util.rb | 9 ++++++++- lib/gitlab/import_export/file_importer.rb | 2 +- lib/gitlab/import_export/project_tree_saver.rb | 4 +++- lib/gitlab/import_export/repo_restorer.rb | 2 +- lib/gitlab/import_export/repo_saver.rb | 2 +- lib/gitlab/import_export/version_saver.rb | 4 +++- lib/gitlab/import_export/wiki_repo_saver.rb | 2 +- spec/features/projects/import_export/export_file_spec.rb | 2 ++ spec/support/import_export/export_file_helper.rb | 4 ++++ 10 files changed, 27 insertions(+), 7 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 64918b89264..c243920283c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -3,6 +3,9 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.13.0 (unreleased) - Speed-up group milestones show page +v 8.12.4 (unreleased) + - Set GitLab project exported file permissions to owner only + v 8.12.2 (unreleased) - Fix Import/Export not recognising correctly the imported services. - Respect the fork_project permission when forking projects diff --git a/lib/gitlab/import_export/command_line_util.rb b/lib/gitlab/import_export/command_line_util.rb index e522a0fc8f6..f00c7460e82 100644 --- a/lib/gitlab/import_export/command_line_util.rb +++ b/lib/gitlab/import_export/command_line_util.rb @@ -1,6 +1,8 @@ module Gitlab module ImportExport module CommandLineUtil + DEFAULT_MODE = 0700 + def tar_czf(archive:, dir:) tar_with_options(archive: archive, dir: dir, options: 'czf') end @@ -21,6 +23,11 @@ module Gitlab execute(%W(#{Gitlab.config.gitlab_shell.path}/bin/create-hooks) + repository_storage_paths_args) end + def mkdir_p(path) + FileUtils.mkdir_p(path, mode: DEFAULT_MODE) + FileUtils.chmod(DEFAULT_MODE, path) + end + private def tar_with_options(archive:, dir:, options:) @@ -45,7 +52,7 @@ module Gitlab # if we are copying files, create the destination folder destination_folder = File.file?(source) ? File.dirname(destination) : destination - FileUtils.mkdir_p(destination_folder) + mkdir_p(destination_folder) FileUtils.copy_entry(source, destination) true end diff --git a/lib/gitlab/import_export/file_importer.rb b/lib/gitlab/import_export/file_importer.rb index eca6e5b6d51..113895ba22c 100644 --- a/lib/gitlab/import_export/file_importer.rb +++ b/lib/gitlab/import_export/file_importer.rb @@ -15,7 +15,7 @@ module Gitlab end def import - FileUtils.mkdir_p(@shared.export_path) + mkdir_p(@shared.export_path) wait_for_archived_file do decompress_archive diff --git a/lib/gitlab/import_export/project_tree_saver.rb b/lib/gitlab/import_export/project_tree_saver.rb index 9153088e966..2fbf437ec26 100644 --- a/lib/gitlab/import_export/project_tree_saver.rb +++ b/lib/gitlab/import_export/project_tree_saver.rb @@ -1,6 +1,8 @@ module Gitlab module ImportExport class ProjectTreeSaver + include Gitlab::ImportExport::CommandLineUtil + attr_reader :full_path def initialize(project:, shared:) @@ -10,7 +12,7 @@ module Gitlab end def save - FileUtils.mkdir_p(@shared.export_path) + mkdir_p(@shared.export_path) File.write(full_path, project_json_tree) true diff --git a/lib/gitlab/import_export/repo_restorer.rb b/lib/gitlab/import_export/repo_restorer.rb index d1e33ea8678..48a9a6fa5e2 100644 --- a/lib/gitlab/import_export/repo_restorer.rb +++ b/lib/gitlab/import_export/repo_restorer.rb @@ -12,7 +12,7 @@ module Gitlab def restore return true unless File.exist?(@path_to_bundle) - FileUtils.mkdir_p(path_to_repo) + mkdir_p(path_to_repo) git_unbundle(repo_path: path_to_repo, bundle_path: @path_to_bundle) && repo_restore_hooks rescue => e diff --git a/lib/gitlab/import_export/repo_saver.rb b/lib/gitlab/import_export/repo_saver.rb index 331e14021e6..a7028a32570 100644 --- a/lib/gitlab/import_export/repo_saver.rb +++ b/lib/gitlab/import_export/repo_saver.rb @@ -20,7 +20,7 @@ module Gitlab private def bundle_to_disk - FileUtils.mkdir_p(@shared.export_path) + mkdir_p(@shared.export_path) git_bundle(repo_path: path_to_repo, bundle_path: @full_path) rescue => e @shared.error(e) diff --git a/lib/gitlab/import_export/version_saver.rb b/lib/gitlab/import_export/version_saver.rb index 9b642d740b7..7cf88298642 100644 --- a/lib/gitlab/import_export/version_saver.rb +++ b/lib/gitlab/import_export/version_saver.rb @@ -1,12 +1,14 @@ module Gitlab module ImportExport class VersionSaver + include Gitlab::ImportExport::CommandLineUtil + def initialize(shared:) @shared = shared end def save - FileUtils.mkdir_p(@shared.export_path) + mkdir_p(@shared.export_path) File.write(version_file, Gitlab::ImportExport.version, mode: 'w') rescue => e diff --git a/lib/gitlab/import_export/wiki_repo_saver.rb b/lib/gitlab/import_export/wiki_repo_saver.rb index 6107420e4dd..1e6722a7bba 100644 --- a/lib/gitlab/import_export/wiki_repo_saver.rb +++ b/lib/gitlab/import_export/wiki_repo_saver.rb @@ -9,7 +9,7 @@ module Gitlab end def bundle_to_disk(full_path) - FileUtils.mkdir_p(@shared.export_path) + mkdir_p(@shared.export_path) git_bundle(repo_path: path_to_repo, bundle_path: full_path) rescue => e @shared.error(e) diff --git a/spec/features/projects/import_export/export_file_spec.rb b/spec/features/projects/import_export/export_file_spec.rb index 27c986c5187..52d08982c7a 100644 --- a/spec/features/projects/import_export/export_file_spec.rb +++ b/spec/features/projects/import_export/export_file_spec.rb @@ -47,6 +47,8 @@ feature 'Import/Export - project export integration test', feature: true, js: tr expect(page).to have_content('Download export') + expect(file_permissions(project.export_path)).to eq(0700) + in_directory_with_expanded_export(project) do |exit_status, tmpdir| expect(exit_status).to eq(0) diff --git a/spec/support/import_export/export_file_helper.rb b/spec/support/import_export/export_file_helper.rb index be0772d6a4a..1b0a4583f5c 100644 --- a/spec/support/import_export/export_file_helper.rb +++ b/spec/support/import_export/export_file_helper.rb @@ -130,4 +130,8 @@ module ExportFileHelper (parsed_model_attributes - parent.keys - excluded_attributes).empty? end + + def file_permissions(file) + File.stat(file).mode & 0777 + end end -- cgit v1.2.1 From 437bebb0ff6e7deba6fd157ec6b55112e125731f Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Tue, 4 Oct 2016 16:35:41 +0200 Subject: Don't send Private-Token headers to Sentry Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/22537 --- CHANGELOG | 1 + Gemfile | 2 +- Gemfile.lock | 6 +++--- config/application.rb | 2 ++ config/initializers/sentry.rb | 2 ++ 5 files changed, 9 insertions(+), 4 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index c243920283c..84a6702907f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,7 @@ v 8.13.0 (unreleased) v 8.12.4 (unreleased) - Set GitLab project exported file permissions to owner only + - Don't send Private-Token (API authentication) headers to Sentry v 8.12.2 (unreleased) - Fix Import/Export not recognising correctly the imported services. diff --git a/Gemfile b/Gemfile index 21b31e8f01d..921554286c3 100644 --- a/Gemfile +++ b/Gemfile @@ -233,7 +233,7 @@ gem 'net-ssh', '~> 3.0.1' gem 'base32', '~> 0.3.0' # Sentry integration -gem 'sentry-raven', '~> 1.1.0' +gem 'sentry-raven', '~> 2.0.0' gem 'premailer-rails', '~> 1.9.0' diff --git a/Gemfile.lock b/Gemfile.lock index 1db8c9dd8c8..66e566de3c1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -664,8 +664,8 @@ GEM activesupport (>= 3.1) select2-rails (3.5.9.3) thor (~> 0.14) - sentry-raven (1.1.0) - faraday (>= 0.7.6) + sentry-raven (2.0.2) + faraday (>= 0.7.6, < 0.10.x) settingslogic (2.0.9) sexp_processor (4.7.0) sham_rack (1.3.6) @@ -950,7 +950,7 @@ DEPENDENCIES sdoc (~> 0.3.20) seed-fu (~> 2.3.5) select2-rails (~> 3.5.9) - sentry-raven (~> 1.1.0) + sentry-raven (~> 2.0.0) settingslogic (~> 2.0.9) sham_rack (~> 1.3.6) shoulda-matchers (~> 2.8.0) diff --git a/config/application.rb b/config/application.rb index 4792f6670a8..f5c900da8cf 100644 --- a/config/application.rb +++ b/config/application.rb @@ -50,6 +50,7 @@ module Gitlab # - Build variables (:variables) # - GitLab Pages SSL cert/key info (:certificate, :encrypted_key) # - Webhook URLs (:hook) + # - GitLab-shell secret token (:secret_token) # - Sentry DSN (:sentry_dsn) # - Deploy keys (:key) config.filter_parameters += %i( @@ -62,6 +63,7 @@ module Gitlab password password_confirmation private_token + secret_token sentry_dsn variables ) diff --git a/config/initializers/sentry.rb b/config/initializers/sentry.rb index 5892c1de024..4f30d1265c8 100644 --- a/config/initializers/sentry.rb +++ b/config/initializers/sentry.rb @@ -18,6 +18,8 @@ if Rails.env.production? # Sanitize fields based on those sanitized from Rails. config.sanitize_fields = Rails.application.config.filter_parameters.map(&:to_s) + # Sanitize authentication headers + config.sanitize_http_headers = %w[Authorization Private-Token] config.tags = { program: Gitlab::Sentry.program_context } end end -- cgit v1.2.1