diff options
25 files changed, 232 insertions, 43 deletions
diff --git a/app/assets/javascripts/due_date_select.js.es6 b/app/assets/javascripts/due_date_select.js.es6 index ab5ce23d261..9169fcd7328 100644 --- a/app/assets/javascripts/due_date_select.js.es6 +++ b/app/assets/javascripts/due_date_select.js.es6 @@ -48,7 +48,7 @@ const calendar = new Pikaday({ field: $dueDateInput.get(0), theme: 'gitlab-theme', - format: 'YYYY-MM-DD', + format: 'yyyy-mm-dd', onSelect: (dateText) => { const formattedDate = dateFormat(new Date(dateText), 'yyyy-mm-dd'); @@ -63,6 +63,7 @@ } }); + calendar.setDate(new Date($dueDateInput.val())); this.$datePicker.append(calendar.el); this.$datePicker.data('pikaday', calendar); } @@ -169,11 +170,12 @@ const calendar = new Pikaday({ field: $datePicker.get(0), theme: 'gitlab-theme', - format: 'YYYY-MM-DD', + format: 'yyyy-mm-dd', onSelect(dateText) { $datePicker.val(dateFormat(new Date(dateText), 'yyyy-mm-dd')); } }); + calendar.setDate(new Date($datePicker.val())); $datePicker.data('pikaday', calendar); }); diff --git a/app/assets/javascripts/issuable_form.js b/app/assets/javascripts/issuable_form.js index 2ec545db665..c7c744ef61f 100644 --- a/app/assets/javascripts/issuable_form.js +++ b/app/assets/javascripts/issuable_form.js @@ -40,11 +40,12 @@ calendar = new Pikaday({ field: $issuableDueDate.get(0), theme: 'gitlab-theme', - format: 'YYYY-MM-DD', + format: 'yyyy-mm-dd', onSelect: function(dateText) { $issuableDueDate.val(dateFormat(new Date(dateText), 'yyyy-mm-dd')); } }); + calendar.setDate(new Date($issuableDueDate.val())); } } diff --git a/app/assets/javascripts/member_expiration_date.js.es6 b/app/assets/javascripts/member_expiration_date.js.es6 index f57d4a20498..efe7c78a8ec 100644 --- a/app/assets/javascripts/member_expiration_date.js.es6 +++ b/app/assets/javascripts/member_expiration_date.js.es6 @@ -19,7 +19,7 @@ const calendar = new Pikaday({ field: $input.get(0), theme: 'gitlab-theme', - format: 'YYYY-MM-DD', + format: 'yyyy-mm-dd', minDate: new Date(), onSelect(dateText) { $input.val(dateFormat(new Date(dateText), 'yyyy-mm-dd')); @@ -30,6 +30,7 @@ }, }); + calendar.setDate(new Date($input.val())); $input.data('pikaday', calendar); }); diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb index 47bef7cd1e4..23b7318827c 100644 --- a/app/uploaders/file_uploader.rb +++ b/app/uploaders/file_uploader.rb @@ -36,7 +36,7 @@ class FileUploader < GitlabUploader escaped_filename = filename.gsub("]", "\\]") markdown = "[#{escaped_filename}](#{self.secure_url})" - markdown.prepend("!") if image_or_video? + markdown.prepend("!") if image_or_video? || dangerous? { alt: filename, diff --git a/app/uploaders/uploader_helper.rb b/app/uploaders/uploader_helper.rb index fbaea2744a3..35fd1ed23f8 100644 --- a/app/uploaders/uploader_helper.rb +++ b/app/uploaders/uploader_helper.rb @@ -1,12 +1,15 @@ # Extra methods for uploader module UploaderHelper - IMAGE_EXT = %w[png jpg jpeg gif bmp tiff svg] + IMAGE_EXT = %w[png jpg jpeg gif bmp tiff] # We recommend using the .mp4 format over .mov. Videos in .mov format can # still be used but you really need to make sure they are served with the # proper MIME type video/mp4 and not video/quicktime or your videos won't play # on IE >= 9. # http://archive.sublimevideo.info/20150912/docs.sublimevideo.net/troubleshooting.html VIDEO_EXT = %w[mp4 m4v mov webm ogv] + # These extension types can contain dangerous code and should only be embedded inline with + # proper filtering. They should always be tagged as "Content-Disposition: attachment", not "inline". + DANGEROUS_EXT = %w[svg] def image? extension_match?(IMAGE_EXT) @@ -20,6 +23,10 @@ module UploaderHelper image? || video? end + def dangerous? + extension_match?(DANGEROUS_EXT) + end + def extension_match?(extensions) return false unless file diff --git a/app/views/profiles/personal_access_tokens/index.html.haml b/app/views/profiles/personal_access_tokens/index.html.haml index 2c006e1712d..b10f5fc08e2 100644 --- a/app/views/profiles/personal_access_tokens/index.html.haml +++ b/app/views/profiles/personal_access_tokens/index.html.haml @@ -91,7 +91,7 @@ new Pikaday({ field: $dateField.get(0), theme: 'gitlab-theme', - format: 'YYYY-MM-DD', + format: 'yyyy-mm-dd', minDate: new Date(), onSelect: function(dateText) { $dateField.val(dateFormat(new Date(dateText), 'yyyy-mm-dd')); diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index 1b9d87e9969..79a0dc1b959 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -28,9 +28,11 @@ .project-clone-holder = render "shared/clone_panel" - - if current_user && can?(current_user, :download_code, @project) - = render 'projects/buttons/download', project: @project, ref: @ref - = render 'projects/buttons/dropdown' + - if current_user + - if can?(current_user, :download_code, @project) + = render 'projects/buttons/download', project: @project, ref: @ref + = render 'projects/buttons/dropdown' + = render 'projects/buttons/koding' + = render 'shared/notifications/button', notification_setting: @notification_setting - = render 'projects/buttons/koding' = render 'shared/members/access_request_buttons', source: @project diff --git a/changelogs/unreleased/26651-cannot-move-project-into-group.yml b/changelogs/unreleased/26651-cannot-move-project-into-group.yml new file mode 100644 index 00000000000..244a19a627d --- /dev/null +++ b/changelogs/unreleased/26651-cannot-move-project-into-group.yml @@ -0,0 +1,4 @@ +--- +title: Specify in the documentation that only projects owners can transfer projects +merge_request: +author: diff --git a/changelogs/unreleased/api-entities.yml b/changelogs/unreleased/api-entities.yml new file mode 100644 index 00000000000..2003d00fd52 --- /dev/null +++ b/changelogs/unreleased/api-entities.yml @@ -0,0 +1,4 @@ +--- +title: "Use an entity for RepoBranch commits and enhance RepoCommit" +merge_request: 7138 +author: Ben Boeckel diff --git a/changelogs/unreleased/fix-gb-notification-settings-when-no-repository.yml b/changelogs/unreleased/fix-gb-notification-settings-when-no-repository.yml new file mode 100644 index 00000000000..17fd1336b8e --- /dev/null +++ b/changelogs/unreleased/fix-gb-notification-settings-when-no-repository.yml @@ -0,0 +1,4 @@ +--- +title: Show notifications settings dropdown even if repository feature is disabled +merge_request: 9180 +author: diff --git a/config/webpack.config.js b/config/webpack.config.js index 5d5e4bb570a..e63775e43ce 100644 --- a/config/webpack.config.js +++ b/config/webpack.config.js @@ -54,7 +54,7 @@ var config = { exclude: /(node_modules|vendor\/assets)/, loader: 'babel-loader', options: { - plugins: ['istanbul'], + plugins: IS_PRODUCTION ? [] : ['istanbul'], presets: [ ["es2015", {"modules": false}], 'stage-2' @@ -83,6 +83,7 @@ var config = { new CompressionPlugin({ asset: '[path].gz[query]', }), + new webpack.IgnorePlugin(/moment/, /pikaday/), ], resolve: { diff --git a/doc/api/branches.md b/doc/api/branches.md index ffcfea41453..5eaa8d2e920 100644 --- a/doc/api/branches.md +++ b/doc/api/branches.md @@ -34,6 +34,8 @@ Example response: "committer_email": "john@example.com", "committer_name": "John Smith", "id": "7b5c3cc8be40ee161ae89a06bba6229da1032a0c", + "short_id": "7b5c3cc", + "title": "add projects API", "message": "add projects API", "parent_ids": [ "4ad91d3c1144c406e50c7b33bae684bd6837faf8" @@ -78,6 +80,8 @@ Example response: "committer_email": "john@example.com", "committer_name": "John Smith", "id": "7b5c3cc8be40ee161ae89a06bba6229da1032a0c", + "short_id": "7b5c3cc", + "title": "add projects API", "message": "add projects API", "parent_ids": [ "4ad91d3c1144c406e50c7b33bae684bd6837faf8" @@ -119,6 +123,8 @@ Example response: "committer_email": "john@example.com", "committer_name": "John Smith", "id": "7b5c3cc8be40ee161ae89a06bba6229da1032a0c", + "short_id": "7b5c3cc", + "title": "add projects API", "message": "add projects API", "parent_ids": [ "4ad91d3c1144c406e50c7b33bae684bd6837faf8" @@ -163,6 +169,8 @@ Example response: "committer_email": "john@example.com", "committer_name": "John Smith", "id": "7b5c3cc8be40ee161ae89a06bba6229da1032a0c", + "short_id": "7b5c3cc", + "title": "add projects API", "message": "add projects API", "parent_ids": [ "4ad91d3c1144c406e50c7b33bae684bd6837faf8" @@ -204,6 +212,8 @@ Example response: "committer_email": "john@example.com", "committer_name": "John Smith", "id": "7b5c3cc8be40ee161ae89a06bba6229da1032a0c", + "short_id": "7b5c3cc", + "title": "add projects API", "message": "add projects API", "parent_ids": [ "4ad91d3c1144c406e50c7b33bae684bd6837faf8" diff --git a/doc/api/commits.md b/doc/api/commits.md index ef2400aaf3c..3223b82f60a 100644 --- a/doc/api/commits.md +++ b/doc/api/commits.md @@ -29,11 +29,15 @@ Example response: "title": "Replace sanitize with escape once", "author_name": "Dmitriy Zaporozhets", "author_email": "dzaporozhets@sphereconsultinginc.com", + "authored_date": "2012-09-20T11:50:22+03:00", "committer_name": "Administrator", "committer_email": "admin@example.com", + "committed_date": "2012-09-20T11:50:22+03:00", "created_at": "2012-09-20T11:50:22+03:00", "message": "Replace sanitize with escape once", - "allow_failure": false + "parent_ids": [ + "6104942438c14ec7bd21c6cd5bd995272b3faff6" + ] }, { "id": "6104942438c14ec7bd21c6cd5bd995272b3faff6", @@ -45,7 +49,9 @@ Example response: "committer_email": "dmitriy.zaporozhets@gmail.com", "created_at": "2012-09-20T09:06:12+03:00", "message": "Sanitize for network graph", - "allow_failure": false + "parent_ids": [ + "ae1d9fb46aa2b07ee9836d49862ec4e2c46fbbba" + ] } ] ``` @@ -214,10 +220,16 @@ Example response: "title": "Feature added", "author_name": "Dmitriy Zaporozhets", "author_email": "dmitriy.zaporozhets@gmail.com", + "authored_date": "2016-12-12T20:10:39.000+01:00", "created_at": "2016-12-12T20:10:39.000+01:00", "committer_name": "Administrator", "committer_email": "admin@example.com", - "message": "Feature added\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n" + "committed_date": "2016-12-12T20:10:39.000+01:00", + "title": "Feature added", + "message": "Feature added\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n", + "parent_ids": [ + "a738f717824ff53aebad8b090c1b79a14f2bd9e8" + ] } ``` diff --git a/doc/workflow/groups.md b/doc/workflow/groups.md index a693cc3d0fd..6237a5d5e18 100644 --- a/doc/workflow/groups.md +++ b/doc/workflow/groups.md @@ -23,7 +23,7 @@ You can use the 'New project' button to add a project to the new group. ## Transferring an existing project into a group -You can transfer an existing project into a group you own from the project settings page. +You can transfer an existing project into a group you own from the project settings page. The option to transfer a project is only available if you are the Owner of the project. First scroll down to the 'Dangerous settings' and click 'Show them to me'. Now you can pick any of the groups you manage as the new namespace for the group. diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 3806470ac94..232f231ddd2 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -155,10 +155,27 @@ module API expose :shared_projects, using: Entities::Project end + class RepoCommit < Grape::Entity + expose :id, :short_id, :title, :created_at + expose :parent_ids + expose :safe_message, as: :message + expose :author_name, :author_email, :authored_date + expose :committer_name, :committer_email, :committed_date + end + + class RepoCommitStats < Grape::Entity + expose :additions, :deletions, :total + end + + class RepoCommitDetail < RepoCommit + expose :stats, using: Entities::RepoCommitStats + expose :status + end + class RepoBranch < Grape::Entity expose :name - expose :commit do |repo_branch, options| + expose :commit, using: Entities::RepoCommit do |repo_branch, options| options[:project].repository.commit(repo_branch.dereferenced_target) end @@ -193,22 +210,6 @@ module API end end - class RepoCommit < Grape::Entity - expose :id, :short_id, :title, :author_name, :author_email, :created_at - expose :committer_name, :committer_email - expose :safe_message, as: :message - end - - class RepoCommitStats < Grape::Entity - expose :additions, :deletions, :total - end - - class RepoCommitDetail < RepoCommit - expose :parent_ids, :committed_date, :authored_date - expose :stats, using: Entities::RepoCommitStats - expose :status - end - class ProjectSnippet < Grape::Entity expose :id, :title, :file_name expose :author, using: Entities::UserBasic diff --git a/lib/gitlab/asciidoc.rb b/lib/gitlab/asciidoc.rb index 0618107e2c3..d575367d81a 100644 --- a/lib/gitlab/asciidoc.rb +++ b/lib/gitlab/asciidoc.rb @@ -36,6 +36,9 @@ module Gitlab html = Banzai.post_process(html, context) + filter = Banzai::Filter::SanitizationFilter.new(html) + html = filter.call.to_s + html.html_safe end diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index ec1318ab33c..9a4ffd28438 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -115,7 +115,7 @@ module Gitlab begin issuable = if gh_issue.pull_request? - MergeRequest.find_by_iid(gh_issue.number) + MergeRequest.find_by(target_project_id: project.id, iid: gh_issue.number) else gh_issue.create! end @@ -212,8 +212,12 @@ module Gitlab comment = CommentFormatter.new(project, raw) # GH does not return info about comment's parent, so we guess it by checking its URL! *_, parent, iid = URI(raw.html_url).path.split('/') - issuable_class = parent == 'issues' ? Issue : MergeRequest - issuable = issuable_class.find_by_iid(iid) + if parent == 'issues' + issuable = Issue.find_by(project_id: project.id, iid: iid) + else + issuable = MergeRequest.find_by(target_project_id: project.id, iid: iid) + end + next unless issuable issuable.notes.create!(comment.attributes) diff --git a/lib/gitlab/other_markup.rb b/lib/gitlab/other_markup.rb index 4e2f8ed5587..e67acf28c94 100644 --- a/lib/gitlab/other_markup.rb +++ b/lib/gitlab/other_markup.rb @@ -17,6 +17,9 @@ module Gitlab html = Banzai.post_process(html, context) + filter = Banzai::Filter::SanitizationFilter.new(html) + html = filter.call.to_s + html.html_safe end end diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index 570d9fa43f8..c9584ddf18c 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -4,6 +4,28 @@ describe UploadsController do let!(:user) { create(:user, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) } describe "GET show" do + context 'Content-Disposition security measures' do + let(:project) { create(:empty_project, :public) } + + context 'for PNG files' do + it 'returns Content-Disposition: inline' do + note = create(:note, :with_attachment, project: project) + get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'image.png' + + expect(response['Content-Disposition']).to start_with('inline;') + end + end + + context 'for SVG files' do + it 'returns Content-Disposition: attachment' do + note = create(:note, :with_svg_attachment, project: project) + get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'image.svg' + + expect(response['Content-Disposition']).to start_with('attachment;') + end + end + end + context "when viewing a user avatar" do context "when signed in" do before do diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index a21da7074f9..5c50cd7f4ad 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -97,7 +97,11 @@ FactoryGirl.define do end trait :with_attachment do - attachment { fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "`/png") } + attachment { fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png") } + end + + trait :with_svg_attachment do + attachment { fixture_file_upload(Rails.root + "spec/fixtures/unsanitized.svg", "image/svg+xml") } end end end diff --git a/spec/lib/gitlab/asciidoc_spec.rb b/spec/lib/gitlab/asciidoc_spec.rb index ba199917f5c..bca57105d1d 100644 --- a/spec/lib/gitlab/asciidoc_spec.rb +++ b/spec/lib/gitlab/asciidoc_spec.rb @@ -41,6 +41,29 @@ module Gitlab render(input, context, asciidoc_opts) end end + + context "XSS" do + links = { + 'links' => { + input: 'link:mylink"onmouseover="alert(1)[Click Here]', + output: "<div>\n<p><a href=\"mylink\">Click Here</a></p>\n</div>" + }, + 'images' => { + input: 'image:https://localhost.com/image.png[Alt text" onerror="alert(7)]', + output: "<div>\n<p><span><img src=\"https://localhost.com/image.png\" alt=\"Alt text\"></span></p>\n</div>" + }, + 'pre' => { + input: '```mypre"><script>alert(3)</script>', + output: "<div>\n<div>\n<pre lang=\"mypre\">\"><code></code></pre>\n</div>\n</div>" + } + } + + links.each do |name, data| + it "does not convert dangerous #{name} into HTML" do + expect(render(data[:input], context)).to eql data[:output] + end + end + end end def render(*args) diff --git a/spec/lib/gitlab/other_markup.rb b/spec/lib/gitlab/other_markup.rb new file mode 100644 index 00000000000..8f5a353b381 --- /dev/null +++ b/spec/lib/gitlab/other_markup.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe Gitlab::OtherMarkup, lib: true do + context "XSS Checks" do + links = { + 'links' => { + file: 'file.rdoc', + input: 'XSS[JaVaScriPt:alert(1)]', + output: '<p><a>XSS</a></p>' + } + } + links.each do |name, data| + it "does not convert dangerous #{name} into HTML" do + expect(render(data[:file], data[:input], context)).to eql data[:output] + end + end + end + + def render(*args) + described_class.render(*args) + end +end diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index 5a3ffc284f2..3e66236f6ae 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -31,7 +31,18 @@ describe API::Branches, api: true do expect(response).to have_http_status(200) expect(json_response['name']).to eq(branch_name) - expect(json_response['commit']['id']).to eq(branch_sha) + json_commit = json_response['commit'] + expect(json_commit['id']).to eq(branch_sha) + expect(json_commit).to have_key('short_id') + expect(json_commit).to have_key('title') + expect(json_commit).to have_key('message') + expect(json_commit).to have_key('author_name') + expect(json_commit).to have_key('author_email') + expect(json_commit).to have_key('authored_date') + expect(json_commit).to have_key('committer_name') + expect(json_commit).to have_key('committer_email') + expect(json_commit).to have_key('committed_date') + expect(json_commit).to have_key('parent_ids') expect(json_response['merged']).to eq(false) expect(json_response['protected']).to eq(false) expect(json_response['developers_can_push']).to eq(false) diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index af9028a8978..3d0d6735359 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -367,11 +367,21 @@ describe API::Commits, api: true do get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user) expect(response).to have_http_status(200) - expect(json_response['id']).to eq(project.repository.commit.id) - expect(json_response['title']).to eq(project.repository.commit.title) - expect(json_response['stats']['additions']).to eq(project.repository.commit.stats.additions) - expect(json_response['stats']['deletions']).to eq(project.repository.commit.stats.deletions) - expect(json_response['stats']['total']).to eq(project.repository.commit.stats.total) + commit = project.repository.commit + expect(json_response['id']).to eq(commit.id) + expect(json_response['short_id']).to eq(commit.short_id) + expect(json_response['title']).to eq(commit.title) + expect(json_response['message']).to eq(commit.safe_message) + expect(json_response['author_name']).to eq(commit.author_name) + expect(json_response['author_email']).to eq(commit.author_email) + expect(json_response['authored_date']).to eq(commit.authored_date.iso8601(3)) + expect(json_response['committer_name']).to eq(commit.committer_name) + expect(json_response['committer_email']).to eq(commit.committer_email) + expect(json_response['committed_date']).to eq(commit.committed_date.iso8601(3)) + expect(json_response['parent_ids']).to eq(commit.parent_ids) + expect(json_response['stats']['additions']).to eq(commit.stats.additions) + expect(json_response['stats']['deletions']).to eq(commit.stats.deletions) + expect(json_response['stats']['total']).to eq(commit.stats.total) end it "returns a 404 error if not found" do diff --git a/spec/views/projects/_home_panel.html.haml_spec.rb b/spec/views/projects/_home_panel.html.haml_spec.rb new file mode 100644 index 00000000000..5af57cdf3b7 --- /dev/null +++ b/spec/views/projects/_home_panel.html.haml_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe 'projects/_home_panel', :view do + let(:project) { create(:empty_project, :public) } + + let(:notification_settings) do + user.notification_settings_for(project) if user + end + + before do + assign(:project, project) + assign(:notification_setting, notification_settings) + + allow(view).to receive(:current_user).and_return(user) + allow(view).to receive(:can?).and_return(false) + end + + context 'when user is signed in' do + let(:user) { create(:user) } + + it 'makes it possible to set notification level' do + render + + expect(view).to render_template('shared/notifications/_button') + expect(rendered).to have_selector('.notification-dropdown') + end + end + + context 'when user is signed out' do + let(:user) { nil } + + it 'is not possible to set notification level' do + render + + expect(rendered).not_to have_selector('.notification_dropdown') + end + end +end |