diff options
author | Stan Hu <stanhu@gmail.com> | 2017-10-14 12:58:11 -0700 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2017-10-14 12:58:11 -0700 |
commit | 9d42830f9f89df2413519b22438373b7678b711a (patch) | |
tree | be9e886efe84e88fa82c6361effdf08441a29bf1 | |
parent | fc451b7dda7e6e145ab566155b4a5cbf3ccde305 (diff) | |
parent | 693285ef637497c0ac4fdb541c843e5ec600ce48 (diff) | |
download | gitlab-ce-9d42830f9f89df2413519b22438373b7678b711a.tar.gz |
Merge remote-tracking branch 'origin/security-9-5' into 3435-backport-9-5
144 files changed, 2015 insertions, 707 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ee2a2cae21..7f6bba82021 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,50 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 9.5.8 (2017-10-04) + +- [FIXED] Fixed fork button being disabled for users who can fork to a group. + +## 9.5.7 (2017-10-03) + +- Fix gitlab rake:import:repos task. + +## 9.5.6 (2017-09-29) + +- [FIXED] Fix MR ready to merge buttons/controls at mobile breakpoint. !14242 +- [FIXED] Fix errors thrown in merge request widget with external CI service/integration. +- [FIXED] Update x/x discussions resolved checkmark icon to be green when all discussions resolved. +- [FIXED] Fix 500 error on merged merge requests when GitLab is restored from a backup. + +## 9.5.5 (2017-09-18) + +- [SECURITY] Upgrade mail and nokogiri gems due to security issues. !13662 (Markus Koller) +- [FIXED] Fix division by zero error in blame age mapping. !13803 (Jeff Stubler) +- [FIXED] Fix problems sanitizing URLs with empty passwords. !14083 +- [FIXED] Fix a wrong `X-Gitlab-Event` header when testing webhooks. !14108 +- [FIXED] Fixes the 500 errors caused by a race condition in GPG's tmp directory handling. !14194 (Alexis Reigel) +- [FIXED] Fix Pipeline Triggers to show triggered label and predefined variables (e.g. CI_PIPELINE_TRIGGERED). !14244 +- [FIXED] Fix project feature being deleted when updating project with invalid visibility level. +- [FIXED] Fix new navigation wrapping and causing height to grow. +- [FIXED] Fix buttons with different height in merge request widget. +- [FIXED] Normalize styles for empty state combo button. +- [FIXED] Fix broken svg in jobs dropdown for success status. +- [FIXED] Improve migrations using triggers. +- [FIXED] Disable GitLab Project Import Button if source disabled. +- [CHANGED] Update the GPG verification semantics: A GPG signature must additionally match the committer in order to be verified. !13771 (Alexis Reigel) +- [OTHER] Fix repository equality check and avoid fetching ref if the commit is already available. This affects merge request creation performance. !13685 +- [OTHER] Update documentation for confidential issue. !14117 + +## 9.5.4 (2017-09-06) + +- [SECURITY] Upgrade mail and nokogiri gems due to security issues. !13662 (Markus Koller) +- [SECURITY] Prevent a persistent XSS in the commit author block. +- Fix XSS issue in go-get handling. +- Resolve CSRF token leakage via pathname manipulation on environments page. +- Fixes race condition in project uploads. +- Disallow arbitrary properties in `th` and `td` `style` attributes. +- Disallow the `name` attribute on all user-provided markup. + ## 9.5.3 (2017-09-03) - [SECURITY] Filter additional secrets from Rails logs. @@ -321,6 +321,7 @@ group :development, :test do gem 'spinach-rerun-reporter', '~> 0.0.2' gem 'rspec_profiling', '~> 0.0.5' gem 'rspec-set', '~> 0.1.3' + gem 'rspec-parameterized' # Prevent occasions where minitest is not bundled in packaged versions of ruby (see #3826) gem 'minitest', '~> 5.7.0' diff --git a/Gemfile.lock b/Gemfile.lock index e4054665def..2809729698f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -2,6 +2,7 @@ GEM remote: https://rubygems.org/ specs: RedCloth (4.3.2) + abstract_type (0.0.7) ace-rails-ap (4.1.2) actionmailer (4.2.8) actionpack (= 4.2.8) @@ -41,6 +42,9 @@ GEM tzinfo (~> 1.1) acts-as-taggable-on (4.0.0) activerecord (>= 4.0) + adamantium (0.2.0) + ice_nine (~> 0.11.0) + memoizable (~> 0.4.0) addressable (2.3.8) after_commit_queue (1.3.0) activerecord (>= 3.0) @@ -124,6 +128,9 @@ GEM coercible (1.0.0) descendants_tracker (~> 0.0.1) colorize (0.7.7) + concord (0.1.5) + adamantium (~> 0.2.0) + equalizer (~> 0.0.9) concurrent-ruby (1.0.5) concurrent-ruby-ext (1.0.5) concurrent-ruby (= 1.0.5) @@ -471,9 +478,12 @@ GEM mime-types (>= 1.16, < 4) mail_room (0.9.1) memoist (0.15.0) + memoizable (0.4.2) + thread_safe (~> 0.3, >= 0.3.1) method_source (0.8.2) mime-types (2.99.3) mimemagic (0.3.0) + mini_mime (0.1.4) mini_portile2 (2.3.0) minitest (5.7.0) mmap2 (2.2.7) @@ -724,6 +734,10 @@ GEM chunky_png rqrcode-rails3 (0.1.7) rqrcode (>= 0.4.2) + rspec (3.6.0) + rspec-core (~> 3.6.0) + rspec-expectations (~> 3.6.0) + rspec-mocks (~> 3.6.0) rspec-core (3.6.0) rspec-support (~> 3.6.0) rspec-expectations (3.6.0) @@ -732,6 +746,12 @@ GEM rspec-mocks (3.6.0) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.6.0) + rspec-parameterized (0.4.0) + binding_of_caller + parser + proc_to_ast + rspec (>= 2.13, < 4) + unparser rspec-rails (3.6.0) actionpack (>= 3.0) activesupport (>= 3.0) @@ -896,6 +916,14 @@ GEM get_process_mem (~> 0) unicorn (>= 4, < 6) uniform_notifier (1.10.0) + unparser (0.2.6) + abstract_type (~> 0.0.7) + adamantium (~> 0.2.0) + concord (~> 0.1.5) + diff-lcs (~> 1.3) + equalizer (~> 0.0.9) + parser (>= 2.3.1.2, < 2.5) + procto (~> 0.0.2) url_safe_base64 (0.2.2) validates_hostname (1.0.6) activerecord (>= 3.0) @@ -1098,6 +1126,7 @@ DEPENDENCIES responders (~> 2.0) rouge (~> 2.0) rqrcode-rails3 (~> 0.1.7) + rspec-parameterized rspec-rails (~> 3.6.0) rspec-retry (~> 0.4.5) rspec-set (~> 0.1.3) @@ -1 +1 @@ -9.5.3 +9.5.8 diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index a0ed5c23ffe..be21116f6fb 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -14,7 +14,6 @@ /* global NotificationsDropdown */ /* global GroupAvatar */ /* global LineHighlighter */ -/* global ProjectFork */ /* global BuildArtifacts */ /* global GroupsSelect */ /* global Search */ @@ -468,7 +467,9 @@ import initChangesDropdown from './init_changes_dropdown'; shortcut_handler = true; break; case 'projects:forks:new': - new ProjectFork(); + import(/* webpackChunkName: 'project_fork' */ './project_fork') + .then(fork => fork.default()) + .catch(() => {}); break; case 'projects:artifacts:browse': new ShortcutsNavigation(); diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 6d7c7e3c930..0af3eba2a87 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -124,7 +124,6 @@ import './preview_markdown'; import './project'; import './project_avatar'; import './project_find_file'; -import './project_fork'; import './project_import'; import './project_label_subscription'; import './project_new'; @@ -248,7 +247,10 @@ $(function () { // Initialize popovers $body.popover({ selector: '[data-toggle="popover"]', - trigger: 'focus' + trigger: 'focus', + // set the viewport to the main content, excluding the navigation bar, so + // the navigation can't overlap the popover + viewport: '.page-with-sidebar' }); $('.trigger-submit').on('change', function () { return $(this).parents('form').submit(); diff --git a/app/assets/javascripts/project_fork.js b/app/assets/javascripts/project_fork.js index 47197db39d3..68cf47fd54e 100644 --- a/app/assets/javascripts/project_fork.js +++ b/app/assets/javascripts/project_fork.js @@ -1,13 +1,8 @@ -/* eslint-disable func-names, space-before-function-paren, wrap-iife, prefer-arrow-callback, max-len */ -(function() { - this.ProjectFork = (function() { - function ProjectFork() { - $('.fork-thumbnail a').on('click', function() { - $('.fork-namespaces').hide(); - return $('.save-project-loader').show(); - }); - } +export default () => { + $('.fork-thumbnail a').on('click', function forkThumbnailClicked() { + if ($(this).hasClass('disabled')) return false; - return ProjectFork; - })(); -}).call(window); + $('.fork-namespaces').hide(); + return $('.save-project-loader').show(); + }); +}; diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.js b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.js index c05a76a3b4a..aaca42e3ebc 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.js @@ -75,18 +75,20 @@ export default { class="btn btn-small inline"> Check out branch </a> - <span class="dropdown inline prepend-left-10"> + <span class="dropdown prepend-left-10"> <a - class="btn btn-xs dropdown-toggle" + class="btn btn-small inline dropdown-toggle" data-toggle="dropdown" aria-label="Download as" role="button"> <i class="fa fa-download" - aria-hidden="true" /> + aria-hidden="true"> + </i> <i class="fa fa-caret-down" - aria-hidden="true" /> + aria-hidden="true"> + </i> </a> <ul class="dropdown-menu dropdown-menu-align-right"> <li> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.js b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.js index 6c2e9ba1d30..c79b5c720eb 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.js @@ -12,6 +12,9 @@ export default { ciIcon, }, computed: { + hasPipeline() { + return this.mr.pipeline && Object.keys(this.mr.pipeline).length > 0; + }, hasCIError() { const { hasCI, ciStatus } = this.mr; @@ -28,7 +31,9 @@ export default { }, }, template: ` - <div class="mr-widget-heading"> + <div + v-if="hasPipeline || hasCIError" + class="mr-widget-heading"> <div class="ci-widget media"> <template v-if="hasCIError"> <div class="ci-status-icon ci-status-icon-failed ci-error js-ci-error append-right-10"> @@ -40,7 +45,7 @@ export default { Could not connect to the CI server. Please check your settings and try again </div> </template> - <template v-else> + <template v-else-if="hasPipeline"> <div class="ci-status-icon append-right-10"> <a class="icon-link" diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js index 65187754009..557b89b2b84 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js @@ -29,6 +29,9 @@ export default { statusIcon, }, computed: { + shouldShowMergeWhenPipelineSucceedsText() { + return this.mr.isPipelineActive; + }, commitMessageLinkTitle() { const withDesc = 'Include description in commit message'; const withoutDesc = "Don't include description in commit message"; @@ -56,7 +59,7 @@ export default { mergeButtonText() { if (this.isMergingImmediately) { return 'Merge in progress'; - } else if (this.mr.isPipelineActive) { + } else if (this.shouldShowMergeWhenPipelineSucceedsText) { return 'Merge when pipeline succeeds'; } @@ -68,7 +71,7 @@ export default { isMergeButtonDisabled() { const { commitMessage } = this; return Boolean(!commitMessage.length - || !this.isMergeAllowed() + || !this.shouldShowMergeControls() || this.isMakingRequest || this.mr.preventMerge); }, @@ -82,7 +85,12 @@ export default { }, methods: { isMergeAllowed() { - return !(this.mr.onlyAllowMergeIfPipelineSucceeds && this.mr.isPipelineFailed); + return !this.mr.onlyAllowMergeIfPipelineSucceeds || + this.mr.isPipelinePassing || + this.mr.isPipelineSkipped; + }, + shouldShowMergeControls() { + return this.isMergeAllowed() || this.shouldShowMergeWhenPipelineSucceedsText; }, updateCommitMessage() { const cmwd = this.mr.commitMessageWithDescription; @@ -202,8 +210,8 @@ export default { <div class="mr-widget-body media"> <status-icon status="success" /> <div class="media-body"> - <div class="media space-children"> - <span class="btn-group"> + <div class="mr-widget-body-controls media space-children"> + <span class="btn-group append-bottom-5"> <button @click="handleMergeButtonClick()" :disabled="isMergeButtonDisabled" @@ -260,8 +268,8 @@ export default { </li> </ul> </span> - <div class="media-body space-children"> - <template v-if="isMergeAllowed()"> + <div class="media-body-wrap space-children"> + <template v-if="shouldShowMergeControls()"> <label> <input id="remove-source-branch-input" @@ -286,7 +294,7 @@ export default { </template> <template v-else> <span class="bold"> - The pipeline for this merge request failed. Please retry the job or push a new commit to fix the failure + The pipeline for this merge request has not succeeded yet </span> </template> </div> diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js index 0042c48816f..2f237262028 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js @@ -57,7 +57,7 @@ export default { return stateMaps.statesToShowHelpWidget.indexOf(this.mr.state) > -1; }, shouldRenderPipelines() { - return Object.keys(this.mr.pipeline).length || this.mr.hasCI; + return this.mr.hasCI; }, shouldRenderRelatedLinks() { return this.mr.relatedLinks; diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index fbea764b739..29464662578 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -85,7 +85,9 @@ export default class MergeRequestStore { this.ciEnvironmentsStatusPath = data.ci_environments_status_path; this.hasCI = data.has_ci; this.ciStatus = data.ci_status; - this.isPipelineFailed = this.ciStatus ? (this.ciStatus === 'failed' || this.ciStatus === 'canceled') : false; + this.isPipelineFailed = this.ciStatus === 'failed' || this.ciStatus === 'canceled'; + this.isPipelinePassing = this.ciStatus === 'success' || this.ciStatus === 'success_with_warnings'; + this.isPipelineSkipped = this.ciStatus === 'skipped'; this.pipelineDetailedStatus = pipelineStatus; this.isPipelineActive = data.pipeline ? data.pipeline.active : false; this.isPipelineBlocked = pipelineStatus ? pipelineStatus.group === 'manual' : false; diff --git a/app/assets/stylesheets/framework/media_object.scss b/app/assets/stylesheets/framework/media_object.scss index b573052c14a..89c561479cc 100644 --- a/app/assets/stylesheets/framework/media_object.scss +++ b/app/assets/stylesheets/framework/media_object.scss @@ -6,3 +6,7 @@ .media-body { flex: 1; } + +.media-body-wrap { + flex-grow: 1; +} diff --git a/app/assets/stylesheets/framework/nav.scss b/app/assets/stylesheets/framework/nav.scss index d386ac5ba9c..5de72a66958 100644 --- a/app/assets/stylesheets/framework/nav.scss +++ b/app/assets/stylesheets/framework/nav.scss @@ -286,11 +286,7 @@ display: flex; max-width: 350px; overflow: hidden; - - @media(max-width: $screen-xs-max) { - width: 100%; - max-width: none; - } + float: right; .new-project-item-link { white-space: nowrap; @@ -303,6 +299,23 @@ } } +.empty-state .project-item-select-holder.btn-group { + float: none; + display: inline-block; + + .btn { + // overrides styles applied to plain `.empty-state .btn` + margin: 10px 0; + max-width: 300px; + width: auto; + + @media(max-width: $screen-xs-max) { + max-width: 250px; + } + + } +} + .new-project-item-select-button .fa-caret-down { margin-left: 2px; } diff --git a/app/assets/stylesheets/new_sidebar.scss b/app/assets/stylesheets/new_sidebar.scss index faedd207e01..25c6715899b 100644 --- a/app/assets/stylesheets/new_sidebar.scss +++ b/app/assets/stylesheets/new_sidebar.scss @@ -70,8 +70,7 @@ $new-sidebar-collapsed-width: 50px; background-color: $white-light; } - .project-title, - .group-title { + .sidebar-context-title { overflow: hidden; text-overflow: ellipsis; } @@ -97,21 +96,28 @@ $new-sidebar-collapsed-width: 50px; top: $header-height; bottom: 0; left: 0; - overflow: auto; background-color: $gray-normal; box-shadow: inset -2px 0 0 $border-color; + transform: translate3d(0, 0, 0); &.sidebar-icons-only { width: $new-sidebar-collapsed-width; - overflow-x: hidden; + + .nav-sidebar-inner-scroll { + overflow-x: hidden; + } .badge, - .project-title { + .sidebar-context-title { display: none; } .nav-item-name { - opacity: 0; + display: none; + } + + .sidebar-top-level-items > li > a { + min-height: 44px; } } @@ -176,6 +182,12 @@ $new-sidebar-collapsed-width: 50px; } } +.nav-sidebar-inner-scroll { + height: 100%; + width: 100%; + overflow: auto; +} + .with-performance-bar .nav-sidebar { top: $header-height + $performance-bar-height; } diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 31fce49e1f8..cb66cda2e67 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -367,6 +367,10 @@ } } +.mr-widget-body-controls { + flex-wrap: wrap; +} + .mr_source_commit, .mr_target_commit { margin-bottom: 0; diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index fbfe5d3c682..701d89cf7fb 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -766,6 +766,7 @@ ul.notes { background-color: transparent; border: none; outline: 0; + color: $gray-darkest; transition: color $general-hover-transition-duration $general-hover-transition-curve; &.is-disabled { @@ -789,7 +790,7 @@ ul.notes { } svg { - fill: $gray-darkest; + fill: currentColor; height: 16px; width: 16px; } diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index d01326637ea..c53ad008567 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -417,7 +417,7 @@ a.deploy-project-label { text-align: center; width: 169px; - &:hover, + &:hover:not(.disabled), &.forked { background-color: $row-hover; border-color: $row-hover-border; @@ -444,6 +444,15 @@ a.deploy-project-label { padding-top: $gl-padding; color: $gl-text-color; + &.disabled { + opacity: .3; + cursor: not-allowed; + + &:hover { + text-decoration: none; + } + } + .caption { min-height: 30px; padding: $gl-padding 0; diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index e2ccabb22db..917ee3955e2 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -212,7 +212,7 @@ class Projects::IssuesController < Projects::ApplicationController end def create_merge_request - result = MergeRequests::CreateFromIssueService.new(project, current_user, issue_iid: issue.iid).execute + result = ::MergeRequests::CreateFromIssueService.new(project, current_user, issue_iid: issue.iid).execute if result[:status] == :success render json: MergeRequestCreateSerializer.new.represent(result[:merge_request]) diff --git a/app/helpers/blame_helper.rb b/app/helpers/blame_helper.rb index d1dc4d94560..089d9e3e387 100644 --- a/app/helpers/blame_helper.rb +++ b/app/helpers/blame_helper.rb @@ -11,11 +11,15 @@ module BlameHelper end def age_map_class(commit_date, duration) - commit_date_days_ago = (duration[:now] - commit_date).to_i / 1.day - # Numbers 0 to 10 come from this calculation, but only commits on the oldest - # day get number 10 (all other numbers can be multiple days), so the range - # is normalized to 0-9 - age_group = [(10 * commit_date_days_ago) / duration[:started_days_ago], 9].min - "blame-commit-age-#{age_group}" + if duration[:started_days_ago] == 0 + "blame-commit-age-0" + else + commit_date_days_ago = (duration[:now] - commit_date).to_i / 1.day + # Numbers 0 to 10 come from this calculation, but only commits on the oldest + # day get number 10 (all other numbers can be multiple days), so the range + # is normalized to 0-9 + age_group = [(10 * commit_date_days_ago) / duration[:started_days_ago], 9].min + "blame-commit-age-#{age_group}" + end end end diff --git a/app/helpers/builds_helper.rb b/app/helpers/builds_helper.rb index 85bc784d53c..aa3a9a055a0 100644 --- a/app/helpers/builds_helper.rb +++ b/app/helpers/builds_helper.rb @@ -30,7 +30,7 @@ module BuildsHelper def build_failed_issue_options { - title: "Build Failed ##{@build.id}", + title: "Job Failed ##{@build.id}", description: project_job_url(@project, @build) } end diff --git a/app/models/commit.rb b/app/models/commit.rb index 71aa93d0979..96605c9168b 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -393,6 +393,6 @@ class Commit end def gpg_commit - @gpg_commit ||= Gitlab::Gpg::Commit.for_commit(self) + @gpg_commit ||= Gitlab::Gpg::Commit.new(self) end end diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index 3df60ddc950..1633acd4fa9 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -56,7 +56,7 @@ class GpgKey < ActiveRecord::Base def verified_user_infos user_infos.select do |user_info| - user_info[:email] == user.email + user.verified_email?(user_info[:email]) end end @@ -64,13 +64,17 @@ class GpgKey < ActiveRecord::Base user_infos.map do |user_info| [ user_info[:email], - user_info[:email] == user.email + user.verified_email?(user_info[:email]) ] end.to_h end def verified? - emails_with_verified_status.any? { |_email, verified| verified } + emails_with_verified_status.values.any? + end + + def verified_and_belongs_to_email?(email) + emails_with_verified_status.fetch(email, false) end def update_invalid_gpg_signatures @@ -78,11 +82,14 @@ class GpgKey < ActiveRecord::Base end def revoke - GpgSignature.where(gpg_key: self, valid_signature: true).update_all( - gpg_key_id: nil, - valid_signature: false, - updated_at: Time.zone.now - ) + GpgSignature + .where(gpg_key: self) + .where.not(verification_status: GpgSignature.verification_statuses[:unknown_key]) + .update_all( + gpg_key_id: nil, + verification_status: GpgSignature.verification_statuses[:unknown_key], + updated_at: Time.zone.now + ) destroy end diff --git a/app/models/gpg_signature.rb b/app/models/gpg_signature.rb index 50fb35c77ec..454c90d5fc4 100644 --- a/app/models/gpg_signature.rb +++ b/app/models/gpg_signature.rb @@ -1,9 +1,21 @@ class GpgSignature < ActiveRecord::Base include ShaAttribute + include IgnorableColumn + + ignore_column :valid_signature sha_attribute :commit_sha sha_attribute :gpg_key_primary_keyid + enum verification_status: { + unverified: 0, + verified: 1, + same_user_different_email: 2, + other_user: 3, + unverified_key: 4, + unknown_key: 5 + } + belongs_to :project belongs_to :gpg_key @@ -20,6 +32,6 @@ class GpgSignature < ActiveRecord::Base end def gpg_commit - Gitlab::Gpg::Commit.new(project, commit_sha) + Gitlab::Gpg::Commit.new(commit) end end diff --git a/app/models/issue.rb b/app/models/issue.rb index 1c948c8957e..cc8289b78b0 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -269,6 +269,10 @@ class Issue < ActiveRecord::Base end end + def update_project_counter_caches + Projects::OpenIssuesCountService.new(project).refresh_cache + end + private # Returns `true` if the given User can read the current Issue. diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 9fc526ec3ef..74ea71a52c3 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -443,7 +443,8 @@ class MergeRequest < ActiveRecord::Base end def reload_diff_if_branch_changed - if source_branch_changed? || target_branch_changed? + if (source_branch_changed? || target_branch_changed?) && + (source_branch_head && target_branch_head) reload_diff end end @@ -794,11 +795,7 @@ class MergeRequest < ActiveRecord::Base end def fetch_ref - target_project.repository.fetch_ref( - source_project.repository.path_to_repo, - "refs/heads/#{source_branch}", - ref_path - ) + write_ref update_column(:ref_fetched, true) end @@ -941,4 +938,19 @@ class MergeRequest < ActiveRecord::Base true end + + def update_project_counter_caches + Projects::OpenMergeRequestsCountService.new(target_project).refresh_cache + end + + private + + def write_ref + target_project.repository.with_repo_branch_commit( + source_project.repository, source_branch) do |commit| + if commit + target_project.repository.write_ref(ref_path, commit.sha) + end + end + end end diff --git a/app/models/project.rb b/app/models/project.rb index 2666bcb4438..b2a75d2b332 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -77,6 +77,7 @@ class Project < ActiveRecord::Base attr_accessor :old_path_with_namespace attr_accessor :template_name attr_writer :pipeline_status + attr_accessor :skip_disk_validation alias_attribute :title, :name @@ -165,7 +166,7 @@ class Project < ActiveRecord::Base has_many :notification_settings, as: :source, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent has_one :import_data, class_name: 'ProjectImportData', inverse_of: :project, autosave: true - has_one :project_feature + has_one :project_feature, inverse_of: :project has_one :statistics, class_name: 'ProjectStatistics' # Container repositories need to remove data from the container registry, @@ -192,7 +193,7 @@ class Project < ActiveRecord::Base has_many :active_runners, -> { active }, through: :runner_projects, source: :runner, class_name: 'Ci::Runner' accepts_nested_attributes_for :variables, allow_destroy: true - accepts_nested_attributes_for :project_feature + accepts_nested_attributes_for :project_feature, update_only: true accepts_nested_attributes_for :import_data delegate :name, to: :owner, allow_nil: true, prefix: true @@ -993,6 +994,7 @@ class Project < ActiveRecord::Base # Check if repository already exists on disk def can_create_repository? + return true if skip_disk_validation return false unless repository_storage_path if gitlab_shell.exists?(repository_storage_path, "#{build_full_path}.git") @@ -1061,13 +1063,16 @@ class Project < ActiveRecord::Base end def change_head(branch) - repository.before_change_head - repository.rugged.references.create('HEAD', - "refs/heads/#{branch}", - force: true) - repository.copy_gitattributes(branch) - repository.after_change_head - reload_default_branch + if repository.branch_exists?(branch) + repository.before_change_head + repository.write_ref('HEAD', "refs/heads/#{branch}") + repository.copy_gitattributes(branch) + repository.after_change_head + reload_default_branch + else + errors.add(:base, "Could not change HEAD: branch '#{branch}' does not exist") + false + end end def forked_from?(project) diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index c8fabb16dc1..9ab69295f68 100644 --- a/app/models/project_feature.rb +++ b/app/models/project_feature.rb @@ -41,6 +41,8 @@ class ProjectFeature < ActiveRecord::Base # http://stackoverflow.com/questions/1540645/how-to-disable-default-scope-for-a-belongs-to belongs_to :project, -> { unscope(where: :pending_delete) } + validates :project, presence: true + validate :repository_children_level default_value_for :builds_access_level, value: ENABLED, allows_nil: false diff --git a/app/models/project_services/pipelines_email_service.rb b/app/models/project_services/pipelines_email_service.rb index 9d37184be2c..6a3118a11b8 100644 --- a/app/models/project_services/pipelines_email_service.rb +++ b/app/models/project_services/pipelines_email_service.rb @@ -80,6 +80,6 @@ class PipelinesEmailService < Service end def retrieve_recipients(data) - recipients.to_s.split(',').reject(&:blank?) + recipients.to_s.split(/[,(?:\r?\n) ]+/).reject(&:empty?) end end diff --git a/app/models/repository.rb b/app/models/repository.rb index ff82b958255..e55daf48dbb 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -58,6 +58,10 @@ class Repository @project = project end + def ==(other) + @disk_path == other.disk_path + end + def raw_repository return nil unless full_path @@ -73,6 +77,10 @@ class Repository ) end + def inspect + "#<#{self.class.name}:#{@disk_path}>" + end + # # Git repository can contains some hidden refs like: # /refs/notes/* @@ -224,7 +232,7 @@ class Repository # This will still fail if the file is corrupted (e.g. 0 bytes) begin - rugged.references.create(keep_around_ref_name(sha), sha, force: true) + write_ref(keep_around_ref_name(sha), sha) rescue Rugged::ReferenceError => ex Rails.logger.error "Unable to create keep-around reference for repository #{path}: #{ex}" rescue Rugged::OSError => ex @@ -237,6 +245,10 @@ class Repository ref_exists?(keep_around_ref_name(sha)) end + def write_ref(ref_path, sha) + rugged.references.create(ref_path, sha, force: true) + end + def diverging_commit_counts(branch) root_ref_hash = raw_repository.rev_parse_target(root_ref).oid cache.fetch(:"diverging_commit_counts_#{branch.name}") do @@ -979,27 +991,26 @@ class Repository end def with_repo_branch_commit(start_repository, start_branch_name) - return yield(nil) if start_repository.empty_repo? + return yield nil if start_repository.empty_repo? - branch_name_or_sha = - if start_repository == self - start_branch_name - else - tmp_ref = "refs/tmp/#{SecureRandom.hex}/head" - - fetch_ref( - start_repository.path_to_repo, - "#{Gitlab::Git::BRANCH_REF_PREFIX}#{start_branch_name}", - tmp_ref - ) + if start_repository == self + yield commit(start_branch_name) + else + start_commit = start_repository.commit(start_branch_name) - start_repository.commit(start_branch_name).sha - end + return yield nil unless start_commit - yield(commit(branch_name_or_sha)) + sha = start_commit.sha - ensure - rugged.references.delete(tmp_ref) if tmp_ref + if branch_commit = commit(sha) + yield branch_commit + else + with_repo_tmp_commit( + start_repository, start_branch_name, sha) do |tmp_commit| + yield tmp_commit + end + end + end end def add_remote(name, url) @@ -1021,7 +1032,12 @@ class Repository def fetch_ref(source_path, source_ref, target_ref) args = %W(fetch --no-tags -f #{source_path} #{source_ref}:#{target_ref}) - run_git(args) + message, status = run_git(args) + + # Make sure ref was created, and raise Rugged::ReferenceError when not + raise Rugged::ReferenceError, message if status != 0 + + target_ref end def create_ref(ref, ref_path) @@ -1203,4 +1219,16 @@ class Repository .commits_by_message(query, revision: ref, path: path, limit: limit, offset: offset) .map { |c| commit(c) } end + + def with_repo_tmp_commit(start_repository, start_branch_name, sha) + tmp_ref = fetch_ref( + start_repository.path_to_repo, + "#{Gitlab::Git::BRANCH_REF_PREFIX}#{start_branch_name}", + "refs/tmp/#{SecureRandom.hex}/head" + ) + + yield commit(sha) + ensure + rugged.references.delete(tmp_ref) if tmp_ref + end end diff --git a/app/models/user.rb b/app/models/user.rb index d09e8478b69..eccb29f8566 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1045,6 +1045,10 @@ class User < ActiveRecord::Base ensure_rss_token! end + def verified_email?(email) + self.email == email + end + protected # override, from Devise::Validatable diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb index 1be7bbe9953..0bf1e12b6db 100644 --- a/app/policies/global_policy.rb +++ b/app/policies/global_policy.rb @@ -11,6 +11,8 @@ class GlobalPolicy < BasePolicy with_options scope: :user, score: 0 condition(:access_locked) { @user.access_locked? } + condition(:can_create_fork, scope: :user) { @user.manageable_namespaces.any? { |namespace| @user.can?(:create_projects, namespace) } } + rule { anonymous }.policy do prevent :log_in prevent :access_api @@ -40,6 +42,10 @@ class GlobalPolicy < BasePolicy enable :create_group end + rule { can_create_fork }.policy do + enable :create_fork + end + rule { access_locked }.policy do prevent :log_in end diff --git a/app/policies/namespace_policy.rb b/app/policies/namespace_policy.rb index 85b67f0a237..92213f0155e 100644 --- a/app/policies/namespace_policy.rb +++ b/app/policies/namespace_policy.rb @@ -1,10 +1,14 @@ class NamespacePolicy < BasePolicy rule { anonymous }.prevent_all + condition(:personal_project, scope: :subject) { @subject.kind == 'user' } + condition(:can_create_personal_project, scope: :user) { @user.can_create_project? } condition(:owner) { @subject.owner == @user } rule { owner | admin }.policy do enable :create_projects enable :admin_namespace end + + rule { personal_project & ~can_create_personal_project }.prevent :create_projects end diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb index 743a08acefe..8c89eea607f 100644 --- a/app/serializers/build_details_entity.rb +++ b/app/serializers/build_details_entity.rb @@ -32,8 +32,8 @@ class BuildDetailsEntity < JobEntity private def build_failed_issue_options - { title: "Build Failed ##{build.id}", - description: project_job_path(project, build) } + { title: "Job Failed ##{build.id}", + description: "Job [##{build.id}](#{project_job_path(project, build)}) failed for #{build.sha}:\n" } end def current_user diff --git a/app/services/ci/pipeline_trigger_service.rb b/app/services/ci/pipeline_trigger_service.rb index 1e5ad28ba57..120af8c1e61 100644 --- a/app/services/ci/pipeline_trigger_service.rb +++ b/app/services/ci/pipeline_trigger_service.rb @@ -14,7 +14,7 @@ module Ci pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: params[:ref]) .execute(:trigger, ignore_skip_ci: true) do |pipeline| - trigger.trigger_requests.create!(pipeline: pipeline) + pipeline.trigger_requests.create!(trigger: trigger) create_pipeline_variables!(pipeline) end diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index ca02b456ae5..876fc78310b 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -28,7 +28,10 @@ module Projects success else - error('Project could not be updated!') + model_errors = project.errors.full_messages.to_sentence + error_message = model_errors.presence || 'Project could not be updated!' + + error(error_message) end end diff --git a/app/services/test_hooks/base_service.rb b/app/services/test_hooks/base_service.rb index 74ba814afff..29b6af41912 100644 --- a/app/services/test_hooks/base_service.rb +++ b/app/services/test_hooks/base_service.rb @@ -9,18 +9,17 @@ module TestHooks end def execute + trigger_key = hook.class::TRIGGERS.key(trigger.to_sym) trigger_data_method = "#{trigger}_data" - if !self.respond_to?(trigger_data_method, true) || - !hook.class::TRIGGERS.value?(trigger.to_sym) - + if trigger_key.nil? || !self.respond_to?(trigger_data_method, true) return error('Testing not available for this hook') end error_message = catch(:validation_error) do sample_data = self.__send__(trigger_data_method) - return hook.execute(sample_data, trigger) + return hook.execute(sample_data, trigger_key) end error(error_message) diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 2825478926a..cd99e0b90f9 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -19,7 +19,7 @@ class WebHookService def initialize(hook, data, hook_name) @hook = hook @data = data - @hook_name = hook_name + @hook_name = hook_name.to_s end def execute diff --git a/app/views/layouts/nav/_new_dashboard.html.haml b/app/views/layouts/nav/_new_dashboard.html.haml index cfdfcbebc9f..f9f557518d3 100644 --- a/app/views/layouts/nav/_new_dashboard.html.haml +++ b/app/views/layouts/nav/_new_dashboard.html.haml @@ -7,7 +7,7 @@ = link_to dashboard_groups_path, class: 'dashboard-shortcuts-groups', title: 'Groups' do Groups - = nav_link(path: 'dashboard#activity', html_options: { class: "hidden-xs hidden-sm" }) do + = nav_link(path: 'dashboard#activity', html_options: { class: "hidden-xs hidden-sm hidden-md" }) do = link_to activity_dashboard_path, class: 'dashboard-shortcuts-activity', title: 'Activity' do Activity @@ -17,7 +17,7 @@ = icon("chevron-down", class: "dropdown-chevron") .dropdown-menu %ul - = nav_link(path: 'dashboard#activity', html_options: { class: "visible-xs visible-sm" }) do + = nav_link(path: 'dashboard#activity', html_options: { class: "visible-xs visible-sm visible-md" }) do = link_to activity_dashboard_path, title: 'Activity' do Activity diff --git a/app/views/projects/buttons/_fork.html.haml b/app/views/projects/buttons/_fork.html.haml index f45cc7f0f45..f880556a9f7 100644 --- a/app/views/projects/buttons/_fork.html.haml +++ b/app/views/projects/buttons/_fork.html.haml @@ -4,12 +4,11 @@ = link_to namespace_project_path(current_user, current_user.fork_of(@project)), title: _('Go to your fork'), class: 'btn has-tooltip' do = custom_icon('icon_fork') %span= s_('GoToYourFork|Fork') - - elsif !current_user.can_create_project? - = link_to new_project_fork_path(@project), title: _('You have reached your project limit'), class: 'btn has-tooltip disabled' do - = custom_icon('icon_fork') - %span= s_('CreateNewFork|Fork') - else - = link_to new_project_fork_path(@project), class: 'btn' do + - can_create_fork = current_user.can?(:create_fork) + = link_to new_project_fork_path(@project), + class: "btn btn-default #{'has-tooltip disabled' unless can_create_fork}", + title: (_('You have reached your project limit') unless can_create_fork) do = custom_icon('icon_fork') %span= s_('CreateNewFork|Fork') .count-with-arrow diff --git a/app/views/projects/commit/_invalid_signature_badge.html.haml b/app/views/projects/commit/_invalid_signature_badge.html.haml deleted file mode 100644 index 3a73aae9d95..00000000000 --- a/app/views/projects/commit/_invalid_signature_badge.html.haml +++ /dev/null @@ -1,9 +0,0 @@ -- title = capture do - .gpg-popover-icon.invalid - = render 'shared/icons/icon_status_notfound_borderless.svg' - %div - This commit was signed with an <strong>unverified</strong> signature. - -- locals = { signature: signature, title: title, label: 'Unverified', css_classes: ['invalid'] } - -= render partial: 'projects/commit/signature_badge', locals: locals diff --git a/app/views/projects/commit/_other_user_signature_badge.html.haml b/app/views/projects/commit/_other_user_signature_badge.html.haml new file mode 100644 index 00000000000..80eca96f7ce --- /dev/null +++ b/app/views/projects/commit/_other_user_signature_badge.html.haml @@ -0,0 +1,6 @@ +- title = capture do + This commit was signed with a different user's verified signature. + +- locals = { signature: signature, title: title, label: 'Unverified', css_class: 'invalid', icon: 'icon_status_notfound_borderless', show_user: true } + += render partial: 'projects/commit/signature_badge', locals: locals diff --git a/app/views/projects/commit/_same_user_different_email_signature_badge.html.haml b/app/views/projects/commit/_same_user_different_email_signature_badge.html.haml new file mode 100644 index 00000000000..e737de48e22 --- /dev/null +++ b/app/views/projects/commit/_same_user_different_email_signature_badge.html.haml @@ -0,0 +1,7 @@ +- title = capture do + This commit was signed with a verified signature, but the committer email + is <strong>not verified</strong> to belong to the same user. + +- locals = { signature: signature, title: title, label: 'Unverified', css_class: ['invalid'], icon: 'icon_status_notfound_borderless', show_user: true } + += render partial: 'projects/commit/signature_badge', locals: locals diff --git a/app/views/projects/commit/_signature.html.haml b/app/views/projects/commit/_signature.html.haml index 60fa52557ef..145bc629380 100644 --- a/app/views/projects/commit/_signature.html.haml +++ b/app/views/projects/commit/_signature.html.haml @@ -1,5 +1,2 @@ - if signature - - if signature.valid_signature? - = render partial: 'projects/commit/valid_signature_badge', locals: { signature: signature } - - else - = render partial: 'projects/commit/invalid_signature_badge', locals: { signature: signature } + = render partial: "projects/commit/#{signature.verification_status}_signature_badge", locals: { signature: signature } diff --git a/app/views/projects/commit/_signature_badge.html.haml b/app/views/projects/commit/_signature_badge.html.haml index a3783b31b86..78a06dd5074 100644 --- a/app/views/projects/commit/_signature_badge.html.haml +++ b/app/views/projects/commit/_signature_badge.html.haml @@ -1,17 +1,27 @@ -- css_classes = commit_signature_badge_classes(css_classes) +- signature = local_assigns.fetch(:signature) +- title = local_assigns.fetch(:title) +- label = local_assigns.fetch(:label) +- css_class = local_assigns.fetch(:css_class) +- icon = local_assigns.fetch(:icon) +- show_user = local_assigns.fetch(:show_user, false) + +- css_classes = commit_signature_badge_classes(css_class) - title = capture do .gpg-popover-status - = title + .gpg-popover-icon{ class: css_class } + = render "shared/icons/#{icon}.svg" + %div + = title - content = capture do - .clearfix - = content + - if show_user + .clearfix + = render partial: 'projects/commit/signature_badge_user', locals: { signature: signature } GPG Key ID: %span.monospace= signature.gpg_key_primary_keyid - = link_to('Learn more about signing commits', help_page_path('user/project/gpg_signed_commits/index.md'), class: 'gpg-popover-help-link') %button{ class: css_classes, data: { toggle: 'popover', html: 'true', placement: 'auto top', title: title, content: content } } diff --git a/app/views/projects/commit/_signature_badge_user.html.haml b/app/views/projects/commit/_signature_badge_user.html.haml new file mode 100644 index 00000000000..b20198e76db --- /dev/null +++ b/app/views/projects/commit/_signature_badge_user.html.haml @@ -0,0 +1,21 @@ +- gpg_key = signature.gpg_key +- user = gpg_key&.user +- user_name = signature.gpg_key_user_name +- user_email = signature.gpg_key_user_email + +- if user + = link_to user_path(user), class: 'gpg-popover-user-link' do + %div + = user_avatar_without_link(user: user, size: 32) + + %div + %strong= user.name + %div= user.to_reference +- else + = mail_to user_email do + %div + = user_avatar_without_link(user_name: user_name, user_email: user_email, size: 32) + + %div + %strong= user_name + %div= user_email diff --git a/app/views/projects/commit/_unknown_key_signature_badge.html.haml b/app/views/projects/commit/_unknown_key_signature_badge.html.haml new file mode 100644 index 00000000000..75c5cf57bcc --- /dev/null +++ b/app/views/projects/commit/_unknown_key_signature_badge.html.haml @@ -0,0 +1 @@ += render partial: 'projects/commit/unverified_signature_badge', locals: { signature: signature } diff --git a/app/views/projects/commit/_unverified_key_signature_badge.html.haml b/app/views/projects/commit/_unverified_key_signature_badge.html.haml new file mode 100644 index 00000000000..75c5cf57bcc --- /dev/null +++ b/app/views/projects/commit/_unverified_key_signature_badge.html.haml @@ -0,0 +1 @@ += render partial: 'projects/commit/unverified_signature_badge', locals: { signature: signature } diff --git a/app/views/projects/commit/_unverified_signature_badge.html.haml b/app/views/projects/commit/_unverified_signature_badge.html.haml new file mode 100644 index 00000000000..1af58027b83 --- /dev/null +++ b/app/views/projects/commit/_unverified_signature_badge.html.haml @@ -0,0 +1,6 @@ +- title = capture do + This commit was signed with an <strong>unverified</strong> signature. + +- locals = { signature: signature, title: title, label: 'Unverified', css_class: 'invalid', icon: 'icon_status_notfound_borderless' } + += render partial: 'projects/commit/signature_badge', locals: locals diff --git a/app/views/projects/commit/_valid_signature_badge.html.haml b/app/views/projects/commit/_valid_signature_badge.html.haml deleted file mode 100644 index db1a41bbf64..00000000000 --- a/app/views/projects/commit/_valid_signature_badge.html.haml +++ /dev/null @@ -1,32 +0,0 @@ -- title = capture do - .gpg-popover-icon.valid - = render 'shared/icons/icon_status_success_borderless.svg' - %div - This commit was signed with a <strong>verified</strong> signature. - -- content = capture do - - gpg_key = signature.gpg_key - - user = gpg_key&.user - - user_name = signature.gpg_key_user_name - - user_email = signature.gpg_key_user_email - - - if user - = link_to user_path(user), class: 'gpg-popover-user-link' do - %div - = user_avatar_without_link(user: user, size: 32) - - %div - %strong= gpg_key.user.name - %div @#{gpg_key.user.username} - - else - = mail_to user_email do - %div - = user_avatar_without_link(user_name: user_name, user_email: user_email, size: 32) - - %div - %strong= user_name - %div= user_email - -- locals = { signature: signature, title: title, content: content, label: 'Verified', css_classes: ['valid'] } - -= render partial: 'projects/commit/signature_badge', locals: locals diff --git a/app/views/projects/commit/_verified_signature_badge.html.haml b/app/views/projects/commit/_verified_signature_badge.html.haml new file mode 100644 index 00000000000..423beba2120 --- /dev/null +++ b/app/views/projects/commit/_verified_signature_badge.html.haml @@ -0,0 +1,7 @@ +- title = capture do + This commit was signed with a <strong>verified</strong> signature and the + committer email is verified to belong to the same user. + +- locals = { signature: signature, title: title, label: 'Verified', css_class: 'valid', icon: 'icon_status_success_borderless', show_user: true } + += render partial: 'projects/commit/signature_badge', locals: locals diff --git a/app/views/projects/forks/new.html.haml b/app/views/projects/forks/new.html.haml index 0f36e1a7353..906774a21e3 100644 --- a/app/views/projects/forks/new.html.haml +++ b/app/views/projects/forks/new.html.haml @@ -13,7 +13,7 @@ - if @namespaces.present? %label.label-light %span - Click to fork the project to a user or group + Click to fork the project - @namespaces.in_groups_of(6, false) do |group| .row - group.each do |namespace| @@ -29,8 +29,12 @@ .caption = namespace.human_name - else - .fork-thumbnail - = link_to project_forks_path(@project, namespace_key: namespace.id), method: "POST" do + - can_create_project = current_user.can?(:create_projects, namespace) + .fork-thumbnail{ class: ("disabled" unless can_create_project) } + = link_to project_forks_path(@project, namespace_key: namespace.id), + method: "POST", + class: ("disabled has-tooltip" unless can_create_project), + title: (_('You have reached your project limit') unless can_create_project) do - if /no_((\w*)_)*avatar/.match(avatar) .no-avatar = icon 'question' diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml index d27e121beb4..807216f4bcb 100644 --- a/app/views/projects/merge_requests/show.html.haml +++ b/app/views/projects/merge_requests/show.html.haml @@ -60,7 +60,10 @@ ":class" => "{ 'has-next-btn': !loggedOut && resolvedDiscussionCount !== discussionCount }" } %span.line-resolve-btn.is-disabled{ type: "button", ":class" => "{ 'is-active': resolvedDiscussionCount === discussionCount }" } - = render "shared/icons/icon_status_success.svg" + %template{ 'v-if' => 'resolvedDiscussionCount === discussionCount' } + = render 'shared/icons/icon_status_success_solid.svg' + %template{ 'v-else' => '' } + = render 'shared/icons/icon_resolve_discussion.svg' %span.line-resolve-text {{ resolvedDiscussionCount }}/{{ discussionCount }} {{ resolvedCountText }} resolved = render "discussions/new_issue_for_all_discussions", merge_request: @merge_request diff --git a/app/views/projects/new.html.haml b/app/views/projects/new.html.haml index 647e0a772b1..5698bb281b4 100644 --- a/app/views/projects/new.html.haml +++ b/app/views/projects/new.html.haml @@ -68,9 +68,10 @@ - if git_import_enabled? %button.btn.js-toggle-button.import_git{ type: "button" } = icon('git', text: 'Repo by URL') - .import_gitlab_project.has-tooltip{ data: { container: 'body' } } - = link_to new_import_gitlab_project_path, class: 'btn btn_import_gitlab_project project-submit' do - = icon('gitlab', text: 'GitLab export') + - if gitlab_project_import_enabled? + .import_gitlab_project.has-tooltip{ data: { container: 'body' } } + = link_to new_import_gitlab_project_path, class: 'btn btn_import_gitlab_project project-submit' do + = icon('gitlab', text: 'GitLab export') .row .col-lg-12 diff --git a/app/views/projects/notes/_actions.html.haml b/app/views/projects/notes/_actions.html.haml index b04f5efe1f9..fb07141d2ac 100644 --- a/app/views/projects/notes/_actions.html.haml +++ b/app/views/projects/notes/_actions.html.haml @@ -31,7 +31,7 @@ %template{ 'v-if' => 'isResolved' } = render 'shared/icons/icon_status_success_solid.svg' %template{ 'v-else' => '' } - = render 'shared/icons/icon_status_success.svg' + = render 'shared/icons/icon_resolve_discussion.svg' - if current_user - if note.emoji_awardable? diff --git a/app/views/shared/_new_project_item_select.html.haml b/app/views/shared/_new_project_item_select.html.haml index 96502d7ce93..2f9b8fb3039 100644 --- a/app/views/shared/_new_project_item_select.html.haml +++ b/app/views/shared/_new_project_item_select.html.haml @@ -1,5 +1,5 @@ - if any_projects?(@projects) - .project-item-select-holder.btn-group.pull-right + .project-item-select-holder.btn-group %a.btn.btn-new.new-project-item-link{ href: '', data: { label: local_assigns[:label] } } = icon('spinner spin') = project_select_tag :project_path, class: "project-item-select", data: { include_groups: local_assigns[:include_groups], order_by: 'last_activity_at', relative_path: local_assigns[:path] }, with_feature_enabled: local_assigns[:with_feature_enabled] diff --git a/app/views/shared/icons/_icon_resolve_discussion.svg b/app/views/shared/icons/_icon_resolve_discussion.svg new file mode 100644 index 00000000000..845562e9320 --- /dev/null +++ b/app/views/shared/icons/_icon_resolve_discussion.svg @@ -0,0 +1 @@ +<svg width="14" height="14" viewBox="0 0 14 14" xmlns="http://www.w3.org/2000/svg"><path d="M0 7a7 7 0 1 1 14 0A7 7 0 0 1 0 7z M13 7A6 6 0 1 0 1 7a6 6 0 0 0 12 0z" fill-rule="evenodd"/><path d="M6.278 7.697L5.045 6.464a.296.296 0 0 0-.42-.002l-.613.614a.298.298 0 0 0 .002.42l1.91 1.909a.5.5 0 0 0 .703.005l.265-.265L9.997 6.04a.291.291 0 0 0-.009-.408l-.614-.614a.29.29 0 0 0-.408-.009L6.278 7.697z"/></svg> diff --git a/app/views/shared/icons/_icon_status_success.svg b/app/views/shared/icons/_icon_status_success.svg index 845562e9320..eed5006bebe 100755 --- a/app/views/shared/icons/_icon_status_success.svg +++ b/app/views/shared/icons/_icon_status_success.svg @@ -1 +1 @@ -<svg width="14" height="14" viewBox="0 0 14 14" xmlns="http://www.w3.org/2000/svg"><path d="M0 7a7 7 0 1 1 14 0A7 7 0 0 1 0 7z M13 7A6 6 0 1 0 1 7a6 6 0 0 0 12 0z" fill-rule="evenodd"/><path d="M6.278 7.697L5.045 6.464a.296.296 0 0 0-.42-.002l-.613.614a.298.298 0 0 0 .002.42l1.91 1.909a.5.5 0 0 0 .703.005l.265-.265L9.997 6.04a.291.291 0 0 0-.009-.408l-.614-.614a.29.29 0 0 0-.408-.009L6.278 7.697z"/></svg> +<svg width="14" height="14" viewBox="0 0 14 14" xmlns="http://www.w3.org/2000/svg"><g fill-rule="evenodd"><path d="M0 7a7 7 0 1 1 14 0A7 7 0 0 1 0 7z"/><path d="M13 7A6 6 0 1 0 1 7a6 6 0 0 0 12 0z" fill="#FFF"/><path d="M6.278 7.697L5.045 6.464a.296.296 0 0 0-.42-.002l-.613.614a.298.298 0 0 0 .002.42l1.91 1.909a.5.5 0 0 0 .703.005l.265-.265L9.997 6.04a.291.291 0 0 0-.009-.408l-.614-.614a.29.29 0 0 0-.408-.009L6.278 7.697z"/></g></svg> diff --git a/app/workers/create_gpg_signature_worker.rb b/app/workers/create_gpg_signature_worker.rb index f34dff2d656..9b5ff17aafa 100644 --- a/app/workers/create_gpg_signature_worker.rb +++ b/app/workers/create_gpg_signature_worker.rb @@ -6,7 +6,11 @@ class CreateGpgSignatureWorker project = Project.find_by(id: project_id) return unless project + commit = project.commit(commit_sha) + + return unless commit + # This calculates and caches the signature in the database - Gitlab::Gpg::Commit.new(project, commit_sha).signature + Gitlab::Gpg::Commit.new(commit).signature end end diff --git a/changelogs/unreleased/29943-environment-folder.yml b/changelogs/unreleased/29943-environment-folder.yml deleted file mode 100644 index 8434d07d86e..00000000000 --- a/changelogs/unreleased/29943-environment-folder.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Resolve CSRF token leakage via pathname manipulation on environments page -merge_request: -author: diff --git a/changelogs/unreleased/37759-also-treat-newlines-as-separator.yml b/changelogs/unreleased/37759-also-treat-newlines-as-separator.yml new file mode 100644 index 00000000000..6894e650c11 --- /dev/null +++ b/changelogs/unreleased/37759-also-treat-newlines-as-separator.yml @@ -0,0 +1,5 @@ +--- +title: Allow using newlines in pipeline email service recipients +merge_request: 14250 +author: +type: fixed diff --git a/changelogs/unreleased/dm-go-get-xss.yml b/changelogs/unreleased/dm-go-get-xss.yml deleted file mode 100644 index bf0a7f4bd3d..00000000000 --- a/changelogs/unreleased/dm-go-get-xss.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Fix XSS issue in go-get handling -merge_request: -author: diff --git a/changelogs/unreleased/fix-escape-commit-block.yml b/changelogs/unreleased/fix-escape-commit-block.yml deleted file mode 100644 index 0665c8e5db2..00000000000 --- a/changelogs/unreleased/fix-escape-commit-block.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Prevent a persistent XSS in the commit author block -merge_request: -author: -type: security diff --git a/changelogs/unreleased/fix-gem-security-updates.yml b/changelogs/unreleased/fix-gem-security-updates.yml deleted file mode 100644 index dce11d08402..00000000000 --- a/changelogs/unreleased/fix-gem-security-updates.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Upgrade mail and nokogiri gems due to security issues -merge_request: 13662 -author: Markus Koller -type: security diff --git a/changelogs/unreleased/race-condition-in-project-uploads-fix-9-4.yml b/changelogs/unreleased/race-condition-in-project-uploads-fix-9-4.yml deleted file mode 100644 index adf9d77bce8..00000000000 --- a/changelogs/unreleased/race-condition-in-project-uploads-fix-9-4.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Fixes race condition in project uploads -merge_request: -author: diff --git a/changelogs/unreleased/rs-issue-36098.yml b/changelogs/unreleased/rs-issue-36098.yml deleted file mode 100644 index 56b92705a80..00000000000 --- a/changelogs/unreleased/rs-issue-36098.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Disallow arbitrary properties in `th` and `td` `style` attributes -merge_request: -author: diff --git a/changelogs/unreleased/rs-issue-36104.yml b/changelogs/unreleased/rs-issue-36104.yml deleted file mode 100644 index b050cd83175..00000000000 --- a/changelogs/unreleased/rs-issue-36104.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Disallow the `name` attribute on all user-provided markup -merge_request: -author: diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 25285525846..67ccdf187d3 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -509,7 +509,7 @@ production: &base failure_count_threshold: 10 # number of failures before stopping attempts failure_wait_time: 30 # Seconds after an access failure before allowing access again failure_reset_time: 1800 # Time in seconds to expire failures - storage_timeout: 5 # Time in seconds to wait before aborting a storage access attempt + storage_timeout: 30 # Time in seconds to wait before aborting a storage access attempt ## Backup settings diff --git a/db/fixtures/development/10_merge_requests.rb b/db/fixtures/development/10_merge_requests.rb index c304e0706dc..30244ee4431 100644 --- a/db/fixtures/development/10_merge_requests.rb +++ b/db/fixtures/development/10_merge_requests.rb @@ -28,6 +28,8 @@ Gitlab::Seeder.quiet do project = Project.find_by_full_path('gitlab-org/gitlab-test') + next if project.empty_repo? # We don't have repository on CI + params = { source_branch: 'feature', target_branch: 'master', diff --git a/db/migrate/20170817123339_add_verification_status_to_gpg_signatures.rb b/db/migrate/20170817123339_add_verification_status_to_gpg_signatures.rb new file mode 100644 index 00000000000..128cd109f8d --- /dev/null +++ b/db/migrate/20170817123339_add_verification_status_to_gpg_signatures.rb @@ -0,0 +1,20 @@ +class AddVerificationStatusToGpgSignatures < ActiveRecord::Migration + DOWNTIME = false + + include Gitlab::Database::MigrationHelpers + disable_ddl_transaction! + + def up + # First we remove all signatures because we need to re-verify them all + # again anyway (because of the updated verification logic). + # + # This makes adding the column with default values faster + truncate(:gpg_signatures) + + add_column_with_default(:gpg_signatures, :verification_status, :smallint, default: 0) + end + + def down + remove_column(:gpg_signatures, :verification_status) + end +end diff --git a/db/post_migrate/20170830084744_destroy_gpg_signatures.rb b/db/post_migrate/20170830084744_destroy_gpg_signatures.rb new file mode 100644 index 00000000000..b04d36f6537 --- /dev/null +++ b/db/post_migrate/20170830084744_destroy_gpg_signatures.rb @@ -0,0 +1,10 @@ +class DestroyGpgSignatures < ActiveRecord::Migration + DOWNTIME = false + + def up + truncate(:gpg_signatures) + end + + def down + end +end diff --git a/db/post_migrate/20170831195038_remove_valid_signature_from_gpg_signatures.rb b/db/post_migrate/20170831195038_remove_valid_signature_from_gpg_signatures.rb new file mode 100644 index 00000000000..9b6745e33d9 --- /dev/null +++ b/db/post_migrate/20170831195038_remove_valid_signature_from_gpg_signatures.rb @@ -0,0 +1,11 @@ +class RemoveValidSignatureFromGpgSignatures < ActiveRecord::Migration + DOWNTIME = false + + def up + remove_column :gpg_signatures, :valid_signature + end + + def down + add_column :gpg_signatures, :valid_signature, :boolean + end +end diff --git a/db/post_migrate/20170913180600_fix_projects_without_project_feature.rb b/db/post_migrate/20170913180600_fix_projects_without_project_feature.rb new file mode 100644 index 00000000000..bfa9ad80c7d --- /dev/null +++ b/db/post_migrate/20170913180600_fix_projects_without_project_feature.rb @@ -0,0 +1,33 @@ +class FixProjectsWithoutProjectFeature < ActiveRecord::Migration + DOWNTIME = false + + def up + # Deletes corrupted project features + sql = "DELETE FROM project_features WHERE project_id IS NULL" + execute(sql) + + # Creates missing project features with private visibility + sql = + %Q{ + INSERT INTO project_features(project_id, repository_access_level, issues_access_level, merge_requests_access_level, wiki_access_level, + builds_access_level, snippets_access_level, created_at, updated_at) + SELECT projects.id as project_id, + 10 as repository_access_level, + 10 as issues_access_level, + 10 as merge_requests_access_level, + 10 as wiki_access_level, + 10 as builds_access_level , + 10 as snippets_access_level, + projects.created_at, + projects.updated_at + FROM projects + LEFT OUTER JOIN project_features ON project_features.project_id = projects.id + WHERE (project_features.id IS NULL) + } + + execute(sql) + end + + def down + end +end diff --git a/db/schema.rb b/db/schema.rb index 3e2c407ddfc..af282fa4bf6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170824162758) do +ActiveRecord::Schema.define(version: 20170913180600) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -596,11 +596,11 @@ ActiveRecord::Schema.define(version: 20170824162758) do t.datetime "updated_at", null: false t.integer "project_id" t.integer "gpg_key_id" - t.boolean "valid_signature" t.binary "commit_sha" t.binary "gpg_key_primary_keyid" t.text "gpg_key_user_name" t.text "gpg_key_user_email" + t.integer "verification_status", limit: 2, default: 0, null: false end add_index "gpg_signatures", ["commit_sha"], name: "index_gpg_signatures_on_commit_sha", unique: true, using: :btree diff --git a/doc/development/testing.md b/doc/development/testing.md index 3d5aa3d45e9..56dc8abd38a 100644 --- a/doc/development/testing.md +++ b/doc/development/testing.md @@ -268,6 +268,43 @@ end - Avoid scenario titles that add no information, such as "successfully". - Avoid scenario titles that repeat the feature title. +### Table-based / Parameterized tests + +This style of testing is used to exercise one piece of code with a comprehensive +range of inputs. By specifying the test case once, alongside a table of inputs +and the expected output for each, your tests can be made easier to read and more +compact. + +We use the [rspec-parameterized](https://github.com/tomykaira/rspec-parameterized) +gem. A short example, using the table syntax and checking Ruby equality for a +range of inputs, might look like this: + +```ruby +describe "#==" do + using Rspec::Parameterized::TableSyntax + + let(:project1) { create(:project) } + let(:project2) { create(:project) } + where(:a, :b, :result) do + 1 | 1 | true + 1 | 2 | false + true | true | true + true | false | false + project1 | project1 | true + project2 | project2 | true + project 1 | project2 | false + end + + with_them do + it { expect(a == b).to eq(result) } + + it 'is isomorphic' do + expect(b == a).to eq(result) + end + end +end +``` + ### Matchers Custom matchers should be created to clarify the intent and/or hide the diff --git a/doc/user/project/gpg_signed_commits/img/project_signed_and_unsigned_commits.png b/doc/user/project/gpg_signed_commits/img/project_signed_and_unsigned_commits.png Binary files differindex 33936a7d6d7..088ecfa6d89 100644 --- a/doc/user/project/gpg_signed_commits/img/project_signed_and_unsigned_commits.png +++ b/doc/user/project/gpg_signed_commits/img/project_signed_and_unsigned_commits.png diff --git a/doc/user/project/gpg_signed_commits/img/project_signed_commit_unverified_signature.png b/doc/user/project/gpg_signed_commits/img/project_signed_commit_unverified_signature.png Binary files differindex 22565cf7c7e..4e3392406b1 100644 --- a/doc/user/project/gpg_signed_commits/img/project_signed_commit_unverified_signature.png +++ b/doc/user/project/gpg_signed_commits/img/project_signed_commit_unverified_signature.png diff --git a/doc/user/project/gpg_signed_commits/img/project_signed_commit_verified_signature.png b/doc/user/project/gpg_signed_commits/img/project_signed_commit_verified_signature.png Binary files differindex 1778b2ddf2b..766970dee81 100644 --- a/doc/user/project/gpg_signed_commits/img/project_signed_commit_verified_signature.png +++ b/doc/user/project/gpg_signed_commits/img/project_signed_commit_verified_signature.png diff --git a/doc/user/project/gpg_signed_commits/index.md b/doc/user/project/gpg_signed_commits/index.md index 3ea2203c895..93d24a60c57 100644 --- a/doc/user/project/gpg_signed_commits/index.md +++ b/doc/user/project/gpg_signed_commits/index.md @@ -22,11 +22,12 @@ GitLab uses its own keyring to verify the GPG signature. It does not access any public key server. In order to have a commit verified on GitLab the corresponding public key needs -to be uploaded to GitLab. For a signature to be verified two prerequisites need +to be uploaded to GitLab. For a signature to be verified three conditions need to be met: 1. The public key needs to be added your GitLab account 1. One of the emails in the GPG key matches your **primary** email +1. The committer's email matches the verified email from the gpg key ## Generating a GPG key diff --git a/doc/user/project/issues/confidential_issues.md b/doc/user/project/issues/confidential_issues.md index 1760b182114..0bf1f396f9d 100644 --- a/doc/user/project/issues/confidential_issues.md +++ b/doc/user/project/issues/confidential_issues.md @@ -9,7 +9,7 @@ keep security vulnerabilities private or prevent surprises from leaking out. ## Making an issue confidential -You can make an issue confidential either by creating a new issue or editing +You can make an issue confidential during issue creation or by editing an existing one. When you create a new issue, a checkbox right below the text area is available @@ -19,11 +19,19 @@ confidential checkbox and hit **Save changes**. ![Creating a new confidential issue](img/confidential_issues_create.png) -## Making an issue non-confidential +## Modifying issue confidentiality -To make an issue non-confidential, all you have to do is edit it and unmark -the confidential checkbox. Once you save the issue, it will gain the default -visibility level you have chosen for your project. +There are two ways to change an issue's confidentiality. + +The first way is to edit the issue and mark/unmark the confidential checkbox. +Once you save the issue, it will change the confidentiality of the issue. + +The second way is to locate the Confidentiality section in the sidebar and click +**Edit**. A popup should appear and give you the option to turn on or turn off confidentiality. + +| Turn off confidentiality | Turn on confidentiality | +| :-----------: | :----------: | +| ![Turn off confidentiality](img/turn_off_confidentiality.png) | ![Turn on confidentiality](img/turn_on_confidentiality.png) | Every change from regular to confidential and vice versa, is indicated by a system note in the issue's comments. @@ -49,6 +57,12 @@ issue you are commenting on is confidential. ![Confidential issue page](img/confidential_issues_issue_page.png) +There is also an indicator on the sidebar denoting confidentiality. + +| Confidential issue | Not confidential issue | +| :-----------: | :----------: | +| ![Sidebar confidential issue](img/sidebar_confidential_issue.png) | ![Sidebar not confidential issue](img/sidebar_not_confidential_issue.png) | + ## Permissions and access to confidential issues There are two kinds of level access for confidential issues. The general rule diff --git a/doc/user/project/issues/img/confidential_issues_index_page.png b/doc/user/project/issues/img/confidential_issues_index_page.png Binary files differindex e4b492a2769..f3efe0ce04e 100755..100644 --- a/doc/user/project/issues/img/confidential_issues_index_page.png +++ b/doc/user/project/issues/img/confidential_issues_index_page.png diff --git a/doc/user/project/issues/img/confidential_issues_issue_page.png b/doc/user/project/issues/img/confidential_issues_issue_page.png Binary files differindex f04ec8ff32b..0f5c774d258 100755..100644 --- a/doc/user/project/issues/img/confidential_issues_issue_page.png +++ b/doc/user/project/issues/img/confidential_issues_issue_page.png diff --git a/doc/user/project/issues/img/sidebar_confidential_issue.png b/doc/user/project/issues/img/sidebar_confidential_issue.png Binary files differnew file mode 100755 index 00000000000..d99a1ca756e --- /dev/null +++ b/doc/user/project/issues/img/sidebar_confidential_issue.png diff --git a/doc/user/project/issues/img/sidebar_not_confidential_issue.png b/doc/user/project/issues/img/sidebar_not_confidential_issue.png Binary files differnew file mode 100755 index 00000000000..2e6cbbc5b3a --- /dev/null +++ b/doc/user/project/issues/img/sidebar_not_confidential_issue.png diff --git a/doc/user/project/issues/img/turn_off_confidentiality.png b/doc/user/project/issues/img/turn_off_confidentiality.png Binary files differnew file mode 100644 index 00000000000..248ae6522d6 --- /dev/null +++ b/doc/user/project/issues/img/turn_off_confidentiality.png diff --git a/doc/user/project/issues/img/turn_on_confidentiality.png b/doc/user/project/issues/img/turn_on_confidentiality.png Binary files differnew file mode 100644 index 00000000000..fac4c833699 --- /dev/null +++ b/doc/user/project/issues/img/turn_on_confidentiality.png diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb index e001d25e7b7..a6ec75da385 100644 --- a/lib/gitlab/database.rb +++ b/lib/gitlab/database.rb @@ -9,6 +9,14 @@ module Gitlab ActiveRecord::Base.configurations[Rails.env] end + def self.username + config['username'] || ENV['USER'] + end + + def self.database_name + config['database'] + end + def self.adapter_name config['adapter'] end diff --git a/lib/gitlab/database/grant.rb b/lib/gitlab/database/grant.rb new file mode 100644 index 00000000000..aee3981e79a --- /dev/null +++ b/lib/gitlab/database/grant.rb @@ -0,0 +1,34 @@ +module Gitlab + module Database + # Model that can be used for querying permissions of a SQL user. + class Grant < ActiveRecord::Base + self.table_name = + if Database.postgresql? + 'information_schema.role_table_grants' + else + 'mysql.user' + end + + def self.scope_to_current_user + if Database.postgresql? + where('grantee = user') + else + where("CONCAT(User, '@', Host) = current_user()") + end + end + + # Returns true if the current user can create and execute triggers on the + # given table. + def self.create_and_execute_trigger?(table) + priv = + if Database.postgresql? + where(privilege_type: 'TRIGGER', table_name: table) + else + where(Trigger_priv: 'Y') + end + + priv.scope_to_current_user.any? + end + end + end +end diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index b83e633c7ed..fb14798efe6 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -358,6 +358,8 @@ module Gitlab raise 'rename_column_concurrently can not be run inside a transaction' end + check_trigger_permissions!(table) + old_col = column_for(table, old) new_type = type || old_col.type @@ -430,6 +432,8 @@ module Gitlab def cleanup_concurrent_column_rename(table, old, new) trigger_name = rename_trigger_name(table, old, new) + check_trigger_permissions!(table) + if Database.postgresql? remove_rename_triggers_for_postgresql(table, trigger_name) else @@ -485,14 +489,14 @@ module Gitlab # Removes the triggers used for renaming a PostgreSQL column concurrently. def remove_rename_triggers_for_postgresql(table, trigger) - execute("DROP TRIGGER #{trigger} ON #{table}") - execute("DROP FUNCTION #{trigger}()") + execute("DROP TRIGGER IF EXISTS #{trigger} ON #{table}") + execute("DROP FUNCTION IF EXISTS #{trigger}()") end # Removes the triggers used for renaming a MySQL column concurrently. def remove_rename_triggers_for_mysql(trigger) - execute("DROP TRIGGER #{trigger}_insert") - execute("DROP TRIGGER #{trigger}_update") + execute("DROP TRIGGER IF EXISTS #{trigger}_insert") + execute("DROP TRIGGER IF EXISTS #{trigger}_update") end # Returns the (base) name to use for triggers when renaming columns. @@ -611,6 +615,44 @@ module Gitlab remove_foreign_key(*args) rescue ArgumentError end + + def sidekiq_queue_migrate(queue_from, to:) + while sidekiq_queue_length(queue_from) > 0 + Sidekiq.redis do |conn| + conn.rpoplpush "queue:#{queue_from}", "queue:#{to}" + end + end + end + + def sidekiq_queue_length(queue_name) + Sidekiq.redis do |conn| + conn.llen("queue:#{queue_name}") + end + end + + def check_trigger_permissions!(table) + unless Grant.create_and_execute_trigger?(table) + dbname = Database.database_name + user = Database.username + + raise <<-EOF +Your database user is not allowed to create, drop, or execute triggers on the +table #{table}. + +If you are using PostgreSQL you can solve this by logging in to the GitLab +database (#{dbname}) using a super user and running: + + ALTER #{user} WITH SUPERUSER + +For MySQL you instead need to run: + + GRANT ALL PRIVILEGES ON *.* TO #{user}@'%' + +Both queries will grant the user super user permissions, ensuring you don't run +into similar problems in the future (e.g. when new tables are created). + EOF + end + end end end end diff --git a/lib/gitlab/gpg.rb b/lib/gitlab/gpg.rb index 45e9f9d65ae..0d5039ddf5f 100644 --- a/lib/gitlab/gpg.rb +++ b/lib/gitlab/gpg.rb @@ -39,7 +39,7 @@ module Gitlab fingerprints = CurrentKeyChain.fingerprints_from_key(key) GPGME::Key.find(:public, fingerprints).flat_map do |raw_key| - raw_key.uids.map { |uid| { name: uid.name, email: uid.email } } + raw_key.uids.map { |uid| { name: uid.name, email: uid.email.downcase } } end end end @@ -69,11 +69,17 @@ module Gitlab def optimistic_using_tmp_keychain previous_dir = current_home_dir - Dir.mktmpdir do |dir| - GPGME::Engine.home_dir = dir - yield - end + tmp_dir = Dir.mktmpdir + GPGME::Engine.home_dir = tmp_dir + yield ensure + # Ignore any errors when removing the tmp directory, as we may run into a + # race condition: + # The `gpg-agent` agent process may clean up some files as well while + # `FileUtils.remove_entry` is iterating the directory and removing all + # its contained files and directories recursively, which could raise an + # error. + FileUtils.remove_entry(tmp_dir, true) GPGME::Engine.home_dir = previous_dir end end diff --git a/lib/gitlab/gpg/commit.rb b/lib/gitlab/gpg/commit.rb index 606c7576f70..86bd9f5b125 100644 --- a/lib/gitlab/gpg/commit.rb +++ b/lib/gitlab/gpg/commit.rb @@ -1,17 +1,12 @@ module Gitlab module Gpg class Commit - def self.for_commit(commit) - new(commit.project, commit.sha) - end - - def initialize(project, sha) - @project = project - @sha = sha + def initialize(commit) + @commit = commit @signature_text, @signed_text = begin - Rugged::Commit.extract_signature(project.repository.rugged, sha) + Rugged::Commit.extract_signature(@commit.project.repository.rugged, @commit.sha) rescue Rugged::OdbError nil end @@ -26,7 +21,7 @@ module Gitlab return @signature if @signature - cached_signature = GpgSignature.find_by(commit_sha: @sha) + cached_signature = GpgSignature.find_by(commit_sha: @commit.sha) return @signature = cached_signature if cached_signature.present? @signature = create_cached_signature! @@ -73,20 +68,31 @@ module Gitlab def attributes(gpg_key) user_infos = user_infos(gpg_key) + verification_status = verification_status(gpg_key) { - commit_sha: @sha, - project: @project, + commit_sha: @commit.sha, + project: @commit.project, gpg_key: gpg_key, gpg_key_primary_keyid: gpg_key&.primary_keyid || verified_signature.fingerprint, gpg_key_user_name: user_infos[:name], gpg_key_user_email: user_infos[:email], - valid_signature: gpg_signature_valid_signature_value(gpg_key) + verification_status: verification_status } end - def gpg_signature_valid_signature_value(gpg_key) - !!(gpg_key && gpg_key.verified? && verified_signature.valid?) + def verification_status(gpg_key) + return :unknown_key unless gpg_key + return :unverified_key unless gpg_key.verified? + return :unverified unless verified_signature.valid? + + if gpg_key.verified_and_belongs_to_email?(@commit.committer_email) + :verified + elsif gpg_key.user.all_emails.include?(@commit.committer_email) + :same_user_different_email + else + :other_user + end end def user_infos(gpg_key) diff --git a/lib/gitlab/gpg/invalid_gpg_signature_updater.rb b/lib/gitlab/gpg/invalid_gpg_signature_updater.rb index a525ee7a9ee..e085eab26c9 100644 --- a/lib/gitlab/gpg/invalid_gpg_signature_updater.rb +++ b/lib/gitlab/gpg/invalid_gpg_signature_updater.rb @@ -8,7 +8,7 @@ module Gitlab def run GpgSignature .select(:id, :commit_sha, :project_id) - .where('gpg_key_id IS NULL OR valid_signature = ?', false) + .where('gpg_key_id IS NULL OR verification_status <> ?', GpgSignature.verification_statuses[:verified]) .where(gpg_key_primary_keyid: @gpg_key.primary_keyid) .find_each { |sig| sig.gpg_commit.update_signature!(sig) } end diff --git a/lib/gitlab/url_sanitizer.rb b/lib/gitlab/url_sanitizer.rb index c81dc7e30d0..703adae12cb 100644 --- a/lib/gitlab/url_sanitizer.rb +++ b/lib/gitlab/url_sanitizer.rb @@ -9,7 +9,7 @@ module Gitlab end def self.valid?(url) - return false unless url + return false unless url.present? Addressable::URI.parse(url.strip) @@ -19,7 +19,12 @@ module Gitlab end def initialize(url, credentials: nil) - @url = Addressable::URI.parse(url.strip) + @url = Addressable::URI.parse(url.to_s.strip) + + %i[user password].each do |symbol| + credentials[symbol] = credentials[symbol].presence if credentials&.key?(symbol) + end + @credentials = credentials end @@ -29,13 +34,13 @@ module Gitlab def masked_url url = @url.dup - url.password = "*****" unless url.password.nil? - url.user = "*****" unless url.user.nil? + url.password = "*****" if url.password.present? + url.user = "*****" if url.user.present? url.to_s end def credentials - @credentials ||= { user: @url.user, password: @url.password } + @credentials ||= { user: @url.user.presence, password: @url.password.presence } end def full_url @@ -47,8 +52,10 @@ module Gitlab def generate_full_url return @url unless valid_credentials? @full_url = @url.dup - @full_url.user = credentials[:user] + @full_url.password = credentials[:password] + @full_url.user = credentials[:user] + @full_url end diff --git a/lib/tasks/gitlab/import.rake b/lib/tasks/gitlab/import.rake index 48bd9139ce8..a7cb3242251 100644 --- a/lib/tasks/gitlab/import.rake +++ b/lib/tasks/gitlab/import.rake @@ -39,7 +39,8 @@ namespace :gitlab do project_params = { name: name, - path: name + path: name, + skip_disk_validation: true } # find group namespace diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 968e097991a..3db7243aad2 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1,7 +1,7 @@ require('spec_helper') describe Projects::IssuesController do - let(:project) { create(:project_empty_repo) } + let(:project) { create(:project) } let(:user) { create(:user) } let(:issue) { create(:issue, project: project) } @@ -841,7 +841,7 @@ describe Projects::IssuesController do describe 'POST #toggle_award_emoji' do before do sign_in(user) - project.team << [user, :developer] + project.add_developer(user) end it "toggles the award emoji" do @@ -855,6 +855,8 @@ describe Projects::IssuesController do end describe 'POST create_merge_request' do + let(:project) { create(:project, :repository) } + before do project.add_developer(user) sign_in(user) diff --git a/spec/factories/deployments.rb b/spec/factories/deployments.rb index 29ad1af9fd9..e5abfd67d60 100644 --- a/spec/factories/deployments.rb +++ b/spec/factories/deployments.rb @@ -10,6 +10,10 @@ FactoryGirl.define do after(:build) do |deployment, evaluator| deployment.project ||= deployment.environment.project + + unless deployment.project.repository_exists? + allow(deployment.project.repository).to receive(:fetch_ref) + end end end end diff --git a/spec/factories/gpg_signature.rb b/spec/factories/gpg_signature.rb index a5aeffbe12d..c0beecf0bea 100644 --- a/spec/factories/gpg_signature.rb +++ b/spec/factories/gpg_signature.rb @@ -6,6 +6,6 @@ FactoryGirl.define do project gpg_key gpg_key_primary_keyid { gpg_key.primary_keyid } - valid_signature true + verification_status :verified end end diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 1bc530d06db..cbec716d6ea 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -68,6 +68,17 @@ FactoryGirl.define do merge_user author end + after(:build) do |merge_request| + target_project = merge_request.target_project + source_project = merge_request.source_project + + # Fake `write_ref` if we don't have repository + # We have too many existing tests replying on this behaviour + unless [target_project, source_project].all?(&:repository_exists?) + allow(merge_request).to receive(:write_ref) + end + end + factory :merged_merge_request, traits: [:merged] factory :closed_merge_request, traits: [:closed] factory :reopened_merge_request, traits: [:opened] diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb index 0c9fcc60d30..479fb713297 100644 --- a/spec/features/commits_spec.rb +++ b/spec/features/commits_spec.rb @@ -203,105 +203,4 @@ describe 'Commits' do end end end - - describe 'GPG signed commits', :js do - it 'changes from unverified to verified when the user changes his email to match the gpg key' do - user = create :user, email: 'unrelated.user@example.org' - project.team << [user, :master] - - Sidekiq::Testing.inline! do - create :gpg_key, key: GpgHelpers::User1.public_key, user: user - end - - sign_in(user) - - visit project_commits_path(project, :'signed-commits') - - within '#commits-list' do - expect(page).to have_content 'Unverified' - expect(page).not_to have_content 'Verified' - end - - # user changes his email which makes the gpg key verified - Sidekiq::Testing.inline! do - user.skip_reconfirmation! - user.update_attributes!(email: GpgHelpers::User1.emails.first) - end - - visit project_commits_path(project, :'signed-commits') - - within '#commits-list' do - expect(page).to have_content 'Unverified' - expect(page).to have_content 'Verified' - end - end - - it 'changes from unverified to verified when the user adds the missing gpg key' do - user = create :user, email: GpgHelpers::User1.emails.first - project.team << [user, :master] - - sign_in(user) - - visit project_commits_path(project, :'signed-commits') - - within '#commits-list' do - expect(page).to have_content 'Unverified' - expect(page).not_to have_content 'Verified' - end - - # user adds the gpg key which makes the signature valid - Sidekiq::Testing.inline! do - create :gpg_key, key: GpgHelpers::User1.public_key, user: user - end - - visit project_commits_path(project, :'signed-commits') - - within '#commits-list' do - expect(page).to have_content 'Unverified' - expect(page).to have_content 'Verified' - end - end - - it 'shows popover badges' do - gpg_user = create :user, email: GpgHelpers::User1.emails.first, username: 'nannie.bernhard', name: 'Nannie Bernhard' - Sidekiq::Testing.inline! do - create :gpg_key, key: GpgHelpers::User1.public_key, user: gpg_user - end - - user = create :user - project.team << [user, :master] - - sign_in(user) - visit project_commits_path(project, :'signed-commits') - - # unverified signature - click_on 'Unverified', match: :first - within '.popover' do - expect(page).to have_content 'This commit was signed with an unverified signature.' - expect(page).to have_content "GPG Key ID: #{GpgHelpers::User2.primary_keyid}" - end - - # verified and the gpg user has a gitlab profile - click_on 'Verified', match: :first - within '.popover' do - expect(page).to have_content 'This commit was signed with a verified signature.' - expect(page).to have_content 'Nannie Bernhard' - expect(page).to have_content '@nannie.bernhard' - expect(page).to have_content "GPG Key ID: #{GpgHelpers::User1.primary_keyid}" - end - - # verified and the gpg user's profile doesn't exist anymore - gpg_user.destroy! - - visit project_commits_path(project, :'signed-commits') - - click_on 'Verified', match: :first - within '.popover' do - expect(page).to have_content 'This commit was signed with a verified signature.' - expect(page).to have_content 'Nannie Bernhard' - expect(page).to have_content 'nannie.bernhard@example.com' - expect(page).to have_content "GPG Key ID: #{GpgHelpers::User1.primary_keyid}" - end - end - end end diff --git a/spec/features/merge_requests/filter_by_labels_spec.rb b/spec/features/merge_requests/filter_by_labels_spec.rb index 1d52a4659ad..9912e8165e6 100644 --- a/spec/features/merge_requests/filter_by_labels_spec.rb +++ b/spec/features/merge_requests/filter_by_labels_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -feature 'Merge Request filtering by Labels', js: true do +feature 'Merge Request filtering by Labels', :js do include FilteredSearchHelpers include MergeRequestHelpers @@ -12,9 +12,9 @@ feature 'Merge Request filtering by Labels', js: true do let!(:feature) { create(:label, project: project, title: 'feature') } let!(:enhancement) { create(:label, project: project, title: 'enhancement') } - let!(:mr1) { create(:merge_request, title: "Bugfix1", source_project: project, target_project: project, source_branch: "bugfix1") } - let!(:mr2) { create(:merge_request, title: "Bugfix2", source_project: project, target_project: project, source_branch: "bugfix2") } - let!(:mr3) { create(:merge_request, title: "Feature1", source_project: project, target_project: project, source_branch: "feature1") } + let!(:mr1) { create(:merge_request, title: "Bugfix1", source_project: project, target_project: project, source_branch: "fix") } + let!(:mr2) { create(:merge_request, title: "Bugfix2", source_project: project, target_project: project, source_branch: "wip") } + let!(:mr3) { create(:merge_request, title: "Feature1", source_project: project, target_project: project, source_branch: "improve/awesome") } before do mr1.labels << bug @@ -25,7 +25,7 @@ feature 'Merge Request filtering by Labels', js: true do mr3.title = "Feature1" mr3.labels << feature - project.team << [user, :master] + project.add_master(user) sign_in(user) visit project_merge_requests_path(project) diff --git a/spec/features/merge_requests/filter_merge_requests_spec.rb b/spec/features/merge_requests/filter_merge_requests_spec.rb index f0019be86ad..3686131fee4 100644 --- a/spec/features/merge_requests/filter_merge_requests_spec.rb +++ b/spec/features/merge_requests/filter_merge_requests_spec.rb @@ -12,7 +12,7 @@ describe 'Filter merge requests' do let!(:wontfix) { create(:label, project: project, title: "Won't fix") } before do - project.team << [user, :master] + project.add_master(user) group.add_developer(user) sign_in(user) create(:merge_request, source_project: project, target_project: project) @@ -170,7 +170,7 @@ describe 'Filter merge requests' do describe 'filter merge requests by text' do before do - create(:merge_request, title: "Bug", source_project: project, target_project: project, source_branch: "bug") + create(:merge_request, title: "Bug", source_project: project, target_project: project, source_branch: "wip") bug_label = create(:label, project: project, title: 'bug') milestone = create(:milestone, title: "8", project: project) @@ -179,7 +179,7 @@ describe 'Filter merge requests' do title: "Bug 2", source_project: project, target_project: project, - source_branch: "bug2", + source_branch: "fix", milestone: milestone, author: user, assignee: user) @@ -259,12 +259,12 @@ describe 'Filter merge requests' do end end - describe 'filter merge requests and sort', js: true do + describe 'filter merge requests and sort', :js do before do bug_label = create(:label, project: project, title: 'bug') - mr1 = create(:merge_request, title: "Frontend", source_project: project, target_project: project, source_branch: "Frontend") - mr2 = create(:merge_request, title: "Bug 2", source_project: project, target_project: project, source_branch: "bug2") + mr1 = create(:merge_request, title: "Frontend", source_project: project, target_project: project, source_branch: "wip") + mr2 = create(:merge_request, title: "Bug 2", source_project: project, target_project: project, source_branch: "fix") mr1.labels << bug_label mr2.labels << bug_label diff --git a/spec/features/merge_requests/reset_filters_spec.rb b/spec/features/merge_requests/reset_filters_spec.rb index c1b90e5f875..eed95816bdf 100644 --- a/spec/features/merge_requests/reset_filters_spec.rb +++ b/spec/features/merge_requests/reset_filters_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -feature 'Merge requests filter clear button', js: true do +feature 'Merge requests filter clear button', :js do include FilteredSearchHelpers include MergeRequestHelpers include IssueHelpers @@ -9,8 +9,8 @@ feature 'Merge requests filter clear button', js: true do let!(:user) { create(:user) } let!(:milestone) { create(:milestone, project: project) } let!(:bug) { create(:label, project: project, name: 'bug')} - let!(:mr1) { create(:merge_request, title: "Feature", source_project: project, target_project: project, source_branch: "Feature", milestone: milestone, author: user, assignee: user) } - let!(:mr2) { create(:merge_request, title: "Bugfix1", source_project: project, target_project: project, source_branch: "Bugfix1") } + let!(:mr1) { create(:merge_request, title: "Feature", source_project: project, target_project: project, source_branch: "improve/awesome", milestone: milestone, author: user, assignee: user) } + let!(:mr2) { create(:merge_request, title: "Bugfix1", source_project: project, target_project: project, source_branch: "fix") } let(:merge_request_css) { '.merge-request' } let(:clear_search_css) { '.filtered-search-box .clear-search' } diff --git a/spec/features/merge_requests/user_lists_merge_requests_spec.rb b/spec/features/merge_requests/user_lists_merge_requests_spec.rb index d62b035b40b..20008b4e7f9 100644 --- a/spec/features/merge_requests/user_lists_merge_requests_spec.rb +++ b/spec/features/merge_requests/user_lists_merge_requests_spec.rb @@ -24,12 +24,10 @@ describe 'Projects > Merge requests > User lists merge requests' do milestone: create(:milestone, due_date: '2013-12-12'), created_at: 2.minutes.ago, updated_at: 2.minutes.ago) - # lfs in itself is not a great choice for the title if one wants to match the whole body content later on - # just think about the scenario when faker generates 'Chester Runolfsson' as the user's name create(:merge_request, - title: 'merge_lfs', + title: 'merge-test', source_project: project, - source_branch: 'merge_lfs', + source_branch: 'merge-test', created_at: 3.minutes.ago, updated_at: 10.seconds.ago) end @@ -38,7 +36,7 @@ describe 'Projects > Merge requests > User lists merge requests' do visit_merge_requests(project, assignee_id: IssuableFinder::NONE) expect(current_path).to eq(project_merge_requests_path(project)) - expect(page).to have_content 'merge_lfs' + expect(page).to have_content 'merge-test' expect(page).not_to have_content 'fix' expect(page).not_to have_content 'markdown' expect(count_merge_requests).to eq(1) @@ -47,7 +45,7 @@ describe 'Projects > Merge requests > User lists merge requests' do it 'filters on a specific assignee' do visit_merge_requests(project, assignee_id: user.id) - expect(page).not_to have_content 'merge_lfs' + expect(page).not_to have_content 'merge-test' expect(page).to have_content 'fix' expect(page).to have_content 'markdown' expect(count_merge_requests).to eq(2) @@ -57,14 +55,14 @@ describe 'Projects > Merge requests > User lists merge requests' do visit_merge_requests(project, sort: sort_value_recently_created) expect(first_merge_request).to include('fix') - expect(last_merge_request).to include('merge_lfs') + expect(last_merge_request).to include('merge-test') expect(count_merge_requests).to eq(3) end it 'sorts by oldest' do visit_merge_requests(project, sort: sort_value_oldest_created) - expect(first_merge_request).to include('merge_lfs') + expect(first_merge_request).to include('merge-test') expect(last_merge_request).to include('fix') expect(count_merge_requests).to eq(3) end @@ -72,7 +70,7 @@ describe 'Projects > Merge requests > User lists merge requests' do it 'sorts by last updated' do visit_merge_requests(project, sort: sort_value_recently_updated) - expect(first_merge_request).to include('merge_lfs') + expect(first_merge_request).to include('merge-test') expect(count_merge_requests).to eq(3) end diff --git a/spec/features/merge_requests/widget_spec.rb b/spec/features/merge_requests/widget_spec.rb index fd991293ee9..443b596b3c6 100644 --- a/spec/features/merge_requests/widget_spec.rb +++ b/spec/features/merge_requests/widget_spec.rb @@ -142,6 +142,24 @@ describe 'Merge request', :js do end end + context 'view merge request where project has CI setup but no CI status' do + before do + pipeline = create(:ci_pipeline, project: project, + sha: merge_request.diff_head_sha, + ref: merge_request.source_branch) + create(:ci_build, pipeline: pipeline) + + visit project_merge_request_path(project, merge_request) + end + + it 'has pipeline error text' do + # Wait for the `ci_status` and `merge_check` requests + wait_for_requests + + expect(page).to have_text('Could not connect to the CI server. Please check your settings and try again') + end + end + context 'view merge request with MWPS enabled but automatically merge fails' do before do merge_request.update( diff --git a/spec/features/profiles/gpg_keys_spec.rb b/spec/features/profiles/gpg_keys_spec.rb index 6edc482b47e..623e4f341c5 100644 --- a/spec/features/profiles/gpg_keys_spec.rb +++ b/spec/features/profiles/gpg_keys_spec.rb @@ -42,7 +42,7 @@ feature 'Profile > GPG Keys' do scenario 'User revokes a key via the key index' do gpg_key = create :gpg_key, user: user, key: GpgHelpers::User2.public_key - gpg_signature = create :gpg_signature, gpg_key: gpg_key, valid_signature: true + gpg_signature = create :gpg_signature, gpg_key: gpg_key, verification_status: :verified visit profile_gpg_keys_path @@ -51,7 +51,7 @@ feature 'Profile > GPG Keys' do expect(page).to have_content('Your GPG keys (0)') expect(gpg_signature.reload).to have_attributes( - valid_signature: false, + verification_status: 'unknown_key', gpg_key: nil ) end diff --git a/spec/features/projects/fork_spec.rb b/spec/features/projects/fork_spec.rb new file mode 100644 index 00000000000..e10d29e5eea --- /dev/null +++ b/spec/features/projects/fork_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +describe 'Project fork' do + let(:user) { create(:user) } + let(:project) { create(:project, :public, :repository) } + + before do + sign_in user + end + + it 'allows user to fork project' do + visit project_path(project) + + expect(page).not_to have_css('a.disabled', text: 'Fork') + end + + it 'disables fork button when user has exceeded project limit' do + user.projects_limit = 0 + user.save! + + visit project_path(project) + + expect(page).to have_css('a.disabled', text: 'Fork') + end + + context 'master in group' do + before do + group = create(:group) + group.add_master(user) + end + + it 'allows user to fork project to group or to user namespace' do + visit project_path(project) + + expect(page).not_to have_css('a.disabled', text: 'Fork') + + click_link 'Fork' + + expect(page).to have_css('.fork-thumbnail', count: 2) + expect(page).not_to have_css('.fork-thumbnail.disabled') + end + + it 'allows user to fork project to group and not user when exceeded project limit' do + user.projects_limit = 0 + user.save! + + visit project_path(project) + + expect(page).not_to have_css('a.disabled', text: 'Fork') + + click_link 'Fork' + + expect(page).to have_css('.fork-thumbnail', count: 2) + expect(page).to have_css('.fork-thumbnail.disabled') + end + end +end diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index 037ac00d39f..2e24455edca 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -164,9 +164,9 @@ feature 'Jobs' do end it 'links to issues/new with the title and description filled in' do - button_title = "Build Failed ##{job.id}" - job_path = project_job_path(project, job) - options = { issue: { title: button_title, description: job_path } } + button_title = "Job Failed ##{job.id}" + job_url = project_job_path(project, job) + options = { issue: { title: button_title, description: "Job [##{job.id}](#{job_url}) failed for #{job.sha}:\n" } } href = new_project_issue_path(project, options) diff --git a/spec/features/signed_commits_spec.rb b/spec/features/signed_commits_spec.rb new file mode 100644 index 00000000000..8efa5b58141 --- /dev/null +++ b/spec/features/signed_commits_spec.rb @@ -0,0 +1,179 @@ +require 'spec_helper' + +describe 'GPG signed commits', :js do + let(:project) { create(:project, :repository) } + + it 'changes from unverified to verified when the user changes his email to match the gpg key' do + user = create :user, email: 'unrelated.user@example.org' + project.team << [user, :master] + + Sidekiq::Testing.inline! do + create :gpg_key, key: GpgHelpers::User1.public_key, user: user + end + + sign_in(user) + + visit project_commits_path(project, :'signed-commits') + + within '#commits-list' do + expect(page).to have_content 'Unverified' + expect(page).not_to have_content 'Verified' + end + + # user changes his email which makes the gpg key verified + Sidekiq::Testing.inline! do + user.skip_reconfirmation! + user.update_attributes!(email: GpgHelpers::User1.emails.first) + end + + visit project_commits_path(project, :'signed-commits') + + within '#commits-list' do + expect(page).to have_content 'Unverified' + expect(page).to have_content 'Verified' + end + end + + it 'changes from unverified to verified when the user adds the missing gpg key' do + user = create :user, email: GpgHelpers::User1.emails.first + project.team << [user, :master] + + sign_in(user) + + visit project_commits_path(project, :'signed-commits') + + within '#commits-list' do + expect(page).to have_content 'Unverified' + expect(page).not_to have_content 'Verified' + end + + # user adds the gpg key which makes the signature valid + Sidekiq::Testing.inline! do + create :gpg_key, key: GpgHelpers::User1.public_key, user: user + end + + visit project_commits_path(project, :'signed-commits') + + within '#commits-list' do + expect(page).to have_content 'Unverified' + expect(page).to have_content 'Verified' + end + end + + context 'shows popover badges' do + let(:user_1) do + create :user, email: GpgHelpers::User1.emails.first, username: 'nannie.bernhard', name: 'Nannie Bernhard' + end + + let(:user_1_key) do + Sidekiq::Testing.inline! do + create :gpg_key, key: GpgHelpers::User1.public_key, user: user_1 + end + end + + let(:user_2) do + create(:user, email: GpgHelpers::User2.emails.first, username: 'bette.cartwright', name: 'Bette Cartwright').tap do |user| + # secondary, unverified email + create :email, user: user, email: GpgHelpers::User2.emails.last + end + end + + let(:user_2_key) do + Sidekiq::Testing.inline! do + create :gpg_key, key: GpgHelpers::User2.public_key, user: user_2 + end + end + + before do + user = create :user + project.team << [user, :master] + + sign_in(user) + end + + it 'unverified signature' do + visit project_commits_path(project, :'signed-commits') + + within(find('.commit', text: 'signed commit by bette cartwright')) do + click_on 'Unverified' + within '.popover' do + expect(page).to have_content 'This commit was signed with an unverified signature.' + expect(page).to have_content "GPG Key ID: #{GpgHelpers::User2.primary_keyid}" + end + end + end + + it 'unverified signature: user email does not match the committer email, but is the same user' do + user_2_key + + visit project_commits_path(project, :'signed-commits') + + within(find('.commit', text: 'signed and authored commit by bette cartwright, different email')) do + click_on 'Unverified' + within '.popover' do + expect(page).to have_content 'This commit was signed with a verified signature, but the committer email is not verified to belong to the same user.' + expect(page).to have_content 'Bette Cartwright' + expect(page).to have_content '@bette.cartwright' + expect(page).to have_content "GPG Key ID: #{GpgHelpers::User2.primary_keyid}" + end + end + end + + it 'unverified signature: user email does not match the committer email' do + user_2_key + + visit project_commits_path(project, :'signed-commits') + + within(find('.commit', text: 'signed commit by bette cartwright')) do + click_on 'Unverified' + within '.popover' do + expect(page).to have_content "This commit was signed with a different user's verified signature." + expect(page).to have_content 'Bette Cartwright' + expect(page).to have_content '@bette.cartwright' + expect(page).to have_content "GPG Key ID: #{GpgHelpers::User2.primary_keyid}" + end + end + end + + it 'verified and the gpg user has a gitlab profile' do + user_1_key + + visit project_commits_path(project, :'signed-commits') + + within(find('.commit', text: 'signed and authored commit by nannie bernhard')) do + click_on 'Verified' + within '.popover' do + expect(page).to have_content 'This commit was signed with a verified signature and the committer email is verified to belong to the same user.' + expect(page).to have_content 'Nannie Bernhard' + expect(page).to have_content '@nannie.bernhard' + expect(page).to have_content "GPG Key ID: #{GpgHelpers::User1.primary_keyid}" + end + end + end + + it "verified and the gpg user's profile doesn't exist anymore" do + user_1_key + + visit project_commits_path(project, :'signed-commits') + + # wait for the signature to get generated + within(find('.commit', text: 'signed and authored commit by nannie bernhard')) do + expect(page).to have_content 'Verified' + end + + user_1.destroy! + + refresh + + within(find('.commit', text: 'signed and authored commit by nannie bernhard')) do + click_on 'Verified' + within '.popover' do + expect(page).to have_content 'This commit was signed with a verified signature and the committer email is verified to belong to the same user.' + expect(page).to have_content 'Nannie Bernhard' + expect(page).to have_content 'nannie.bernhard@example.com' + expect(page).to have_content "GPG Key ID: #{GpgHelpers::User1.primary_keyid}" + end + end + end + end +end diff --git a/spec/features/task_lists_spec.rb b/spec/features/task_lists_spec.rb index c14826df55a..580258f77eb 100644 --- a/spec/features/task_lists_spec.rb +++ b/spec/features/task_lists_spec.rb @@ -52,8 +52,8 @@ feature 'Task Lists' do before do Warden.test_mode! - project.team << [user, :master] - project.team << [user2, :guest] + project.add_master(user) + project.add_guest(user2) login_as(user) end diff --git a/spec/finders/environments_finder_spec.rb b/spec/finders/environments_finder_spec.rb index 0c063f6d5ee..3a8a1e7de74 100644 --- a/spec/finders/environments_finder_spec.rb +++ b/spec/finders/environments_finder_spec.rb @@ -12,7 +12,7 @@ describe EnvironmentsFinder do context 'tagged deployment' do before do - create(:deployment, environment: environment, ref: '1.0', tag: true, sha: project.commit.id) + create(:deployment, environment: environment, ref: 'v1.1.0', tag: true, sha: project.commit.id) end it 'returns environment when with_tags is set' do diff --git a/spec/helpers/blame_helper_spec.rb b/spec/helpers/blame_helper_spec.rb index b4368516d83..722d21c566f 100644 --- a/spec/helpers/blame_helper_spec.rb +++ b/spec/helpers/blame_helper_spec.rb @@ -35,25 +35,32 @@ describe BlameHelper do end describe '#age_map_class' do - let(:dates) do - [Time.zone.local(2014, 3, 17, 0, 0, 0)] - end - let(:blame_groups) do - [ - { commit: double(committed_date: dates[0]) } - ] - end + let(:date) { Time.zone.local(2014, 3, 17, 0, 0, 0) } + let(:blame_groups) { [{ commit: double(committed_date: date) }] } let(:duration) do - project = double(created_at: dates[0]) + project = double(created_at: date) helper.age_map_duration(blame_groups, project) end it 'returns blame-commit-age-9 when oldest' do - expect(helper.age_map_class(dates[0], duration)).to eq 'blame-commit-age-9' + expect(helper.age_map_class(date, duration)).to eq 'blame-commit-age-9' end it 'returns blame-commit-age-0 class when newest' do expect(helper.age_map_class(duration[:now], duration)).to eq 'blame-commit-age-0' end + + context 'when called on the same day as project creation' do + let(:same_day_duration) do + project = double(created_at: now) + helper.age_map_duration(today_blame_groups, project) + end + let(:today_blame_groups) { [{ commit: double(committed_date: now) }] } + let(:now) { Time.zone.now } + + it 'returns blame-commit-age-0 class' do + expect(helper.age_map_class(duration[:now], same_day_duration)).to eq 'blame-commit-age-0' + end + end end end diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js index c763487d12f..690665ae12c 100644 --- a/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js +++ b/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js @@ -37,6 +37,26 @@ describe('MRWidgetPipeline', () => { }); }); + describe('hasPipeline', () => { + it('should return true when there is a pipeline', () => { + expect(Object.keys(mockData.pipeline).length).toBeGreaterThan(0); + + const vm = createComponent({ + pipeline: mockData.pipeline, + }); + + expect(vm.hasPipeline).toBeTruthy(); + }); + + it('should return false when there is no pipeline', () => { + const vm = createComponent({ + pipeline: null, + }); + + expect(vm.hasPipeline).toBeFalsy(); + }); + }); + describe('hasCIError', () => { it('should return false when there is no CI error', () => { const vm = createComponent({ diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js index c607c9746a4..8f69458ffb0 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js @@ -11,6 +11,7 @@ const createComponent = (customConfig = {}) => { isPipelineActive: false, pipeline: null, isPipelineFailed: false, + isPipelinePassing: false, onlyAllowMergeIfPipelineSucceeds: false, hasCI: false, ciStatus: null, @@ -68,6 +69,18 @@ describe('MRWidgetReadyToMerge', () => { }); describe('computed', () => { + describe('shouldShowMergeWhenPipelineSucceedsText', () => { + it('should return true with active pipeline', () => { + vm.mr.isPipelineActive = true; + expect(vm.shouldShowMergeWhenPipelineSucceedsText).toBeTruthy(); + }); + + it('should return false with inactive pipeline', () => { + vm.mr.isPipelineActive = false; + expect(vm.shouldShowMergeWhenPipelineSucceedsText).toBeFalsy(); + }); + }); + describe('commitMessageLinkTitle', () => { const withDesc = 'Include description in commit message'; const withoutDesc = "Don't include description in commit message"; @@ -203,20 +216,55 @@ describe('MRWidgetReadyToMerge', () => { describe('methods', () => { describe('isMergeAllowed', () => { - it('should return false with initial data', () => { + it('should return true when no pipeline and not required to succeed', () => { + vm.mr.onlyAllowMergeIfPipelineSucceeds = false; + vm.mr.isPipelinePassing = false; expect(vm.isMergeAllowed()).toBeTruthy(); }); - it('should return false when MR is set only merge when pipeline succeeds', () => { - vm.mr.onlyAllowMergeIfPipelineSucceeds = true; + it('should return true when pipeline failed and not required to succeed', () => { + vm.mr.onlyAllowMergeIfPipelineSucceeds = false; + vm.mr.isPipelinePassing = false; expect(vm.isMergeAllowed()).toBeTruthy(); }); - it('should return true true', () => { + it('should return false when pipeline failed and required to succeed', () => { vm.mr.onlyAllowMergeIfPipelineSucceeds = true; - vm.mr.isPipelineFailed = true; + vm.mr.isPipelinePassing = false; expect(vm.isMergeAllowed()).toBeFalsy(); }); + + it('should return true when pipeline succeeded and required to succeed', () => { + vm.mr.onlyAllowMergeIfPipelineSucceeds = true; + vm.mr.isPipelinePassing = true; + expect(vm.isMergeAllowed()).toBeTruthy(); + }); + }); + + describe('shouldShowMergeControls', () => { + it('should return false when an external pipeline is running and required to succeed', () => { + spyOn(vm, 'isMergeAllowed').and.returnValue(false); + vm.mr.isPipelineActive = false; + expect(vm.shouldShowMergeControls()).toBeFalsy(); + }); + + it('should return true when the build succeeded or build not required to succeed', () => { + spyOn(vm, 'isMergeAllowed').and.returnValue(true); + vm.mr.isPipelineActive = false; + expect(vm.shouldShowMergeControls()).toBeTruthy(); + }); + + it('should return true when showing the MWPS button and a pipeline is running that needs to be successful', () => { + spyOn(vm, 'isMergeAllowed').and.returnValue(false); + vm.mr.isPipelineActive = true; + expect(vm.shouldShowMergeControls()).toBeTruthy(); + }); + + it('should return true when showing the MWPS button but not required for the pipeline to succeed', () => { + spyOn(vm, 'isMergeAllowed').and.returnValue(true); + vm.mr.isPipelineActive = true; + expect(vm.shouldShowMergeControls()).toBeTruthy(); + }); }); describe('updateCommitMessage', () => { diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js index 669ee248bf1..da66c7504cb 100644 --- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js @@ -59,23 +59,15 @@ describe('mrWidgetOptions', () => { }); describe('shouldRenderPipelines', () => { - it('should return true for the initial data', () => { - expect(vm.shouldRenderPipelines).toBeTruthy(); - }); + it('should return true when hasCI is true', () => { + vm.mr.hasCI = true; - it('should return true when pipeline is empty but MR.hasCI is set to true', () => { - vm.mr.pipeline = {}; expect(vm.shouldRenderPipelines).toBeTruthy(); }); - it('should return true when pipeline available', () => { + it('should return false when hasCI is false', () => { vm.mr.hasCI = false; - expect(vm.shouldRenderPipelines).toBeTruthy(); - }); - it('should return false when there is no pipeline', () => { - vm.mr.pipeline = {}; - vm.mr.hasCI = false; expect(vm.shouldRenderPipelines).toBeFalsy(); }); }); diff --git a/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js b/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js index 56dd0198ae2..8e5614b20f0 100644 --- a/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js +++ b/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js @@ -18,5 +18,39 @@ describe('MergeRequestStore', () => { store.setData({ ...mockData, work_in_progress: !mockData.work_in_progress }); expect(store.hasSHAChanged).toBe(false); }); + + describe('isPipelinePassing', () => { + it('is true when the CI status is `success`', () => { + store.setData({ ...mockData, ci_status: 'success' }); + expect(store.isPipelinePassing).toBe(true); + }); + + it('is true when the CI status is `success_with_warnings`', () => { + store.setData({ ...mockData, ci_status: 'success_with_warnings' }); + expect(store.isPipelinePassing).toBe(true); + }); + + it('is false when the CI status is `failed`', () => { + store.setData({ ...mockData, ci_status: 'failed' }); + expect(store.isPipelinePassing).toBe(false); + }); + + it('is false when the CI status is anything except `success`', () => { + store.setData({ ...mockData, ci_status: 'foobarbaz' }); + expect(store.isPipelinePassing).toBe(false); + }); + }); + + describe('isPipelineSkipped', () => { + it('should set isPipelineSkipped=true when the CI status is `skipped`', () => { + store.setData({ ...mockData, ci_status: 'skipped' }); + expect(store.isPipelineSkipped).toBe(true); + }); + + it('should set isPipelineSkipped=false when the CI status is anything except `skipped`', () => { + store.setData({ ...mockData, ci_status: 'foobarbaz' }); + expect(store.isPipelineSkipped).toBe(false); + }); + }); }); }); diff --git a/spec/lib/gitlab/database/grant_spec.rb b/spec/lib/gitlab/database/grant_spec.rb new file mode 100644 index 00000000000..651da3e8476 --- /dev/null +++ b/spec/lib/gitlab/database/grant_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +describe Gitlab::Database::Grant do + describe '.scope_to_current_user' do + it 'scopes the relation to the current user' do + user = Gitlab::Database.username + column = Gitlab::Database.postgresql? ? :grantee : :User + names = described_class.scope_to_current_user.pluck(column).uniq + + expect(names).to eq([user]) + end + end + + describe '.create_and_execute_trigger' do + it 'returns true when the user can create and execute a trigger' do + # We assume the DB/user is set up correctly so that triggers can be + # created, which is necessary anyway for other tests to work. + expect(described_class.create_and_execute_trigger?('users')).to eq(true) + end + + it 'returns false when the user can not create and/or execute a trigger' do + allow(described_class).to receive(:scope_to_current_user) + .and_return(described_class.none) + + result = described_class.create_and_execute_trigger?('kittens') + + expect(result).to eq(false) + end + end +end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index ec2274a70aa..40ac4ef319d 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -452,6 +452,8 @@ describe Gitlab::Database::MigrationHelpers do it 'renames a column concurrently' do allow(Gitlab::Database).to receive(:postgresql?).and_return(false) + expect(model).to receive(:check_trigger_permissions!).with(:users) + expect(model).to receive(:install_rename_triggers_for_mysql) .with(trigger_name, 'users', 'old', 'new') @@ -479,6 +481,8 @@ describe Gitlab::Database::MigrationHelpers do it 'renames a column concurrently' do allow(Gitlab::Database).to receive(:postgresql?).and_return(true) + expect(model).to receive(:check_trigger_permissions!).with(:users) + expect(model).to receive(:install_rename_triggers_for_postgresql) .with(trigger_name, 'users', 'old', 'new') @@ -508,6 +512,8 @@ describe Gitlab::Database::MigrationHelpers do it 'cleans up the renaming procedure for PostgreSQL' do allow(Gitlab::Database).to receive(:postgresql?).and_return(true) + expect(model).to receive(:check_trigger_permissions!).with(:users) + expect(model).to receive(:remove_rename_triggers_for_postgresql) .with(:users, /trigger_.{12}/) @@ -519,6 +525,8 @@ describe Gitlab::Database::MigrationHelpers do it 'cleans up the renaming procedure for MySQL' do allow(Gitlab::Database).to receive(:postgresql?).and_return(false) + expect(model).to receive(:check_trigger_permissions!).with(:users) + expect(model).to receive(:remove_rename_triggers_for_mysql) .with(/trigger_.{12}/) @@ -575,8 +583,8 @@ describe Gitlab::Database::MigrationHelpers do describe '#remove_rename_triggers_for_postgresql' do it 'removes the function and trigger' do - expect(model).to receive(:execute).with('DROP TRIGGER foo ON bar') - expect(model).to receive(:execute).with('DROP FUNCTION foo()') + expect(model).to receive(:execute).with('DROP TRIGGER IF EXISTS foo ON bar') + expect(model).to receive(:execute).with('DROP FUNCTION IF EXISTS foo()') model.remove_rename_triggers_for_postgresql('bar', 'foo') end @@ -584,8 +592,8 @@ describe Gitlab::Database::MigrationHelpers do describe '#remove_rename_triggers_for_mysql' do it 'removes the triggers' do - expect(model).to receive(:execute).with('DROP TRIGGER foo_insert') - expect(model).to receive(:execute).with('DROP TRIGGER foo_update') + expect(model).to receive(:execute).with('DROP TRIGGER IF EXISTS foo_insert') + expect(model).to receive(:execute).with('DROP TRIGGER IF EXISTS foo_update') model.remove_rename_triggers_for_mysql('foo') end @@ -845,4 +853,67 @@ describe Gitlab::Database::MigrationHelpers do end end end + + describe 'sidekiq migration helpers', :sidekiq, :redis do + let(:worker) do + Class.new do + include Sidekiq::Worker + sidekiq_options queue: 'test' + end + end + + describe '#sidekiq_queue_length' do + context 'when queue is empty' do + it 'returns zero' do + Sidekiq::Testing.disable! do + expect(model.sidekiq_queue_length('test')).to eq 0 + end + end + end + + context 'when queue contains jobs' do + it 'returns correct size of the queue' do + Sidekiq::Testing.disable! do + worker.perform_async('Something', [1]) + worker.perform_async('Something', [2]) + + expect(model.sidekiq_queue_length('test')).to eq 2 + end + end + end + end + + describe '#migrate_sidekiq_queue' do + it 'migrates jobs from one sidekiq queue to another' do + Sidekiq::Testing.disable! do + worker.perform_async('Something', [1]) + worker.perform_async('Something', [2]) + + expect(model.sidekiq_queue_length('test')).to eq 2 + expect(model.sidekiq_queue_length('new_test')).to eq 0 + + model.sidekiq_queue_migrate('test', to: 'new_test') + + expect(model.sidekiq_queue_length('test')).to eq 0 + expect(model.sidekiq_queue_length('new_test')).to eq 2 + end + end + end + end + + describe '#check_trigger_permissions!' do + it 'does nothing when the user has the correct permissions' do + expect { model.check_trigger_permissions!('users') } + .not_to raise_error(RuntimeError) + end + + it 'raises RuntimeError when the user does not have the correct permissions' do + allow(Gitlab::Database::Grant).to receive(:create_and_execute_trigger?) + .with('kittens') + .and_return(false) + + expect { model.check_trigger_permissions!('kittens') } + .to raise_error(RuntimeError, /Your database user is not allowed/) + end + end end diff --git a/spec/lib/gitlab/gpg/commit_spec.rb b/spec/lib/gitlab/gpg/commit_spec.rb index e521fcc6dc1..b07462e4978 100644 --- a/spec/lib/gitlab/gpg/commit_spec.rb +++ b/spec/lib/gitlab/gpg/commit_spec.rb @@ -2,45 +2,9 @@ require 'rails_helper' describe Gitlab::Gpg::Commit do describe '#signature' do - let!(:project) { create :project, :repository, path: 'sample-project' } - let!(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' } - - context 'unsigned commit' do - it 'returns nil' do - expect(described_class.new(project, commit_sha).signature).to be_nil - end - end - - context 'known and verified public key' do - let!(:gpg_key) do - create :gpg_key, key: GpgHelpers::User1.public_key, user: create(:user, email: GpgHelpers::User1.emails.first) - end - - before do - allow(Rugged::Commit).to receive(:extract_signature) - .with(Rugged::Repository, commit_sha) - .and_return( - [ - GpgHelpers::User1.signed_commit_signature, - GpgHelpers::User1.signed_commit_base_data - ] - ) - end - - it 'returns a valid signature' do - expect(described_class.new(project, commit_sha).signature).to have_attributes( - commit_sha: commit_sha, - project: project, - gpg_key: gpg_key, - gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - gpg_key_user_name: GpgHelpers::User1.names.first, - gpg_key_user_email: GpgHelpers::User1.emails.first, - valid_signature: true - ) - end - + shared_examples 'returns the cached signature on second call' do it 'returns the cached signature on second call' do - gpg_commit = described_class.new(project, commit_sha) + gpg_commit = described_class.new(commit) expect(gpg_commit).to receive(:using_keychain).and_call_original gpg_commit.signature @@ -51,11 +15,140 @@ describe Gitlab::Gpg::Commit do end end - context 'known but unverified public key' do - let!(:gpg_key) { create :gpg_key, key: GpgHelpers::User1.public_key } + let!(:project) { create :project, :repository, path: 'sample-project' } + let!(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' } - before do - allow(Rugged::Commit).to receive(:extract_signature) + context 'unsigned commit' do + let!(:commit) { create :commit, project: project, sha: commit_sha } + + it 'returns nil' do + expect(described_class.new(commit).signature).to be_nil + end + end + + context 'known key' do + context 'user matches the key uid' do + context 'user email matches the email committer' do + let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User1.emails.first } + + let!(:user) { create(:user, email: GpgHelpers::User1.emails.first) } + + let!(:gpg_key) do + create :gpg_key, key: GpgHelpers::User1.public_key, user: user + end + + before do + allow(Rugged::Commit).to receive(:extract_signature) + .with(Rugged::Repository, commit_sha) + .and_return( + [ + GpgHelpers::User1.signed_commit_signature, + GpgHelpers::User1.signed_commit_base_data + ] + ) + end + + it 'returns a valid signature' do + expect(described_class.new(commit).signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + gpg_key_user_name: GpgHelpers::User1.names.first, + gpg_key_user_email: GpgHelpers::User1.emails.first, + verification_status: 'verified' + ) + end + + it_behaves_like 'returns the cached signature on second call' + end + + context 'user email does not match the committer email, but is the same user' do + let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User2.emails.first } + + let(:user) do + create(:user, email: GpgHelpers::User1.emails.first).tap do |user| + create :email, user: user, email: GpgHelpers::User2.emails.first + end + end + + let!(:gpg_key) do + create :gpg_key, key: GpgHelpers::User1.public_key, user: user + end + + before do + allow(Rugged::Commit).to receive(:extract_signature) + .with(Rugged::Repository, commit_sha) + .and_return( + [ + GpgHelpers::User1.signed_commit_signature, + GpgHelpers::User1.signed_commit_base_data + ] + ) + end + + it 'returns an invalid signature' do + expect(described_class.new(commit).signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + gpg_key_user_name: GpgHelpers::User1.names.first, + gpg_key_user_email: GpgHelpers::User1.emails.first, + verification_status: 'same_user_different_email' + ) + end + + it_behaves_like 'returns the cached signature on second call' + end + + context 'user email does not match the committer email' do + let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User2.emails.first } + + let(:user) { create(:user, email: GpgHelpers::User1.emails.first) } + + let!(:gpg_key) do + create :gpg_key, key: GpgHelpers::User1.public_key, user: user + end + + before do + allow(Rugged::Commit).to receive(:extract_signature) + .with(Rugged::Repository, commit_sha) + .and_return( + [ + GpgHelpers::User1.signed_commit_signature, + GpgHelpers::User1.signed_commit_base_data + ] + ) + end + + it 'returns an invalid signature' do + expect(described_class.new(commit).signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + gpg_key_user_name: GpgHelpers::User1.names.first, + gpg_key_user_email: GpgHelpers::User1.emails.first, + verification_status: 'other_user' + ) + end + + it_behaves_like 'returns the cached signature on second call' + end + end + + context 'user does not match the key uid' do + let!(:commit) { create :commit, project: project, sha: commit_sha } + + let(:user) { create(:user, email: GpgHelpers::User2.emails.first) } + + let!(:gpg_key) do + create :gpg_key, key: GpgHelpers::User1.public_key, user: user + end + + before do + allow(Rugged::Commit).to receive(:extract_signature) .with(Rugged::Repository, commit_sha) .and_return( [ @@ -63,33 +156,27 @@ describe Gitlab::Gpg::Commit do GpgHelpers::User1.signed_commit_base_data ] ) - end - - it 'returns an invalid signature' do - expect(described_class.new(project, commit_sha).signature).to have_attributes( - commit_sha: commit_sha, - project: project, - gpg_key: gpg_key, - gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - gpg_key_user_name: GpgHelpers::User1.names.first, - gpg_key_user_email: GpgHelpers::User1.emails.first, - valid_signature: false - ) - end - - it 'returns the cached signature on second call' do - gpg_commit = described_class.new(project, commit_sha) - - expect(gpg_commit).to receive(:using_keychain).and_call_original - gpg_commit.signature + end + + it 'returns an invalid signature' do + expect(described_class.new(commit).signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + gpg_key_user_name: GpgHelpers::User1.names.first, + gpg_key_user_email: GpgHelpers::User1.emails.first, + verification_status: 'unverified_key' + ) + end - # consecutive call - expect(gpg_commit).not_to receive(:using_keychain).and_call_original - gpg_commit.signature + it_behaves_like 'returns the cached signature on second call' end end - context 'unknown public key' do + context 'unknown key' do + let!(:commit) { create :commit, project: project, sha: commit_sha } + before do allow(Rugged::Commit).to receive(:extract_signature) .with(Rugged::Repository, commit_sha) @@ -102,27 +189,18 @@ describe Gitlab::Gpg::Commit do end it 'returns an invalid signature' do - expect(described_class.new(project, commit_sha).signature).to have_attributes( + expect(described_class.new(commit).signature).to have_attributes( commit_sha: commit_sha, project: project, gpg_key: nil, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, gpg_key_user_name: nil, gpg_key_user_email: nil, - valid_signature: false + verification_status: 'unknown_key' ) end - it 'returns the cached signature on second call' do - gpg_commit = described_class.new(project, commit_sha) - - expect(gpg_commit).to receive(:using_keychain).and_call_original - gpg_commit.signature - - # consecutive call - expect(gpg_commit).not_to receive(:using_keychain).and_call_original - gpg_commit.signature - end + it_behaves_like 'returns the cached signature on second call' end end end diff --git a/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb b/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb index 4de4419de27..b9fd4d02156 100644 --- a/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb +++ b/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb @@ -4,8 +4,29 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do describe '#run' do let!(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' } let!(:project) { create :project, :repository, path: 'sample-project' } + let!(:raw_commit) do + raw_commit = double( + :raw_commit, + signature: [ + GpgHelpers::User1.signed_commit_signature, + GpgHelpers::User1.signed_commit_base_data + ], + sha: commit_sha, + committer_email: GpgHelpers::User1.emails.first + ) + + allow(raw_commit).to receive :save! + + raw_commit + end + + let!(:commit) do + create :commit, git_commit: raw_commit, project: project + end before do + allow_any_instance_of(Project).to receive(:commit).and_return(commit) + allow(Rugged::Commit).to receive(:extract_signature) .with(Rugged::Repository, commit_sha) .and_return( @@ -25,7 +46,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: nil, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: true + verification_status: 'verified' end it 'assigns the gpg key to the signature when the missing gpg key is added' do @@ -39,7 +60,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: gpg_key, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: true + verification_status: 'verified' ) end @@ -54,7 +75,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: nil, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: true + verification_status: 'verified' ) end end @@ -68,7 +89,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: nil, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: false + verification_status: 'unknown_key' end it 'updates the signature to being valid when the missing gpg key is added' do @@ -82,7 +103,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: gpg_key, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: true + verification_status: 'verified' ) end @@ -97,7 +118,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: nil, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: false + verification_status: 'unknown_key' ) end end @@ -115,7 +136,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: nil, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: false + verification_status: 'unknown_key' end it 'updates the signature to being valid when the user updates the email address' do @@ -123,7 +144,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do key: GpgHelpers::User1.public_key, user: user - expect(invalid_gpg_signature.reload.valid_signature).to be_falsey + expect(invalid_gpg_signature.reload.verification_status).to eq 'unverified_key' # InvalidGpgSignatureUpdater is called by the after_update hook user.update_attributes!(email: GpgHelpers::User1.emails.first) @@ -133,7 +154,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: gpg_key, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: true + verification_status: 'verified' ) end @@ -147,7 +168,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: gpg_key, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: false + verification_status: 'unverified_key' ) # InvalidGpgSignatureUpdater is called by the after_update hook @@ -158,7 +179,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: gpg_key, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: false + verification_status: 'unverified_key' ) end end diff --git a/spec/lib/gitlab/gpg_spec.rb b/spec/lib/gitlab/gpg_spec.rb index 30ad033b204..11a2aea1915 100644 --- a/spec/lib/gitlab/gpg_spec.rb +++ b/spec/lib/gitlab/gpg_spec.rb @@ -42,6 +42,21 @@ describe Gitlab::Gpg do described_class.user_infos_from_key('bogus') ).to eq [] end + + it 'downcases the email' do + public_key = double(:key) + fingerprints = double(:fingerprints) + uid = double(:uid, name: 'Nannie Bernhard', email: 'NANNIE.BERNHARD@EXAMPLE.COM') + raw_key = double(:raw_key, uids: [uid]) + allow(Gitlab::Gpg::CurrentKeyChain).to receive(:fingerprints_from_key).with(public_key).and_return(fingerprints) + allow(GPGME::Key).to receive(:find).with(:public, anything).and_return([raw_key]) + + user_infos = described_class.user_infos_from_key(public_key) + expect(user_infos).to eq([{ + name: 'Nannie Bernhard', + email: 'nannie.bernhard@example.com' + }]) + end end describe '.current_home_dir' do diff --git a/spec/lib/gitlab/url_sanitizer_spec.rb b/spec/lib/gitlab/url_sanitizer_spec.rb index 308b1a128be..fdc3990132a 100644 --- a/spec/lib/gitlab/url_sanitizer_spec.rb +++ b/spec/lib/gitlab/url_sanitizer_spec.rb @@ -1,11 +1,7 @@ require 'spec_helper' describe Gitlab::UrlSanitizer do - let(:credentials) { { user: 'blah', password: 'password' } } - let(:url_sanitizer) do - described_class.new("https://github.com/me/project.git", credentials: credentials) - end - let(:user) { double(:user, username: 'john.doe') } + using RSpec::Parameterized::TableSyntax describe '.sanitize' do def sanitize_url(url) @@ -16,83 +12,166 @@ describe Gitlab::UrlSanitizer do }) end - it 'mask the credentials from HTTP URLs' do - filtered_content = sanitize_url('http://user:pass@test.com/root/repoC.git/') + where(:input, :output) do + 'http://user:pass@test.com/root/repoC.git/' | 'http://*****:*****@test.com/root/repoC.git/' + 'https://user:pass@test.com/root/repoA.git/' | 'https://*****:*****@test.com/root/repoA.git/' + 'ssh://user@host.test/path/to/repo.git' | 'ssh://*****@host.test/path/to/repo.git' - expect(filtered_content).to include("http://*****:*****@test.com/root/repoC.git/") - end + # git protocol does not support authentication but clean any details anyway + 'git://user:pass@host.test/path/to/repo.git' | 'git://*****:*****@host.test/path/to/repo.git' + 'git://host.test/path/to/repo.git' | 'git://host.test/path/to/repo.git' - it 'mask the credentials from HTTPS URLs' do - filtered_content = sanitize_url('https://user:pass@test.com/root/repoA.git/') + # SCP-style URLs are left unmodified + 'user@server:project.git' | 'user@server:project.git' + 'user:pass@server:project.git' | 'user:pass@server:project.git' - expect(filtered_content).to include("https://*****:*****@test.com/root/repoA.git/") + # return an empty string for invalid URLs + 'ssh://' | '' end - it 'mask credentials from SSH URLs' do - filtered_content = sanitize_url('ssh://user@host.test/path/to/repo.git') - - expect(filtered_content).to include("ssh://*****@host.test/path/to/repo.git") + with_them do + it { expect(sanitize_url(input)).to include("repository '#{output}' not found") } end + end - it 'does not modify Git URLs' do - # git protocol does not support authentication - filtered_content = sanitize_url('git://host.test/path/to/repo.git') + describe '.valid?' do + where(:value, :url) do + false | nil + false | '' + false | '123://invalid:url' + true | 'valid@project:url.git' + true | 'ssh://example.com' + true | 'ssh://:@example.com' + true | 'ssh://foo@example.com' + true | 'ssh://foo:bar@example.com' + true | 'ssh://foo:bar@example.com/group/group/project.git' + true | 'git://example.com/group/group/project.git' + true | 'git://foo:bar@example.com/group/group/project.git' + true | 'http://foo:bar@example.com/group/group/project.git' + true | 'https://foo:bar@example.com/group/group/project.git' + end - expect(filtered_content).to include("git://host.test/path/to/repo.git") + with_them do + it { expect(described_class.valid?(url)).to eq(value) } end + end + + describe '#sanitized_url' do + context 'credentials in hash' do + where(username: ['foo', '', nil], password: ['bar', '', nil]) - it 'does not modify scp-like URLs' do - filtered_content = sanitize_url('user@server:project.git') + with_them do + let(:credentials) { { user: username, password: password } } + subject { described_class.new('http://example.com', credentials: credentials).sanitized_url } - expect(filtered_content).to include("user@server:project.git") + it { is_expected.to eq('http://example.com') } + end end - it 'returns an empty string for invalid URLs' do - filtered_content = sanitize_url('ssh://') + context 'credentials in URL' do + where(userinfo: %w[foo:bar@ foo@ :bar@ :@ @] + [nil]) - expect(filtered_content).to include("repository '' not found") - end - end + with_them do + subject { described_class.new("http://#{userinfo}example.com").sanitized_url } - describe '.valid?' do - it 'validates url strings' do - expect(described_class.valid?(nil)).to be(false) - expect(described_class.valid?('valid@project:url.git')).to be(true) - expect(described_class.valid?('123://invalid:url')).to be(false) + it { is_expected.to eq('http://example.com') } + end end end - describe '#sanitized_url' do - it { expect(url_sanitizer.sanitized_url).to eq("https://github.com/me/project.git") } - end - describe '#credentials' do - it { expect(url_sanitizer.credentials).to eq(credentials) } + context 'credentials in hash' do + where(:input, :output) do + { user: 'foo', password: 'bar' } | { user: 'foo', password: 'bar' } + { user: 'foo', password: '' } | { user: 'foo', password: nil } + { user: 'foo', password: nil } | { user: 'foo', password: nil } + { user: '', password: 'bar' } | { user: nil, password: 'bar' } + { user: '', password: '' } | { user: nil, password: nil } + { user: '', password: nil } | { user: nil, password: nil } + { user: nil, password: 'bar' } | { user: nil, password: 'bar' } + { user: nil, password: '' } | { user: nil, password: nil } + { user: nil, password: nil } | { user: nil, password: nil } + end - context 'when user is given to #initialize' do - let(:url_sanitizer) do - described_class.new("https://github.com/me/project.git", credentials: { user: user.username }) + with_them do + subject { described_class.new('user@example.com:path.git', credentials: input).credentials } + + it { is_expected.to eq(output) } end - it { expect(url_sanitizer.credentials).to eq({ user: 'john.doe' }) } + it 'overrides URL-provided credentials' do + sanitizer = described_class.new('http://a:b@example.com', credentials: { user: 'c', password: 'd' }) + + expect(sanitizer.credentials).to eq(user: 'c', password: 'd') + end + end + + context 'credentials in URL' do + where(:url, :credentials) do + 'http://foo:bar@example.com' | { user: 'foo', password: 'bar' } + 'http://:bar@example.com' | { user: nil, password: 'bar' } + 'http://foo:@example.com' | { user: 'foo', password: nil } + 'http://foo@example.com' | { user: 'foo', password: nil } + 'http://:@example.com' | { user: nil, password: nil } + 'http://@example.com' | { user: nil, password: nil } + 'http://example.com' | { user: nil, password: nil } + + # Credentials from SCP-style URLs are not supported at present + 'foo@example.com:path' | { user: nil, password: nil } + 'foo:bar@example.com:path' | { user: nil, password: nil } + + # Other invalid URLs + nil | { user: nil, password: nil } + '' | { user: nil, password: nil } + 'no' | { user: nil, password: nil } + end + + with_them do + subject { described_class.new(url).credentials } + + it { is_expected.to eq(credentials) } + end end end describe '#full_url' do - it { expect(url_sanitizer.full_url).to eq("https://blah:password@github.com/me/project.git") } + context 'credentials in hash' do + where(:credentials, :userinfo) do + { user: 'foo', password: 'bar' } | 'foo:bar@' + { user: 'foo', password: '' } | 'foo@' + { user: 'foo', password: nil } | 'foo@' + { user: '', password: 'bar' } | ':bar@' + { user: '', password: '' } | nil + { user: '', password: nil } | nil + { user: nil, password: 'bar' } | ':bar@' + { user: nil, password: '' } | nil + { user: nil, password: nil } | nil + end - it 'supports scp-like URLs' do - sanitizer = described_class.new('user@server:project.git') + with_them do + subject { described_class.new('http://example.com', credentials: credentials).full_url } - expect(sanitizer.full_url).to eq('user@server:project.git') + it { is_expected.to eq("http://#{userinfo}example.com") } + end end - context 'when user is given to #initialize' do - let(:url_sanitizer) do - described_class.new("https://github.com/me/project.git", credentials: { user: user.username }) + context 'credentials in URL' do + where(:input, :output) do + nil | '' + '' | :same + 'git@example.com' | :same + 'http://example.com' | :same + 'http://foo@example.com' | :same + 'http://foo:@example.com' | 'http://foo@example.com' + 'http://:bar@example.com' | :same + 'http://foo:bar@example.com' | :same end - it { expect(url_sanitizer.full_url).to eq("https://john.doe@github.com/me/project.git") } + with_them do + let(:expected) { output == :same ? input : output } + + it { expect(described_class.new(input).full_url).to eq(expected) } + end end end end diff --git a/spec/models/gpg_key_spec.rb b/spec/models/gpg_key_spec.rb index e48f20bf53b..9c99c3e5c08 100644 --- a/spec/models/gpg_key_spec.rb +++ b/spec/models/gpg_key_spec.rb @@ -99,14 +99,14 @@ describe GpgKey do end describe '#verified?' do - it 'returns true one of the email addresses in the key belongs to the user' do + it 'returns true if one of the email addresses in the key belongs to the user' do user = create :user, email: 'bette.cartwright@example.com' gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user expect(gpg_key.verified?).to be_truthy end - it 'returns false if one of the email addresses in the key does not belong to the user' do + it 'returns false if none of the email addresses in the key does not belong to the user' do user = create :user, email: 'someone.else@example.com' gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user @@ -114,6 +114,32 @@ describe GpgKey do end end + describe 'verified_and_belongs_to_email?' do + it 'returns false if none of the email addresses in the key does not belong to the user' do + user = create :user, email: 'someone.else@example.com' + gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user + + expect(gpg_key.verified?).to be_falsey + expect(gpg_key.verified_and_belongs_to_email?('someone.else@example.com')).to be_falsey + end + + it 'returns false if one of the email addresses in the key belongs to the user and does not match the provided email' do + user = create :user, email: 'bette.cartwright@example.com' + gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user + + expect(gpg_key.verified?).to be_truthy + expect(gpg_key.verified_and_belongs_to_email?('bette.cartwright@example.net')).to be_falsey + end + + it 'returns true if one of the email addresses in the key belongs to the user and matches the provided email' do + user = create :user, email: 'bette.cartwright@example.com' + gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user + + expect(gpg_key.verified?).to be_truthy + expect(gpg_key.verified_and_belongs_to_email?('bette.cartwright@example.com')).to be_truthy + end + end + describe 'notification', :mailer do let(:user) { create(:user) } @@ -129,15 +155,15 @@ describe GpgKey do describe '#revoke' do it 'invalidates all associated gpg signatures and destroys the key' do gpg_key = create :gpg_key - gpg_signature = create :gpg_signature, valid_signature: true, gpg_key: gpg_key + gpg_signature = create :gpg_signature, verification_status: :verified, gpg_key: gpg_key unrelated_gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key - unrelated_gpg_signature = create :gpg_signature, valid_signature: true, gpg_key: unrelated_gpg_key + unrelated_gpg_signature = create :gpg_signature, verification_status: :verified, gpg_key: unrelated_gpg_key gpg_key.revoke expect(gpg_signature.reload).to have_attributes( - valid_signature: false, + verification_status: 'unknown_key', gpg_key: nil ) @@ -145,7 +171,7 @@ describe GpgKey do # unrelated signature is left untouched expect(unrelated_gpg_signature.reload).to have_attributes( - valid_signature: true, + verification_status: 'verified', gpg_key: unrelated_gpg_key ) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 5baa7c81ecc..0028b60749b 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1660,12 +1660,40 @@ describe MergeRequest do end describe '#fetch_ref' do - it 'sets "ref_fetched" flag to true' do + before do subject.update!(ref_fetched: nil) + end - subject.fetch_ref + context 'when the branch exists' do + it 'writes the ref' do + expect(subject.target_project.repository).to receive(:write_ref).with(subject.ref_path, /\h{40}/) - expect(subject.reload.ref_fetched).to be_truthy + subject.fetch_ref + end + + it 'sets "ref_fetched" flag to true' do + subject.fetch_ref + + expect(subject.reload.ref_fetched).to be_truthy + end + end + + context 'when the branch does not exist' do + before do + subject.source_branch = 'definitely-not-master' + end + + it 'does not write the ref' do + expect(subject.target_project.repository).not_to receive(:write_ref) + + subject.fetch_ref + end + + it 'sets "ref_fetched" flag to true' do + subject.fetch_ref + + expect(subject.reload.ref_fetched).to be_truthy + end end end diff --git a/spec/models/project_services/pipelines_email_service_spec.rb b/spec/models/project_services/pipelines_email_service_spec.rb index 5faab9ba38b..be07ca2d945 100644 --- a/spec/models/project_services/pipelines_email_service_spec.rb +++ b/spec/models/project_services/pipelines_email_service_spec.rb @@ -6,7 +6,8 @@ describe PipelinesEmailService, :mailer do end let(:project) { create(:project, :repository) } - let(:recipient) { 'test@gitlab.com' } + let(:recipients) { 'test@gitlab.com' } + let(:receivers) { [recipients] } let(:data) do Gitlab::DataBuilder::Pipeline.build(pipeline) @@ -48,18 +49,24 @@ describe PipelinesEmailService, :mailer do shared_examples 'sending email' do before do + subject.recipients = recipients + perform_enqueued_jobs do run end end it 'sends email' do - should_only_email(double(notification_email: recipient), kind: :bcc) + emails = receivers.map { |r| double(notification_email: r) } + + should_only_email(*emails, kind: :bcc) end end shared_examples 'not sending email' do before do + subject.recipients = recipients + perform_enqueued_jobs do run end @@ -75,10 +82,6 @@ describe PipelinesEmailService, :mailer do subject.test(data) end - before do - subject.recipients = recipient - end - context 'when pipeline is failed' do before do data[:object_attributes][:status] = 'failed' @@ -104,10 +107,6 @@ describe PipelinesEmailService, :mailer do end context 'with recipients' do - before do - subject.recipients = recipient - end - context 'with failed pipeline' do before do data[:object_attributes][:status] = 'failed' @@ -152,9 +151,7 @@ describe PipelinesEmailService, :mailer do end context 'with empty recipients list' do - before do - subject.recipients = ' ,, ' - end + let(:recipients) { ' ,, ' } context 'with failed pipeline' do before do @@ -165,5 +162,19 @@ describe PipelinesEmailService, :mailer do it_behaves_like 'not sending email' end end + + context 'with recipients list separating with newlines' do + let(:recipients) { "\ntest@gitlab.com, \r\nexample@gitlab.com" } + let(:receivers) { %w[test@gitlab.com example@gitlab.com] } + + context 'with failed pipeline' do + before do + data[:object_attributes][:status] = 'failed' + pipeline.update(status: 'failed') + end + + it_behaves_like 'sending email' + end + end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 98861c8c81a..f55d287445c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1650,6 +1650,17 @@ describe Project do end end + describe '#can_create_repository?' do + let(:project) { build(:project) } + + it 'skips gitlab-shell exists?' do + project.skip_disk_validation = true + + expect(project.gitlab_shell).not_to receive(:exists?) + expect(project.can_create_repository?).to be_truthy + end + end + describe '#latest_successful_builds_for' do def create_pipeline(status = 'success') create(:ci_pipeline, project: project, diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index cfa77648338..131340cee4c 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -922,13 +922,16 @@ describe Repository, models: true do describe '#update_branch_with_hooks' do let(:old_rev) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } # git rev-parse feature let(:new_rev) { 'a74ae73c1ccde9b974a70e82b901588071dc142a' } # commit whose parent is old_rev + let(:updating_ref) { 'refs/heads/feature' } + let(:target_project) { project } + let(:target_repository) { target_project.repository } context 'when pre hooks were successful' do before do service = GitHooksService.new expect(GitHooksService).to receive(:new).and_return(service) expect(service).to receive(:execute) - .with(user, project, old_rev, new_rev, 'refs/heads/feature') + .with(user, target_repository, old_rev, new_rev, updating_ref) .and_yield(service).and_return(true) end @@ -959,6 +962,58 @@ describe Repository, models: true do expect(repository.find_branch('feature').dereferenced_target.id).to eq(new_rev) end end + + context 'when target project does not have the commit' do + let(:target_project) { create(:project, :empty_repo) } + let(:old_rev) { Gitlab::Git::BLANK_SHA } + let(:new_rev) { project.commit('feature').sha } + let(:updating_ref) { 'refs/heads/master' } + + it 'fetch_ref and create the branch' do + expect(target_project.repository).to receive(:fetch_ref) + .and_call_original + + GitOperationService.new(user, target_repository) + .with_branch( + 'master', + start_project: project, + start_branch_name: 'feature') { new_rev } + + expect(target_repository.branch_names).to contain_exactly('master') + end + end + + context 'when target project already has the commit' do + let(:target_project) { create(:project, :repository) } + + it 'does not fetch_ref and just pass the commit' do + expect(target_repository).not_to receive(:fetch_ref) + + GitOperationService.new(user, target_repository) + .with_branch('feature', start_project: project) { new_rev } + end + end + end + + context 'when temporary ref failed to be created from other project' do + let(:target_project) { create(:project, :empty_repo) } + + before do + expect(target_project.repository).to receive(:run_git) + end + + it 'raises Rugged::ReferenceError' do + raise_reference_error = raise_error(Rugged::ReferenceError) do |err| + expect(err.cause).to be_nil + end + + expect do + GitOperationService.new(user, target_project.repository) + .with_branch('feature', + start_project: project, + &:itself) + end.to raise_reference_error + end end context 'when the update adds more than one commit' do @@ -2055,4 +2110,51 @@ describe Repository, models: true do end end end + + describe '#with_repo_branch_commit' do + context 'when comparing with the same repository' do + let(:start_repository) { repository } + + context 'when the branch exists' do + let(:start_branch_name) { 'master' } + + it 'yields the commit' do + expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } + .to yield_with_args(an_instance_of(::Commit)) + end + end + + context 'when the branch does not exist' do + let(:start_branch_name) { 'definitely-not-master' } + + it 'yields nil' do + expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } + .to yield_with_args(nil) + end + end + end + + context 'when comparing with another repository' do + let(:forked_project) { create(:project, :repository) } + let(:start_repository) { forked_project.repository } + + context 'when the branch exists' do + let(:start_branch_name) { 'master' } + + it 'yields the commit' do + expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } + .to yield_with_args(an_instance_of(::Commit)) + end + end + + context 'when the branch does not exist' do + let(:start_branch_name) { 'definitely-not-master' } + + it 'yields nil' do + expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } + .to yield_with_args(nil) + end + end + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e25ffa4d228..3bfdc8e0d17 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2074,4 +2074,18 @@ describe User do end end end + + describe '#verified_email?' do + it 'returns true when the email is the primary email' do + user = build :user, email: 'email@example.com' + + expect(user.verified_email?('email@example.com')).to be true + end + + it 'returns false when the email is not the primary email' do + user = build :user, email: 'email@example.com' + + expect(user.verified_email?('other_email@example.com')).to be false + end + end end diff --git a/spec/policies/global_policy_spec.rb b/spec/policies/global_policy_spec.rb index a6bf70c1e09..0ca1d03c2d3 100644 --- a/spec/policies/global_policy_spec.rb +++ b/spec/policies/global_policy_spec.rb @@ -51,4 +51,27 @@ describe GlobalPolicy do end end end + + describe "create fork" do + context "when user has not exceeded project limit" do + it { is_expected.to be_allowed(:create_fork) } + end + + context "when user has exceeded project limit" do + let(:current_user) { create(:user, projects_limit: 0) } + + it { is_expected.not_to be_allowed(:create_fork) } + end + + context "when user is a master in a group" do + let(:group) { create(:group) } + let(:current_user) { create(:user, projects_limit: 0) } + + before do + group.add_master(current_user) + end + + it { is_expected.to be_allowed(:create_fork) } + end + end end diff --git a/spec/policies/namespace_policy_spec.rb b/spec/policies/namespace_policy_spec.rb new file mode 100644 index 00000000000..e52ff02e5f0 --- /dev/null +++ b/spec/policies/namespace_policy_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe NamespacePolicy do + let(:current_user) { create(:user) } + let(:namespace) { current_user.namespace } + + subject { described_class.new(current_user, namespace) } + + context "create projects" do + context "user namespace" do + it { is_expected.to be_allowed(:create_projects) } + end + + context "user who has exceeded project limit" do + let(:current_user) { create(:user, projects_limit: 0) } + + it { is_expected.not_to be_allowed(:create_projects) } + end + end +end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 9eda6836ded..db17a78b109 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -31,7 +31,7 @@ describe API::MergeRequests do it 'returns authentication error' do get api('/merge_requests') - expect(response).to have_http_status(401) + expect(response).to have_gitlab_http_status(401) end end @@ -43,7 +43,7 @@ describe API::MergeRequests do it 'returns an array of all merge requests' do get api('/merge_requests', user), scope: :all - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.map { |mr| mr['id'] }) @@ -56,7 +56,7 @@ describe API::MergeRequests do get api('/merge_requests', user), scope: :all - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.map { |mr| mr['id'] }) @@ -68,7 +68,7 @@ describe API::MergeRequests do get api('/merge_requests', user2) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(1) expect(json_response.first['id']).to eq(merge_request3.id) @@ -79,7 +79,7 @@ describe API::MergeRequests do get api('/merge_requests', user), author_id: user2.id, scope: :all - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(1) expect(json_response.first['id']).to eq(merge_request3.id) @@ -90,7 +90,7 @@ describe API::MergeRequests do get api('/merge_requests', user), assignee_id: user2.id, scope: :all - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(1) expect(json_response.first['id']).to eq(merge_request3.id) @@ -101,7 +101,7 @@ describe API::MergeRequests do get api('/merge_requests', user2), scope: 'assigned-to-me' - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(1) expect(json_response.first['id']).to eq(merge_request3.id) @@ -112,7 +112,7 @@ describe API::MergeRequests do get api('/merge_requests', user2), scope: 'created-by-me' - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(1) expect(json_response.first['id']).to eq(merge_request3.id) @@ -125,7 +125,7 @@ describe API::MergeRequests do it "returns authentication error" do get api("/projects/#{project.id}/merge_requests") - expect(response).to have_http_status(401) + expect(response).to have_gitlab_http_status(401) end end @@ -145,7 +145,7 @@ describe API::MergeRequests do it "returns an array of all merge_requests" do get api("/projects/#{project.id}/merge_requests", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) @@ -166,7 +166,7 @@ describe API::MergeRequests do it "returns an array of all merge_requests using simple mode" do get api("/projects/#{project.id}/merge_requests?view=simple", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response.last.keys).to match_array(%w(id iid title web_url created_at description project_id state updated_at)) expect(json_response).to be_an Array @@ -182,7 +182,7 @@ describe API::MergeRequests do it "returns an array of all merge_requests" do get api("/projects/#{project.id}/merge_requests?state", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) @@ -192,7 +192,7 @@ describe API::MergeRequests do it "returns an array of open merge_requests" do get api("/projects/#{project.id}/merge_requests?state=opened", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(1) @@ -202,7 +202,7 @@ describe API::MergeRequests do it "returns an array of closed merge_requests" do get api("/projects/#{project.id}/merge_requests?state=closed", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(1) @@ -212,7 +212,7 @@ describe API::MergeRequests do it "returns an array of merged merge_requests" do get api("/projects/#{project.id}/merge_requests?state=merged", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(1) @@ -222,7 +222,7 @@ describe API::MergeRequests do it 'returns merge_request by "iids" array' do get api("/projects/#{project.id}/merge_requests", user), iids: [merge_request.iid, merge_request_closed.iid] - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(2) expect(json_response.first['title']).to eq merge_request_closed.title @@ -232,14 +232,14 @@ describe API::MergeRequests do it 'matches V4 response schema' do get api("/projects/#{project.id}/merge_requests", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to match_response_schema('public_api/v4/merge_requests') end it 'returns an empty array if no issue matches milestone' do get api("/projects/#{project.id}/merge_requests", user), milestone: '1.0.0' - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(0) end @@ -247,7 +247,7 @@ describe API::MergeRequests do it 'returns an empty array if milestone does not exist' do get api("/projects/#{project.id}/merge_requests", user), milestone: 'foo' - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(0) end @@ -262,7 +262,7 @@ describe API::MergeRequests do it 'returns an array of merge requests matching state in milestone' do get api("/projects/#{project.id}/merge_requests", user), milestone: '0.9', state: 'closed' - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(1) expect(json_response.first['id']).to eq(merge_request_closed.id) @@ -271,7 +271,7 @@ describe API::MergeRequests do it 'returns an array of labeled merge requests' do get api("/projects/#{project.id}/merge_requests?labels=#{label.title}", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(1) expect(json_response.first['labels']).to eq([label2.title, label.title]) @@ -280,7 +280,7 @@ describe API::MergeRequests do it 'returns an array of labeled merge requests where all labels match' do get api("/projects/#{project.id}/merge_requests?labels=#{label.title},foo,bar", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(0) end @@ -288,7 +288,7 @@ describe API::MergeRequests do it 'returns an empty array if no merge request matches labels' do get api("/projects/#{project.id}/merge_requests?labels=foo,bar", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(0) end @@ -307,7 +307,7 @@ describe API::MergeRequests do get api("/projects/#{project.id}/merge_requests?labels=#{bug_label.title}&milestone=#{milestone1.title}&state=merged", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(1) expect(json_response.first['id']).to eq(mr2.id) @@ -322,7 +322,7 @@ describe API::MergeRequests do it "returns an array of merge_requests in ascending order" do get api("/projects/#{project.id}/merge_requests?sort=asc", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) @@ -333,7 +333,7 @@ describe API::MergeRequests do it "returns an array of merge_requests in descending order" do get api("/projects/#{project.id}/merge_requests?sort=desc", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) @@ -344,7 +344,7 @@ describe API::MergeRequests do it "returns an array of merge_requests ordered by updated_at" do get api("/projects/#{project.id}/merge_requests?order_by=updated_at", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) @@ -355,7 +355,7 @@ describe API::MergeRequests do it "returns an array of merge_requests ordered by created_at" do get api("/projects/#{project.id}/merge_requests?order_by=created_at&sort=asc", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) @@ -370,7 +370,7 @@ describe API::MergeRequests do it 'exposes known attributes' do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['id']).to eq(merge_request.id) expect(json_response['iid']).to eq(merge_request.iid) expect(json_response['project_id']).to eq(merge_request.project.id) @@ -398,7 +398,7 @@ describe API::MergeRequests do it "returns merge_request" do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['title']).to eq(merge_request.title) expect(json_response['iid']).to eq(merge_request.iid) expect(json_response['work_in_progress']).to eq(false) @@ -409,13 +409,13 @@ describe API::MergeRequests do it "returns a 404 error if merge_request_iid not found" do get api("/projects/#{project.id}/merge_requests/999", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it "returns a 404 error if merge_request `id` is used instead of iid" do get api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end context 'Work in Progress' do @@ -423,7 +423,7 @@ describe API::MergeRequests do it "returns merge_request" do get api("/projects/#{project.id}/merge_requests/#{merge_request_wip.iid}", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['work_in_progress']).to eq(true) end end @@ -434,7 +434,7 @@ describe API::MergeRequests do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/commits", user) commit = merge_request.commits.first - expect(response.status).to eq 200 + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.size).to eq(merge_request.commits.size) @@ -444,13 +444,13 @@ describe API::MergeRequests do it 'returns a 404 when merge_request_iid not found' do get api("/projects/#{project.id}/merge_requests/999/commits", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it 'returns a 404 when merge_request id is used instead of iid' do get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/commits", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end end @@ -458,19 +458,19 @@ describe API::MergeRequests do it 'returns the change information of the merge_request' do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/changes", user) - expect(response.status).to eq 200 + expect(response).to have_gitlab_http_status(200) expect(json_response['changes'].size).to eq(merge_request.diffs.size) end it 'returns a 404 when merge_request_iid not found' do get api("/projects/#{project.id}/merge_requests/999/changes", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it 'returns a 404 when merge_request id is used instead of iid' do get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/changes", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end end @@ -485,7 +485,7 @@ describe API::MergeRequests do labels: 'label, label2', milestone_id: milestone.id - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['title']).to eq('Test merge_request') expect(json_response['labels']).to eq(%w(label label2)) expect(json_response['milestone']['id']).to eq(milestone.id) @@ -495,25 +495,25 @@ describe API::MergeRequests do it "returns 422 when source_branch equals target_branch" do post api("/projects/#{project.id}/merge_requests", user), title: "Test merge_request", source_branch: "master", target_branch: "master", author: user - expect(response).to have_http_status(422) + expect(response).to have_gitlab_http_status(422) end it "returns 400 when source_branch is missing" do post api("/projects/#{project.id}/merge_requests", user), title: "Test merge_request", target_branch: "master", author: user - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end it "returns 400 when target_branch is missing" do post api("/projects/#{project.id}/merge_requests", user), title: "Test merge_request", source_branch: "markdown", author: user - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end it "returns 400 when title is missing" do post api("/projects/#{project.id}/merge_requests", user), target_branch: 'master', source_branch: 'markdown' - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end it 'allows special label names' do @@ -523,7 +523,7 @@ describe API::MergeRequests do target_branch: 'master', author: user, labels: 'label, label?, label&foo, ?, &' - expect(response.status).to eq(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['labels']).to include 'label' expect(json_response['labels']).to include 'label?' expect(json_response['labels']).to include 'label&foo' @@ -549,7 +549,7 @@ describe API::MergeRequests do target_branch: 'master', author: user end.to change { MergeRequest.count }.by(0) - expect(response).to have_http_status(409) + expect(response).to have_gitlab_http_status(409) end end @@ -580,15 +580,17 @@ describe API::MergeRequests do let!(:fork_project) { create(:project, forked_from_project: project, namespace: user2.namespace, creator_id: user2.id) } let!(:unrelated_project) { create(:project, namespace: create(:user).namespace, creator_id: user2.id) } - before :each do |each| - fork_project.team << [user2, :reporter] + before do + fork_project.add_reporter(user2) + + allow_any_instance_of(MergeRequest).to receive(:write_ref) end it "returns merge_request" do post api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', source_branch: "feature_conflict", target_branch: "master", author: user2, target_project_id: project.id, description: 'Test description for Test merge_request' - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['title']).to eq('Test merge_request') expect(json_response['description']).to eq('Test description for Test merge_request') end @@ -599,7 +601,7 @@ describe API::MergeRequests do expect(fork_project.forked_from_project).to eq(project) post api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', source_branch: "master", target_branch: "master", author: user2, target_project_id: project.id - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['title']).to eq('Test merge_request') end @@ -613,25 +615,25 @@ describe API::MergeRequests do author: user2, target_project_id: project.id - expect(response).to have_http_status(422) + expect(response).to have_gitlab_http_status(422) end it "returns 400 when source_branch is missing" do post api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', target_branch: "master", author: user2, target_project_id: project.id - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end it "returns 400 when target_branch is missing" do post api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', target_branch: "master", author: user2, target_project_id: project.id - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end it "returns 400 when title is missing" do post api("/projects/#{fork_project.id}/merge_requests", user2), target_branch: 'master', source_branch: 'markdown', author: user2, target_project_id: project.id - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end context 'when target_branch is specified' do @@ -642,7 +644,7 @@ describe API::MergeRequests do source_branch: 'markdown', author: user, target_project_id: fork_project.id - expect(response).to have_http_status(422) + expect(response).to have_gitlab_http_status(422) end it 'returns 422 if targeting a different fork' do @@ -652,14 +654,14 @@ describe API::MergeRequests do source_branch: 'markdown', author: user2, target_project_id: unrelated_project.id - expect(response).to have_http_status(422) + expect(response).to have_gitlab_http_status(422) end end it "returns 201 when target_branch is specified and for the same project" do post api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', target_branch: 'master', source_branch: 'markdown', author: user2, target_project_id: fork_project.id - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) end end end @@ -674,7 +676,7 @@ describe API::MergeRequests do it "denies the deletion of the merge request" do delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", developer) - expect(response).to have_http_status(403) + expect(response).to have_gitlab_http_status(403) end end @@ -682,19 +684,19 @@ describe API::MergeRequests do it "destroys the merge request owners can destroy" do delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) - expect(response).to have_http_status(204) + expect(response).to have_gitlab_http_status(204) end it "returns 404 for an invalid merge request IID" do delete api("/projects/#{project.id}/merge_requests/12345", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it "returns 404 if the merge request id is used instead of iid" do delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end end end @@ -705,7 +707,7 @@ describe API::MergeRequests do it "returns merge_request in case of success" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) end it "returns 406 if branch can't be merged" do @@ -714,21 +716,21 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) - expect(response).to have_http_status(406) + expect(response).to have_gitlab_http_status(406) expect(json_response['message']).to eq('Branch cannot be merged') end it "returns 405 if merge_request is not open" do merge_request.close put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) - expect(response).to have_http_status(405) + expect(response).to have_gitlab_http_status(405) expect(json_response['message']).to eq('405 Method Not Allowed') end it "returns 405 if merge_request is a work in progress" do merge_request.update_attribute(:title, "WIP: #{merge_request.title}") put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) - expect(response).to have_http_status(405) + expect(response).to have_gitlab_http_status(405) expect(json_response['message']).to eq('405 Method Not Allowed') end @@ -737,7 +739,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) - expect(response).to have_http_status(405) + expect(response).to have_gitlab_http_status(405) expect(json_response['message']).to eq('405 Method Not Allowed') end @@ -745,21 +747,21 @@ describe API::MergeRequests do user2 = create(:user) project.team << [user2, :reporter] put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user2) - expect(response).to have_http_status(401) + expect(response).to have_gitlab_http_status(401) expect(json_response['message']).to eq('401 Unauthorized') end it "returns 409 if the SHA parameter doesn't match" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), sha: merge_request.diff_head_sha.reverse - expect(response).to have_http_status(409) + expect(response).to have_gitlab_http_status(409) expect(json_response['message']).to start_with('SHA does not match HEAD of source branch') end it "succeeds if the SHA parameter matches" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), sha: merge_request.diff_head_sha - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) end it "enables merge when pipeline succeeds if the pipeline is active" do @@ -768,7 +770,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), merge_when_pipeline_succeeds: true - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['title']).to eq('Test') expect(json_response['merge_when_pipeline_succeeds']).to eq(true) end @@ -780,7 +782,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), merge_when_pipeline_succeeds: true - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['title']).to eq('Test') expect(json_response['merge_when_pipeline_succeeds']).to eq(true) end @@ -788,13 +790,13 @@ describe API::MergeRequests do it "returns 404 for an invalid merge request IID" do put api("/projects/#{project.id}/merge_requests/12345/merge", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it "returns 404 if the merge request id is used instead of iid" do put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end end @@ -803,39 +805,39 @@ describe API::MergeRequests do it "returns merge_request" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), state_event: "close" - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['state']).to eq('closed') end end it "updates title and returns merge_request" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), title: "New title" - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['title']).to eq('New title') end it "updates description and returns merge_request" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), description: "New description" - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['description']).to eq('New description') end it "updates milestone_id and returns merge_request" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), milestone_id: milestone.id - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['milestone']['id']).to eq(milestone.id) end it "returns merge_request with renamed target_branch" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), target_branch: "wiki" - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['target_branch']).to eq('wiki') end it "returns merge_request that removes the source branch" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), remove_source_branch: true - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['force_remove_source_branch']).to be_truthy end @@ -856,7 +858,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), state_event: 'close', title: nil merge_request.reload - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) expect(merge_request.state).to eq('opened') end @@ -864,20 +866,20 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), state_event: 'close', target_branch: nil merge_request.reload - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) expect(merge_request.state).to eq('opened') end it "returns 404 for an invalid merge request IID" do put api("/projects/#{project.id}/merge_requests/12345", user), state_event: "close" - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it "returns 404 if the merge request id is used instead of iid" do put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), state_event: "close" - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end end @@ -890,7 +892,7 @@ describe API::MergeRequests do get api("/projects/#{project.id}/merge_requests/#{mr.iid}/closes_issues", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(1) @@ -900,7 +902,7 @@ describe API::MergeRequests do it 'returns an empty array when there are no issues to be closed' do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/closes_issues", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(0) @@ -916,7 +918,7 @@ describe API::MergeRequests do get api("/projects/#{jira_project.id}/merge_requests/#{merge_request.iid}/closes_issues", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(2) @@ -936,19 +938,19 @@ describe API::MergeRequests do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/closes_issues", guest) - expect(response).to have_http_status(403) + expect(response).to have_gitlab_http_status(403) end it "returns 404 for an invalid merge request IID" do get api("/projects/#{project.id}/merge_requests/12345/closes_issues", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it "returns 404 if the merge request id is used instead of iid" do get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/closes_issues", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end end @@ -956,26 +958,26 @@ describe API::MergeRequests do it 'subscribes to a merge request' do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/subscribe", admin) - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['subscribed']).to eq(true) end it 'returns 304 if already subscribed' do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/subscribe", user) - expect(response).to have_http_status(304) + expect(response).to have_gitlab_http_status(304) end it 'returns 404 if the merge request is not found' do post api("/projects/#{project.id}/merge_requests/123/subscribe", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it 'returns 404 if the merge request id is used instead of iid' do post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscribe", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it 'returns 403 if user has no access to read code' do @@ -984,7 +986,7 @@ describe API::MergeRequests do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/subscribe", guest) - expect(response).to have_http_status(403) + expect(response).to have_gitlab_http_status(403) end end @@ -992,26 +994,26 @@ describe API::MergeRequests do it 'unsubscribes from a merge request' do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unsubscribe", user) - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['subscribed']).to eq(false) end it 'returns 304 if not subscribed' do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unsubscribe", admin) - expect(response).to have_http_status(304) + expect(response).to have_gitlab_http_status(304) end it 'returns 404 if the merge request is not found' do post api("/projects/#{project.id}/merge_requests/123/unsubscribe", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it 'returns 404 if the merge request id is used instead of iid' do post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/unsubscribe", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it 'returns 403 if user has no access to read code' do @@ -1020,7 +1022,7 @@ describe API::MergeRequests do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unsubscribe", guest) - expect(response).to have_http_status(403) + expect(response).to have_gitlab_http_status(403) end end diff --git a/spec/requests/api/v3/merge_requests_spec.rb b/spec/requests/api/v3/merge_requests_spec.rb index ef7516fc28f..68052ef1035 100644 --- a/spec/requests/api/v3/merge_requests_spec.rb +++ b/spec/requests/api/v3/merge_requests_spec.rb @@ -315,15 +315,17 @@ describe API::MergeRequests do let!(:fork_project) { create(:project, forked_from_project: project, namespace: user2.namespace, creator_id: user2.id) } let!(:unrelated_project) { create(:project, namespace: create(:user).namespace, creator_id: user2.id) } - before :each do |each| - fork_project.team << [user2, :reporter] + before do + fork_project.add_reporter(user2) + + allow_any_instance_of(MergeRequest).to receive(:write_ref) end it "returns merge_request" do post v3_api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', source_branch: "feature_conflict", target_branch: "master", author: user2, target_project_id: project.id, description: 'Test description for Test merge_request' - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['title']).to eq('Test merge_request') expect(json_response['description']).to eq('Test description for Test merge_request') end @@ -334,7 +336,7 @@ describe API::MergeRequests do expect(fork_project.forked_from_project).to eq(project) post v3_api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', source_branch: "master", target_branch: "master", author: user2, target_project_id: project.id - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['title']).to eq('Test merge_request') end @@ -348,25 +350,25 @@ describe API::MergeRequests do author: user2, target_project_id: project.id - expect(response).to have_http_status(422) + expect(response).to have_gitlab_http_status(422) end it "returns 400 when source_branch is missing" do post v3_api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', target_branch: "master", author: user2, target_project_id: project.id - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end it "returns 400 when target_branch is missing" do post v3_api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', target_branch: "master", author: user2, target_project_id: project.id - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end it "returns 400 when title is missing" do post v3_api("/projects/#{fork_project.id}/merge_requests", user2), target_branch: 'master', source_branch: 'markdown', author: user2, target_project_id: project.id - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end context 'when target_branch is specified' do @@ -377,7 +379,7 @@ describe API::MergeRequests do source_branch: 'markdown', author: user, target_project_id: fork_project.id - expect(response).to have_http_status(422) + expect(response).to have_gitlab_http_status(422) end it 'returns 422 if targeting a different fork' do @@ -387,14 +389,14 @@ describe API::MergeRequests do source_branch: 'markdown', author: user2, target_project_id: unrelated_project.id - expect(response).to have_http_status(422) + expect(response).to have_gitlab_http_status(422) end end it "returns 201 when target_branch is specified and for the same project" do post v3_api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', target_branch: 'master', source_branch: 'markdown', author: user2, target_project_id: fork_project.id - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) end end end diff --git a/spec/services/ci/pipeline_trigger_service_spec.rb b/spec/services/ci/pipeline_trigger_service_spec.rb index 9a6875e448c..f4ff818c479 100644 --- a/spec/services/ci/pipeline_trigger_service_spec.rb +++ b/spec/services/ci/pipeline_trigger_service_spec.rb @@ -34,6 +34,8 @@ describe Ci::PipelineTriggerService do expect(result[:pipeline].ref).to eq('master') expect(result[:pipeline].project).to eq(project) expect(result[:pipeline].user).to eq(trigger.owner) + expect(result[:pipeline].trigger_requests.to_a) + .to eq(result[:pipeline].builds.map(&:trigger_request).uniq) expect(result[:status]).to eq(:success) end diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb index 049b082277a..08267d6e6a0 100644 --- a/spec/services/create_deployment_service_spec.rb +++ b/spec/services/create_deployment_service_spec.rb @@ -20,6 +20,10 @@ describe CreateDeploymentService do let(:service) { described_class.new(job) } + before do + allow_any_instance_of(Deployment).to receive(:create_ref) + end + describe '#execute' do subject { service.execute } diff --git a/spec/services/issues/resolve_discussions_spec.rb b/spec/services/issues/resolve_discussions_spec.rb index fac66791ffb..67a86c50fc1 100644 --- a/spec/services/issues/resolve_discussions_spec.rb +++ b/spec/services/issues/resolve_discussions_spec.rb @@ -20,7 +20,7 @@ describe Issues::ResolveDiscussions do describe "for resolving discussions" do let(:discussion) { create(:diff_note_on_merge_request, project: project, note: "Almost done").to_discussion } let(:merge_request) { discussion.noteable } - let(:other_merge_request) { create(:merge_request, source_project: project, source_branch: "other") } + let(:other_merge_request) { create(:merge_request, source_project: project, source_branch: "fix") } describe "#merge_request_for_resolving_discussion" do let(:service) { DummyService.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid) } diff --git a/spec/services/merge_requests/get_urls_service_spec.rb b/spec/services/merge_requests/get_urls_service_spec.rb index 672d86e4028..25599dea19f 100644 --- a/spec/services/merge_requests/get_urls_service_spec.rb +++ b/spec/services/merge_requests/get_urls_service_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" describe MergeRequests::GetUrlsService do let(:project) { create(:project, :public, :repository) } let(:service) { described_class.new(project) } - let(:source_branch) { "my_branch" } + let(:source_branch) { "merge-test" } let(:new_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=#{source_branch}" } let(:show_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/#{merge_request.iid}" } let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" } @@ -111,9 +111,9 @@ describe MergeRequests::GetUrlsService do end context 'pushing new branch and existing branch (with merge request created) at once' do - let!(:merge_request) { create(:merge_request, source_project: project, source_branch: "existing_branch") } + let!(:merge_request) { create(:merge_request, source_project: project, source_branch: "markdown") } let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/new_branch" } - let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/existing_branch" } + let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/markdown" } let(:changes) { "#{new_branch_changes}\n#{existing_branch_changes}" } let(:new_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch" } @@ -124,7 +124,7 @@ describe MergeRequests::GetUrlsService do url: new_merge_request_url, new_merge_request: true }, { - branch_name: "existing_branch", + branch_name: "markdown", url: show_merge_request_url, new_merge_request: false }]) diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index dbf3c49c67d..95ff976e198 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -208,6 +208,15 @@ describe Projects::CreateService, '#execute' do end end + context 'when skip_disk_validation is used' do + it 'sets the project attribute' do + opts[:skip_disk_validation] = true + project = create_project(user, opts) + + expect(project.skip_disk_validation).to be_truthy + end + end + def create_project(user, opts) Projects::CreateService.new(user, opts).execute end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 3036ced3ef8..faf24cc9ca4 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -57,6 +57,21 @@ describe Projects::UpdateService, '#execute' do end end end + + context 'when project visibility is higher than parent group' do + let(:group) { create(:group, visibility_level: Gitlab::VisibilityLevel::INTERNAL) } + + before do + project.update(namespace: group, visibility_level: group.visibility_level) + end + + it 'does not update project visibility level' do + result = update_project(project, admin, visibility_level: Gitlab::VisibilityLevel::PUBLIC) + + expect(result).to eq({ status: :error, message: 'Visibility level public is not allowed in a internal group.' }) + expect(project.reload).to be_internal + end + end end describe 'when updating project that has forks' do @@ -151,8 +166,10 @@ describe Projects::UpdateService, '#execute' do it 'returns an error result when record cannot be updated' do result = update_project(project, admin, { name: 'foo&bar' }) - expect(result).to eq({ status: :error, - message: 'Project could not be updated!' }) + expect(result).to eq({ + status: :error, + message: "Name can contain only letters, digits, emojis, '_', '.', dash, space. It must start with letter, digit, emoji or '_'." + }) end end diff --git a/spec/services/test_hooks/project_service_spec.rb b/spec/services/test_hooks/project_service_spec.rb index 4218c15a3ce..28dfa9cf59c 100644 --- a/spec/services/test_hooks/project_service_spec.rb +++ b/spec/services/test_hooks/project_service_spec.rb @@ -21,6 +21,7 @@ describe TestHooks::ProjectService do context 'push_events' do let(:trigger) { 'push_events' } + let(:trigger_key) { :push_hooks } it 'returns error message if not enough data' do allow(project).to receive(:empty_repo?).and_return(true) @@ -33,13 +34,14 @@ describe TestHooks::ProjectService do allow(project).to receive(:empty_repo?).and_return(false) allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data) - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result) expect(service.execute).to include(success_result) end end context 'tag_push_events' do let(:trigger) { 'tag_push_events' } + let(:trigger_key) { :tag_push_hooks } it 'returns error message if not enough data' do allow(project).to receive(:empty_repo?).and_return(true) @@ -52,13 +54,14 @@ describe TestHooks::ProjectService do allow(project).to receive(:empty_repo?).and_return(false) allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data) - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result) expect(service.execute).to include(success_result) end end context 'note_events' do let(:trigger) { 'note_events' } + let(:trigger_key) { :note_hooks } it 'returns error message if not enough data' do expect(hook).not_to receive(:execute) @@ -69,13 +72,14 @@ describe TestHooks::ProjectService do allow(project).to receive(:notes).and_return([Note.new]) allow(Gitlab::DataBuilder::Note).to receive(:build).and_return(sample_data) - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result) expect(service.execute).to include(success_result) end end context 'issues_events' do let(:trigger) { 'issues_events' } + let(:trigger_key) { :issue_hooks } let(:issue) { build(:issue) } it 'returns error message if not enough data' do @@ -87,13 +91,14 @@ describe TestHooks::ProjectService do allow(project).to receive(:issues).and_return([issue]) allow(issue).to receive(:to_hook_data).and_return(sample_data) - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result) expect(service.execute).to include(success_result) end end context 'confidential_issues_events' do let(:trigger) { 'confidential_issues_events' } + let(:trigger_key) { :confidential_issue_hooks } let(:issue) { build(:issue) } it 'returns error message if not enough data' do @@ -105,13 +110,14 @@ describe TestHooks::ProjectService do allow(project).to receive(:issues).and_return([issue]) allow(issue).to receive(:to_hook_data).and_return(sample_data) - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result) expect(service.execute).to include(success_result) end end context 'merge_requests_events' do let(:trigger) { 'merge_requests_events' } + let(:trigger_key) { :merge_request_hooks } it 'returns error message if not enough data' do expect(hook).not_to receive(:execute) @@ -122,13 +128,14 @@ describe TestHooks::ProjectService do create(:merge_request, source_project: project) allow_any_instance_of(MergeRequest).to receive(:to_hook_data).and_return(sample_data) - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result) expect(service.execute).to include(success_result) end end context 'job_events' do let(:trigger) { 'job_events' } + let(:trigger_key) { :job_hooks } it 'returns error message if not enough data' do expect(hook).not_to receive(:execute) @@ -139,13 +146,14 @@ describe TestHooks::ProjectService do create(:ci_build, project: project) allow(Gitlab::DataBuilder::Build).to receive(:build).and_return(sample_data) - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result) expect(service.execute).to include(success_result) end end context 'pipeline_events' do let(:trigger) { 'pipeline_events' } + let(:trigger_key) { :pipeline_hooks } it 'returns error message if not enough data' do expect(hook).not_to receive(:execute) @@ -156,13 +164,14 @@ describe TestHooks::ProjectService do create(:ci_empty_pipeline, project: project) allow(Gitlab::DataBuilder::Pipeline).to receive(:build).and_return(sample_data) - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result) expect(service.execute).to include(success_result) end end context 'wiki_page_events' do let(:trigger) { 'wiki_page_events' } + let(:trigger_key) { :wiki_page_hooks } it 'returns error message if wiki disabled' do allow(project).to receive(:wiki_enabled?).and_return(false) @@ -180,7 +189,7 @@ describe TestHooks::ProjectService do create(:wiki_page, wiki: project.wiki) allow(Gitlab::DataBuilder::WikiPage).to receive(:build).and_return(sample_data) - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result) expect(service.execute).to include(success_result) end end diff --git a/spec/services/test_hooks/system_service_spec.rb b/spec/services/test_hooks/system_service_spec.rb index a15708bf82f..ff8b9595538 100644 --- a/spec/services/test_hooks/system_service_spec.rb +++ b/spec/services/test_hooks/system_service_spec.rb @@ -24,36 +24,39 @@ describe TestHooks::SystemService do context 'push_events' do let(:trigger) { 'push_events' } + let(:trigger_key) { :push_hooks } it 'executes hook' do allow(project).to receive(:empty_repo?).and_return(false) expect(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original - expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger_key).and_return(success_result) expect(service.execute).to include(success_result) end end context 'tag_push_events' do let(:trigger) { 'tag_push_events' } + let(:trigger_key) { :tag_push_hooks } it 'executes hook' do allow(project.repository).to receive(:tags).and_return(['tag']) expect(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original - expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger_key).and_return(success_result) expect(service.execute).to include(success_result) end end context 'repository_update_events' do let(:trigger) { 'repository_update_events' } + let(:trigger_key) { :repository_update_hooks } it 'executes hook' do allow(project).to receive(:empty_repo?).and_return(false) expect(Gitlab::DataBuilder::Repository).to receive(:sample_data).and_call_original - expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Repository::SAMPLE_DATA, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Repository::SAMPLE_DATA, trigger_key).and_return(success_result) expect(service.execute).to include(success_result) end end diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index 79d90defd78..d55919e7cbb 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -12,7 +12,7 @@ describe WebHookService do let(:data) do { before: 'oldrev', after: 'newrev', ref: 'ref' } end - let(:service_instance) { described_class.new(project_hook, data, 'push_hooks') } + let(:service_instance) { described_class.new(project_hook, data, :push_hooks) } describe '#execute' do before(:each) do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2580776afb0..2533337b67c 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -8,6 +8,7 @@ require File.expand_path("../../config/environment", __FILE__) require 'rspec/rails' require 'shoulda/matchers' require 'rspec/retry' +require 'rspec-parameterized' rspec_profiling_is_configured = ENV['RSPEC_PROFILING_POSTGRES_URL'].present? || diff --git a/spec/support/issuables_list_metadata_shared_examples.rb b/spec/support/issuables_list_metadata_shared_examples.rb index a60d3b0d22d..75982432ab4 100644 --- a/spec/support/issuables_list_metadata_shared_examples.rb +++ b/spec/support/issuables_list_metadata_shared_examples.rb @@ -2,12 +2,12 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil| before do @issuable_ids = [] - 2.times do |n| + %w[fix improve/awesome].each do |source_branch| issuable = if issuable_type == :issue create(issuable_type, project: project) else - create(issuable_type, source_project: project, source_branch: "#{n}-feature") + create(issuable_type, source_project: project, source_branch: source_branch) end @issuable_ids << issuable.id diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 1e39f80699c..290ded3ff7e 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -5,7 +5,7 @@ module TestEnv # When developing the seed repository, comment out the branch you will modify. BRANCH_SHA = { - 'signed-commits' => '5d4a1cb', + 'signed-commits' => '2d1096e', 'not-merged-branch' => 'b83d6e3', 'branch-merged' => '498214d', 'empty-branch' => '7efb185', diff --git a/spec/workers/create_gpg_signature_worker_spec.rb b/spec/workers/create_gpg_signature_worker_spec.rb index 54978baca88..aa6c347d738 100644 --- a/spec/workers/create_gpg_signature_worker_spec.rb +++ b/spec/workers/create_gpg_signature_worker_spec.rb @@ -7,9 +7,14 @@ describe CreateGpgSignatureWorker do let(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' } it 'calls Gitlab::Gpg::Commit#signature' do - expect(Gitlab::Gpg::Commit).to receive(:new).with(project, commit_sha).and_call_original + commit = instance_double(Commit) + gpg_commit = instance_double(Gitlab::Gpg::Commit) - expect_any_instance_of(Gitlab::Gpg::Commit).to receive(:signature) + allow(Project).to receive(:find_by).with(id: project.id).and_return(project) + allow(project).to receive(:commit).with(commit_sha).and_return(commit) + + expect(Gitlab::Gpg::Commit).to receive(:new).with(commit).and_return(gpg_commit) + expect(gpg_commit).to receive(:signature) described_class.new.perform(commit_sha, project.id) end |