diff options
169 files changed, 1768 insertions, 713 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index b4fa22ad70e..a1c928aedf3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,31 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 11.6.1 (2018-12-28) + +### Security (15 changes) + +- Escape label and milestone titles to prevent XSS in GFM autocomplete. !2740 +- Prevent private snippets from being embeddable. +- Add subresources removal to member destroy service. +- Escape html entities in LabelReferenceFilter when no label found. +- Allow changing group CI/CD settings only for owners. +- Authorize before reading job information via API. +- Prevent leaking protected variables for ambiguous refs. +- Ensure that build token is only used when running. +- Issuable no longer is visible to users when project can't be viewed. +- Don't expose cross project repositories through diffs when creating merge reqeusts. +- Fix SSRF with import_url and remote mirror url. +- Fix persistent symlink in project import. +- Set URL rel attribute for broken URLs. +- Project guests no longer are able to see refs page. +- Delete confidential todos for user when downgraded to Guest. + +### Other (1 change) + +- Fix due date test. !23845 + + ## 11.6.0 (2018-12-22) ### Security (24 changes, 1 of them is from the community) @@ -1,6 +1,6 @@ source 'https://rubygems.org' -gem 'rails', '5.0.7' +gem 'rails', '5.0.7.1' gem 'rails-deprecated_sanitizer', '~> 1.0.3' # Improves copy-on-write performance for MRI @@ -34,7 +34,7 @@ gem 'omniauth-cas3', '~> 1.1.4' gem 'omniauth-facebook', '~> 4.0.0' gem 'omniauth-github', '~> 1.3' gem 'omniauth-gitlab', '~> 1.0.2' -gem 'omniauth-google-oauth2', '~> 0.5.3' +gem 'omniauth-google-oauth2', '~> 0.6.0' gem 'omniauth-kerberos', '~> 0.3.0', group: :kerberos gem 'omniauth-oauth2-generic', '~> 0.2.2' gem 'omniauth-saml', '~> 1.10' @@ -43,7 +43,7 @@ gem 'omniauth-twitter', '~> 1.4' gem 'omniauth_crowd', '~> 2.2.0' gem 'omniauth-authentiq', '~> 0.3.3' gem 'rack-oauth2', '~> 1.2.1' -gem 'jwt', '~> 1.5.6' +gem 'jwt', '~> 2.1.0' # Spam and anti-bot protection gem 'recaptcha', '~> 3.0', require: 'recaptcha/rails' @@ -328,7 +328,7 @@ group :development, :test do gem 'awesome_print', require: false gem 'fuubar', '~> 2.2.0' - gem 'database_cleaner', '~> 1.5.0' + gem 'database_cleaner', '~> 1.7.0' gem 'factory_bot_rails', '~> 4.8.2' gem 'rspec-rails', '~> 3.7.0' gem 'rspec-retry', '~> 0.4.5' diff --git a/Gemfile.lock b/Gemfile.lock index 942dbac27a6..1f7f58d59e0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -4,41 +4,41 @@ GEM RedCloth (4.3.2) abstract_type (0.0.7) ace-rails-ap (4.1.2) - actioncable (5.0.7) - actionpack (= 5.0.7) + actioncable (5.0.7.1) + actionpack (= 5.0.7.1) nio4r (>= 1.2, < 3.0) websocket-driver (~> 0.6.1) - actionmailer (5.0.7) - actionpack (= 5.0.7) - actionview (= 5.0.7) - activejob (= 5.0.7) + actionmailer (5.0.7.1) + actionpack (= 5.0.7.1) + actionview (= 5.0.7.1) + activejob (= 5.0.7.1) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (5.0.7) - actionview (= 5.0.7) - activesupport (= 5.0.7) + actionpack (5.0.7.1) + actionview (= 5.0.7.1) + activesupport (= 5.0.7.1) rack (~> 2.0) rack-test (~> 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.0.2) - actionview (5.0.7) - activesupport (= 5.0.7) + actionview (5.0.7.1) + activesupport (= 5.0.7.1) builder (~> 3.1) erubis (~> 2.7.0) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.0.3) - activejob (5.0.7) - activesupport (= 5.0.7) + activejob (5.0.7.1) + activesupport (= 5.0.7.1) globalid (>= 0.3.6) - activemodel (5.0.7) - activesupport (= 5.0.7) - activerecord (5.0.7) - activemodel (= 5.0.7) - activesupport (= 5.0.7) + activemodel (5.0.7.1) + activesupport (= 5.0.7.1) + activerecord (5.0.7.1) + activemodel (= 5.0.7.1) + activesupport (= 5.0.7.1) arel (~> 7.0) activerecord_sane_schema_dumper (1.0) rails (>= 5, < 6) - activesupport (5.0.7) + activesupport (5.0.7.1) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 0.7, < 2) minitest (~> 5.1) @@ -140,7 +140,7 @@ GEM css_parser (1.5.0) addressable daemons (1.2.6) - database_cleaner (1.5.3) + database_cleaner (1.7.0) debug_inspector (0.0.3) debugger-ruby_core_source (1.3.8) deckar01-task_list (2.0.0) @@ -381,7 +381,7 @@ GEM json (~> 1.8) multi_xml (>= 0.5.2) httpclient (2.8.3) - i18n (1.1.1) + i18n (1.2.0) concurrent-ruby (~> 1.0) icalendar (2.4.1) ice_nine (0.11.2) @@ -403,7 +403,7 @@ GEM bindata json-schema (2.8.0) addressable (>= 2.4) - jwt (1.5.6) + jwt (2.1.0) kaminari (1.0.1) activesupport (>= 4.1.0) kaminari-actionview (= 1.0.1) @@ -449,7 +449,7 @@ GEM loofah (2.2.3) crass (~> 1.0.2) nokogiri (>= 1.5.9) - mail (2.7.0) + mail (2.7.1) mini_mime (>= 0.1.1) mail_room (0.9.1) memoist (0.16.0) @@ -483,24 +483,24 @@ GEM nokogiri numerizer (0.1.1) oauth (0.5.4) - oauth2 (1.4.0) - faraday (>= 0.8, < 0.13) - jwt (~> 1.0) + oauth2 (1.4.1) + faraday (>= 0.8, < 0.16.0) + jwt (>= 1.0, < 3.0) multi_json (~> 1.3) multi_xml (~> 0.5) rack (>= 1.2, < 3) octokit (4.9.0) sawyer (~> 0.8.0, >= 0.5.3) - omniauth (1.8.1) - hashie (>= 3.4.6, < 3.6.0) + omniauth (1.9.0) + hashie (>= 3.4.6, < 3.7.0) rack (>= 1.6.2, < 3) omniauth-auth0 (2.0.0) omniauth-oauth2 (~> 1.4) omniauth-authentiq (0.3.3) jwt (>= 1.5) omniauth-oauth2 (>= 1.5) - omniauth-azure-oauth2 (0.0.9) - jwt (~> 1.0) + omniauth-azure-oauth2 (0.0.10) + jwt (>= 1.0, < 3.0) omniauth (~> 1.0) omniauth-oauth2 (~> 1.4) omniauth-cas3 (1.1.4) @@ -515,8 +515,8 @@ GEM omniauth-gitlab (1.0.3) omniauth (~> 1.0) omniauth-oauth2 (~> 1.0) - omniauth-google-oauth2 (0.5.3) - jwt (>= 1.5) + omniauth-google-oauth2 (0.6.0) + jwt (>= 2.0) omniauth (>= 1.1.1) omniauth-oauth2 (>= 1.5) omniauth-kerberos (0.3.0) @@ -527,9 +527,9 @@ GEM omniauth-oauth (1.1.0) oauth omniauth (~> 1.0) - omniauth-oauth2 (1.5.0) + omniauth-oauth2 (1.6.0) oauth2 (~> 1.1) - omniauth (~> 1.2) + omniauth (~> 1.9) omniauth-oauth2-generic (0.2.2) omniauth-oauth2 (~> 1.0) omniauth-saml (1.10.0) @@ -624,17 +624,17 @@ GEM rack rack-test (0.6.3) rack (>= 1.0) - rails (5.0.7) - actioncable (= 5.0.7) - actionmailer (= 5.0.7) - actionpack (= 5.0.7) - actionview (= 5.0.7) - activejob (= 5.0.7) - activemodel (= 5.0.7) - activerecord (= 5.0.7) - activesupport (= 5.0.7) + rails (5.0.7.1) + actioncable (= 5.0.7.1) + actionmailer (= 5.0.7.1) + actionpack (= 5.0.7.1) + actionview (= 5.0.7.1) + activejob (= 5.0.7.1) + activemodel (= 5.0.7.1) + activerecord (= 5.0.7.1) + activesupport (= 5.0.7.1) bundler (>= 1.3.0) - railties (= 5.0.7) + railties (= 5.0.7.1) sprockets-rails (>= 2.0.0) rails-controller-testing (1.0.2) actionpack (~> 5.x, >= 5.0.1) @@ -650,15 +650,15 @@ GEM rails-i18n (5.1.1) i18n (>= 0.7, < 2) railties (>= 5.0, < 6) - railties (5.0.7) - actionpack (= 5.0.7) - activesupport (= 5.0.7) + railties (5.0.7.1) + actionpack (= 5.0.7.1) + activesupport (= 5.0.7.1) method_source rake (>= 0.8.7) thor (>= 0.18.1, < 2.0) rainbow (3.0.0) raindrops (0.18.0) - rake (12.3.1) + rake (12.3.2) rb-fsevent (0.10.2) rb-inotify (0.9.10) ffi (>= 0.5.0, < 2) @@ -973,7 +973,7 @@ DEPENDENCIES concurrent-ruby (~> 1.1) connection_pool (~> 2.0) creole (~> 0.5.0) - database_cleaner (~> 1.5.0) + database_cleaner (~> 1.7.0) deckar01-task_list (= 2.0.0) device_detector devise (~> 4.4) @@ -1041,7 +1041,7 @@ DEPENDENCIES jquery-atwho-rails (~> 1.3.2) js_regex (~> 2.2.1) json-schema (~> 2.8.0) - jwt (~> 1.5.6) + jwt (~> 2.1.0) kaminari (~> 1.0) knapsack (~> 1.17) kubeclient (~> 4.0.0) @@ -1070,7 +1070,7 @@ DEPENDENCIES omniauth-facebook (~> 4.0.0) omniauth-github (~> 1.3) omniauth-gitlab (~> 1.0.2) - omniauth-google-oauth2 (~> 0.5.3) + omniauth-google-oauth2 (~> 0.6.0) omniauth-kerberos (~> 0.3.0) omniauth-oauth2-generic (~> 0.2.2) omniauth-saml (~> 1.10) @@ -1096,7 +1096,7 @@ DEPENDENCIES rack-cors (~> 1.0.0) rack-oauth2 (~> 1.2.1) rack-proxy (~> 0.6.0) - rails (= 5.0.7) + rails (= 5.0.7.1) rails-controller-testing rails-deprecated_sanitizer (~> 1.0.3) rails-i18n (~> 5.1) diff --git a/app/assets/javascripts/diffs/components/compare_versions_dropdown.vue b/app/assets/javascripts/diffs/components/compare_versions_dropdown.vue index 8da02ed0b7c..b9b1ee02697 100644 --- a/app/assets/javascripts/diffs/components/compare_versions_dropdown.vue +++ b/app/assets/javascripts/diffs/components/compare_versions_dropdown.vue @@ -129,7 +129,7 @@ export default { </strong> </div> <div> - <small class="commit-sha"> {{ version.truncated_commit_sha }} </small> + <small class="commit-sha"> {{ version.short_commit_sha }} </small> </div> <div> <small> diff --git a/app/assets/javascripts/gfm_auto_complete.js b/app/assets/javascripts/gfm_auto_complete.js index c14eb936930..8178821be3d 100644 --- a/app/assets/javascripts/gfm_auto_complete.js +++ b/app/assets/javascripts/gfm_auto_complete.js @@ -256,7 +256,7 @@ class GfmAutoComplete { displayTpl(value) { let tmpl = GfmAutoComplete.Loading.template; if (value.title != null) { - tmpl = GfmAutoComplete.Milestones.template; + tmpl = GfmAutoComplete.Milestones.templateFunction(value.title); } return tmpl; }, @@ -323,7 +323,7 @@ class GfmAutoComplete { searchKey: 'search', data: GfmAutoComplete.defaultLoadingData, displayTpl(value) { - let tmpl = GfmAutoComplete.Labels.template; + let tmpl = GfmAutoComplete.Labels.templateFunction(value.color, value.title); if (GfmAutoComplete.isLoading(value)) { tmpl = GfmAutoComplete.Loading.template; } @@ -588,9 +588,11 @@ GfmAutoComplete.Members = { }, }; GfmAutoComplete.Labels = { - template: - // eslint-disable-next-line no-template-curly-in-string - '<li><span class="dropdown-label-box" style="background: ${color}"></span> ${title}</li>', + templateFunction(color, title) { + return `<li><span class="dropdown-label-box" style="background: ${_.escape( + color, + )}"></span> ${_.escape(title)}</li>`; + }, }; // Issues, MergeRequests and Snippets GfmAutoComplete.Issues = { @@ -600,8 +602,9 @@ GfmAutoComplete.Issues = { }; // Milestones GfmAutoComplete.Milestones = { - // eslint-disable-next-line no-template-curly-in-string - template: '<li>${title}</li>', + templateFunction(title) { + return `<li>${_.escape(title)}</li>`; + }, }; GfmAutoComplete.Loading = { template: diff --git a/app/assets/javascripts/notes/components/discussion_filter.vue b/app/assets/javascripts/notes/components/discussion_filter.vue index 86c114a761a..f5c410211b6 100644 --- a/app/assets/javascripts/notes/components/discussion_filter.vue +++ b/app/assets/javascripts/notes/components/discussion_filter.vue @@ -2,7 +2,11 @@ import $ from 'jquery'; import { mapGetters, mapActions } from 'vuex'; import Icon from '~/vue_shared/components/icon.vue'; -import { DISCUSSION_FILTERS_DEFAULT_VALUE, HISTORY_ONLY_FILTER_VALUE } from '../constants'; +import { + DISCUSSION_FILTERS_DEFAULT_VALUE, + HISTORY_ONLY_FILTER_VALUE, + DISCUSSION_TAB_LABEL, +} from '../constants'; export default { components: { @@ -23,6 +27,7 @@ export default { return { currentValue: this.selectedValue, defaultValue: DISCUSSION_FILTERS_DEFAULT_VALUE, + displayFilters: true, }; }, computed: { @@ -32,6 +37,14 @@ export default { return this.filters.find(filter => filter.value === this.currentValue); }, }, + created() { + if (window.mrTabs) { + const { eventHub, currentTab } = window.mrTabs; + + eventHub.$on('MergeRequestTabChange', this.toggleFilters); + this.toggleFilters(currentTab); + } + }, mounted() { this.toggleCommentsForm(); }, @@ -51,12 +64,15 @@ export default { toggleCommentsForm() { this.setCommentsDisabled(this.currentValue === HISTORY_ONLY_FILTER_VALUE); }, + toggleFilters(tab) { + this.displayFilters = tab === DISCUSSION_TAB_LABEL; + }, }, }; </script> <template> - <div class="discussion-filter-container d-inline-block align-bottom"> + <div v-if="displayFilters" class="discussion-filter-container d-inline-block align-bottom"> <button id="discussion-filter-dropdown" ref="dropdownToggle" diff --git a/app/assets/javascripts/notes/constants.js b/app/assets/javascripts/notes/constants.js index 3147dc64c27..78d365fe94b 100644 --- a/app/assets/javascripts/notes/constants.js +++ b/app/assets/javascripts/notes/constants.js @@ -17,6 +17,7 @@ export const RESOLVE_NOTE_METHOD_NAME = 'post'; export const DESCRIPTION_TYPE = 'changed the description'; export const HISTORY_ONLY_FILTER_VALUE = 2; export const DISCUSSION_FILTERS_DEFAULT_VALUE = 0; +export const DISCUSSION_TAB_LABEL = 'show'; export const NOTEABLE_TYPE_MAPPING = { Issue: ISSUE_NOTEABLE_TYPE, diff --git a/app/assets/javascripts/pages/users/user_overview_block.js b/app/assets/javascripts/pages/users/user_overview_block.js index eec2b5ca8e5..e9ecec717d6 100644 --- a/app/assets/javascripts/pages/users/user_overview_block.js +++ b/app/assets/javascripts/pages/users/user_overview_block.js @@ -29,18 +29,21 @@ export default class UserOverviewBlock { render(data) { const { html, count } = data; - const contentList = document.querySelector(`${this.container} .overview-content-list`); + const containerEl = document.querySelector(this.container); + const contentList = containerEl.querySelector('.overview-content-list'); contentList.innerHTML += html; - const loadingEl = document.querySelector(`${this.container} .loading`); + const loadingEl = containerEl.querySelector('.loading'); if (count && count > 0) { - document.querySelector(`${this.container} .js-view-all`).classList.remove('hide'); + containerEl.querySelector('.js-view-all').classList.remove('hide'); } else { - document - .querySelector(`${this.container} .nothing-here-block`) - .classList.add('text-left', 'p-0'); + const nothingHereBlock = containerEl.querySelector('.nothing-here-block'); + + if (nothingHereBlock) { + nothingHereBlock.classList.add('text-left', 'p-0'); + } } loadingEl.classList.add('hide'); diff --git a/app/controllers/groups/settings/ci_cd_controller.rb b/app/controllers/groups/settings/ci_cd_controller.rb index c1dcc463de7..f476f428fdb 100644 --- a/app/controllers/groups/settings/ci_cd_controller.rb +++ b/app/controllers/groups/settings/ci_cd_controller.rb @@ -4,7 +4,7 @@ module Groups module Settings class CiCdController < Groups::ApplicationController skip_cross_project_access_check :show - before_action :authorize_admin_pipeline! + before_action :authorize_admin_group! def show define_ci_variables @@ -26,8 +26,8 @@ module Groups .map { |variable| variable.present(current_user: current_user) } end - def authorize_admin_pipeline! - return render_404 unless can?(current_user, :admin_pipeline, group) + def authorize_admin_group! + return render_404 unless can?(current_user, :admin_group, group) end end end diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb index a44acb12bdf..255f1f3569a 100644 --- a/app/controllers/projects/snippets_controller.rb +++ b/app/controllers/projects/snippets_controller.rb @@ -75,7 +75,14 @@ class Projects::SnippetsController < Projects::ApplicationController format.json do render_blob_json(blob) end - format.js { render 'shared/snippets/show'} + + format.js do + if @snippet.embeddable? + render 'shared/snippets/show' + else + head :not_found + end + end end end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 8bf93bfd68d..878816475b2 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -19,6 +19,7 @@ class ProjectsController < Projects::ApplicationController before_action :lfs_blob_ids, only: [:show], if: [:repo_exists?, :project_view_files?] before_action :project_export_enabled, only: [:export, :download_export, :remove_export, :generate_new_export] before_action :present_project, only: [:edit] + before_action :authorize_download_code!, only: [:refs] # Authorize before_action :authorize_admin_project!, only: [:edit, :update, :housekeeping, :download_export, :export, :remove_export, :generate_new_export] diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb index dd9bf17cf0c..8ea5450b4e8 100644 --- a/app/controllers/snippets_controller.rb +++ b/app/controllers/snippets_controller.rb @@ -80,7 +80,13 @@ class SnippetsController < ApplicationController render_blob_json(blob) end - format.js { render 'shared/snippets/show' } + format.js do + if @snippet.embeddable? + render 'shared/snippets/show' + else + head :not_found + end + end end end diff --git a/app/helpers/snippets_helper.rb b/app/helpers/snippets_helper.rb index 8fded0efe4d..ecb2b2d707b 100644 --- a/app/helpers/snippets_helper.rb +++ b/app/helpers/snippets_helper.rb @@ -130,12 +130,4 @@ module SnippetsHelper link_to external_snippet_icon('download'), download_url, class: 'btn', target: '_blank', title: 'Download', rel: 'noopener noreferrer' end - - def public_snippet? - if @snippet.project_id? - can?(nil, :read_project_snippet, @snippet) - else - can?(nil, :read_personal_snippet, @snippet) - end - end end diff --git a/app/helpers/sorting_helper.rb b/app/helpers/sorting_helper.rb index 6ac1f42c321..02762897c89 100644 --- a/app/helpers/sorting_helper.rb +++ b/app/helpers/sorting_helper.rb @@ -234,7 +234,7 @@ module SortingHelper end def sort_title_milestone - s_('SortOptions|Milestone') + s_('SortOptions|Milestone due date') end def sort_title_milestone_later diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 16a72c680fa..b128a254b96 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -10,6 +10,7 @@ module Ci include Importable include Gitlab::Utils::StrongMemoize include Deployable + include HasRef belongs_to :project, inverse_of: :builds belongs_to :runner @@ -640,11 +641,11 @@ module Ci def secret_group_variables return [] unless project.group - project.group.ci_variables_for(ref, project) + project.group.ci_variables_for(git_ref, project) end def secret_project_variables(environment: persisted_environment) - project.ci_variables_for(ref: ref, environment: environment) + project.ci_variables_for(ref: git_ref, environment: environment) end def steps diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 25937065011..1f5017cc3c3 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -11,6 +11,7 @@ module Ci include Gitlab::Utils::StrongMemoize include AtomicInternalId include EnumWithNil + include HasRef belongs_to :project, inverse_of: :all_pipelines belongs_to :user @@ -380,7 +381,7 @@ module Ci end def branch? - !tag? && !merge_request? + super && !merge_request? end def stuck? @@ -580,7 +581,7 @@ module Ci end def protected_ref? - strong_memoize(:protected_ref) { project.protected_for?(ref) } + strong_memoize(:protected_ref) { project.protected_for?(git_ref) } end def legacy_trigger @@ -712,14 +713,10 @@ module Ci end def git_ref - if branch? + if merge_request? Gitlab::Git::BRANCH_REF_PREFIX + ref.to_s - elsif merge_request? - Gitlab::Git::BRANCH_REF_PREFIX + ref.to_s - elsif tag? - Gitlab::Git::TAG_REF_PREFIX + ref.to_s else - raise ArgumentError, 'Invalid pipeline type!' + super end end diff --git a/app/models/concerns/has_ref.rb b/app/models/concerns/has_ref.rb new file mode 100644 index 00000000000..d7089294efc --- /dev/null +++ b/app/models/concerns/has_ref.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module HasRef + extend ActiveSupport::Concern + + def branch? + !tag? + end + + def git_ref + if branch? + Gitlab::Git::BRANCH_REF_PREFIX + ref.to_s + elsif tag? + Gitlab::Git::TAG_REF_PREFIX + ref.to_s + end + end +end diff --git a/app/models/project.rb b/app/models/project.rb index 09e2a6114fe..cd558752080 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -324,10 +324,9 @@ class Project < ActiveRecord::Base validates :namespace, presence: true validates :name, uniqueness: { scope: :namespace_id } - validates :import_url, url: { protocols: ->(project) { project.persisted? ? VALID_MIRROR_PROTOCOLS : VALID_IMPORT_PROTOCOLS }, - ports: ->(project) { project.persisted? ? VALID_MIRROR_PORTS : VALID_IMPORT_PORTS }, - allow_localhost: false, - enforce_user: true }, if: [:external_import?, :import_url_changed?] + validates :import_url, public_url: { protocols: ->(project) { project.persisted? ? VALID_MIRROR_PROTOCOLS : VALID_IMPORT_PROTOCOLS }, + ports: ->(project) { project.persisted? ? VALID_MIRROR_PORTS : VALID_IMPORT_PORTS }, + enforce_user: true }, if: [:external_import?, :import_url_changed?] validates :star_count, numericality: { greater_than_or_equal_to: 0 } validate :check_limit, on: :create validate :check_repository_path_availability, on: :update, if: ->(project) { project.renamed? } @@ -1734,10 +1733,21 @@ class Project < ActiveRecord::Base end def protected_for?(ref) - if repository.branch_exists?(ref) - ProtectedBranch.protected?(self, ref) - elsif repository.tag_exists?(ref) - ProtectedTag.protected?(self, ref) + raise Repository::AmbiguousRefError if repository.ambiguous_ref?(ref) + + resolved_ref = repository.expand_ref(ref) || ref + return false unless Gitlab::Git.tag_ref?(resolved_ref) || Gitlab::Git.branch_ref?(resolved_ref) + + ref_name = if resolved_ref == ref + Gitlab::Git.ref_name(resolved_ref) + else + ref + end + + if Gitlab::Git.branch_ref?(resolved_ref) + ProtectedBranch.protected?(self, ref_name) + elsif Gitlab::Git.tag_ref?(resolved_ref) + ProtectedTag.protected?(self, ref_name) end end diff --git a/app/models/remote_mirror.rb b/app/models/remote_mirror.rb index 5a6895aefab..a3fa67c72bf 100644 --- a/app/models/remote_mirror.rb +++ b/app/models/remote_mirror.rb @@ -17,7 +17,7 @@ class RemoteMirror < ActiveRecord::Base belongs_to :project, inverse_of: :remote_mirrors - validates :url, presence: true, url: { protocols: %w(ssh git http https), allow_blank: true, enforce_user: true } + validates :url, presence: true, public_url: { protocols: %w(ssh git http https), allow_blank: true, enforce_user: true } before_save :set_new_remote_name, if: :mirror_url_changed? diff --git a/app/models/repository.rb b/app/models/repository.rb index 015a179f374..b19ae2e0e6a 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -25,6 +25,7 @@ class Repository delegate :bundle_to_disk, to: :raw_repository CreateTreeError = Class.new(StandardError) + AmbiguousRefError = Class.new(StandardError) # Methods that cache data from the Git repository. # @@ -181,6 +182,18 @@ class Repository tags.find { |tag| tag.name == name } end + def ambiguous_ref?(ref) + tag_exists?(ref) && branch_exists?(ref) + end + + def expand_ref(ref) + if tag_exists?(ref) + Gitlab::Git::TAG_REF_PREFIX + ref + elsif branch_exists?(ref) + Gitlab::Git::BRANCH_REF_PREFIX + ref + end + end + def add_branch(user, branch_name, ref) branch = raw_repository.add_branch(branch_name, user: user, target: ref) diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 11856b55902..f9b23bbbf6c 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -175,6 +175,12 @@ class Snippet < ActiveRecord::Base :visibility_level end + def embeddable? + ability = project_id? ? :read_project_snippet : :read_personal_snippet + + Ability.allowed?(nil, ability, self) + end + def notes_with_associations notes.includes(:author) end diff --git a/app/models/todo.rb b/app/models/todo.rb index 7b64615f699..d9b86d941b6 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -4,6 +4,11 @@ class Todo < ActiveRecord::Base include Sortable include FromUnion + # Time to wait for todos being removed when not visible for user anymore. + # Prevents TODOs being removed by mistake, for example, removing access from a user + # and giving it back again. + WAIT_FOR_DELETE = 1.hour + ASSIGNED = 1 MENTIONED = 2 BUILD_FAILED = 3 diff --git a/app/policies/issuable_policy.rb b/app/policies/issuable_policy.rb index 6d8b575102e..ecb2797d1d9 100644 --- a/app/policies/issuable_policy.rb +++ b/app/policies/issuable_policy.rb @@ -11,7 +11,7 @@ class IssuablePolicy < BasePolicy @user && @subject.assignee_or_author?(@user) end - rule { assignee_or_author }.policy do + rule { can?(:guest_access) & assignee_or_author }.policy do enable :read_issue enable :update_issue enable :reopen_issue diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index 31d3c844ad5..de78a3f7b27 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -31,7 +31,7 @@ module Groups def after_update if group.previous_changes.include?(:visibility_level) && group.private? # don't enqueue immediately to prevent todos removal in case of a mistake - TodosDestroyer::GroupPrivateWorker.perform_in(1.hour, group.id) + TodosDestroyer::GroupPrivateWorker.perform_in(Todo::WAIT_FOR_DELETE, group.id) end end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index a1d0cc0e568..e992d682c79 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -44,7 +44,7 @@ module Issues if issue.previous_changes.include?('confidential') # don't enqueue immediately to prevent todos removal in case of a mistake - TodosDestroyer::ConfidentialIssueWorker.perform_in(1.hour, issue.id) if issue.confidential? + TodosDestroyer::ConfidentialIssueWorker.perform_in(Todo::WAIT_FOR_DELETE, issue.id) if issue.confidential? create_confidentiality_note(issue) end diff --git a/app/services/members/base_service.rb b/app/services/members/base_service.rb index d734571f835..e78affff797 100644 --- a/app/services/members/base_service.rb +++ b/app/services/members/base_service.rb @@ -47,5 +47,11 @@ module Members raise "Unknown action '#{action}' on #{member}!" end end + + def enqueue_delete_todos(member) + type = member.is_a?(GroupMember) ? 'Group' : 'Project' + # don't enqueue immediately to prevent todos removal in case of a mistake + TodosDestroyer::EntityLeaveWorker.perform_in(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, type) + end end end diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index c186a5971dc..ae0c644e6c0 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -15,7 +15,7 @@ module Members notification_service.decline_access_request(member) end - enqeue_delete_todos(member) + enqueue_delete_todos(member) after_execute(member: member) @@ -24,12 +24,6 @@ module Members private - def enqeue_delete_todos(member) - type = member.is_a?(GroupMember) ? 'Group' : 'Project' - # don't enqueue immediately to prevent todos removal in case of a mistake - TodosDestroyer::EntityLeaveWorker.perform_in(1.hour, member.user_id, member.source_id, type) - end - def can_destroy_member?(member) can?(current_user, destroy_member_permission(member), member) end diff --git a/app/services/members/update_service.rb b/app/services/members/update_service.rb index 1f5618dae53..ff8d5c1d8c9 100644 --- a/app/services/members/update_service.rb +++ b/app/services/members/update_service.rb @@ -10,9 +10,18 @@ module Members if member.update(params) after_execute(action: permission, old_access_level: old_access_level, member: member) + + # Deletes only confidential issues todos for guests + enqueue_delete_todos(member) if downgrading_to_guest? end member end + + private + + def downgrading_to_guest? + params[:access_level] == Gitlab::Access::GUEST + end end end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 36767621d74..48419da98ad 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -18,7 +18,7 @@ module MergeRequests merge_request.source_project = find_source_project merge_request.target_project = find_target_project merge_request.target_branch = find_target_branch - merge_request.can_be_created = branches_valid? + merge_request.can_be_created = projects_and_branches_valid? # compare branches only if branches are valid, otherwise # compare_branches may raise an error @@ -49,15 +49,19 @@ module MergeRequests to: :merge_request def find_source_project - return source_project if source_project.present? && can?(current_user, :read_project, source_project) + return source_project if source_project.present? && can?(current_user, :create_merge_request_from, source_project) project end def find_target_project - return target_project if target_project.present? && can?(current_user, :read_project, target_project) + return target_project if target_project.present? && can?(current_user, :create_merge_request_in, target_project) - project.default_merge_request_target + target_project = project.default_merge_request_target + + return target_project if target_project.present? && can?(current_user, :create_merge_request_in, target_project) + + project end def find_target_branch @@ -72,10 +76,11 @@ module MergeRequests params[:target_branch].present? end - def branches_valid? + def projects_and_branches_valid? + return false if source_project.nil? || target_project.nil? return false unless source_branch_specified? || target_branch_specified? - validate_branches + validate_projects_and_branches errors.blank? end @@ -94,7 +99,12 @@ module MergeRequests end end - def validate_branches + def validate_projects_and_branches + merge_request.validate_target_project + merge_request.validate_fork + + return if errors.any? + add_error('You must select source and target branch') unless branches_present? add_error('You must select different branches') if same_source_and_target? add_error("Source branch \"#{source_branch}\" does not exist") unless source_branch_exists? diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 33d8299c8b6..86a04587f79 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -46,11 +46,13 @@ module MergeRequests end if merge_request.previous_changes.include?('assignee_id') + reassigned_merge_request_args = [merge_request, current_user] + old_assignee_id = merge_request.previous_changes['assignee_id'].first - old_assignee = User.find(old_assignee_id) if old_assignee_id + reassigned_merge_request_args << User.find(old_assignee_id) if old_assignee_id create_assignee_note(merge_request) - notification_service.async.reassigned_merge_request(merge_request, current_user, old_assignee) + notification_service.async.reassigned_merge_request(*reassigned_merge_request_args) todo_service.reassigned_merge_request(merge_request, current_user) end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index ff035fea216..e1cf327209b 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -188,7 +188,7 @@ class NotificationService # * merge_request assignee if their notification level is not Disabled # * users with custom level checked with "reassign merge request" # - def reassigned_merge_request(merge_request, current_user, previous_assignee) + def reassigned_merge_request(merge_request, current_user, previous_assignee = nil) recipients = NotificationRecipientService.build_recipients( merge_request, current_user, diff --git a/app/services/projects/lfs_pointers/lfs_download_service.rb b/app/services/projects/lfs_pointers/lfs_download_service.rb index f9b9781ad5f..b5128443435 100644 --- a/app/services/projects/lfs_pointers/lfs_download_service.rb +++ b/app/services/projects/lfs_pointers/lfs_download_service.rb @@ -12,28 +12,43 @@ module Projects return if LfsObject.exists?(oid: oid) - sanitized_uri = Gitlab::UrlSanitizer.new(url) - Gitlab::UrlBlocker.validate!(sanitized_uri.sanitized_url, protocols: VALID_PROTOCOLS) + sanitized_uri = sanitize_url!(url) with_tmp_file(oid) do |file| - size = download_and_save_file(file, sanitized_uri) - lfs_object = LfsObject.new(oid: oid, size: size, file: file) + download_and_save_file(file, sanitized_uri) + lfs_object = LfsObject.new(oid: oid, size: file.size, file: file) project.all_lfs_objects << lfs_object end + rescue Gitlab::UrlBlocker::BlockedUrlError => e + Rails.logger.error("LFS file with oid #{oid} couldn't be downloaded: #{e.message}") rescue StandardError => e - Rails.logger.error("LFS file with oid #{oid} could't be downloaded from #{sanitized_uri.sanitized_url}: #{e.message}") + Rails.logger.error("LFS file with oid #{oid} couldn't be downloaded from #{sanitized_uri.sanitized_url}: #{e.message}") end # rubocop: enable CodeReuse/ActiveRecord private + def sanitize_url!(url) + Gitlab::UrlSanitizer.new(url).tap do |sanitized_uri| + # Just validate that HTTP/HTTPS protocols are used. The + # subsequent Gitlab::HTTP.get call will do network checks + # based on the settings. + Gitlab::UrlBlocker.validate!(sanitized_uri.sanitized_url, + protocols: VALID_PROTOCOLS) + end + end + def download_and_save_file(file, sanitized_uri) - IO.copy_stream(open(sanitized_uri.sanitized_url, headers(sanitized_uri)), file) # rubocop:disable Security/Open + response = Gitlab::HTTP.get(sanitized_uri.sanitized_url, headers(sanitized_uri)) do |fragment| + file.write(fragment) + end + + raise StandardError, "Received error code #{response.code}" unless response.success? end def headers(sanitized_uri) - {}.tap do |headers| + query_options.tap do |headers| credentials = sanitized_uri.credentials if credentials[:user].present? || credentials[:password].present? @@ -43,10 +58,14 @@ module Projects end end + def query_options + { stream_body: true } + end + def with_tmp_file(oid) create_tmp_storage_dir - File.open(File.join(tmp_storage_dir, oid), 'w') { |file| yield file } + File.open(File.join(tmp_storage_dir, oid), 'wb') { |file| yield file } end def create_tmp_storage_dir diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 93e48fc0199..dd1b9680ece 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -61,9 +61,9 @@ module Projects if project.previous_changes.include?(:visibility_level) && project.private? # don't enqueue immediately to prevent todos removal in case of a mistake - TodosDestroyer::ProjectPrivateWorker.perform_in(1.hour, project.id) + TodosDestroyer::ProjectPrivateWorker.perform_in(Todo::WAIT_FOR_DELETE, project.id) elsif (project_changed_feature_keys & todos_features_changes).present? - TodosDestroyer::PrivateFeaturesWorker.perform_in(1.hour, project.id) + TodosDestroyer::PrivateFeaturesWorker.perform_in(Todo::WAIT_FOR_DELETE, project.id) end if project.previous_changes.include?('path') diff --git a/app/views/events/_events.html.haml b/app/views/events/_events.html.haml index 68c19df092d..6ae4c334f7f 100644 --- a/app/views/events/_events.html.haml +++ b/app/views/events/_events.html.haml @@ -1 +1,4 @@ -= render partial: 'events/event', collection: @events +- if @events.present? + = render partial: 'events/event', collection: @events +- else + .nothing-here-block= _("No activities found") diff --git a/app/views/shared/snippets/_header.html.haml b/app/views/shared/snippets/_header.html.haml index 10bfc30492a..a43296aa806 100644 --- a/app/views/shared/snippets/_header.html.haml +++ b/app/views/shared/snippets/_header.html.haml @@ -30,7 +30,7 @@ - if @snippet.updated_at != @snippet.created_at = edited_time_ago_with_tooltip(@snippet, placement: 'bottom', html_class: 'snippet-edited-ago', exclude_author: true) - - if public_snippet? + - if @snippet.embeddable? .embed-snippet .input-group .input-group-prepend diff --git a/app/workers/mail_scheduler/notification_service_worker.rb b/app/workers/mail_scheduler/notification_service_worker.rb index 4726e416182..c8ccaf0c487 100644 --- a/app/workers/mail_scheduler/notification_service_worker.rb +++ b/app/workers/mail_scheduler/notification_service_worker.rb @@ -8,14 +8,35 @@ module MailScheduler include MailSchedulerQueue def perform(meth, *args) - deserialized_args = ActiveJob::Arguments.deserialize(args) + check_arguments!(args) + deserialized_args = ActiveJob::Arguments.deserialize(args) notification_service.public_send(meth, *deserialized_args) # rubocop:disable GitlabSecurity/PublicSend rescue ActiveJob::DeserializationError + # No-op. + # This exception gets raised when an argument + # is correct (deserializeable), but it still cannot be deserialized. + # This can happen when an object has been deleted after + # rails passes this job to sidekiq, but before + # sidekiq gets it for execution. + # In this case just do nothing. end def self.perform_async(*args) super(*ActiveJob::Arguments.serialize(args)) end + + private + + # If an argument is in the ActiveJob::Arguments::TYPE_WHITELIST list, + # it means the argument cannot be deserialized. + # Which means there's something wrong with our code. + def check_arguments!(args) + args.each do |arg| + if arg.class.in?(ActiveJob::Arguments::TYPE_WHITELIST) + raise(ArgumentError, "Argument `#{arg}` cannot be deserialized because of its type") + end + end + end end end diff --git a/changelogs/unreleased/54206-show-the-activity-filter-dropdown-in-discussion-tab-only.yml b/changelogs/unreleased/54206-show-the-activity-filter-dropdown-in-discussion-tab-only.yml new file mode 100644 index 00000000000..e29987b0935 --- /dev/null +++ b/changelogs/unreleased/54206-show-the-activity-filter-dropdown-in-discussion-tab-only.yml @@ -0,0 +1,5 @@ +--- +title: Discussion filter only displayed in discussions tab for merge requests +merge_request: 24082 +author: +type: changed diff --git a/changelogs/unreleased/54427-label-xss.yml b/changelogs/unreleased/54427-label-xss.yml new file mode 100644 index 00000000000..090d1832af2 --- /dev/null +++ b/changelogs/unreleased/54427-label-xss.yml @@ -0,0 +1,5 @@ +--- +title: Escape html entities in LabelReferenceFilter when no label found +merge_request: +author: +type: security diff --git a/changelogs/unreleased/55369-update-milestone-sort-to-say-say-milestone-due-date.yml b/changelogs/unreleased/55369-update-milestone-sort-to-say-say-milestone-due-date.yml new file mode 100644 index 00000000000..7476b9caa93 --- /dev/null +++ b/changelogs/unreleased/55369-update-milestone-sort-to-say-say-milestone-due-date.yml @@ -0,0 +1,5 @@ +--- +title: Renames Milestone sort into Milestone due date +merge_request: 24080 +author: Jacopo Beschi @jacopo-beschi +type: changed diff --git a/changelogs/unreleased/55755-user-activity-is-stuck-loading-when-there-is-none.yml b/changelogs/unreleased/55755-user-activity-is-stuck-loading-when-there-is-none.yml new file mode 100644 index 00000000000..5362a781281 --- /dev/null +++ b/changelogs/unreleased/55755-user-activity-is-stuck-loading-when-there-is-none.yml @@ -0,0 +1,5 @@ +--- +title: Hide spinner on empty activites list on user profile overview +merge_request: 24063 +author: +type: other diff --git a/changelogs/unreleased/55838-remove-gem-install-bundler-from-docker-based-ruby-environments.yml b/changelogs/unreleased/55838-remove-gem-install-bundler-from-docker-based-ruby-environments.yml new file mode 100644 index 00000000000..08f60d205df --- /dev/null +++ b/changelogs/unreleased/55838-remove-gem-install-bundler-from-docker-based-ruby-environments.yml @@ -0,0 +1,5 @@ +--- +title: Remove gem install bundler from Docker-based Ruby environments +merge_request: 24093 +author: Takuya Noguchi +type: other diff --git a/changelogs/unreleased/blackst0ne-bump-rails-cve-2018-16476.yml b/changelogs/unreleased/blackst0ne-bump-rails-cve-2018-16476.yml new file mode 100644 index 00000000000..dfa94c69ce0 --- /dev/null +++ b/changelogs/unreleased/blackst0ne-bump-rails-cve-2018-16476.yml @@ -0,0 +1,5 @@ +--- +title: Bump Ruby on Rails to 5.0.7.1 +merge_request: 23396 +author: "@blackst0ne" +type: security diff --git a/changelogs/unreleased/s3-directories-get.yml b/changelogs/unreleased/s3-directories-get.yml new file mode 100644 index 00000000000..9f76af2bb09 --- /dev/null +++ b/changelogs/unreleased/s3-directories-get.yml @@ -0,0 +1,6 @@ +--- +title: Allow 'rake gitlab:cleanup:remote_upload_files' to read bucket files without + having permissions to see all buckets. +merge_request: 23981 +author: +type: fixed diff --git a/changelogs/unreleased/security-48259-private-snippet.yml b/changelogs/unreleased/security-48259-private-snippet.yml new file mode 100644 index 00000000000..6cf1e5dc694 --- /dev/null +++ b/changelogs/unreleased/security-48259-private-snippet.yml @@ -0,0 +1,5 @@ +--- +title: Prevent private snippets from being embeddable +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-53543-user-keeps-access-to-mr-issue-when-removed-from-team.yml b/changelogs/unreleased/security-53543-user-keeps-access-to-mr-issue-when-removed-from-team.yml new file mode 100644 index 00000000000..ab12ba539c1 --- /dev/null +++ b/changelogs/unreleased/security-53543-user-keeps-access-to-mr-issue-when-removed-from-team.yml @@ -0,0 +1,5 @@ +--- +title: Issuable no longer is visible to users when project can't be viewed +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-54377-label-milestone-name-xss.yml b/changelogs/unreleased/security-54377-label-milestone-name-xss.yml new file mode 100644 index 00000000000..76589b2eb4f --- /dev/null +++ b/changelogs/unreleased/security-54377-label-milestone-name-xss.yml @@ -0,0 +1,5 @@ +--- +title: Escape label and milestone titles to prevent XSS in GFM autocomplete +merge_request: 2693 +author: +type: security diff --git a/changelogs/unreleased/security-bvl-fix-cross-project-mr-exposure.yml b/changelogs/unreleased/security-bvl-fix-cross-project-mr-exposure.yml new file mode 100644 index 00000000000..11aae4428fb --- /dev/null +++ b/changelogs/unreleased/security-bvl-fix-cross-project-mr-exposure.yml @@ -0,0 +1,5 @@ +--- +title: Don't expose cross project repositories through diffs when creating merge reqeusts +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-fix-ssrf-import-url-remote-mirror.yml b/changelogs/unreleased/security-fix-ssrf-import-url-remote-mirror.yml new file mode 100644 index 00000000000..7ba7aa21090 --- /dev/null +++ b/changelogs/unreleased/security-fix-ssrf-import-url-remote-mirror.yml @@ -0,0 +1,5 @@ +--- +title: Fix SSRF with import_url and remote mirror url +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-master-group-cicd-settings-accessible-to-maintainer.yml b/changelogs/unreleased/security-master-group-cicd-settings-accessible-to-maintainer.yml new file mode 100644 index 00000000000..5586fa6cd8e --- /dev/null +++ b/changelogs/unreleased/security-master-group-cicd-settings-accessible-to-maintainer.yml @@ -0,0 +1,5 @@ +--- +title: Allow changing group CI/CD settings only for owners. +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-master-guests-jobs-api.yml b/changelogs/unreleased/security-master-guests-jobs-api.yml new file mode 100644 index 00000000000..83022e91aca --- /dev/null +++ b/changelogs/unreleased/security-master-guests-jobs-api.yml @@ -0,0 +1,5 @@ +--- +title: Authorize before reading job information via API. +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-master-secret-ci-variables-exposed.yml b/changelogs/unreleased/security-master-secret-ci-variables-exposed.yml new file mode 100644 index 00000000000..702181065f5 --- /dev/null +++ b/changelogs/unreleased/security-master-secret-ci-variables-exposed.yml @@ -0,0 +1,5 @@ +--- +title: Prevent leaking protected variables for ambiguous refs. +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-master-url-rel.yml b/changelogs/unreleased/security-master-url-rel.yml new file mode 100644 index 00000000000..75f599f6bcd --- /dev/null +++ b/changelogs/unreleased/security-master-url-rel.yml @@ -0,0 +1,5 @@ +--- +title: Set URL rel attribute for broken URLs. +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-refs-available-to-project-guest.yml b/changelogs/unreleased/security-refs-available-to-project-guest.yml new file mode 100644 index 00000000000..eb6804c52d3 --- /dev/null +++ b/changelogs/unreleased/security-refs-available-to-project-guest.yml @@ -0,0 +1,5 @@ +--- +title: Project guests no longer are able to see refs page +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-todos_not_redacted_for_guests.yml b/changelogs/unreleased/security-todos_not_redacted_for_guests.yml new file mode 100644 index 00000000000..be0ae9a7193 --- /dev/null +++ b/changelogs/unreleased/security-todos_not_redacted_for_guests.yml @@ -0,0 +1,5 @@ +--- +title: Delete confidential todos for user when downgraded to Guest +merge_request: +author: +type: security diff --git a/changelogs/unreleased/sh-bump-omniauth-google-gem.yml b/changelogs/unreleased/sh-bump-omniauth-google-gem.yml new file mode 100644 index 00000000000..2b31a55f8b2 --- /dev/null +++ b/changelogs/unreleased/sh-bump-omniauth-google-gem.yml @@ -0,0 +1,5 @@ +--- +title: Upgrade Omniauth and JWT gems to switch away from Google+ API +merge_request: 24068 +author: +type: changed diff --git a/changelogs/unreleased/sh-fix-issue-55822.yml b/changelogs/unreleased/sh-fix-issue-55822.yml new file mode 100644 index 00000000000..1267b2ace2f --- /dev/null +++ b/changelogs/unreleased/sh-fix-issue-55822.yml @@ -0,0 +1,5 @@ +--- +title: Fix commit SHA not showing in merge request compare dropdown +merge_request: 24084 +author: +type: fixed diff --git a/doc/api/lint.md b/doc/api/lint.md index bd5a216a99d..c37a8bff749 100644 --- a/doc/api/lint.md +++ b/doc/api/lint.md @@ -13,7 +13,7 @@ POST /lint | `content` | string | yes | the .gitlab-ci.yaml content| ```bash -curl --header "Content-Type: application/json" https://gitlab.example.com/api/v4/ci/lint --data '{"content": "{ \"image\": \"ruby:2.1\", \"services\": [\"postgres\"], \"before_script\": [\"gem install bundler\", \"bundle install\", \"bundle exec rake db:create\"], \"variables\": {\"DB_NAME\": \"postgres\"}, \"types\": [\"test\", \"deploy\", \"notify\"], \"rspec\": { \"script\": \"rake spec\", \"tags\": [\"ruby\", \"postgres\"], \"only\": [\"branches\"]}}"}' +curl --header "Content-Type: application/json" https://gitlab.example.com/api/v4/ci/lint --data '{"content": "{ \"image\": \"ruby:2.6\", \"services\": [\"postgres\"], \"before_script\": [\"bundle install\", \"bundle exec rake db:create\"], \"variables\": {\"DB_NAME\": \"postgres\"}, \"types\": [\"test\", \"deploy\", \"notify\"], \"rspec\": { \"script\": \"rake spec\", \"tags\": [\"ruby\", \"postgres\"], \"only\": [\"branches\"]}}"}' ``` Be sure to copy paste the exact contents of `.gitlab-ci.yml` as YAML is very picky about indentation and spaces. diff --git a/doc/api/templates/gitlab_ci_ymls.md b/doc/api/templates/gitlab_ci_ymls.md index cecfc8cd9b9..11ec7360e06 100644 --- a/doc/api/templates/gitlab_ci_ymls.md +++ b/doc/api/templates/gitlab_ci_ymls.md @@ -120,6 +120,6 @@ Example response: ```json { "name": "Ruby", - "content": "# This file is a template, and might need editing before it works on your project.\n# Official language image. Look for the different tagged releases at:\n# https://hub.docker.com/r/library/ruby/tags/\nimage: \"ruby:2.3\"\n\n# Pick zero or more services to be used on all builds.\n# Only needed when using a docker container to run your tests in.\n# Check out: http://docs.gitlab.com/ce/ci/docker/using_docker_images.html#what-is-service\nservices:\n - mysql:latest\n - redis:latest\n - postgres:latest\n\nvariables:\n POSTGRES_DB: database_name\n\n# Cache gems in between builds\ncache:\n paths:\n - vendor/ruby\n\n# This is a basic example for a gem or script which doesn't use\n# services such as redis or postgres\nbefore_script:\n - ruby -v # Print out ruby version for debugging\n # Uncomment next line if your rails app needs a JS runtime:\n # - apt-get update -q && apt-get install nodejs -yqq\n - gem install bundler --no-ri --no-rdoc # Bundler is not installed with the image\n - bundle install -j $(nproc) --path vendor # Install dependencies into ./vendor/ruby\n\n# Optional - Delete if not using `rubocop`\nrubocop:\n script:\n - rubocop\n\nrspec:\n script:\n - rspec spec\n\nrails:\n variables:\n DATABASE_URL: \"postgresql://postgres:postgres@postgres:5432/$POSTGRES_DB\"\n script:\n - bundle exec rake db:migrate\n - bundle exec rake db:seed\n - bundle exec rake test\n" + "content": "# This file is a template, and might need editing before it works on your project.\n# Official language image. Look for the different tagged releases at:\n# https://hub.docker.com/r/library/ruby/tags/\nimage: \"ruby:2.5\"\n\n# Pick zero or more services to be used on all builds.\n# Only needed when using a docker container to run your tests in.\n# Check out: http://docs.gitlab.com/ce/ci/docker/using_docker_images.html#what-is-a-service\nservices:\n - mysql:latest\n - redis:latest\n - postgres:latest\n\nvariables:\n POSTGRES_DB: database_name\n\n# Cache gems in between builds\ncache:\n paths:\n - vendor/ruby\n\n# This is a basic example for a gem or script which doesn't use\n# services such as redis or postgres\nbefore_script:\n - ruby -v # Print out ruby version for debugging\n # Uncomment next line if your rails app needs a JS runtime:\n # - apt-get update -q && apt-get install nodejs -yqq\n - bundle install -j $(nproc) --path vendor # Install dependencies into ./vendor/ruby\n\n# Optional - Delete if not using `rubocop`\nrubocop:\n script:\n - rubocop\n\nrspec:\n script:\n - rspec spec\n\nrails:\n variables:\n DATABASE_URL: \"postgresql://postgres:postgres@postgres:5432/$POSTGRES_DB\"\n script:\n - rails db:migrate\n - rails db:seed\n - rails test\n\n# This deploy job uses a simple deploy flow to Heroku, other providers, e.g. AWS Elastic Beanstalk\n# are supported too: https://github.com/travis-ci/dpl\ndeploy:\n type: deploy\n environment: production\n script:\n - gem install dpl\n - dpl --provider=heroku --app=$HEROKU_APP_NAME --api-key=$HEROKU_PRODUCTION_KEY\n" } ``` diff --git a/doc/ci/caching/index.md b/doc/ci/caching/index.md index f93ccc4e3c1..495ec099111 100644 --- a/doc/ci/caching/index.md +++ b/doc/ci/caching/index.md @@ -300,7 +300,6 @@ cache: before_script: - ruby -v # Print out ruby version for debugging - - gem install bundler --no-ri --no-rdoc # Bundler is not installed with the image - bundle install -j $(nproc) --path vendor # Install dependencies into ./vendor/ruby rspec: diff --git a/doc/ci/docker/using_kaniko.md b/doc/ci/docker/using_kaniko.md index 66f0d429165..aa6b387bc58 100644 --- a/doc/ci/docker/using_kaniko.md +++ b/doc/ci/docker/using_kaniko.md @@ -57,3 +57,26 @@ build: only: - tags ``` + +## Using a registry with a custom certificate + +When trying to push to a Docker registry that uses a certificate that is signed +by a custom CA, you might get the following error: + +```sh +$ /kaniko/executor --context $CI_PROJECT_DIR --dockerfile $CI_PROJECT_DIR/Dockerfile --no-push +INFO[0000] Downloading base image registry.gitlab.example.com/group/docker-image +error building image: getting stage builder for stage 0: Get https://registry.gitlab.example.com/v2/: x509: certificate signed by unknown authority +``` + +This can be solved by adding your CA's certificate to the kaniko certificate +store: + +```yaml + before_script: + - echo "{\"auths\":{\"$CI_REGISTRY\":{\"username\":\"$CI_REGISTRY_USER\",\"password\":\"$CI_REGISTRY_PASSWORD\"}}}" > /kaniko/.docker/config.json + - | + echo "-----BEGIN CERTIFICATE----- + ... + -----END CERTIFICATE-----" >> /kaniko/ssl/certs/ca-certificates.crt +``` diff --git a/doc/ci/quick_start/README.md b/doc/ci/quick_start/README.md index bdc593493ea..1ec8a8c89c9 100644 --- a/doc/ci/quick_start/README.md +++ b/doc/ci/quick_start/README.md @@ -77,7 +77,7 @@ before_script: - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - ruby -v - which ruby - - gem install bundler --no-ri --no-rdoc + - gem install bundler --no-document - bundle install --jobs $(nproc) "${FLAGS[@]}" rspec: diff --git a/doc/ci/services/mysql.md b/doc/ci/services/mysql.md index b76f9618fc9..2902c30c7c0 100644 --- a/doc/ci/services/mysql.md +++ b/doc/ci/services/mysql.md @@ -16,7 +16,7 @@ services: - mysql:latest variables: - # Configure mysql environment variables (https://hub.docker.com/r/_/mysql/) + # Configure mysql environment variables (https://hub.docker.com/_/mysql/) MYSQL_DATABASE: el_duderino MYSQL_ROOT_PASSWORD: mysql_strong_password ``` @@ -114,5 +114,5 @@ available [shared runners](../runners/README.md). Want to hack on it? Simply fork it, commit and push your changes. Within a few moments the changes will be picked by a public runner and the job will begin. -[hub-mysql]: https://hub.docker.com/r/_/mysql/ +[hub-mysql]: https://hub.docker.com/_/mysql/ [mysql-example-repo]: https://gitlab.com/gitlab-examples/mysql diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 440254e58bd..c74f5e5b3f9 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -1660,7 +1660,7 @@ automatically fetched and evaluated along with the content of `.gitlab-ci.yml`: before_script: - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - - gem install bundler --no-ri --no-rdoc + - gem install bundler --no-document - bundle install --jobs $(nproc) "${FLAGS[@]}" ``` diff --git a/doc/development/documentation/feature-change-workflow.md b/doc/development/documentation/feature-change-workflow.md new file mode 100644 index 00000000000..b5b325683a3 --- /dev/null +++ b/doc/development/documentation/feature-change-workflow.md @@ -0,0 +1,112 @@ +--- +description: How to add docs for new or enhanced GitLab features. +--- + +# Documentation process at GitLab + +At GitLab, developers contribute new or updated documentation along with their code, but product managers and technical writers also have essential roles in the process. + +- **Developers**: Author/update documentation in the same MR as their code, and +merge it by the feature freeze for the assigned milestone. Request technical writer +assistance if needed. +- **Product Managers** (PMs): In the issue for all new and enhanced features, +confirm the documentation requirements, plus the mentioned feature description +and use cases, which can be reused in docs. They can bring in a technical +writer for discussion or help, and can be called upon themselves as a doc reviewer. +- **Technical Writers**: Review doc requirements in issues, track issues and MRs +that contain docs changes, help with any questions throughout the authoring/editing process, +and review all new and updated docs content after it's merged (unless a pre-merge +review request is made). + +Beyond this process, any member of the GitLab community can also author documentation +improvements that are not associated with a new or changed feature. See the [Documentation improvement workflow](improvement-workflow.md). + +## When documentation is required + +Documentation must be delivered whenever: + +- A new or enhanced feature is shipped that impacts the user/admin experience +- There are changes to the UI or API +- A process, workflow, or previously documented feature is changed +- A feature is deprecated or removed + +Documentation is not required when a feature is changed on the backend +only and does not directly affect the way that any user or +administrator would interact with GitLab. For example, a UI restyling that offers +no difference in functionality may require documentation updates if screenshots +are now needed, or need to be updated. + +NOTE: **Note:** +When revamping documentation, if unrelated to the feature change, this should be submitted +in its own MR (using the [documentation improvement workflow](improvement-workflow.md)) +so that we can ensure the more time-sensitive doc updates are merged with code by the freeze. + +## Documenting a new or changed feature + +To follow a consistent workflow every month, documentation changes +involve the Product Managers, the developer who shipped the feature, +and the Technical Writing team. Each role is described below. + +### 1. Product Manager's role + +The Product Manager (PM) should confirm or add the following items in the issue: + +- New or updated feature name, overview/description, and use cases, all required per the [Documentation structure and template](structure.md). +- The documentation requirements for the developer working on the docs. + - What new page, new subsection of an existing page, or other update to an existing page/subsection is needed. + - Just one page/section/update or multiple (perhaps there's an end user and admin change needing docs, or we need to update a previously recommended workflow, or we want to link the new feature from various places; consider and mention all ways documentation should be affected. + - Suggested title of any page or subsection, if applicable. +- Label the issue with `Documentation` and `docs:P1` in addition to the `Deliverable` label and correct milestone. + +Anyone is welcome to draft the items above in the issue, but a product manager must review and update them whenever the issue is assigned a specific milestone. + +### 2. Developer's role + +As a developer, you must ship the documentation with the code of the feature that +you are creating or updating. The documentation is an essential part of the product. + +- New and edited docs should be included in the MR introducing the code, and planned +in the issue that proposed the feature. However, if the new or changed doc requires +extensive collaboration or conversation, a separate, linked issue can be used for the planning process. +- Use the [Documentation guidelines](index.md), as well as other resources linked from there, +including the [Structure and template](structure.md) page, [Style Guide](styleguide.md), and [Markdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/). +- If you need any help to choose the correct place for a doc, discuss a documentation +idea or outline, or request any other help, ping the Technical Writer for the relevant +[DevOps stage](https://about.gitlab.com/handbook/product/categories/#devops-stages) +in your issue or MR, or write within `#docs` on the GitLab Slack. +- The docs must be merged with the code **by the feature freeze date**, otherwise +- the feature cannot be included with the release.<!-- TODO: Policy/process for feature-flagged issues --> + +Prior to merge, documentation changes commited by the developer must be reviewed by: +* the person reviewing the code and merging the MR. +* optionally: others involved in the work (such as other devs, the PM, or a technical writer), if requested. + +After merging, documentation changing are reviewed by: +* a technical writer (for clarity, structure, grammar, etc). +* optionally: by the PM (for accuracy and to ensure it's consistent with the vision for how the product will be used). +Any party can raise the item to the PM for review at any point: the dev, the technical writer, or the PM, who can request/plan a review at the outset. + +### 3. Technical Writer's role + +**Planning** +- Once an issue contains a Documentation label and an upcoming milestone, a +technical writer reviews the listed documentation requirements, which should have +already been reviewed by the PM. (These are non-blocking reviews; developers should +not wait to work on docs.) +- Monitor the documentation needs of issues assigned to the current and next milestone, +and participate in any needed discussion on docs planning with the dev, PM, and others. + +**Review** +- Techncial writers provide non-blocking reviews of all documentation changes, +typically after the change is merged. However, if the docs are ready in the MR while +we are awaiting other work in order to merge, the technical writer's review can commence early. +- The technical writer will confirm that the doc is clear, grammatically correct, +and discoverable, while avoiding redundancy, bad file locations, typos, broken links, +etc. The technical writer will review the documentation for the following, which +the developer and code reviewer should have already made a good-faith effort to ensure: + - Clarity. + - Relevance (make sure the content is appropriate given the impact of the feature). + - Location (make sure the doc is in the correct dir and has the correct name). + - Syntax, typos, and broken links. + - Improvements to the content. + - Accordance to the [Documentation Style Guide](styleguide.md) and [structure/template](structure.md). diff --git a/doc/development/documentation/improvement-workflow.md b/doc/development/documentation/improvement-workflow.md new file mode 100644 index 00000000000..ef6392c6f7f --- /dev/null +++ b/doc/development/documentation/improvement-workflow.md @@ -0,0 +1,49 @@ +--- +description: How to improve GitLab's documentation. +--- + +# Documentation improvement workflow + +Anyone can contribute a merge request or create an issue for GitLab's documentation. + +This page covers the process for any contributions to GitLab's docs that are +not part of feature development. If you are looking for information on updating +GitLab's docs as is required with the development and release of a new feature +or feature enhancement, see the [feature-change documentation workflow](feature-change-workflow.md). + +## Who updates the docs + +Anyone can contribute! You can create a merge request with documentation +when you find errors or other room for improvement in an existing doc, or when you +have an idea for all-new documentation that would help a GitLab user or admin +to achieve or improve their DevOps workflows. + +## How to update the docs + +- Follow the described standards and processes listed on the [GitLab Documentation guidelines](index.md) page, +including linked resources: the [Structure and template](structure.md) page, [Style Guide](styleguide.md), and [Markdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/). +- Follow GitLab's [Merge Request Guidelines](../contributing/merge_request_workflow.md#merge-request-guidelines). +- If you need any help to choose the correct place for a doc, discuss a documentation +idea or outline, or request any other help, ping the Technical Writer for the relevant +[DevOps stage](https://about.gitlab.com/handbook/product/categories/#devops-stages) +in your issue or MR, or write within `#docs` if you are a member of GitLab's Slack workspace. + +## Merging + +Anyone with master access to the affected GitLab project can merge documentation changes. +This person must make a good-faith effort to ensure that the content is clear +(sufficiently easy for the intended audience to navigate and understand) and +that it meets the [Documentation Guidelines](index.md) and [Style Guide](styleguide.md). + +If the author or reviewer has any questions, or would like a techncial writer's review +before merging, mention the writer who is assigned to the relevant [DevOps stage](https://about.gitlab.com/handbook/product/categories/#devops-stages). + +## Technical Writer review + +The technical writing team reviews changes after they are merged, unless a prior +review is requested. + +## Other ways to help + +If you have ideas for further documentation resources that would be best +considered/handled by technical writers, devs, and other SMEs, please create an issue. diff --git a/doc/development/documentation/index.md b/doc/development/documentation/index.md index 4e5b4a85a97..256e0476c2f 100644 --- a/doc/development/documentation/index.md +++ b/doc/development/documentation/index.md @@ -4,48 +4,49 @@ description: Learn how to contribute to GitLab Documentation. # GitLab Documentation guidelines -- **General Documentation**: written by the [developers responsible by creating features](#contributing-to-docs). Should be submitted in the same merge request containing code. Feature proposals (by GitLab contributors) should also be accompanied by its respective documentation. They can be later improved by PMs and Technical Writers. -- **[Technical Articles](#technical-articles)**: written by any [GitLab Team](https://about.gitlab.com/team/) member, GitLab contributors, or [Community Writers](https://about.gitlab.com/handbook/product/technical-writing/community-writers/). -- **Indexes per topic**: initially prepared by the Technical Writing Team, and kept up-to-date by developers and PMs in the same merge request containing code. They gather all resources for that topic in a single page (user and admin documentation, articles, and third-party docs). +GitLab's documentation is [intended as the single source of truth (SSOT)](https://about.gitab.com/handbook/documentation/) for information about how to configure, use, and troubleshoot GitLab. The documentation contains use cases and usage instructions covering every GitLab feature, organized by product area and subject. This includes topics and workflows that span multiple GitLab features, as well as the use of GitLab with other applications. -## Contributing to docs +In addition to this page, the following resources to help craft and contribute documentation are available: + +- [Style Guide](styleguide.md) - What belongs in the docs, language guidelines, and more. +- [Structure and template](structure.md) - Learn the typical parts of a doc page and how to write each one. +- [Workflow](workflow.md) - A landing page for our key workflows: + - [Feature-change documentation workflow](feature-change-workflow.md) - Adding required documentation when developing a GitLab feature. + - [Documentation improvement worflow](improvement-workflow.md) - New content not associated with a new feature. +- [Markdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/) - A reference for the markdown implementation used by GitLab's documentation site and about.gitlab.com. +- [Site architecture](/doc/development/documentation/site_architecture/index.md) - How docs.gitlab.com is built. + +## Source and rendered locations -Whenever a feature is changed, updated, introduced, or deprecated, the merge -request introducing these changes must be accompanied by the documentation -(either updating existing ones or creating new ones). This is also valid when -changes are introduced to the UI. +Documentation for GitLab Community Edition (CE) and Enterprise Edition (EE), along with GitLab Runner and Omnibus, is published to [docs.gitlab.com](https://docs.gitlab.com). The documentation for CE and EE is also published within the application at `/help` on the domain of the GitLab instance. -The one responsible for writing the first piece of documentation is the developer who -wrote the code. It's the job of the Product Manager to ensure all features are -shipped with its docs, whether is a small or big change. At the pace GitLab evolves, -this is the only way to keep the docs up-to-date. If you have any questions about it, -ask a Technical Writer. Otherwise, when your content is ready, assign one of -them to review it for you. +At `/help`, only content for your current edition and version is included, whereas multiple versions' content is available at docs.gitlab.com. -We use the [monthly release blog post](https://about.gitlab.com/handbook/marketing/blog/release-posts/#monthly-releases) as a changelog checklist to ensure everything -is documented. +The source of the documentation is maintained in the following repository locations: -Whenever you submit a merge request for the documentation, use the -"Documentation" MR description template. If you're changing documentation -location, use the MR description template called "Change documentation -location" instead. +| Project | Path | +| --- | --- | +| [GitLab Community Edition](https://gitlab.com/gitlab-org/gitlab-ce/) | [`/doc`](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/doc) | +| [GitLab Enterprise Edition](https://gitlab.com/gitlab-org/gitlab-ce/) | [`/doc`](https://gitlab.com/gitlab-org/gitlab-ee/tree/master/doc) | +| [GitLab Runner](https://gitlab.com/gitlab-org/gitlab-runner/) | [`/docs`](https://gitlab.com/gitlab-org/gitlab-runner/tree/master/docs) | +| [Omnibus GitLab](https://gitlab.com/gitlab-org/omnibus-gitlab/) | [`/doc`](https://gitlab.com/gitlab-org/gitlab-ee/tree/master/doc) | -## Documentation workflow +Documentation issues and merge requests are part of their respective repositories and all have the label `Documentation`. -Please read through the [documentation workflow](workflow.md) before getting started. +## Contributing to docs + +[Contributions to GitLab docs](workflow.md) are welcome from the entire GitLab community. -## Documentation structure +To ensure that GitLab docs keep up with changes to the product, special processes and responsibilities are in place concerning all [feature changes](feature-change-workflow.md)—i.e. development work that impacts the appearance, usage, or administration of a feature. -Follow through the [documentation structure guide](structure.md) for learning -how to structure GitLab docs. +Meanwhile, anyone can contribute [documentation improvements](improvement-workflow.md) large or small that are not associated with a feature change. For example, adding a new doc on how to accomplish a use case that's already possible with GitLab or with third-party tools and GitLab. ## Markdown and styles [GitLab docs](https://gitlab.com/gitlab-com/gitlab-docs) uses [GitLab Kramdown](https://gitlab.com/gitlab-org/gitlab_kramdown) -as markdown engine. Check the [GitLab Markdown Kramdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/) -for a complete Kramdown reference. +as its markdown rendering engine. See the [GitLab Markdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/) for a complete Kramdown reference. -Follow the [documentation style guidelines](styleguide.md) strictly. +Adhere to the [Documentation Style Guide](styleguide.md). If a style standard is missing, you are welcome to suggest one via a merge request. ## Documentation directory structure @@ -58,7 +59,7 @@ all docs should be linked. Every new document should be cross-linked to its rela The directories `/workflow/`, `/gitlab-basics/`, `/university/`, and `/articles/` have been **deprecated** and the majority their docs have been moved to their correct location -in small iterations. Please don't create new docs in these folders. +in small iterations. Please do not create new docs in these folders. Organize docs by product area and subject, not type. ### Documentation files @@ -71,18 +72,23 @@ in small iterations. Please don't create new docs in these folders. ### Location and naming documents -The documentation hierarchy can be vastly improved by providing a better layout -and organization of directories. - -Having a structured document layout, we will be able to have meaningful URLs -like `docs.gitlab.com/user/project/merge_requests/index.html`. With this pattern, -you can immediately tell that you are navigating a user related documentation -and is about the project and its merge requests. - -Do not create summaries of similar types of content (e.g. an index of all articles, videos, etc.), -rather organize content by its subject (e.g. everything related to CI goes together) +Our goal is to have a clear hierarchical structure with meaningful URLs +like `docs.gitlab.com/user/project/merge_requests/`. With this pattern, +you can immediately tell that you are navigating to user-related documentation +about project features; specifically about merge requests. Our site's paths match +those of our repository, so the clear structure also makes documentation easier to update. + +While the documentation is home to a variety of content types, we do not organize by content type. +For example, do not create groupings of similar media types (e.g. indexes of all articles, videos, etc.). +Similarly, we do not use glossaries or FAQs. Such grouping of content by type makes +it difficult to browse for the information you need and difficult to maintain up-to-date content. +Instead, organize content by its subject (e.g. everything related to CI goes together) and cross-link between any related content. +Do not simply link out to GitLab technical blog posts. There should be an up-to-date +single source of truth on the topic within the documentation, and the top of the +blog post should be updated to link to that doc. + The table below shows what kind of documentation goes where. | Directory | What belongs here | @@ -90,14 +96,12 @@ The table below shows what kind of documentation goes where. | `doc/user/` | User related documentation. Anything that can be done within the GitLab UI goes here including `/admin`. | | `doc/administration/` | Documentation that requires the user to have access to the server where GitLab is installed. The admin settings that can be accessed via GitLab's interface go under `doc/user/admin_area/`. | | `doc/api/` | API related documentation. | -| `doc/development/` | Documentation related to the development of GitLab. Any styleguides should go here. | +| `doc/development/` | Documentation related to the development of GitLab. Related process and style guides should go here. | | `doc/legal/` | Legal documents about contributing to GitLab. | | `doc/install/` | Probably the most visited directory, since `installation.md` is there. Ideally this should go under `doc/administration/`, but it's best to leave it as-is in order to avoid confusion (still debated though). | | `doc/update/` | Same with `doc/install/`. Should be under `administration/`, but this is a well known location, better leave as-is, at least for now. | | `doc/topics/` | Indexes per Topic (`doc/topics/topic-name/index.md`): all resources for that topic (user and admin documentation, articles, and third-party docs) | ---- - **General rules & best practices:** 1. When creating a new document and it has more than one word in its name, @@ -126,18 +130,23 @@ The table below shows what kind of documentation goes where. `doc/topics/topic-name/subtopic-name/index.md` when subtopics become necessary. General user- and admin- related documentation, should be placed accordingly. -If you are unsure where a document should live, you can ping `@axil` or `@marcia` in your -merge request. +If you are unsure where a document or a content addition should live, this should +not stop you from authoring and contributing. You can use your best judgment and +then ask the reviewer of your MR to confirm your decision, and/or ask a technical writer +at any stage in the process. The techncial writing team will review all documentation +changes, regardless, and can move content if there is a better place for it. ### Changing document location -Changing a document's location is not to be taken lightly. Remember that the -documentation is available to all installations under `help/` and not only to -GitLab.com or <http://docs.gitlab.com>. Make sure this is discussed with the -Documentation team beforehand. +Changing a document's location requires specific steps to be followed to ensure that +users can seamlessly access the new doc page, whether they are accesing content +on a GitLab instance domain at `/help` or at docs.gitlab.com. Be sure to ping a +GitLab technical writer if you have any questions during the process (such as +whether the move is necessary), and ensure that a technical writer reviews this +change prior to merging. -If you indeed need to change a document's location, do NOT remove the old -document, but rather replace all of its contents with a new line: +If you indeed need to change a document's location, do not remove the old +document, but rather replace all of its content with a new line: ```md This document was moved to [another location](path/to/new_doc.md). @@ -175,6 +184,7 @@ Things to note: - Since we also use inline documentation, except for the documentation itself, the document might also be referenced in the views of GitLab (`app/`) which will render when visiting `/help`, and sometimes in the testing suite (`spec/`). + You must search these paths for references to the doc and update them as well. - The above `git grep` command will search recursively in the directory you run it in for `workflow/lfs/lfs_administration` and `lfs/lfs_administration` and will print the file and the line where this file is mentioned. @@ -202,7 +212,7 @@ This redirection method will not provide a redirect fallback on GitLab `/help`. it, make sure to add a link to the new page on the doc, otherwise it's a dead end for users that land on the doc via `/help`. -### Redirections for pages with Disqus comments +#### Redirections for pages with Disqus comments If the documentation page being relocated already has any Disqus comments, we need to preserve the Disqus thread. @@ -240,20 +250,26 @@ choices: | Ending in `-docs` | `123-update-api-issues-docs` | If your branch name matches any of the above, it will run only the docs -tests. If it doesn't, the whole test suite will run (including docs). +tests. If it does not, the whole application test suite will run (including docs tests). ## Merge requests for GitLab documentation Before getting started, make sure you read the introductory section "[contributing to docs](#contributing-to-docs)" above and the -[tech writing workflow](https://about.gitlab.com/handbook/product/technical-writing/workflow/) -for GitLab Team members. +[documentation workflow](workflow.md). - Use the current [merge request description template](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/.gitlab/merge_request_templates/Documentation.md) - Use the correct [branch name](#branch-naming) - Label the MR `Documentation` - Assign the correct milestone (see note below) +Documentation will be merged if it is an improvement on existing content, +represents a good-faith effort to follow the template and style standards, +and is believed to be accurate. + +Further needs for what would make the doc even better should be immediately addressed +in a follow-up MR or issue. + NOTE: **Note:** If the release version you want to add the documentation to has already been frozen or released, use the label `Pick into X.Y` to get it merged into @@ -380,80 +396,14 @@ to merge changes that will break `master` from a merge request with a successful ## Docs site architecture -Read through [docs architecture](site_architecture/index.md) to learn -how we architecture, build, and deploy the docs site, <https://docs.gitlab.com>, and -to check all the assets and libraries available. +See the [Docs site architecture](site_architecture/index.md) page to learn +how we build and deploy the site at [docs.gitlab.com](https://docs.gitlab.com), and +to review all the assets and libraries in use. ### Global navigation -Read through the [global navigation](site_architecture/global_nav.md) doc. - -## General Documentation vs Technical Articles - -### General documentation - -General documentation is categorized by _User_, _Admin_, and _Contributor_, and describe what that feature is, what it does, and its available settings. - -### Technical Articles - -Technical articles replace technical content that once lived in the [GitLab Blog](https://about.gitlab.com/blog/), where they got out-of-date and weren't easily found. - -They are topic-related documentation, written with an user-friendly approach and language, aiming to provide the community with guidance on specific processes to achieve certain objectives. - -A technical article guides users and/or admins to achieve certain objectives (within guides and tutorials), or provide an overview of that particular topic or feature (within technical overviews). It can also describe the use, implementation, or integration of third-party tools with GitLab. - -They should be placed in a new directory named `/article-title/index.md` under a topic-related folder, and their images should be placed in `/article-title/img/`. For example, a new article on GitLab Pages should be placed in `doc/user/project/pages/article-title/` and a new article on GitLab CI/CD should be placed in `doc/ci/examples/article-title/`. - -#### Types of Technical Articles - -- **User guides**: technical content to guide regular users from point A to point B -- **Admin guides**: technical content to guide administrators of GitLab instances from point A to point B -- **Technical Overviews**: technical content describing features, solutions, and third-party integrations -- **Tutorials**: technical content provided step-by-step on how to do things, or how to reach specific objectives - -#### Understanding guides, tutorials, and technical overviews - -Suppose there's a process to go from point A to point B in 5 steps: `(A) 1 > 2 > 3 > 4 > 5 (B)`. - -A **guide** can be understood as a description of certain processes to achieve a particular objective. A guide brings you from A to B describing the characteristics of that process, but not necessarily going over each step. It can mention, for example, steps 2 and 3, but does not necessarily explain how to accomplish them. - -- Live example: "[Static sites and GitLab Pages domains (Part 1)](../../user/project/pages/getting_started_part_one.md) to [Creating and Tweaking GitLab CI/CD for GitLab Pages (Part 4)](../../user/project/pages/getting_started_part_four.md)" - -A **tutorial** requires a clear **step-by-step** guidance to achieve a singular objective. It brings you from A to B, describing precisely all the necessary steps involved in that process, showing each of the 5 steps to go from A to B. -It does not only describes steps 2 and 3, but also shows you how to accomplish them. - -- Live example (on the blog): [Hosting on GitLab.com with GitLab Pages](https://about.gitlab.com/2016/04/07/gitlab-pages-setup/) - -A **technical overview** is a description of what a certain feature is, and what it does, but does not walk -through the process of how to use it systematically. - -- Live example (on the blog): [GitLab Workflow, an overview](https://about.gitlab.com/2016/10/25/gitlab-workflow-an-overview/) - -#### Special format - -Every **Technical Article** contains a frontmatter at the beginning of the doc -with the following information: - -- **Type of article** (user guide, admin guide, technical overview, tutorial) -- **Knowledge level** expected from the reader to be able to follow through (beginner, intermediate, advanced) -- **Author's name** and **GitLab.com handle** -- **Publication date** (ISO format YYYY-MM-DD) - -For example: - -```yaml ---- -author: John Doe -author_gitlab: johnDoe -level: beginner -article_type: user guide -date: 2017-02-01 ---- -``` - -#### Technical Articles - Writing Method - -Use the [writing method](https://about.gitlab.com/handbook/product/technical-writing/#writing-method) defined by the Technical Writing team. +See the [Global navigation](site_architecture/global_nav.md) doc for information +on how the left-side navigation menu is built and updated. ## Previewing the changes live @@ -468,13 +418,13 @@ The live preview is currently enabled for the following projects: - <https://gitlab.com/gitlab-org/gitlab-runner> If your branch contains only documentation changes, you can use -[special branch names](#branch-naming) to avoid long running pipelines. +[special branch names](#branch-naming) to avoid long-running pipelines. For [docs-only changes](#branch-naming), the review app is run automatically. For all other branches, you can use the manual `review-docs-deploy-manual` job in your merge request. You will need at least Maintainer permissions to be able -to run it. In the mini pipeline graph, you should see an `>>` icon. Clicking on it will -reveal the `review-docs-deploy-manual` job. Hit the play button for the job to start. +to run it. In the mini pipeline graph, you should see a `>>` icon. Clicking it will +reveal the `review-docs-deploy-manual` job. Click the play button to start the job.  @@ -630,9 +580,10 @@ A file with `proselint` configuration must be placed in a #### `markdownlint` `markdownlint` checks that certain rules ([example](https://github.com/DavidAnson/markdownlint/blob/master/README.md#rules--aliases)) - are followed for Markdown syntax. Our [style guidelines](styleguide.md) elaborate on which choices - must be made when selecting Markdown syntax for GitLab documentation and this tool helps - catch deviations from those guidelines. + are followed for Markdown syntax. + Our [Documentation Style Guide](styleguide.md) and [Markdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/) + elaborate on which choices must be made when selecting Markdown syntax for + GitLab documentation. This tool helps catch deviations from those guidelines. `markdownlint` can be used [on the command line](https://github.com/igorshubovych/markdownlint-cli#markdownlint-cli--), either on a single Markdown file or on all Markdown files in a project. For example, to run @@ -655,7 +606,7 @@ markdownlint **/*.md The following sample `markdownlint` configuration modifies the available default rules to: -- Adhere to the [style guidelines](styleguide.md). +- Adhere to the [Documentation Style Guide](styleguide.md). - Apply conventions found in the GitLab documentation. - Allow the flexibility of using some inline HTML. @@ -694,9 +645,10 @@ For [`markdownlint`](https://github.com/DavidAnson/markdownlint/), this configur placed in a [valid location](https://github.com/igorshubovych/markdownlint-cli#configuration). For example, `~/.markdownlintrc`. -## Danger bot +## Danger Bot -GitLab uses [danger bot](https://github.com/danger/danger) for some elements in -code review. For docs changes in merge requests, whenever a change under `/doc` -is made, the bot leaves a comment for the author to mention `@gl-docsteam`, so -that the docs can be properly reviewed. +GitLab uses [Danger](https://github.com/danger/danger) for some elements in +code review. For docs changes in merge requests, whenever a change to files under `/doc` +is made, Danger Bot leaves a comment with further instructions about the documentation +process. This is configured in the Dangerfile in the GitLab CE and EE repo under +[/danger/documentation/](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/danger/documentation). diff --git a/doc/development/documentation/site_architecture/index.md b/doc/development/documentation/site_architecture/index.md index 956bf90a5d9..9d4d2d3a28b 100644 --- a/doc/development/documentation/site_architecture/index.md +++ b/doc/development/documentation/site_architecture/index.md @@ -2,11 +2,18 @@ description: "Learn how GitLab's documentation website is architectured." --- -# Docs site architecture +# Documentation site architecture Learn how we build and architecture [`gitlab-docs`](https://gitlab.com/gitlab-com/gitlab-docs) and deploy it to <https://docs.gitlab.com>. +## Repository + +While the source of the documentation content is stored in GitLab's respective product +repositories, the source that is used to build the documentation site _from that content_ +is located at https://gitlab.com/gitlab-com/gitlab-docs. See the README there for +detailed information. + ## Assets To provide an optimized site structure, design, and a search-engine friendly diff --git a/doc/development/documentation/structure.md b/doc/development/documentation/structure.md index 607ad21d459..ee3bd5606a5 100644 --- a/doc/development/documentation/structure.md +++ b/doc/development/documentation/structure.md @@ -1,147 +1,121 @@ --- -description: Learn the how to correctly structure GitLab documentation. +description: What to include in GitLab documentation pages. --- -# Documentation structure +# Documentation structure and template -For consistency throughout the documentation, it's important to maintain the same -structure among the docs. +This document will help you determine how to structure a page within GitLab's +documentation and what content to include. These standards help ensure consistency +and completeness throughout the documentation, and they make it easier to contribute. -Before getting started, read through the following docs: +Before getting started, familiarize yourself with [GitLab's Documentation guidelines](index.md) +and the section on Content in the [Style Guide](styleguide.md). -- [Contributing to GitLab documentation](index.md#contributing-to-docs) -- [Merge requests for GitLab documentation](index.md#merge-requests-for-gitlab-documentation) -- [Branch naming for docs-only changes](index.md#branch-naming) -- [Documentation directory structure](index.md#documentation-directory-structure) -- [Documentation style guidelines](styleguide.md) -- [Documentation workflow](workflow.md) +## Components of a documentation page -## Documentation blurb +Most pages will be dedicated to a specifig GitLab feature or to a use case that involves +one or more features, potentially in conjunction with third-party tools. -Every document should include the following content in the following sequence: +Every feature or use case document should include the following content in the following sequence, +with exceptions and details noted below and in the template included on this page. -- **Feature name**: defines an intuitive name for the feature that clearly - states what it is and is consistent with any relevant UI text. -- **Feature overview** and description: describe what it is, what it does, and in what context it should be used. -- **Use cases**: describes real use case scenarios for that feature. -- **Requirements**: describes what software and/or configuration is required to be able to - use the feature and, if applicable, prerequisite knowledge for being able to follow/implement the tutorial. - For example, familiarity with GitLab CI/CD, an account on a third-party service, dependencies installed, etc. - Link each one to its most relevant resource; i.e., where the reader can go to begin to fullfil that requirement. - (Another doc page, a third party application's site, etc.) -- **Instructions**: clearly describes the steps to use the feature, leaving no gaps. -- **Troubleshooting** guide (recommended but not required): if you know beforehand what issues - one might have when setting it up, or when something is changed, or on upgrading, it's - important to describe those too. Think of things that may go wrong and include them in the - docs. This is important to minimize requests for support, and to avoid doc comments with - questions that you know someone might ask. Answering them beforehand only makes your - document better and more approachable. +- **Title**: Top-level heading with the feature name, or a use case name, which would start with +a verb, like Configuring, Enabling, etc. +- **Introduction**: A couple sentences about the subject matter and what's to be found on this page. +- **Overview** Describe what it is, what it does, and in what context it should be used. +- **Use cases**: describes real use case scenarios for that feature/configuration. +- **Requirements**: describes what software, configuration, account, or knowledge is required. +- **Instructions**: One or more sets of detailed instructions to follow. +- **Troubleshooting** guide (recommended but not required). -For additional details, see the subsections below, as well as the [Documentation template for new docs](#Documentation-template-for-new-docs). +For additional details on each, see the [template for new docs](#template-for-new-docs), +below. -### Feature overview and use cases +Note that you can include additional subsections, as appropriate, such as 'How it Works', 'Architecture', +and other logicial divisions such as pre- and post-deployment steps. -Every major feature (regardless if present in GitLab Community or Enterprise editions) -should present, at the beginning of the document, two main sections: **overview** and -**use cases**. Every GitLab EE-only feature should also contain these sections. - -**Overview**: as the name suggests, the goal here is to provide an overview of the feature. -Describe what is it, what it does, why it is important/cool/nice-to-have, -what problem it solves, and what you can do with this feature that you couldn't -do before. - -**Use cases**: provide at least two, ideally three, use cases for every major feature. -You should answer this question: what can you do with this feature/change? Use cases -are examples of how this feature or change can be used in real life. - -Examples: - -- CE and EE: [Issues](../../user/project/issues/index.md#use-cases) -- CE and EE: [Merge Requests](../../user/project/merge_requests/index.md) -- EE-only: [Geo](https://docs.gitlab.com/ee/administration/geo/replication/index.html) -- EE-only: [Jenkins integration](https://docs.gitlab.com/ee/integration/jenkins.html) - -Note that if you don't have anything to add between the doc title (`<h1>`) and -the header `## Overview`, you can omit the header, but keep the content of the -overview there. - -> **Overview** and **use cases** are required to **every** Enterprise Edition feature, -and for every **major** feature present in Community Edition. - -### Discoverability - -Your new document will be discoverable by the user only if: - -- Crosslinked from the higher-level index (e.g., Issue Boards docs - should be linked from Issues; Prometheus docs should be linked from - Monitoring; CI/CD tutorials should be linked from CI/CD examples). - - When referencing other GitLab products and features, link to their - respective docs; when referencing third-party products or technologies, - link out to their external sites, documentation, and resources. -- The headings are clear. E.g., "App testing" is a bad heading, "Testing - an application with GitLab CI/CD" is much better. Think of something - someone will search for and use these keywords in the headings. - -## Documentation template for new docs +## Template for new docs To start a new document, respect the file tree and file name guidelines, as well as the style guidelines. Use the following template: ```md +<!--Follow the Style Guide when working on this document. https://docs.gitlab.com/ee/development/documentation/styleguide.html +When done, remove all of this commented-out text, except a commented-out Troubleshooting section, +which, if empty, can be left in place to encourage future use.--> --- -description: "short document description." # Up to ~200 chars long. They will be displayed in Google Search Snippets. +description: "Short document description." # Up to ~200 chars long. They will be displayed in Google Search snippets. It may help to write the page intro first, and then reuse it here. --- -# Feature Name **[TIER]** (1) +# Feature Name or Use Case Name **[TIER]** (1) +<!--If writing about a use case, drop the tier, and start with a verb, e.g. 'Configuring', 'Implementing', + the goal/scenario--> -> [Introduced](link_to_issue_or_mr) in GitLab Tier X.Y (2). +<!--For pages on newly introduced features, add the following line. If only some aspects of the feature have been introduced, specify what parts of the feature.--> +> [Introduced](link_to_issue_or_mr) in GitLab (Tier) X.Y (2). -A short description for the feature (can be the same used in the frontmatter's -`description`). +An introduction -- without its own additional header -- goes here. +Offer a very short description of the feature or use case, and what to expect on this page. +(You can reuse this content, or part of it, for the front matter's `description` at the top of this file). ## Overview -To write the feature overview, you should consider answering the following questions: +The feature overview should answer the following questions: -- What is it? +- What is this feature or use case? - Who is it for? - What is the context in which it is used and are there any prerequisites/requirements? -- What can the user do with it? (Be sure to consider multiple audiences, like GitLab admin and developer-user.) -- What are the benefits to using it over any alternatives? +- What can the audience do with this? (Be sure to consider all applicable audiences, like GitLab admin and developer-user.) +- What are the benefits to using this over any alternatives? ## Use cases -Describe one to three use cases for that feature. Give real-life examples. +Describe some use cases, typically in bulleted form. Include real-life examples for each. + +If the page itself is dedicated to a use case, this section can usually include more specific scenarios +for use (e.g. variations on the main use case), but if that's not applicable, the section can be omitted. + +Examples of use cases on feature pages: +- CE and EE: [Issues](../../user/project/issues/index.md#use-cases) +- CE and EE: [Merge Requests](../../user/project/merge_requests/index.md) +- EE-only: [Geo](https://docs.gitlab.com/ee/administration/geo/replication/index.html) +- EE-only: [Jenkins integration](https://docs.gitlab.com/ee/integration/jenkins.html) ## Requirements -State any requirements, if any, for using the feature and/or following along with the tutorial. +State any requirements for using the feature and/or following along with the instructions. -The only assumption that is redundant and doesn't need to be mentioned is having an account -on GitLab. +These can include both: +- technical requirements (e.g. an account on a third party service, an amount of storage space, prior configuration of another feature) +- prerequisite knowledge (e.g. familiarity with certain GitLab features, cloud technologies) + +Link each one to an appropriate place for more information. ## Instructions -("Instructions" is not necessarily the name of the heading) +"Instructions" is usually not the name of the heading. +This is the part of the document where you can include one or more sets of instructions, each to accomplish a specific task. +Headers should describe the task the reader will achieve by following the instructions within, typically starting with a verb. +Larger instruction sets may have subsections covering specific phases of the process. - Write a step-by-step guide, with no gaps between the steps. - Start with an h2 (`##`), break complex steps into small steps using -subheadings h3 > h4 > h5 > h6. _Never skip the hierarchy level, such +subheadings h3 > h4 > h5 > h6. _Never skip a hierarchy level, such as h2 > h4_, as it will break the TOC and may affect the breadcrumbs. - Use short and descriptive headings (up to ~50 chars). You can use one -single heading `## How it works` for the instructions when the feature +single heading like `## Configuring X` for instructions when the feature is simple and the document is short. -- Be clear, concise, and stick to the goal of the doc: explain how to -use that feature. -- Use inclusive language and avoid jargons, as well as uncommon and -fancy words. The docs should be clear and easy to understand. -- Write in the 3rd person (use "we", "you", "us", "one", instead of "I" or "me"). -- Always provide internal and external reference links. -- Always link the doc from its higher-level index. <!-- ## Troubleshooting -Add a troubleshooting guide when possible/applicable. --> +Include any troubleshooting steps that you can foresee. If you know beforehand what issues +one might have when setting this up, or when something is changed, or on upgrading, it's +important to describe those, too. Think of things that may go wrong and include them here. +This is important to minimize requests for support, and to avoid doc comments with +questions that you know someone might ask. + +Each scenario can be a third-level heading, e.g. `### Getting error message X`. +If you have none to add when creating a doc, leave this section in place +but commented out to help encourage others to add to it in the future. --> --- diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index 8309ba9a72c..8760cd194d4 100644 --- a/doc/development/documentation/styleguide.md +++ b/doc/development/documentation/styleguide.md @@ -1,16 +1,16 @@ --- -description: 'Writing styles, markup, formatting, and reusing regular expressions throughout the GitLab Documentation.' +description: 'Writing styles, markup, formatting, and other standards for GitLab Documentation.' --- -# Documentation style guidelines +# Documentation Style Guide The documentation style guide defines the markup structure used in GitLab documentation. Check the [documentation guidelines](index.md) for general development instructions. -Check the GitLab handbook for the [writing styles guidelines](https://about.gitlab.com/handbook/communication/#writing-style-guidelines). +See the GitLab handbook for the [writing style guidelines](https://about.gitlab.com/handbook/communication/#writing-style-guidelines). -For help adhering to the guidelines, see [linting](index.md#linting). +For programmatic help adhering to the guidelines, see [linting](index.md#linting). ## Files @@ -40,13 +40,36 @@ Use Kramdown markup wisely: do not overuse its specific markup (e.g., `{:.class} ## Content -- Make sure that the documentation is added in the correct - [directory](index.md#documentation-directory-structure), linked from its - higher-level index, and linked from other related pages. +These guidelines help toward the goal of having every user's search of documentation +yield a useful result, and ensuring content is helpful and easy to consume. + +- What to include: + - Any and all helpful information, processes, and tips for implementing, +using, and troubleshooting GitLab features. [The documentation is the single source of truth](https://about.gitlab.com/handbook/documentation/#documentation-as-single-source-of-truth-ssot) +for this information. + - 'Risky' or niche problem-solving steps. There is no reason to withhold these or +store them elsewhere; simply include them along with the rest of the docs including all necessary +detail, such as specific warnings and caveats about potential ramifications. + - Any content types/sources, if relevant to users or admins. You can freely +include presentations, videos, etc.; no matter who it was originally written for, +if it is helpful to any of our audiences, we can include it. If an outside source +that's under copyright, rephrase, or summarize and link out; do not copy and paste. + - All applicable subsections as described on the [structure and template](structure.md) page, +with files organized in the [correct directory](index.md#documentation-directory-structure). +- To ensure discoverability, link to each doc from its higher-level index page and other related pages. +- When referencing other GitLab products and features, link to their + respective docs; when referencing third-party products or technologies, + link out to their external sites, documentation, and resources. - Do not duplicate information. -- Be brief and clear. -- Unless there's a logical reason not to, structure the document in alphabetical order -(headings, tables, and lists). +- Structure content in alphabetical order in tables, lists, etc., unless there is +a logical reason not to (for example, when mirroring the UI or an ordered sequence). + +## Language + +- Use inclusive language and avoid jargon, as well as uncommon +words. The docs should be clear and easy to understand. +- Write in the 3rd person (use "we", "you", "us", "one", instead of "I" or "me"). +- Be clear, concise, and stick to the goal of the doc. - Write in US English. - Capitalize "G" and "L" in GitLab. - Use title case when referring to [features](https://about.gitlab.com/features/) or diff --git a/doc/development/documentation/workflow.md b/doc/development/documentation/workflow.md index 75ce8640e87..7c32c92b147 100644 --- a/doc/development/documentation/workflow.md +++ b/doc/development/documentation/workflow.md @@ -1,185 +1,10 @@ --- -description: Learn the process of shipping documentation for GitLab. +description: Learn the processes for contributing to GitLab's documentation. --- -# Documentation process at GitLab +# Documentation workflows at GitLab -At GitLab, developers contribute new or updated documentation along with their code, but product managers and technical writers also have essential roles in the process. +Documentation workflows at GitLab differ depending on the reason for the change. The two types of documentation changes are: -- Product Managers (PMs): in the issue for all new and updated features, - PMs include specific documentation requirements that the developer who is - writing or updating the docs must meet, along with feature descriptions - and use cases. They call out any specific areas where collaborating with - a technical writer is recommended, and usually act as the first reviewer - of the docs. -- Developers: author documentation and merge it on time (up to a week after - the feature freeze). -- Technical Writers: review each issue to ensure PM's requirements are complete, - help developers with any questions throughout the process, and act as the final - reviewer of all new and updated docs content before it's merged. - -## Requirements - -Documentation must be delivered whenever: - -- A new feature is shipped -- There are changes to the UI -- A process, workflow, or previously documented feature is changed - -Documentation is not required when a feature is changed on the backend -only and does not directly affect the way that any regular user or -administrator would interact with GitLab. - -NOTE: **Note:** -When refactoring documentation, it should be submitted in its own MR. -**Do not** join new features' MRs with refactoring existing docs, as they might have -different priorities. - -NOTE: **Note:** -[Smaller MRs are better](https://gitlab.com/gitlab-com/blog-posts/issues/185#note_4401010)! Do not mix subjects, and ship the smallest MR possible. - -### Documentation review process - -The docs shipped by the developer should be reviewed by the PM (for accuracy) and a Technical Writer (for clarity and structure). - -#### Documentation updates that require Technical Writer review - -Every documentation change that meets the criteria below must be reviewed by a Technical Writer -to ensure clarity and discoverability, and avoid redundancy, bad file locations, typos, broken links, etc. -Within the GitLab issue or MR, ping the relevant technical writer for the subject area. If you're not sure who that is, -ping any of them or all of them (`@gl\-docsteam`). - -A Technical Writer must review documentation updates that involve: - -- Docs introducing new features -- Changing documentation location -- Refactoring existing documentation -- Creating new documentation files - -If you need any help to choose the correct place for a doc, discuss a documentation -idea or outline, or request any other help, ping a Technical Writer on your issue, MR, -or on Slack in `#docs`. - -#### Skip the PM's review - -When there's a non-significant change to the docs, you can skip the review -of the PM. Add the same labels as you would for a regular doc change and -assign the correct milestone. In these cases, assign a Technical Writer -for approval/merge, or mention `@gl\-docsteam` in case you don't know -which Tech Writer to assign for. - -#### Skip the entire review - -When the MR only contains corrections to the content (typos, grammar, -broken links, etc), it can be merged without the PM's and Tech Writer's review. - -## Documentation structure - -Read through the [documentation structure](structure.md) docs for an overview. - -## Documentation workflow - -To follow a consistent workflow every month, documentation changes -involve the Product Managers, the developer who shipped the feature, -and the Technical Writing team. Each role is described below. - -### 1. Product Manager's role in the documentation process - -The Product Manager (PM) should add to the feature issue: - -- Feature name, overview/description, and use cases, for the [documentation blurb](structure.md#documentation-blurb) -- The documentation requirements for the developer working on the docs - - What new page, new subsection of an existing page, or other update to an existing page/subsection is needed. - - Just one page/section/update or multiple (perhaps there's an end user and admin change needing docs, or we need to update a previously recommended workflow, or we want to link the new feature from various places; consider and mention all ways documentation should be affected - - Suggested title of any page or subsection, if applicable -- Label the issue with `Documentation`, `Deliverable`, `docs:P1`, and assign - the correct milestone - -### 2. Developer's role in the documentation process - -As a developer, or as a community contributor, you should ship the documentation -with the feature, as in GitLab the documentation is part of the product. - -The docs can either be shipped along with the MR introducing the code, or, -alternatively, created from a follow-up issue and MR. - -The docs should be shipped **by the feature freeze date**. Justified -exceptions are accepted, as long as the [following process](#documentation-shipped-late) -and the missed-deliverable due date (the 14th of each month) are both respected. - -#### Documentation shipped in the feature MR - -The developer should add to the feature MR the documentation containing: - -- The [documentation blurb](structure.md#documentation-blurb): copy the - feature name, overview/description, and use cases from the feature issue -- Instructions: write how to use the feature, step by step, with no gaps. -- [Crosslink for discoverability](structure.md#discoverability): link with - internal docs and external resources (if applicable) -- Index: link the new doc or the new heading from the higher-level index - for [discoverability](#discoverability) -- [Screenshots](styleguide.md#images): when necessary, add screenshots for: - - Illustrating a step of the process - - Indicating the location of a navigation menu -- Label the MR with `Documentation`, `Deliverable`, `docs-P1`, and assign - the correct milestone -- Assign the PM for review -- When done, mention the `@gl\-docsteam` in the MR asking for review -- **Due date**: feature freeze date and time - -#### Documentation shipped in a follow-up MR - -If the docs aren't being shipped within the feature MR: - -- Create a new issue mentioning "docs" or "documentation" in the title (use the Documentation issue description template) -- Label the issue with: `Documentation`, `Deliverable`, `docs-P1`, `<product-label>` - (product label == CI/CD, Pages, Prometheus, etc) -- Add the correct milestone -- Create a new MR for shipping the docs changes and follow the same - process [described above](#documentation-shipped-in-the-feature-mr) -- Use the MR description template called "Documentation" -- Add the same labels and milestone as you did for the issue -- Assign the PM for review -- When done, mention the `@gl\-docsteam` in the MR asking for review -- **Due date**: feature freeze date and time - -#### Documentation shipped late - -Shipping late means that you are affecting the whole feature workflow -as well as other teams' priorities (PMs, tech writers, release managers, -release post reviewers), so every effort should be made to avoid this. - -If you did not ship the docs within the feature freeze, proceed as -[described above](#documentation-shipped-in-a-follow-up-mr) and, -besides the regular labels, include the labels `Pick into X.Y` and -`missed-deliverable` in the issue and the MR, and assign them the correct -milestone. - -The **due date** for **merging** `missed-deliverable` MRs is on the -**14th** of each month. - -### 3. Technical Writer's role in the documentation process - -- **Planning** - - Once an issue contains a Documentation label and the current milestone, a - technical writer reviews the Product Manager's documentation requirements. - - Once the documentation requirements are approved, the technical writer can - work with the developer to discuss any documentation questions and plans/outlines, as needed. - -- **Review** - A technical writer must review the documentation for: - - Clarity - - Relevance (make sure the content is appropriate given the impact of the feature) - - Location (make sure the doc is in the correct dir and has the correct name) - - Syntax, typos, and broken links - - Improvements to the content - - Accordance to the [docs style guide](styleguide.md) - -<!-- TBA: issue and MR description templates as part of the process --> - -<!-- -## New features vs feature updates - -- TBA: - - Describe the difference between new features and feature updates - - Creating a new doc vs updating an existing doc ---> +- [Feature-change documentation workflow](feature-change-workflow.md) - The documentation is being created or updated as part of the development and release of a new or enhanced feature. This process involves the developer of the feature (who includes new/updated documentation files as part of the same merge request containing the feature's code) and also involves the product manager and technical writer who are listed for the feature's [DevOps stage](https://about.gitlab.com/handbook/product/categories/#devops-stages). +- [Documentation improvement workflow](improvement-workflow.md) - All documentation additions not associated with a feature release. Documentation is being created or updated to improve accuracy, completeness, ease of use, or any reason other than a feature change. Anyone (and everyone) can contribute a merge request for this type of change at any time. diff --git a/doc/install/installation.md b/doc/install/installation.md index d041bfa863a..b7781dea384 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -142,7 +142,7 @@ Download Ruby and compile it: Then install the Bundler Gem: - sudo gem install bundler --no-ri --no-rdoc + sudo gem install bundler --no-document ## 3. Go diff --git a/doc/integration/google.md b/doc/integration/google.md index b91d40d4bd4..d2b4e119978 100644 --- a/doc/integration/google.md +++ b/doc/integration/google.md @@ -35,7 +35,6 @@ In Google's side: 1. You should now be able to see a Client ID and Client secret. Note them down or keep this page open as you will need them later. -1. From the **Dashboard** select **ENABLE APIS AND SERVICES > Social > Google+ API > Enable** 1. To enable projects to access [Google Kubernetes Engine](../user/project/clusters/index.md), you must also enable these APIs: - Google Kubernetes Engine API diff --git a/doc/update/10.0-to-10.1.md b/doc/update/10.0-to-10.1.md index af815d26a74..10cf02a984f 100644 --- a/doc/update/10.0-to-10.1.md +++ b/doc/update/10.0-to-10.1.md @@ -49,7 +49,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/10.1-to-10.2.md b/doc/update/10.1-to-10.2.md index 632e8befa74..20895a05567 100644 --- a/doc/update/10.1-to-10.2.md +++ b/doc/update/10.1-to-10.2.md @@ -49,7 +49,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/10.2-to-10.3.md b/doc/update/10.2-to-10.3.md index f8fe4a4b6bf..441a241d053 100644 --- a/doc/update/10.2-to-10.3.md +++ b/doc/update/10.2-to-10.3.md @@ -49,7 +49,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/10.3-to-10.4.md b/doc/update/10.3-to-10.4.md index 083f6090a8a..9f3efdd790e 100644 --- a/doc/update/10.3-to-10.4.md +++ b/doc/update/10.3-to-10.4.md @@ -51,7 +51,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/10.4-to-10.5.md b/doc/update/10.4-to-10.5.md index 313419ed13d..3766645a141 100644 --- a/doc/update/10.4-to-10.5.md +++ b/doc/update/10.4-to-10.5.md @@ -51,7 +51,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/10.5-to-10.6.md b/doc/update/10.5-to-10.6.md index 2f90fb62c4a..986ecbf5ef0 100644 --- a/doc/update/10.5-to-10.6.md +++ b/doc/update/10.5-to-10.6.md @@ -51,7 +51,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/10.6-to-10.7.md b/doc/update/10.6-to-10.7.md index b9c14395a3a..10d29837bfb 100644 --- a/doc/update/10.6-to-10.7.md +++ b/doc/update/10.6-to-10.7.md @@ -51,7 +51,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/10.7-to-10.8.md b/doc/update/10.7-to-10.8.md index 7bb628f9740..0cc46fc5aa9 100644 --- a/doc/update/10.7-to-10.8.md +++ b/doc/update/10.7-to-10.8.md @@ -52,7 +52,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/10.8-to-11.0.md b/doc/update/10.8-to-11.0.md index 22a0c9f950c..ad3305d8ebd 100644 --- a/doc/update/10.8-to-11.0.md +++ b/doc/update/10.8-to-11.0.md @@ -51,7 +51,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/11.0-to-11.1.md b/doc/update/11.0-to-11.1.md index 3f10a7edb8a..5b2dd48a744 100644 --- a/doc/update/11.0-to-11.1.md +++ b/doc/update/11.0-to-11.1.md @@ -51,7 +51,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/11.1-to-11.2.md b/doc/update/11.1-to-11.2.md index 3edc7e6923e..cb09d0a2505 100644 --- a/doc/update/11.1-to-11.2.md +++ b/doc/update/11.1-to-11.2.md @@ -51,7 +51,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/11.2-to-11.3.md b/doc/update/11.2-to-11.3.md index f2b8efc3e6e..228ff6cb70e 100644 --- a/doc/update/11.2-to-11.3.md +++ b/doc/update/11.2-to-11.3.md @@ -51,7 +51,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/11.3-to-11.4.md b/doc/update/11.3-to-11.4.md index fddec45e57a..5f64bf81127 100644 --- a/doc/update/11.3-to-11.4.md +++ b/doc/update/11.3-to-11.4.md @@ -51,7 +51,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/11.4-to-11.5.md b/doc/update/11.4-to-11.5.md index 44105348d14..fd7a8e5c2ae 100644 --- a/doc/update/11.4-to-11.5.md +++ b/doc/update/11.4-to-11.5.md @@ -51,7 +51,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/11.5-to-11.6.md b/doc/update/11.5-to-11.6.md index 031abc434ca..2e9ec5d71de 100644 --- a/doc/update/11.5-to-11.6.md +++ b/doc/update/11.5-to-11.6.md @@ -51,7 +51,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/6.9-to-7.0.md b/doc/update/6.9-to-7.0.md index 27063948028..781c90e4198 100644 --- a/doc/update/6.9-to-7.0.md +++ b/doc/update/6.9-to-7.0.md @@ -47,7 +47,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 3. Get latest code diff --git a/doc/update/6.x-or-7.x-to-7.14.md b/doc/update/6.x-or-7.x-to-7.14.md index 61854b91aa2..6fcec5b7974 100644 --- a/doc/update/6.x-or-7.x-to-7.14.md +++ b/doc/update/6.x-or-7.x-to-7.14.md @@ -67,7 +67,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ## 3. Get latest code diff --git a/doc/update/7.0-to-7.1.md b/doc/update/7.0-to-7.1.md index 308e8aeb985..fb4710faad5 100644 --- a/doc/update/7.0-to-7.1.md +++ b/doc/update/7.0-to-7.1.md @@ -47,7 +47,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 3. Get latest code diff --git a/doc/update/8.10-to-8.11.md b/doc/update/8.10-to-8.11.md index df3e34f5cc6..12a465e1602 100644 --- a/doc/update/8.10-to-8.11.md +++ b/doc/update/8.10-to-8.11.md @@ -47,7 +47,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Get latest code diff --git a/doc/update/8.11-to-8.12.md b/doc/update/8.11-to-8.12.md index 9d6a1f42375..b9a7986d5ba 100644 --- a/doc/update/8.11-to-8.12.md +++ b/doc/update/8.11-to-8.12.md @@ -47,7 +47,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Get latest code diff --git a/doc/update/8.12-to-8.13.md b/doc/update/8.12-to-8.13.md index 6225dee9802..37e61794e7e 100644 --- a/doc/update/8.12-to-8.13.md +++ b/doc/update/8.12-to-8.13.md @@ -47,7 +47,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Get latest code diff --git a/doc/update/8.13-to-8.14.md b/doc/update/8.13-to-8.14.md index d2508e3f980..927f453b9bf 100644 --- a/doc/update/8.13-to-8.14.md +++ b/doc/update/8.13-to-8.14.md @@ -47,7 +47,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Get latest code diff --git a/doc/update/8.14-to-8.15.md b/doc/update/8.14-to-8.15.md index daf8d0f2ca6..d98a60d31c8 100644 --- a/doc/update/8.14-to-8.15.md +++ b/doc/update/8.14-to-8.15.md @@ -50,7 +50,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Get latest code diff --git a/doc/update/8.15-to-8.16.md b/doc/update/8.15-to-8.16.md index 3668142edd2..94b0102ed48 100644 --- a/doc/update/8.15-to-8.16.md +++ b/doc/update/8.15-to-8.16.md @@ -50,7 +50,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Get latest code diff --git a/doc/update/8.16-to-8.17.md b/doc/update/8.16-to-8.17.md index ee2e31c2aec..5a4f620a164 100644 --- a/doc/update/8.16-to-8.17.md +++ b/doc/update/8.16-to-8.17.md @@ -50,7 +50,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/8.17-to-9.0.md b/doc/update/8.17-to-9.0.md index 3c73bc573a6..38f7d22437a 100644 --- a/doc/update/8.17-to-9.0.md +++ b/doc/update/8.17-to-9.0.md @@ -49,7 +49,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/9.0-to-9.1.md b/doc/update/9.0-to-9.1.md index 7c9dacc9b90..a4d2e7be23c 100644 --- a/doc/update/9.0-to-9.1.md +++ b/doc/update/9.0-to-9.1.md @@ -49,7 +49,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/9.1-to-9.2.md b/doc/update/9.1-to-9.2.md index b815242ab4e..dd808c51985 100644 --- a/doc/update/9.1-to-9.2.md +++ b/doc/update/9.1-to-9.2.md @@ -49,7 +49,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/9.2-to-9.3.md b/doc/update/9.2-to-9.3.md index a58b12cb81c..d2bcf45a28e 100644 --- a/doc/update/9.2-to-9.3.md +++ b/doc/update/9.2-to-9.3.md @@ -49,7 +49,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/9.3-to-9.4.md b/doc/update/9.3-to-9.4.md index 0c87468334b..dae2162a964 100644 --- a/doc/update/9.3-to-9.4.md +++ b/doc/update/9.3-to-9.4.md @@ -49,7 +49,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/9.4-to-9.5.md b/doc/update/9.4-to-9.5.md index 6a655f77a55..f2811e9471f 100644 --- a/doc/update/9.4-to-9.5.md +++ b/doc/update/9.4-to-9.5.md @@ -49,7 +49,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/update/9.5-to-10.0.md b/doc/update/9.5-to-10.0.md index 7790d192a82..333a6e35714 100644 --- a/doc/update/9.5-to-10.0.md +++ b/doc/update/9.5-to-10.0.md @@ -49,7 +49,7 @@ sudo make install Install Bundler: ```bash -sudo gem install bundler --no-ri --no-rdoc +sudo gem install bundler --no-document ``` ### 4. Update Node diff --git a/doc/user/project/clusters/index.md b/doc/user/project/clusters/index.md index 6bdafc60949..6f334af4fb7 100644 --- a/doc/user/project/clusters/index.md +++ b/doc/user/project/clusters/index.md @@ -60,7 +60,7 @@ new Kubernetes cluster to your project: **Sign in with Google** button. 1. From there on, choose your cluster's settings: - **Kubernetes cluster name** - The name you wish to give the cluster. - - **Environment scope** - The [associated environment](#setting-the-environment-scope) to this cluster. + - **Environment scope** - The [associated environment](#setting-the-environment-scope-premium) to this cluster. - **Google Cloud Platform project** - Choose the project you created in your GCP console that will host the Kubernetes cluster. Learn more about [Google Cloud Platform projects](https://cloud.google.com/resource-manager/docs/creating-managing-projects). diff --git a/doc/workflow/todos.md b/doc/workflow/todos.md index f94d592d0db..830f17aa7f2 100644 --- a/doc/workflow/todos.md +++ b/doc/workflow/todos.md @@ -35,6 +35,9 @@ A Todo appears in your Todos dashboard when: - the author, or - have set it to automatically merge once pipeline succeeds. +NOTE: **Note:** +When an user no longer has access to a resource related to a Todo like an issue, merge request, project or group the related Todos, for security reasons, gets deleted within the next hour. The delete is delayed to prevent data loss in case user got their access revoked by mistake. + ### Directly addressed Todos > [Introduced][ce-7926] in GitLab 9.0. diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb index 80a5cbd6b19..45c694b6448 100644 --- a/lib/api/jobs.rb +++ b/lib/api/jobs.rb @@ -38,6 +38,8 @@ module API end # rubocop: disable CodeReuse/ActiveRecord get ':id/jobs' do + authorize_read_builds! + builds = user_project.builds.order('id DESC') builds = filter_builds(builds, params[:scope]) @@ -56,7 +58,10 @@ module API end # rubocop: disable CodeReuse/ActiveRecord get ':id/pipelines/:pipeline_id/jobs' do + authorize!(:read_pipeline, user_project) pipeline = user_project.ci_pipelines.find(params[:pipeline_id]) + authorize!(:read_build, pipeline) + builds = pipeline.builds builds = filter_builds(builds, params[:scope]) builds = builds.preload(:job_artifacts_archive, :job_artifacts, project: [:namespace]) diff --git a/lib/banzai/filter/external_link_filter.rb b/lib/banzai/filter/external_link_filter.rb index 2e6d742de27..4f60b6f84c6 100644 --- a/lib/banzai/filter/external_link_filter.rb +++ b/lib/banzai/filter/external_link_filter.rb @@ -9,11 +9,10 @@ module Banzai def call links.each do |node| uri = uri(node['href'].to_s) - next unless uri - node.set_attribute('href', uri.to_s) + node.set_attribute('href', uri.to_s) if uri - if SCHEMES.include?(uri.scheme) && external_url?(uri) + if SCHEMES.include?(uri&.scheme) && !internal_url?(uri) node.set_attribute('rel', 'nofollow noreferrer noopener') node.set_attribute('target', '_blank') end @@ -35,11 +34,12 @@ module Banzai doc.xpath(query) end - def external_url?(uri) + def internal_url?(uri) + return false if uri.nil? # Relative URLs miss a hostname - return false unless uri.hostname + return true unless uri.hostname - uri.hostname != internal_url.hostname + uri.hostname == internal_url.hostname end def internal_url diff --git a/lib/banzai/filter/label_reference_filter.rb b/lib/banzai/filter/label_reference_filter.rb index 04ec38209c7..f90a35952e5 100644 --- a/lib/banzai/filter/label_reference_filter.rb +++ b/lib/banzai/filter/label_reference_filter.rb @@ -29,7 +29,7 @@ module Banzai if label yield match, label.id, project, namespace, $~ else - match + escape_html_entities(match) end end end @@ -102,6 +102,10 @@ module Banzai CGI.unescapeHTML(text.to_s) end + def escape_html_entities(text) + CGI.escapeHTML(text.to_s) + end + def object_link_title(object, matches) # use title of wrapped element instead nil diff --git a/lib/gitlab/ci/pipeline/chain/command.rb b/lib/gitlab/ci/pipeline/chain/command.rb index 100b9521412..90208352c55 100644 --- a/lib/gitlab/ci/pipeline/chain/command.rb +++ b/lib/gitlab/ci/pipeline/chain/command.rb @@ -54,7 +54,13 @@ module Gitlab def protected_ref? strong_memoize(:protected_ref) do - project.protected_for?(ref) + project.protected_for?(origin_ref) + end + end + + def ambiguous_ref? + strong_memoize(:ambiguous_ref) do + project.repository.ambiguous_ref?(origin_ref) end end end diff --git a/lib/gitlab/ci/pipeline/chain/validate/repository.rb b/lib/gitlab/ci/pipeline/chain/validate/repository.rb index d88851d8245..9c6c2bc8e25 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/repository.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/repository.rb @@ -16,6 +16,10 @@ module Gitlab unless @command.sha return error('Commit not found') end + + if @command.ambiguous_ref? + return error('Ref is ambiguous') + end end def break? diff --git a/lib/gitlab/ci/templates/Ruby.gitlab-ci.yml b/lib/gitlab/ci/templates/Ruby.gitlab-ci.yml index 93cb31f48c0..0d12cbc6460 100644 --- a/lib/gitlab/ci/templates/Ruby.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Ruby.gitlab-ci.yml @@ -24,7 +24,6 @@ before_script: - ruby -v # Print out ruby version for debugging # Uncomment next line if your rails app needs a JS runtime: # - apt-get update -q && apt-get install nodejs -yqq - - gem install bundler --no-ri --no-rdoc # Bundler is not installed with the image - bundle install -j $(nproc) --path vendor # Install dependencies into ./vendor/ruby # Optional - Delete if not using `rubocop` diff --git a/lib/gitlab/cleanup/remote_uploads.rb b/lib/gitlab/cleanup/remote_uploads.rb index eba1faacc3a..03298d960a4 100644 --- a/lib/gitlab/cleanup/remote_uploads.rb +++ b/lib/gitlab/cleanup/remote_uploads.rb @@ -67,7 +67,7 @@ module Gitlab end def remote_directory - connection.directories.get(configuration['remote_directory']) + connection.directories.new(key: configuration['remote_directory']) end def connection diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb index c4aac228b2f..44a62586a23 100644 --- a/lib/gitlab/git.rb +++ b/lib/gitlab/git.rb @@ -54,11 +54,11 @@ module Gitlab end def tag_ref?(ref) - ref.start_with?(TAG_REF_PREFIX) + ref =~ /^#{TAG_REF_PREFIX}.+/ end def branch_ref?(ref) - ref.start_with?(BRANCH_REF_PREFIX) + ref =~ /^#{BRANCH_REF_PREFIX}.+/ end def blank_ref?(ref) diff --git a/lib/json_web_token/hmac_token.rb b/lib/json_web_token/hmac_token.rb index ceb1b9c913f..ec0917ab49d 100644 --- a/lib/json_web_token/hmac_token.rb +++ b/lib/json_web_token/hmac_token.rb @@ -18,7 +18,7 @@ module JSONWebToken end def encoded - JWT.encode(payload, secret, JWT_ALGORITHM) + JWT.encode(payload, secret, JWT_ALGORITHM, { typ: 'JWT' }) end private diff --git a/lib/json_web_token/rsa_token.rb b/lib/json_web_token/rsa_token.rb index 160e1e506f1..bcce811cd28 100644 --- a/lib/json_web_token/rsa_token.rb +++ b/lib/json_web_token/rsa_token.rb @@ -11,7 +11,8 @@ module JSONWebToken def encoded headers = { - kid: kid + kid: kid, + typ: 'JWT' } JWT.encode(payload, key, 'RS256', headers) end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 88eff3b6645..ddfd5599883 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4416,6 +4416,9 @@ msgstr "" msgid "No" msgstr "" +msgid "No activities found" +msgstr "" + msgid "No assignee" msgstr "" @@ -6239,7 +6242,7 @@ msgstr "" msgid "SortOptions|Least popular" msgstr "" -msgid "SortOptions|Milestone" +msgid "SortOptions|Milestone due date" msgstr "" msgid "SortOptions|Milestone due later" diff --git a/qa/Dockerfile b/qa/Dockerfile index 9956ced0ef6..ca7f9accb70 100644 --- a/qa/Dockerfile +++ b/qa/Dockerfile @@ -1,4 +1,4 @@ -FROM ruby:2.4-stretch +FROM ruby:2.5-stretch LABEL maintainer "Grzegorz Bizon <grzegorz@gitlab.com>" ENV DEBIAN_FRONTEND noninteractive diff --git a/qa/qa/support/page/logging.rb b/qa/qa/support/page/logging.rb index df3b794b14b..cfccbb910b7 100644 --- a/qa/qa/support/page/logging.rb +++ b/qa/qa/support/page/logging.rb @@ -37,8 +37,11 @@ module QA exists end - def find_element(name, wait: Capybara.default_max_wait_time) - log("finding :#{name} (wait: #{wait})") + def find_element(name, text_filter = nil, wait: Capybara.default_max_wait_time) + msg = ["finding :#{name}"] + msg << %Q(with text_filter "#{text_filter}") if text_filter + msg << "(wait: #{wait})" + log(msg.compact.join(' ')) element = super diff --git a/qa/spec/page/logging_spec.rb b/qa/spec/page/logging_spec.rb index a54ff424f53..f108a5ca318 100644 --- a/qa/spec/page/logging_spec.rb +++ b/qa/spec/page/logging_spec.rb @@ -47,6 +47,15 @@ describe QA::Support::Page::Logging do it 'logs find_element' do expect { subject.find_element(:element) } + .to output(/finding :element/).to_stdout_from_any_process + expect { subject.find_element(:element) } + .to output(/found :element/).to_stdout_from_any_process + end + + it 'logs find_element with text_filter' do + expect { subject.find_element(:element, 'foo') } + .to output(/finding :element with text_filter "foo"/).to_stdout_from_any_process + expect { subject.find_element(:element, 'foo') } .to output(/found :element/).to_stdout_from_any_process end diff --git a/spec/controllers/groups/settings/ci_cd_controller_spec.rb b/spec/controllers/groups/settings/ci_cd_controller_spec.rb index b7f04f732b9..40673d10b91 100644 --- a/spec/controllers/groups/settings/ci_cd_controller_spec.rb +++ b/spec/controllers/groups/settings/ci_cd_controller_spec.rb @@ -5,30 +5,65 @@ describe Groups::Settings::CiCdController do let(:user) { create(:user) } before do - group.add_maintainer(user) sign_in(user) end describe 'GET #show' do - it 'renders show with 200 status code' do - get :show, params: { group_id: group } + context 'when user is owner' do + before do + group.add_owner(user) + end - expect(response).to have_gitlab_http_status(200) - expect(response).to render_template(:show) + it 'renders show with 200 status code' do + get :show, params: { group_id: group } + + expect(response).to have_gitlab_http_status(200) + expect(response).to render_template(:show) + end + end + + context 'when user is not owner' do + before do + group.add_maintainer(user) + end + + it 'renders a 404' do + get :show, params: { group_id: group } + + expect(response).to have_gitlab_http_status(404) + end end end describe 'PUT #reset_registration_token' do subject { put :reset_registration_token, params: { group_id: group } } - it 'resets runner registration token' do - expect { subject }.to change { group.reload.runners_token } + context 'when user is owner' do + before do + group.add_owner(user) + end + + it 'resets runner registration token' do + expect { subject }.to change { group.reload.runners_token } + end + + it 'redirects the user to admin runners page' do + subject + + expect(response).to redirect_to(group_settings_ci_cd_path) + end end - it 'redirects the user to admin runners page' do - subject + context 'when user is not owner' do + before do + group.add_maintainer(user) + end + + it 'renders a 404' do + subject - expect(response).to redirect_to(group_settings_ci_cd_path) + expect(response).to have_gitlab_http_status(404) + end end end end diff --git a/spec/controllers/projects/ci/lints_controller_spec.rb b/spec/controllers/projects/ci/lints_controller_spec.rb index 82c1374aa4f..cfa010c2d1c 100644 --- a/spec/controllers/projects/ci/lints_controller_spec.rb +++ b/spec/controllers/projects/ci/lints_controller_spec.rb @@ -51,7 +51,6 @@ describe Projects::Ci::LintsController do - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - ruby -v - which ruby - - gem install bundler --no-ri --no-rdoc - bundle install --jobs $(nproc) "${FLAGS[@]}" HEREDOC end diff --git a/spec/controllers/projects/snippets_controller_spec.rb b/spec/controllers/projects/snippets_controller_spec.rb index 1a3fb4da15f..e4b78aff25d 100644 --- a/spec/controllers/projects/snippets_controller_spec.rb +++ b/spec/controllers/projects/snippets_controller_spec.rb @@ -379,6 +379,46 @@ describe Projects::SnippetsController do end end + describe "GET #show for embeddable content" do + let(:project_snippet) { create(:project_snippet, snippet_permission, project: project, author: user) } + + before do + sign_in(user) + + get :show, namespace_id: project.namespace, project_id: project, id: project_snippet.to_param, format: :js + end + + context 'when snippet is private' do + let(:snippet_permission) { :private } + + it 'responds with status 404' do + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when snippet is public' do + let(:snippet_permission) { :public } + + it 'responds with status 200' do + expect(assigns(:snippet)).to eq(project_snippet) + expect(response).to have_gitlab_http_status(200) + end + end + + context 'when the project is private' do + let(:project) { create(:project_empty_repo, :private) } + + context 'when snippet is public' do + let(:project_snippet) { create(:project_snippet, :public, project: project, author: user) } + + it 'responds with status 404' do + expect(assigns(:snippet)).to eq(project_snippet) + expect(response).to have_gitlab_http_status(404) + end + end + end + end + describe 'GET #raw' do let(:project_snippet) do create( diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index ea067a01295..4747d837273 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -621,10 +621,10 @@ describe ProjectsController do end describe "GET refs" do - let(:public_project) { create(:project, :public, :repository) } + let(:project) { create(:project, :public, :repository) } it 'gets a list of branches and tags' do - get :refs, params: { namespace_id: public_project.namespace, id: public_project, sort: 'updated_desc' } + get :refs, params: { namespace_id: project.namespace, id: project, sort: 'updated_desc' } parsed_body = JSON.parse(response.body) expect(parsed_body['Branches']).to include('master') @@ -634,7 +634,7 @@ describe ProjectsController do end it "gets a list of branches, tags and commits" do - get :refs, params: { namespace_id: public_project.namespace, id: public_project, ref: "123456" } + get :refs, params: { namespace_id: project.namespace, id: project, ref: "123456" } parsed_body = JSON.parse(response.body) expect(parsed_body["Branches"]).to include("master") @@ -649,7 +649,7 @@ describe ProjectsController do end it "gets a list of branches, tags and commits" do - get :refs, params: { namespace_id: public_project.namespace, id: public_project, ref: "123456" } + get :refs, params: { namespace_id: project.namespace, id: project, ref: "123456" } parsed_body = JSON.parse(response.body) expect(parsed_body["Branches"]).to include("master") @@ -657,6 +657,22 @@ describe ProjectsController do expect(parsed_body["Commits"]).to include("123456") end end + + context 'when private project' do + let(:project) { create(:project, :repository) } + + context 'as a guest' do + it 'renders forbidden' do + user = create(:user) + project.add_guest(user) + + sign_in(user) + get :refs, namespace_id: project.namespace, id: project + + expect(response).to have_gitlab_http_status(404) + end + end + end end describe 'POST #preview_markdown' do diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index d2a56518f65..d762531da7e 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -80,6 +80,12 @@ describe SnippetsController do expect(assigns(:snippet)).to eq(personal_snippet) expect(response).to have_gitlab_http_status(200) end + + it 'responds with status 404 when embeddable content is requested' do + get :show, id: personal_snippet.to_param, format: :js + + expect(response).to have_gitlab_http_status(404) + end end end @@ -106,6 +112,12 @@ describe SnippetsController do expect(assigns(:snippet)).to eq(personal_snippet) expect(response).to have_gitlab_http_status(200) end + + it 'responds with status 404 when embeddable content is requested' do + get :show, id: personal_snippet.to_param, format: :js + + expect(response).to have_gitlab_http_status(404) + end end context 'when not signed in' do @@ -131,6 +143,13 @@ describe SnippetsController do expect(assigns(:snippet)).to eq(personal_snippet) expect(response).to have_gitlab_http_status(200) end + + it 'responds with status 200 when embeddable content is requested' do + get :show, id: personal_snippet.to_param, format: :js + + expect(assigns(:snippet)).to eq(personal_snippet) + expect(response).to have_gitlab_http_status(200) + end end context 'when not signed in' do diff --git a/spec/features/group_variables_spec.rb b/spec/features/group_variables_spec.rb index 89e0cdd8ed7..57e3ddfb39c 100644 --- a/spec/features/group_variables_spec.rb +++ b/spec/features/group_variables_spec.rb @@ -7,7 +7,7 @@ describe 'Group variables', :js do let(:page_path) { group_settings_ci_cd_path(group) } before do - group.add_maintainer(user) + group.add_owner(user) gitlab_sign_in(user) visit page_path diff --git a/spec/features/issues/gfm_autocomplete_spec.rb b/spec/features/issues/gfm_autocomplete_spec.rb index d7531d5fcd9..3b7a17ef355 100644 --- a/spec/features/issues/gfm_autocomplete_spec.rb +++ b/spec/features/issues/gfm_autocomplete_spec.rb @@ -3,6 +3,8 @@ require 'rails_helper' describe 'GFM autocomplete', :js do let(:issue_xss_title) { 'This will execute alert<img src=x onerror=alert(2)<img src=x onerror=alert(1)>' } let(:user_xss_title) { 'eve <img src=x onerror=alert(2)<img src=x onerror=alert(1)>' } + let(:label_xss_title) { 'alert label <img src=x onerror="alert(\'Hello xss\');" a'} + let(:milestone_xss_title) { 'alert milestone <img src=x onerror="alert(\'Hello xss\');" a' } let(:user_xss) { create(:user, name: user_xss_title, username: 'xss.user') } let(:user) { create(:user, name: '💃speciąl someone💃', username: 'someone.special') } @@ -25,10 +27,14 @@ describe 'GFM autocomplete', :js do simulate_input('#issue-description', "@#{user.name[0...3]}") + wait_for_requests + find('.atwho-view .cur').click click_button 'Save changes' + wait_for_requests + expect(find('.description')).to have_content(user.to_reference) end @@ -47,6 +53,8 @@ describe 'GFM autocomplete', :js do find('#note-body').native.send_keys('#') end + wait_for_requests + expect(page).to have_selector('.atwho-container') page.within '.atwho-container #at-view-issues' do @@ -59,6 +67,8 @@ describe 'GFM autocomplete', :js do find('#note-body').native.send_keys('@ev') end + wait_for_requests + expect(page).to have_selector('.atwho-container') page.within '.atwho-container #at-view-users' do @@ -66,6 +76,22 @@ describe 'GFM autocomplete', :js do end end + it 'opens autocomplete menu for Milestone when field starts with text with item escaping HTML characters' do + create(:milestone, project: project, title: milestone_xss_title) + + page.within '.timeline-content-form' do + find('#note-body').native.send_keys('%') + end + + wait_for_requests + + expect(page).to have_selector('.atwho-container') + + page.within '.atwho-container #at-view-milestones' do + expect(find('li').text).to have_content('alert milestone') + end + end + it 'doesnt open autocomplete menu character is prefixed with text' do page.within '.timeline-content-form' do find('#note-body').native.send_keys('testing') @@ -258,12 +284,28 @@ describe 'GFM autocomplete', :js do let!(:bug) { create(:label, project: project, title: 'bug') } let!(:feature_proposal) { create(:label, project: project, title: 'feature proposal') } + it 'opens autocomplete menu for Labels when field starts with text with item escaping HTML characters' do + create(:label, project: project, title: label_xss_title) + + note = find('#note-body') + + # It should show all the labels on "~". + type(note, '~') + + wait_for_requests + + page.within '.atwho-container #at-view-labels' do + expect(find('.atwho-view-ul').text).to have_content('alert label') + end + end + context 'when no labels are assigned' do it 'shows labels' do note = find('#note-body') # It should show all the labels on "~". type(note, '~') + wait_for_requests expect_labels(shown: [backend, bug, feature_proposal]) # It should show all the labels on "/label ~". @@ -290,6 +332,7 @@ describe 'GFM autocomplete', :js do # It should show all the labels on "~". type(note, '~') + wait_for_requests expect_labels(shown: [backend, bug, feature_proposal]) # It should show only unset labels on "/label ~". @@ -316,6 +359,7 @@ describe 'GFM autocomplete', :js do # It should show all the labels on "~". type(note, '~') + wait_for_requests expect_labels(shown: [backend, bug, feature_proposal]) # It should show no labels on "/label ~". diff --git a/spec/features/merge_request/user_sees_versions_spec.rb b/spec/features/merge_request/user_sees_versions_spec.rb index f7512294bef..63d8decc2d2 100644 --- a/spec/features/merge_request/user_sees_versions_spec.rb +++ b/spec/features/merge_request/user_sees_versions_spec.rb @@ -64,6 +64,17 @@ describe 'Merge request > User sees versions', :js do end end + it 'shows the commit SHAs for every version in the dropdown' do + page.within '.mr-version-dropdown' do + find('.btn-default').click + + page.within('.dropdown-content') do + shas = merge_request.merge_request_diffs.map { |diff| Commit.truncate_sha(diff.head_commit_sha) } + shas.each { |sha| expect(page).to have_content(sha) } + end + end + end + it 'shows comments that were last relevant at that version' do expect(page).to have_content '5 changed files' diff --git a/spec/features/merge_request/user_tries_to_access_private_repository_through_new_mr_spec.rb b/spec/features/merge_request/user_tries_to_access_private_repository_through_new_mr_spec.rb new file mode 100644 index 00000000000..9318b5f1ebb --- /dev/null +++ b/spec/features/merge_request/user_tries_to_access_private_repository_through_new_mr_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe 'Merge Request > Tries to access private repo of public project' do + let(:current_user) { create(:user) } + let(:private_project) do + create(:project, :public, :repository, + path: 'nothing-to-see-here', + name: 'nothing to see here', + repository_access_level: ProjectFeature::PRIVATE) + end + let(:owned_project) do + create(:project, :public, :repository, + namespace: current_user.namespace, + creator: current_user) + end + + context 'when the user enters the querystring info for the other project' do + let(:mr_path) do + project_new_merge_request_diffs_path( + owned_project, + merge_request: { + source_project_id: private_project.id, + source_branch: 'feature' + } + ) + end + + before do + sign_in current_user + visit mr_path + end + + it "does not mention the project the user can't see the repo of" do + expect(page).not_to have_content('nothing-to-see-here') + end + end +end diff --git a/spec/features/runners_spec.rb b/spec/features/runners_spec.rb index cb7a912946c..09de983f669 100644 --- a/spec/features/runners_spec.rb +++ b/spec/features/runners_spec.rb @@ -259,8 +259,9 @@ describe 'Runners' do context 'group runners in group settings' do let(:group) { create(:group) } + before do - group.add_maintainer(user) + group.add_owner(user) end context 'group with no runners' do diff --git a/spec/features/users/overview_spec.rb b/spec/features/users/overview_spec.rb index 873de85708a..8748230fa0c 100644 --- a/spec/features/users/overview_spec.rb +++ b/spec/features/users/overview_spec.rb @@ -33,6 +33,8 @@ describe 'Overview tab on a user profile', :js do it 'does not show any entries in the list of activities' do page.within('.activities-block') do + expect(page).to have_selector('.loading', visible: false) + expect(page).to have_content('No activities found') expect(page).not_to have_selector('.event-item') end end @@ -93,6 +95,7 @@ describe 'Overview tab on a user profile', :js do it 'it shows an empty project list with an info message' do page.within('.projects-block') do + expect(page).to have_selector('.loading', visible: false) expect(page).to have_content('No projects found') expect(page).not_to have_selector('.project-row') end diff --git a/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml b/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml index 0bab94a7c2e..1e88cd120aa 100644 --- a/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml +++ b/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml @@ -2,7 +2,6 @@ before_script: - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - ruby -v - which ruby - - gem install bundler --no-ri --no-rdoc - bundle install --jobs $(nproc) "${FLAGS[@]}" rspec: diff --git a/spec/javascripts/notes/components/discussion_filter_spec.js b/spec/javascripts/notes/components/discussion_filter_spec.js index 9070d968cfd..5efcab436e4 100644 --- a/spec/javascripts/notes/components/discussion_filter_spec.js +++ b/spec/javascripts/notes/components/discussion_filter_spec.js @@ -7,8 +7,9 @@ import { discussionFiltersMock, discussionMock } from '../mock_data'; describe('DiscussionFilter component', () => { let vm; let store; + let eventHub; - beforeEach(() => { + const mountComponent = () => { store = createStore(); const discussions = [ @@ -22,7 +23,7 @@ describe('DiscussionFilter component', () => { const selectedValue = discussionFiltersMock[0].value; store.state.discussions = discussions; - vm = mountComponentWithStore(Component, { + return mountComponentWithStore(Component, { el: null, store, props: { @@ -30,6 +31,11 @@ describe('DiscussionFilter component', () => { selectedValue, }, }); + }; + + beforeEach(() => { + window.mrTabs = undefined; + vm = mountComponent(); }); afterEach(() => { @@ -83,4 +89,30 @@ describe('DiscussionFilter component', () => { expect(defaultFilter.lastChild.classList).toContain('dropdown-divider'); }); + + describe('Merge request tabs', () => { + eventHub = new Vue(); + + beforeEach(() => { + window.mrTabs = { + eventHub, + currentTab: 'show', + }; + + vm = mountComponent(); + }); + + afterEach(() => { + window.mrTabs = undefined; + }); + + it('only renders when discussion tab is active', done => { + eventHub.$emit('MergeRequestTabChange', 'commit'); + + vm.$nextTick(() => { + expect(vm.$el.querySelector).toBeUndefined(); + done(); + }); + }); + }); }); diff --git a/spec/lib/banzai/filter/external_link_filter_spec.rb b/spec/lib/banzai/filter/external_link_filter_spec.rb index 2a3c0cd78b8..e6dae8d5382 100644 --- a/spec/lib/banzai/filter/external_link_filter_spec.rb +++ b/spec/lib/banzai/filter/external_link_filter_spec.rb @@ -49,16 +49,16 @@ describe Banzai::Filter::ExternalLinkFilter do end context 'for invalid urls' do - it 'skips broken hrefs' do + it 'adds rel and target attributes to broken hrefs' do doc = filter %q(<p><a href="don't crash on broken urls">Google</a></p>) - expected = %q(<p><a href="don't%20crash%20on%20broken%20urls">Google</a></p>) + expected = %q(<p><a href="don't%20crash%20on%20broken%20urls" rel="nofollow noreferrer noopener" target="_blank">Google</a></p>) expect(doc.to_html).to eq(expected) end - it 'skips improperly formatted mailtos' do + it 'adds rel and target to improperly formatted mailtos' do doc = filter %q(<p><a href="mailto://jblogs@example.com">Email</a></p>) - expected = %q(<p><a href="mailto://jblogs@example.com">Email</a></p>) + expected = %q(<p><a href="mailto://jblogs@example.com" rel="nofollow noreferrer noopener" target="_blank">Email</a></p>) expect(doc.to_html).to eq(expected) end diff --git a/spec/lib/banzai/filter/label_reference_filter_spec.rb b/spec/lib/banzai/filter/label_reference_filter_spec.rb index 00257ed7904..9cfdb9e53a2 100644 --- a/spec/lib/banzai/filter/label_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/label_reference_filter_spec.rb @@ -236,6 +236,24 @@ describe Banzai::Filter::LabelReferenceFilter do end end + context 'References with html entities' do + let!(:label) { create(:label, name: '<html>', project: project) } + + it 'links to a valid reference' do + doc = reference_filter('See ~"<html>"') + + expect(doc.css('a').first.attr('href')).to eq urls + .project_issues_url(project, label_name: label.name) + expect(doc.text).to eq 'See <html>' + end + + it 'ignores invalid label names and escapes entities' do + act = %(Label #{Label.reference_prefix}"<non valid>") + + expect(reference_filter(act).to_html).to eq act + end + end + describe 'consecutive references' do let(:bug) { create(:label, name: 'bug', project: project) } let(:feature_proposal) { create(:label, name: 'feature proposal', project: project) } diff --git a/spec/lib/gitlab/ci/config/external/file/local_spec.rb b/spec/lib/gitlab/ci/config/external/file/local_spec.rb index 541deb13b97..645db642e29 100644 --- a/spec/lib/gitlab/ci/config/external/file/local_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/local_spec.rb @@ -44,7 +44,6 @@ describe Gitlab::Ci::Config::External::File::Local do - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - ruby -v - which ruby - - gem install bundler --no-ri --no-rdoc - bundle install --jobs $(nproc) "${FLAGS[@]}" HEREDOC end diff --git a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb index 7c1a1c38736..eaf621e4140 100644 --- a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb @@ -11,7 +11,6 @@ describe Gitlab::Ci::Config::External::File::Remote do - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - ruby -v - which ruby - - gem install bundler --no-ri --no-rdoc - bundle install --jobs $(nproc) "${FLAGS[@]}" HEREDOC end diff --git a/spec/lib/gitlab/ci/config/external/processor_spec.rb b/spec/lib/gitlab/ci/config/external/processor_spec.rb index 1a05f716247..dbd28e9745c 100644 --- a/spec/lib/gitlab/ci/config/external/processor_spec.rb +++ b/spec/lib/gitlab/ci/config/external/processor_spec.rb @@ -51,7 +51,6 @@ describe Gitlab::Ci::Config::External::Processor do - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - ruby -v - which ruby - - gem install bundler --no-ri --no-rdoc - bundle install --jobs $(nproc) "${FLAGS[@]}" rspec: @@ -86,7 +85,6 @@ describe Gitlab::Ci::Config::External::Processor do - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - ruby -v - which ruby - - gem install bundler --no-ri --no-rdoc - bundle install --jobs $(nproc) "${FLAGS[@]}" HEREDOC end diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 975e11e8cc1..ea6f1e20014 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -170,7 +170,6 @@ describe Gitlab::Ci::Config do before_script_values = [ "apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs", "ruby -v", "which ruby", - "gem install bundler --no-ri --no-rdoc", "bundle install --jobs $(nproc) \"${FLAGS[@]}\"" ] variables = { diff --git a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb index 75a177d2d1f..6aa802ce6fd 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb @@ -182,4 +182,24 @@ describe Gitlab::Ci::Pipeline::Chain::Command do it { is_expected.to eq(false) } end end + + describe '#ambiguous_ref' do + let(:project) { create(:project, :repository) } + let(:command) { described_class.new(project: project, origin_ref: 'ref') } + + subject { command.ambiguous_ref? } + + context 'when ref is not ambiguous' do + it { is_expected. to eq(false) } + end + + context 'when ref is ambiguous' do + before do + project.repository.add_tag(project.creator, 'ref', 'master') + project.repository.add_branch(project.creator, 'ref', 'master') + end + + it { is_expected. to eq(true) } + end + end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb index 284aed91e29..1b014ecfaa4 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb @@ -14,6 +14,7 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do Gitlab::Ci::Pipeline::Chain::Command.new( project: project, current_user: user, + origin_ref: 'master', seeds_block: nil) end @@ -106,6 +107,7 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do Gitlab::Ci::Pipeline::Chain::Command.new( project: project, current_user: user, + origin_ref: 'master', seeds_block: seeds_block) end diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb index fb1b53fc55c..a7cad423d09 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb @@ -42,6 +42,27 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do end end + context 'when ref is ambiguous' do + let(:project) do + create(:project, :repository).tap do |proj| + proj.repository.add_tag(user, 'master', 'master') + end + end + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: 'master') + end + + it 'breaks the chain' do + expect(step.break?).to be true + end + + it 'adds an error about missing ref' do + expect(pipeline.errors.to_a) + .to include 'Ref is ambiguous' + end + end + context 'when does not have existing SHA set' do let(:command) do Gitlab::Ci::Pipeline::Chain::Command.new( diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb index fffa727c2ed..2cf812b26dc 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb @@ -1,7 +1,8 @@ require 'spec_helper' describe Gitlab::Ci::Pipeline::Seed::Build do - let(:pipeline) { create(:ci_empty_pipeline) } + let(:project) { create(:project, :repository) } + let(:pipeline) { create(:ci_empty_pipeline, project: project) } let(:attributes) do { name: 'rspec', diff --git a/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb index 05ce3412fd8..82f741845db 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb @@ -1,7 +1,8 @@ require 'spec_helper' describe Gitlab::Ci::Pipeline::Seed::Stage do - let(:pipeline) { create(:ci_empty_pipeline) } + let(:project) { create(:project, :repository) } + let(:pipeline) { create(:ci_empty_pipeline, project: project) } let(:attributes) do { name: 'test', diff --git a/spec/lib/gitlab/cleanup/remote_uploads_spec.rb b/spec/lib/gitlab/cleanup/remote_uploads_spec.rb index 8d03baeb07b..35642cd6e50 100644 --- a/spec/lib/gitlab/cleanup/remote_uploads_spec.rb +++ b/spec/lib/gitlab/cleanup/remote_uploads_spec.rb @@ -25,7 +25,7 @@ describe Gitlab::Cleanup::RemoteUploads do expect(::Fog::Storage).to receive(:new).and_return(connection) - expect(connection).to receive(:directories).and_return(double(get: directory)) + expect(connection).to receive(:directories).and_return(double(new: directory)) expect(directory).to receive(:files).and_return(remote_files) end diff --git a/spec/lib/json_web_token/rsa_token_spec.rb b/spec/lib/json_web_token/rsa_token_spec.rb index d6edc964844..a3c54651e80 100644 --- a/spec/lib/json_web_token/rsa_token_spec.rb +++ b/spec/lib/json_web_token/rsa_token_spec.rb @@ -25,7 +25,7 @@ describe JSONWebToken::RSAToken do rsa_token['key'] = 'value' end - subject { JWT.decode(rsa_encoded, rsa_key) } + subject { JWT.decode(rsa_encoded, rsa_key, true, { algorithm: 'RS256' }) } it { expect {subject}.not_to raise_error } it { expect(subject.first).to include('key' => 'value') } @@ -39,7 +39,7 @@ describe JSONWebToken::RSAToken do context 'for invalid key to raise an exception' do let(:new_key) { OpenSSL::PKey::RSA.generate(512) } - subject { JWT.decode(rsa_encoded, new_key) } + subject { JWT.decode(rsa_encoded, new_key, true, { algorithm: 'RS256' }) } it { expect {subject}.to raise_error(JWT::DecodeError) } end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index fe7f5f8e1e3..7baf4d93804 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2386,6 +2386,8 @@ describe Ci::Build do end context 'when protected variable is defined' do + let(:ref) { Gitlab::Git::BRANCH_REF_PREFIX + build.ref } + let(:protected_variable) do { key: 'PROTECTED_KEY', value: 'protected_value', public: false } end @@ -2398,7 +2400,7 @@ describe Ci::Build do context 'when the branch is protected' do before do - allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true) + allow(build.project).to receive(:protected_for?).with(ref).and_return(true) end it { is_expected.to include(protected_variable) } @@ -2406,7 +2408,7 @@ describe Ci::Build do context 'when the tag is protected' do before do - allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true) + allow(build.project).to receive(:protected_for?).with(ref).and_return(true) end it { is_expected.to include(protected_variable) } @@ -2431,6 +2433,8 @@ describe Ci::Build do end context 'when group protected variable is defined' do + let(:ref) { Gitlab::Git::BRANCH_REF_PREFIX + build.ref } + let(:protected_variable) do { key: 'PROTECTED_KEY', value: 'protected_value', public: false } end @@ -2443,7 +2447,7 @@ describe Ci::Build do context 'when the branch is protected' do before do - allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true) + allow(build.project).to receive(:protected_for?).with(ref).and_return(true) end it { is_expected.to include(protected_variable) } @@ -2451,7 +2455,7 @@ describe Ci::Build do context 'when the tag is protected' do before do - allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true) + allow(build.project).to receive(:protected_for?).with(ref).and_return(true) end it { is_expected.to include(protected_variable) } diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index b67c6a4cffa..17f33785fda 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -397,6 +397,10 @@ describe Ci::Pipeline, :mailer do end describe '#protected_ref?' do + before do + pipeline.project = create(:project, :repository) + end + it 'delegates method to project' do expect(pipeline).not_to be_protected_ref end diff --git a/spec/models/concerns/has_ref_spec.rb b/spec/models/concerns/has_ref_spec.rb new file mode 100644 index 00000000000..8aed72d77a4 --- /dev/null +++ b/spec/models/concerns/has_ref_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe HasRef do + describe '#branch?' do + let(:build) { create(:ci_build) } + + subject { build.branch? } + + context 'is not a tag' do + before do + build.tag = false + end + + it 'return true when tag is set to false' do + is_expected.to be_truthy + end + end + + context 'is not a tag' do + before do + build.tag = true + end + + it 'return false when tag is set to true' do + is_expected.to be_falsey + end + end + end + + describe '#git_ref' do + subject { build.git_ref } + + context 'when tag is true' do + let(:build) { create(:ci_build, tag: true) } + + it 'returns a tag ref' do + is_expected.to start_with(Gitlab::Git::TAG_REF_PREFIX) + end + end + + context 'when tag is false' do + let(:build) { create(:ci_build, tag: false) } + + it 'returns a branch ref' do + is_expected.to start_with(Gitlab::Git::BRANCH_REF_PREFIX) + end + end + + context 'when tag is nil' do + let(:build) { create(:ci_build, tag: nil) } + + it 'returns a branch ref' do + is_expected.to start_with(Gitlab::Git::BRANCH_REF_PREFIX) + end + end + end +end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 81748681528..a64720f1876 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -243,6 +243,20 @@ describe Event do expect(event.visible_to_user?(admin)).to eq true end end + + context 'private project' do + let(:project) { create(:project, :private) } + let(:target) { note_on_issue } + + it do + expect(event.visible_to_user?(non_member)).to eq false + expect(event.visible_to_user?(author)).to eq false + expect(event.visible_to_user?(assignee)).to eq false + expect(event.visible_to_user?(member)).to eq true + expect(event.visible_to_user?(guest)).to eq true + expect(event.visible_to_user?(admin)).to eq true + end + end end context 'merge request diff note event' do @@ -265,8 +279,8 @@ describe Event do it do expect(event.visible_to_user?(non_member)).to eq false - expect(event.visible_to_user?(author)).to eq true - expect(event.visible_to_user?(assignee)).to eq true + expect(event.visible_to_user?(author)).to eq false + expect(event.visible_to_user?(assignee)).to eq false expect(event.visible_to_user?(member)).to eq true expect(event.visible_to_user?(guest)).to eq false expect(event.visible_to_user?(admin)).to eq true diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index a01f76a5bab..4b6592020c1 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -299,6 +299,13 @@ describe Project do expect(project.errors[:import_url].first).to include('Requests to localhost are not allowed') end + it 'does not allow import_url pointing to the local network' do + project = build(:project, import_url: 'https://192.168.1.1') + + expect(project).to be_invalid + expect(project.errors[:import_url].first).to include('Requests to the local network are not allowed') + end + it "does not allow import_url with invalid ports for new projects" do project = build(:project, import_url: 'http://github.com:25/t.git') @@ -2543,6 +2550,10 @@ describe Project do end context 'when the ref is not protected' do + before do + allow(project).to receive(:protected_for?).with('ref').and_return(false) + end + it 'contains only the CI variables' do is_expected.to contain_exactly(ci_variable) end @@ -2582,42 +2593,139 @@ describe Project do end describe '#protected_for?' do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } - subject { project.protected_for?('ref') } + subject { project.protected_for?(ref) } - context 'when the ref is not protected' do + shared_examples 'ref is not protected' do before do stub_application_setting( default_branch_protection: Gitlab::Access::PROTECTION_NONE) end it 'returns false' do - is_expected.to be_falsey + is_expected.to be false end end - context 'when the ref is a protected branch' do + shared_examples 'ref is protected branch' do before do - allow(project).to receive(:repository).and_call_original - allow(project).to receive_message_chain(:repository, :branch_exists?).and_return(true) - create(:protected_branch, name: 'ref', project: project) + create(:protected_branch, name: 'master', project: project) end it 'returns true' do - is_expected.to be_truthy + is_expected.to be true end end - context 'when the ref is a protected tag' do + shared_examples 'ref is protected tag' do before do - allow(project).to receive_message_chain(:repository, :branch_exists?).and_return(false) - allow(project).to receive_message_chain(:repository, :tag_exists?).and_return(true) - create(:protected_tag, name: 'ref', project: project) + create(:protected_tag, name: 'v1.0.0', project: project) end it 'returns true' do - is_expected.to be_truthy + is_expected.to be true + end + end + + context 'when ref is nil' do + let(:ref) { nil } + + it 'returns false' do + is_expected.to be false + end + end + + context 'when ref is ref name' do + context 'when ref is ambiguous' do + let(:ref) { 'ref' } + + before do + project.repository.add_branch(project.creator, 'ref', 'master') + project.repository.add_tag(project.creator, 'ref', 'master') + end + + it 'raises an error' do + expect { subject }.to raise_error(Repository::AmbiguousRefError) + end + end + + context 'when the ref is not protected' do + let(:ref) { 'master' } + + it_behaves_like 'ref is not protected' + end + + context 'when the ref is a protected branch' do + let(:ref) { 'master' } + + it_behaves_like 'ref is protected branch' + end + + context 'when the ref is a protected tag' do + let(:ref) { 'v1.0.0' } + + it_behaves_like 'ref is protected tag' + end + + context 'when ref does not exist' do + let(:ref) { 'something' } + + it 'returns false' do + is_expected.to be false + end + end + end + + context 'when ref is full ref' do + context 'when the ref is not protected' do + let(:ref) { 'refs/heads/master' } + + it_behaves_like 'ref is not protected' + end + + context 'when the ref is a protected branch' do + let(:ref) { 'refs/heads/master' } + + it_behaves_like 'ref is protected branch' + end + + context 'when the ref is a protected tag' do + let(:ref) { 'refs/tags/v1.0.0' } + + it_behaves_like 'ref is protected tag' + end + + context 'when branch ref name is a full tag ref' do + let(:ref) { 'refs/tags/something' } + + before do + project.repository.add_branch(project.creator, ref, 'master') + end + + context 'when ref is not protected' do + it 'returns false' do + is_expected.to be false + end + end + + context 'when ref is a protected branch' do + before do + create(:protected_branch, name: 'refs/tags/something', project: project) + end + + it 'returns true' do + is_expected.to be true + end + end + end + + context 'when ref does not exist' do + let(:ref) { 'refs/heads/something' } + + it 'returns false' do + is_expected.to be false + end end end end @@ -2837,7 +2945,7 @@ describe Project do it 'shows full error updating an invalid MR' do error_message = 'Failed to replace merge_requests because one or more of the new records could not be saved.'\ - ' Validate fork Source project is not a fork of the target project' + ' Validate fork Source project is not a fork of the target project' expect { project.append_or_update_attribute(:merge_requests, [create(:merge_request)]) } .to raise_error(ActiveRecord::RecordNotSaved, error_message) diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index 5d3c25062d5..224bc9ed935 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -24,6 +24,20 @@ describe RemoteMirror, :mailer do expect(remote_mirror).to be_invalid expect(remote_mirror.errors[:url].first).to include('Username needs to start with an alphanumeric character') end + + it 'does not allow url pointing to localhost' do + remote_mirror = build(:remote_mirror, url: 'http://127.0.0.2/t.git') + + expect(remote_mirror).to be_invalid + expect(remote_mirror.errors[:url].first).to include('Requests to loopback addresses are not allowed') + end + + it 'does not allow url pointing to the local network' do + remote_mirror = build(:remote_mirror, url: 'https://192.168.1.1') + + expect(remote_mirror).to be_invalid + expect(remote_mirror.errors[:url].first).to include('Requests to the local network are not allowed') + end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index f09b4b67061..2063b4bbe75 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1005,6 +1005,67 @@ describe Repository do end end + describe '#ambiguous_ref?' do + let(:ref) { 'ref' } + + subject { repository.ambiguous_ref?(ref) } + + context 'when ref is ambiguous' do + before do + repository.add_tag(project.creator, ref, 'master') + repository.add_branch(project.creator, ref, 'master') + end + + it 'should be true' do + is_expected.to eq(true) + end + end + + context 'when ref is not ambiguous' do + before do + repository.add_tag(project.creator, ref, 'master') + end + + it 'should be false' do + is_expected.to eq(false) + end + end + end + + describe '#expand_ref' do + let(:ref) { 'ref' } + + subject { repository.expand_ref(ref) } + + context 'when ref is not tag or branch name' do + let(:ref) { 'refs/heads/master' } + + it 'returns nil' do + is_expected.to eq(nil) + end + end + + context 'when ref is tag name' do + before do + repository.add_tag(project.creator, ref, 'master') + end + + it 'returns the tag ref' do + is_expected.to eq("refs/tags/#{ref}") + end + end + + context 'when ref is branch name' do + before do + repository.add_branch(project.creator, ref, 'master') + end + + it 'returns the branch ref' do + is_expected.to eq("refs/heads/#{ref}") + end + end + end + describe '#add_branch' do let(:branch_name) { 'new_feature' } let(:target) { 'master' } diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 7a7272ccb60..664dc3fa145 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -423,4 +423,41 @@ describe Snippet do expect(blob.data).to eq(snippet.content) end end + + describe '#embeddable?' do + context 'project snippet' do + [ + { project: :public, snippet: :public, embeddable: true }, + { project: :internal, snippet: :public, embeddable: false }, + { project: :private, snippet: :public, embeddable: false }, + { project: :public, snippet: :internal, embeddable: false }, + { project: :internal, snippet: :internal, embeddable: false }, + { project: :private, snippet: :internal, embeddable: false }, + { project: :public, snippet: :private, embeddable: false }, + { project: :internal, snippet: :private, embeddable: false }, + { project: :private, snippet: :private, embeddable: false } + ].each do |combination| + it 'only returns true when both project and snippet are public' do + project = create(:project, combination[:project]) + snippet = create(:project_snippet, combination[:snippet], project: project) + + expect(snippet.embeddable?).to eq(combination[:embeddable]) + end + end + end + + context 'personal snippet' do + [ + { snippet: :public, embeddable: true }, + { snippet: :internal, embeddable: false }, + { snippet: :private, embeddable: false } + ].each do |combination| + it 'only returns true when snippet is public' do + snippet = create(:personal_snippet, combination[:snippet]) + + expect(snippet.embeddable?).to eq(combination[:embeddable]) + end + end + end + end end diff --git a/spec/policies/issuable_policy_spec.rb b/spec/policies/issuable_policy_spec.rb index d1bf98995e7..db3df760472 100644 --- a/spec/policies/issuable_policy_spec.rb +++ b/spec/policies/issuable_policy_spec.rb @@ -7,6 +7,33 @@ describe IssuablePolicy, models: true do let(:policies) { described_class.new(user, issue) } describe '#rules' do + context 'when user is author of issuable' do + let(:merge_request) { create(:merge_request, source_project: project, author: user) } + let(:policies) { described_class.new(user, merge_request) } + + context 'when user is able to read project' do + it 'enables user to read and update issuables' do + expect(policies).to be_allowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request) + end + end + + context 'when project is private' do + let(:project) { create(:project, :private) } + + context 'when user belongs to the projects team' do + it 'enables user to read and update issuables' do + project.add_maintainer(user) + + expect(policies).to be_allowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request) + end + end + + it 'disallows user from reading and updating issuables from that project' do + expect(policies).to be_disallowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request) + end + end + end + context 'when discussion is locked for the issuable' do let(:issue) { create(:issue, project: project, discussion_locked: true) } diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 73131dba542..97aa71bf231 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -142,10 +142,20 @@ describe API::Jobs do end context 'unauthorized user' do - let(:api_user) { nil } + context 'when user is not logged in' do + let(:api_user) { nil } - it 'does not return project jobs' do - expect(response).to have_gitlab_http_status(401) + it 'does not return project jobs' do + expect(response).to have_gitlab_http_status(401) + end + end + + context 'when user is guest' do + let(:api_user) { guest } + + it 'does not return project jobs' do + expect(response).to have_gitlab_http_status(403) + end end end @@ -241,10 +251,20 @@ describe API::Jobs do end context 'unauthorized user' do - let(:api_user) { nil } + context 'when user is not logged in' do + let(:api_user) { nil } - it 'does not return jobs' do - expect(response).to have_gitlab_http_status(401) + it 'does not return jobs' do + expect(response).to have_gitlab_http_status(401) + end + end + + context 'when user is guest' do + let(:api_user) { guest } + + it 'does not return jobs' do + expect(response).to have_gitlab_http_status(403) + end end end end diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index f2e9799452a..8021bd338e0 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -5,7 +5,7 @@ describe Auth::ContainerRegistryAuthenticationService do let(:current_user) { nil } let(:current_params) { {} } let(:rsa_key) { OpenSSL::PKey::RSA.generate(512) } - let(:payload) { JWT.decode(subject[:token], rsa_key).first } + let(:payload) { JWT.decode(subject[:token], rsa_key, true, { algorithm: 'RS256' }).first } let(:authentication_abilities) do [:read_container_image, :create_container_image, :admin_container_image] diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index ffa47d527f7..9fc2cc8b7d6 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -667,7 +667,7 @@ describe Ci::CreatePipelineService do stub_ci_pipeline_yaml_file(YAML.dump(config)) end - let(:ref_name) { 'feature' } + let(:ref_name) { 'refs/heads/feature' } context 'when source is merge request' do let(:source) { :merge_request } @@ -696,7 +696,7 @@ describe Ci::CreatePipelineService do let(:merge_request) do create(:merge_request, source_project: project, - source_branch: ref_name, + source_branch: Gitlab::Git.ref_name(ref_name), target_project: project, target_branch: 'master') end @@ -709,7 +709,7 @@ describe Ci::CreatePipelineService do end context 'when ref is tag' do - let(:ref_name) { 'v1.1.0' } + let(:ref_name) { 'refs/tags/v1.1.0' } it 'does not create a merge request pipeline' do expect(pipeline).not_to be_persisted @@ -721,7 +721,7 @@ describe Ci::CreatePipelineService do let(:merge_request) do create(:merge_request, source_project: project, - source_branch: ref_name, + source_branch: Gitlab::Git.ref_name(ref_name), target_project: target_project, target_branch: 'master') end @@ -786,7 +786,7 @@ describe Ci::CreatePipelineService do let(:merge_request) do create(:merge_request, source_project: project, - source_branch: ref_name, + source_branch: Gitlab::Git.ref_name(ref_name), target_project: project, target_branch: 'master') end @@ -928,7 +928,7 @@ describe Ci::CreatePipelineService do let(:merge_request) do create(:merge_request, source_project: project, - source_branch: ref_name, + source_branch: Gitlab::Git.ref_name(ref_name), target_project: project, target_branch: 'master') end diff --git a/spec/services/groups/update_service_spec.rb b/spec/services/groups/update_service_spec.rb index 84cfa53ea05..d87a7dd234d 100644 --- a/spec/services/groups/update_service_spec.rb +++ b/spec/services/groups/update_service_spec.rb @@ -56,7 +56,7 @@ describe Groups::UpdateService do create(:project, :private, group: internal_group) expect(TodosDestroyer::GroupPrivateWorker).to receive(:perform_in) - .with(1.hour, internal_group.id) + .with(Todo::WAIT_FOR_DELETE, internal_group.id) end it "changes permission level to private" do diff --git a/spec/services/issuable/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb index f0b0f7956ce..ca366cdf1df 100644 --- a/spec/services/issuable/bulk_update_service_spec.rb +++ b/spec/services/issuable/bulk_update_service_spec.rb @@ -28,6 +28,33 @@ describe Issuable::BulkUpdateService do expect(project.issues.opened).to be_empty expect(project.issues.closed).not_to be_empty end + + context 'when issue for a different project is created' do + let(:private_project) { create(:project, :private) } + let(:issue) { create(:issue, project: private_project, author: user) } + + context 'when user has access to the project' do + it 'closes all issues passed' do + private_project.add_maintainer(user) + + bulk_update(issues + [issue], state_event: 'close') + + expect(project.issues.opened).to be_empty + expect(project.issues.closed).not_to be_empty + expect(private_project.issues.closed).not_to be_empty + end + end + + context 'when user does not have access to project' do + it 'only closes all issues that the user has access to' do + bulk_update(issues + [issue], state_event: 'close') + + expect(project.issues.opened).to be_empty + expect(project.issues.closed).not_to be_empty + expect(private_project.issues.closed).to be_empty + end + end + end end describe 'reopen issues' do diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index bd519e7f077..ce20bf2bef6 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -77,7 +77,7 @@ describe Issues::UpdateService, :mailer do end it 'enqueues ConfidentialIssueWorker when an issue is made confidential' do - expect(TodosDestroyer::ConfidentialIssueWorker).to receive(:perform_in).with(1.hour, issue.id) + expect(TodosDestroyer::ConfidentialIssueWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, issue.id) update_issue(confidential: true) end diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb index 0a5220c7c61..5aa7165e135 100644 --- a/spec/services/members/destroy_service_spec.rb +++ b/spec/services/members/destroy_service_spec.rb @@ -22,7 +22,7 @@ describe Members::DestroyService do shared_examples 'a service destroying a member' do before do type = member.is_a?(GroupMember) ? 'Group' : 'Project' - expect(TodosDestroyer::EntityLeaveWorker).to receive(:perform_in).with(1.hour, member.user_id, member.source_id, type) + expect(TodosDestroyer::EntityLeaveWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, type) end it 'destroys the member' do diff --git a/spec/services/members/update_service_spec.rb b/spec/services/members/update_service_spec.rb index 6d19a95ffeb..599ed39ca37 100644 --- a/spec/services/members/update_service_spec.rb +++ b/spec/services/members/update_service_spec.rb @@ -20,11 +20,28 @@ describe Members::UpdateService do shared_examples 'a service updating a member' do it 'updates the member' do + expect(TodosDestroyer::EntityLeaveWorker).not_to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, source.class.name) + updated_member = described_class.new(current_user, params).execute(member, permission: permission) expect(updated_member).to be_valid expect(updated_member.access_level).to eq(Gitlab::Access::MAINTAINER) end + + context 'when member is downgraded to guest' do + let(:params) do + { access_level: Gitlab::Access::GUEST } + end + + it 'schedules to delete confidential todos' do + expect(TodosDestroyer::EntityLeaveWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, source.class.name).once + + updated_member = described_class.new(current_user, params).execute(member, permission: permission) + + expect(updated_member).to be_valid + expect(updated_member.access_level).to eq(Gitlab::Access::GUEST) + end + end end before do diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb index 1894d8c8d0e..536d0d345a4 100644 --- a/spec/services/merge_requests/build_service_spec.rb +++ b/spec/services/merge_requests/build_service_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' describe MergeRequests::BuildService do using RSpec::Parameterized::TableSyntax include RepoHelpers + include ProjectForksHelper let(:project) { create(:project, :repository) } let(:source_project) { nil } @@ -49,7 +50,7 @@ describe MergeRequests::BuildService do describe '#execute' do it 'calls the compare service with the correct arguments' do - allow_any_instance_of(described_class).to receive(:branches_valid?).and_return(true) + allow_any_instance_of(described_class).to receive(:projects_and_branches_valid?).and_return(true) expect(CompareService).to receive(:new) .with(project, Gitlab::Git::BRANCH_REF_PREFIX + source_branch) .and_call_original @@ -393,11 +394,27 @@ describe MergeRequests::BuildService do end end + context 'target_project is set but repo is not accessible by current_user' do + let(:target_project) do + create(:project, :public, :repository, repository_access_level: ProjectFeature::PRIVATE) + end + + it 'sets target project correctly' do + expect(merge_request.target_project).to eq(project) + end + end + context 'source_project is set and accessible by current_user' do let(:source_project) { create(:project, :public, :repository)} let(:commits) { Commit.decorate([commit_1], project) } - it 'sets target project correctly' do + before do + # To create merge requests _from_ a project the user needs at least + # developer access + source_project.add_developer(user) + end + + it 'sets source project correctly' do expect(merge_request.source_project).to eq(source_project) end end @@ -406,11 +423,43 @@ describe MergeRequests::BuildService do let(:source_project) { create(:project, :private, :repository)} let(:commits) { Commit.decorate([commit_1], project) } - it 'sets target project correctly' do + it 'sets source project correctly' do + expect(merge_request.source_project).to eq(project) + end + end + + context 'source_project is set but the user cannot create merge requests from the project' do + let(:source_project) do + create(:project, :public, :repository, merge_requests_access_level: ProjectFeature::PRIVATE) + end + + it 'sets the source_project correctly' do expect(merge_request.source_project).to eq(project) end end + context 'target_project is not in the fork network of source_project' do + let(:target_project) { create(:project, :public, :repository) } + + it 'adds an error to the merge request' do + expect(merge_request.errors[:validate_fork]).to contain_exactly('Source project is not a fork of the target project') + end + end + + context 'target_project is in the fork network of source project but no longer accessible' do + let!(:project) { fork_project(target_project, user, namespace: user.namespace, repository: true) } + let(:source_project) { project } + let(:target_project) { create(:project, :public, :repository) } + + before do + target_project.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + end + + it 'sets the target_project correctly' do + expect(merge_request.target_project).to eq(project) + end + end + context 'when specifying target branch in the description' do let(:description) { "A merge request targeting another branch\n\n/target_branch with-codeowners" } diff --git a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb index d7d7f1874eb..95c9b6e63b8 100644 --- a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb +++ b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb @@ -4,17 +4,15 @@ describe Projects::LfsPointers::LfsDownloadService do let(:project) { create(:project) } let(:oid) { '9e548e25631dd9ce6b43afd6359ab76da2819d6a5b474e66118c7819e1d8b3e8' } let(:download_link) { "http://gitlab.com/#{oid}" } - let(:lfs_content) do - <<~HEREDOC - whatever - HEREDOC - end + let(:lfs_content) { SecureRandom.random_bytes(10) } subject { described_class.new(project) } before do allow(project).to receive(:lfs_enabled?).and_return(true) WebMock.stub_request(:get, download_link).to_return(body: lfs_content) + + allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_hooks_and_services?).and_return(false) end describe '#execute' do @@ -32,7 +30,7 @@ describe Projects::LfsPointers::LfsDownloadService do it 'stores the content' do subject.execute(oid, download_link) - expect(File.read(LfsObject.first.file.file.file)).to eq lfs_content + expect(File.binread(LfsObject.first.file.file.file)).to eq lfs_content end end @@ -54,18 +52,61 @@ describe Projects::LfsPointers::LfsDownloadService do end end + context 'when localhost requests are allowed' do + let(:download_link) { 'http://192.168.2.120' } + + before do + allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_hooks_and_services?).and_return(true) + end + + it 'downloads the file' do + expect(subject).to receive(:download_and_save_file).and_call_original + + expect { subject.execute(oid, download_link) }.to change { LfsObject.count }.by(1) + end + end + context 'when a bad URL is used' do - where(download_link: ['/etc/passwd', 'ftp://example.com', 'http://127.0.0.2']) + where(download_link: ['/etc/passwd', 'ftp://example.com', 'http://127.0.0.2', 'http://192.168.2.120']) with_them do it 'does not download the file' do - expect(subject).not_to receive(:download_and_save_file) - expect { subject.execute(oid, download_link) }.not_to change { LfsObject.count } end end end + context 'when the URL points to a redirected URL' do + context 'that is blocked' do + where(redirect_link: ['ftp://example.com', 'http://127.0.0.2', 'http://192.168.2.120']) + + with_them do + before do + WebMock.stub_request(:get, download_link).to_return(status: 301, headers: { 'Location' => redirect_link }) + end + + it 'does not follow the redirection' do + expect(Rails.logger).to receive(:error).with(/LFS file with oid #{oid} couldn't be downloaded/) + + expect { subject.execute(oid, download_link) }.not_to change { LfsObject.count } + end + end + end + + context 'that is valid' do + let(:redirect_link) { "http://example.com/"} + + before do + WebMock.stub_request(:get, download_link).to_return(status: 301, headers: { 'Location' => redirect_link }) + WebMock.stub_request(:get, redirect_link).to_return(body: lfs_content) + end + + it 'follows the redirection' do + expect { subject.execute(oid, download_link) }.to change { LfsObject.count }.from(0).to(1) + end + end + end + context 'when an lfs object with the same oid already exists' do before do create(:lfs_object, oid: 'oid') diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index d58ff2cedc0..8adfc63222e 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -41,7 +41,7 @@ describe Projects::UpdateService do end it 'updates the project to private' do - expect(TodosDestroyer::ProjectPrivateWorker).to receive(:perform_in).with(1.hour, project.id) + expect(TodosDestroyer::ProjectPrivateWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, project.id) result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE) @@ -191,7 +191,7 @@ describe Projects::UpdateService do context 'when changing feature visibility to private' do it 'updates the visibility correctly' do expect(TodosDestroyer::PrivateFeaturesWorker) - .to receive(:perform_in).with(1.hour, project.id) + .to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, project.id) result = update_project(project, user, project_feature_attributes: { issues_access_level: ProjectFeature::PRIVATE } diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index c52515aefd8..253f2e44d10 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -19,6 +19,7 @@ describe TodoService do before do project.add_guest(guest) project.add_developer(author) + project.add_developer(assignee) project.add_developer(member) project.add_developer(john_doe) project.add_developer(skipped) diff --git a/spec/support/gitlab_stubs/gitlab_ci.yml b/spec/support/gitlab_stubs/gitlab_ci.yml index e55a61b2b94..f3755e52b2c 100644 --- a/spec/support/gitlab_stubs/gitlab_ci.yml +++ b/spec/support/gitlab_stubs/gitlab_ci.yml @@ -1,9 +1,8 @@ -image: ruby:2.1 +image: ruby:2.6 services: - postgres before_script: - - gem install bundler - bundle install - bundle exec rake db:create diff --git a/spec/workers/mail_scheduler/notification_service_worker_spec.rb b/spec/workers/mail_scheduler/notification_service_worker_spec.rb index f725c8763a0..1033557ee88 100644 --- a/spec/workers/mail_scheduler/notification_service_worker_spec.rb +++ b/spec/workers/mail_scheduler/notification_service_worker_spec.rb @@ -17,10 +17,21 @@ describe MailScheduler::NotificationServiceWorker do end context 'when the arguments cannot be deserialized' do - it 'does nothing' do - expect(worker.notification_service).not_to receive(method) + context 'when the arguments are not deserializeable' do + it 'raises exception' do + expect(worker.notification_service).not_to receive(method) + expect { worker.perform(method, key.to_global_id.to_s.succ) }.to raise_exception(ArgumentError) + end + end + + context 'when the arguments are deserializeable' do + it 'does nothing' do + serialized_arguments = *serialize(key) + key.destroy! - worker.perform(method, key.to_global_id.to_s.succ) + expect(worker.notification_service).not_to receive(method) + expect { worker.perform(method, serialized_arguments) }.not_to raise_exception + end end end |