diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-01-30 21:08:47 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-01-30 21:08:47 +0000 |
| commit | c8f773a8593926f4f2dec6f446a3b3e59e9c9909 (patch) | |
| tree | 4e5ea1d3b861ff99015f6112da567de7873868aa | |
| parent | 929b887e5391dea7cb53b88b77b9a35351c87d99 (diff) | |
| download | gitlab-ce-c8f773a8593926f4f2dec6f446a3b3e59e9c9909.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
96 files changed, 1631 insertions, 375 deletions
diff --git a/CHANGELOG-EE.md b/CHANGELOG-EE.md index 4ec5e9c98fd..ab7d706fc7b 100644 --- a/CHANGELOG-EE.md +++ b/CHANGELOG-EE.md @@ -1,5 +1,9 @@ Please view this file on the master branch, on stable branches it's out of date. +## 12.7.3 + +- No changes. + ## 12.7.1 ### Fixed (1 change) @@ -95,6 +99,14 @@ Please view this file on the master branch, on stable branches it's out of date. - Remove "creations" in gitlab_subscription_histories on gitlab.com. !22278 +## 12.6.6 + +- No changes. + +## 12.6.5 + +- No changes. + ## 12.6.4 - No changes. @@ -207,6 +219,10 @@ Please view this file on the master branch, on stable branches it's out of date. - Update the alerts used in the Dependency List to follow GitLab design guidelines. !21760 +## 12.5.8 + +- No changes. + ## 12.5.5 - No changes. diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index 401cc8dd3e7..ba624dacf9a 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -8.19.0 +8.20.0 @@ -65,7 +65,7 @@ gem 'u2f', '~> 0.2.1' # GitLab Pages gem 'validates_hostname', '~> 1.0.6' -gem 'rubyzip', '~> 1.3.0', require: 'zip' +gem 'rubyzip', '~> 2.0.0', require: 'zip' # GitLab Pages letsencrypt support gem 'acme-client', '~> 2.0.5' diff --git a/Gemfile.lock b/Gemfile.lock index c52150cc675..58a4f1ad53d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -274,7 +274,7 @@ GEM et-orbi (1.2.1) tzinfo eventmachine (1.2.7) - excon (0.62.0) + excon (0.71.1) execjs (2.6.0) expression_parser (0.9.0) extended-markdown-filter (0.6.0) @@ -961,7 +961,7 @@ GEM sexp_processor (~> 4.9) rubyntlm (0.6.2) rubypants (0.2.0) - rubyzip (1.3.0) + rubyzip (2.0.0) rugged (0.28.4.1) safe_yaml (1.0.4) sanitize (4.6.6) @@ -1361,7 +1361,7 @@ DEPENDENCIES ruby-prof (~> 1.0.0) ruby-progressbar ruby_parser (~> 3.8) - rubyzip (~> 1.3.0) + rubyzip (~> 2.0.0) rugged (~> 0.28) sanitize (~> 4.6) sassc-rails (~> 2.1.0) diff --git a/app/assets/javascripts/frequent_items/components/app.vue b/app/assets/javascripts/frequent_items/components/app.vue index 8cf939254c1..2ffecce0a56 100644 --- a/app/assets/javascripts/frequent_items/components/app.vue +++ b/app/assets/javascripts/frequent_items/components/app.vue @@ -5,7 +5,7 @@ import AccessorUtilities from '~/lib/utils/accessor'; import eventHub from '../event_hub'; import store from '../store/'; import { FREQUENT_ITEMS, STORAGE_KEY } from '../constants'; -import { isMobile, updateExistingFrequentItem } from '../utils'; +import { isMobile, updateExistingFrequentItem, sanitizeItem } from '../utils'; import FrequentItemsSearchInput from './frequent_items_search_input.vue'; import FrequentItemsList from './frequent_items_list.vue'; import frequentItemsMixin from './frequent_items_mixin'; @@ -64,7 +64,9 @@ export default { this.fetchFrequentItems(); } }, - logItemAccess(storageKey, item) { + logItemAccess(storageKey, unsanitizedItem) { + const item = sanitizeItem(unsanitizedItem); + if (!AccessorUtilities.isLocalStorageAccessSafe()) { return false; } diff --git a/app/assets/javascripts/frequent_items/components/frequent_items_list.vue b/app/assets/javascripts/frequent_items/components/frequent_items_list.vue index 67ffa97a046..0ece64692ae 100644 --- a/app/assets/javascripts/frequent_items/components/frequent_items_list.vue +++ b/app/assets/javascripts/frequent_items/components/frequent_items_list.vue @@ -1,6 +1,7 @@ <script> import FrequentItemsListItem from './frequent_items_list_item.vue'; import frequentItemsMixin from './frequent_items_mixin'; +import { sanitizeItem } from '../utils'; export default { components: { @@ -48,6 +49,9 @@ export default { ? this.translations.itemListErrorMessage : this.translations.itemListEmptyMessage; }, + sanitizedItems() { + return this.items.map(sanitizeItem); + }, }, }; </script> @@ -59,7 +63,7 @@ export default { {{ listEmptyMessage }} </li> <frequent-items-list-item - v-for="item in items" + v-for="item in sanitizedItems" v-else :key="item.id" :item-id="item.id" diff --git a/app/assets/javascripts/frequent_items/utils.js b/app/assets/javascripts/frequent_items/utils.js index cc1668b1a0d..5188d6118ac 100644 --- a/app/assets/javascripts/frequent_items/utils.js +++ b/app/assets/javascripts/frequent_items/utils.js @@ -1,5 +1,6 @@ import _ from 'underscore'; import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils'; +import sanitize from 'sanitize-html'; import { FREQUENT_ITEMS, HOUR_IN_MS } from './constants'; export const isMobile = () => ['md', 'sm', 'xs'].includes(bp.getBreakpointSize()); @@ -43,3 +44,9 @@ export const updateExistingFrequentItem = (frequentItem, item) => { lastAccessedOn: accessedOverHourAgo ? Date.now() : frequentItem.lastAccessedOn, }; }; + +export const sanitizeItem = item => ({ + ...item, + name: sanitize(item.name.toString(), { allowedTags: [] }), + namespace: sanitize(item.namespace.toString(), { allowedTags: [] }), +}); diff --git a/app/assets/javascripts/groups_select.js b/app/assets/javascripts/groups_select.js index a5e38022b8d..4daa8c60e58 100644 --- a/app/assets/javascripts/groups_select.js +++ b/app/assets/javascripts/groups_select.js @@ -1,6 +1,7 @@ import $ from 'jquery'; import axios from './lib/utils/axios_utils'; import Api from './api'; +import { escape } from 'lodash'; import { normalizeHeaders } from './lib/utils/common_utils'; import { __ } from '~/locale'; @@ -75,10 +76,12 @@ const groupsSelect = () => { } }, formatResult(object) { - return `<div class='group-result'> <div class='group-name'>${object.full_name}</div> <div class='group-path'>${object.full_path}</div> </div>`; + return `<div class='group-result'> <div class='group-name'>${escape( + object.full_name, + )}</div> <div class='group-path'>${object.full_path}</div> </div>`; }, formatSelection(object) { - return object.full_name; + return escape(object.full_name); }, dropdownCssClass: 'ajax-groups-dropdown select2-infinite', // we do not want to escape markup since we are displaying html in results diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue index be2adb07526..762228dd138 100644 --- a/app/assets/javascripts/notes/components/notes_app.vue +++ b/app/assets/javascripts/notes/components/notes_app.vue @@ -14,7 +14,7 @@ import placeholderSystemNote from '../../vue_shared/components/notes/placeholder import skeletonLoadingContainer from '../../vue_shared/components/notes/skeleton_note.vue'; import highlightCurrentUser from '~/behaviors/markdown/highlight_current_user'; import { __ } from '~/locale'; -import initUserPopovers from '../../user_popovers'; +import initUserPopovers from '~/user_popovers'; export default { name: 'NotesApp', diff --git a/app/assets/javascripts/user_popovers.js b/app/assets/javascripts/user_popovers.js index 157d89a3a40..5b9e3817f3a 100644 --- a/app/assets/javascripts/user_popovers.js +++ b/app/assets/javascripts/user_popovers.js @@ -3,108 +3,92 @@ import Vue from 'vue'; import UsersCache from './lib/utils/users_cache'; import UserPopover from './vue_shared/components/user_popover/user_popover.vue'; -let renderedPopover; -let renderFn; - -const handleUserPopoverMouseOut = event => { - const { target } = event; - target.removeEventListener('mouseleave', handleUserPopoverMouseOut); - - if (renderFn) { - clearTimeout(renderFn); - } - if (renderedPopover) { - renderedPopover.$destroy(); - renderedPopover = null; - } - target.removeAttribute('aria-describedby'); +const removeTitle = el => { + // Removing titles so its not showing tooltips also + + el.dataset.originalTitle = ''; + el.setAttribute('title', ''); +}; + +const getPreloadedUserInfo = dataset => { + const userId = dataset.user || dataset.userId; + const { username, name, avatarUrl } = dataset; + + return { + userId, + username, + name, + avatarUrl, + }; }; /** * Adds a UserPopover component to the body, hands over as much data as the target element has in data attributes. * loads based on data-user-id more data about a user from the API and sets it on the popover */ -const handleUserPopoverMouseOver = event => { - const { target } = event; - // Add listener to actually remove it again - target.addEventListener('mouseleave', handleUserPopoverMouseOut); - - renderFn = setTimeout(() => { - // Helps us to use current markdown setup without maybe breaking or duplicating for now - if (target.dataset.user) { - target.dataset.userId = target.dataset.user; - // Removing titles so its not showing tooltips also - target.dataset.originalTitle = ''; - target.setAttribute('title', ''); - } - - const { userId, username, name, avatarUrl } = target.dataset; +const populateUserInfo = user => { + const { userId } = user; + + return Promise.all([UsersCache.retrieveById(userId), UsersCache.retrieveStatusById(userId)]).then( + ([userData, status]) => { + if (userData) { + Object.assign(user, { + avatarUrl: userData.avatar_url, + username: userData.username, + name: userData.name, + location: userData.location, + bio: userData.bio, + organization: userData.organization, + loaded: true, + }); + } + + if (status) { + Object.assign(user, { + status, + }); + } + + return user; + }, + ); +}; + +export default (elements = document.querySelectorAll('.js-user-link')) => { + const userLinks = Array.from(elements); + + return userLinks.map(el => { + const UserPopoverComponent = Vue.extend(UserPopover); const user = { - userId, - username, - name, - avatarUrl, location: null, bio: null, organization: null, status: null, loaded: false, }; - if (userId || username) { - const UserPopoverComponent = Vue.extend(UserPopover); - renderedPopover = new UserPopoverComponent({ - propsData: { - target, - user, - }, - }); - - renderedPopover.$mount(); - - UsersCache.retrieveById(userId) - .then(userData => { - if (!userData) { - return undefined; - } - - Object.assign(user, { - avatarUrl: userData.avatar_url, - username: userData.username, - name: userData.name, - location: userData.location, - bio: userData.bio, - organization: userData.organization, - status: userData.status, - loaded: true, - }); - - if (userData.status) { - return Promise.resolve(); - } - - return UsersCache.retrieveStatusById(userId); - }) - .then(status => { - if (!status) { - return; - } - - Object.assign(user, { - status, - }); - }) - .catch(() => { - renderedPopover.$destroy(); - renderedPopover = null; - }); - } - }, 200); // 200ms delay so not every mouseover triggers Popover + API Call -}; + const renderedPopover = new UserPopoverComponent({ + propsData: { + target: el, + user, + }, + }); + + renderedPopover.$mount(); + + el.addEventListener('mouseenter', ({ target }) => { + removeTitle(target); + const preloadedUserInfo = getPreloadedUserInfo(target.dataset); + + Object.assign(user, preloadedUserInfo); -export default elements => { - const userLinks = elements || [...document.querySelectorAll('.js-user-link')]; + if (preloadedUserInfo.userId) { + populateUserInfo(user); + } + }); + el.addEventListener('mouseleave', ({ target }) => { + target.removeAttribute('aria-describedby'); + }); - userLinks.forEach(el => { - el.addEventListener('mouseenter', handleUserPopoverMouseOver); + return renderedPopover; }); }; diff --git a/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue b/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue index 37e3643bf6c..ca25d9ee738 100644 --- a/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue +++ b/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue @@ -56,19 +56,16 @@ export default { </script> <template> - <gl-popover :target="target" boundary="viewport" placement="top" offset="0, 1" show> + <!-- 200ms delay so not every mouseover triggers Popover --> + <gl-popover :target="target" :delay="200" boundary="viewport" triggers="hover" placement="top"> <div class="user-popover d-flex"> <div class="p-1 flex-shrink-1"> <user-avatar-image :img-src="user.avatarUrl" :size="60" css-classes="mr-2" /> </div> <div class="p-1 w-100"> <h5 class="m-0"> - {{ user.name }} - <gl-skeleton-loading - v-if="nameIsLoading" - :lines="1" - class="animation-container-small mb-1" - /> + <span v-if="user.name">{{ user.name }}</span> + <gl-skeleton-loading v-else :lines="1" class="animation-container-small mb-1" /> </h5> <div class="text-secondary mb-2"> <span v-if="user.username">@{{ user.username }}</span> diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 789bccf268a..fa88ca91170 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -5,6 +5,7 @@ require 'fogbugz' class ApplicationController < ActionController::Base include Gitlab::GonHelper + include Gitlab::NoCacheHeaders include GitlabRoutingHelper include PageLayoutHelper include SafeParamsHelper @@ -55,7 +56,6 @@ class ApplicationController < ActionController::Base # Adds `no-store` to the DEFAULT_CACHE_CONTROL, to prevent security # concerns due to caching private data. DEFAULT_GITLAB_CACHE_CONTROL = "#{ActionDispatch::Http::Cache::Response::DEFAULT_CACHE_CONTROL}, no-store" - DEFAULT_GITLAB_CONTROL_NO_CACHE = "#{DEFAULT_GITLAB_CACHE_CONTROL}, no-cache" rescue_from Encoding::CompatibilityError do |exception| log_exception(exception) @@ -247,9 +247,9 @@ class ApplicationController < ActionController::Base end def no_cache_headers - headers['Cache-Control'] = DEFAULT_GITLAB_CONTROL_NO_CACHE - headers['Pragma'] = 'no-cache' # HTTP 1.0 compatibility - headers['Expires'] = 'Fri, 01 Jan 1990 00:00:00 GMT' + DEFAULT_GITLAB_NO_CACHE_HEADERS.each do |k, v| + headers[k] = v + end end def default_headers diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index 1a97b39d3ae..1668cf004f8 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -19,7 +19,7 @@ class DashboardController < Dashboard::ApplicationController format.json do load_events - pager_json("events/_events", @events.count) + pager_json('events/_events', @events.count { |event| event.visible_to_user?(current_user) }) end end end @@ -37,6 +37,7 @@ class DashboardController < Dashboard::ApplicationController @events = EventCollection .new(projects, offset: params[:offset].to_i, filter: event_filter) .to_a + .map(&:present) Events::RenderService.new(current_user).execute(@events) end diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 0953ca96317..958dc27984f 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -91,7 +91,7 @@ class GroupsController < Groups::ApplicationController format.json do load_events - pager_json("events/_events", @events.count) + pager_json("events/_events", @events.count { |event| event.visible_to_user?(current_user) }) end end end @@ -209,8 +209,9 @@ class GroupsController < Groups::ApplicationController .includes(:namespace) @events = EventCollection - .new(projects, offset: params[:offset].to_i, filter: event_filter, groups: groups) - .to_a + .new(projects, offset: params[:offset].to_i, filter: event_filter, groups: groups) + .to_a + .map(&:present) Events::RenderService .new(current_user) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index f4f2a16b82b..d39a4c373ff 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -118,7 +118,7 @@ class ProjectsController < Projects::ApplicationController format.html format.json do load_events - pager_json('events/_events', @events.count) + pager_json('events/_events', @events.count { |event| event.visible_to_user?(current_user) }) end end end @@ -343,6 +343,7 @@ class ProjectsController < Projects::ApplicationController @events = EventCollection .new(projects, offset: params[:offset].to_i, filter: event_filter) .to_a + .map(&:present) Events::RenderService.new(current_user).execute(@events, atom_request: request.format.atom?) end diff --git a/app/graphql/types/base_field.rb b/app/graphql/types/base_field.rb index efeee4a7a4d..3ade1300c2d 100644 --- a/app/graphql/types/base_field.rb +++ b/app/graphql/types/base_field.rb @@ -10,6 +10,8 @@ module Types @calls_gitaly = !!kwargs.delete(:calls_gitaly) @constant_complexity = !!kwargs[:complexity] kwargs[:complexity] ||= field_complexity(kwargs[:resolver_class]) + @feature_flag = kwargs[:feature_flag] + kwargs = check_feature_flag(kwargs) super(*args, **kwargs, &block) end @@ -28,8 +30,27 @@ module Types @constant_complexity end + def visible?(context) + return false if feature_flag.present? && !Feature.enabled?(feature_flag) + + super + end + private + attr_reader :feature_flag + + def feature_documentation_message(key, description) + "#{description}. Available only when feature flag #{key} is enabled." + end + + def check_feature_flag(args) + args[:description] = feature_documentation_message(args[:feature_flag], args[:description]) if args[:feature_flag].present? + args.delete(:feature_flag) + + args + end + def field_complexity(resolver_class) if resolver_class field_resolver_complexity diff --git a/app/graphql/types/grafana_integration_type.rb b/app/graphql/types/grafana_integration_type.rb index e6c865fea53..f234008ee0d 100644 --- a/app/graphql/types/grafana_integration_type.rb +++ b/app/graphql/types/grafana_integration_type.rb @@ -10,14 +10,19 @@ module Types description: 'Internal ID of the Grafana integration' field :grafana_url, GraphQL::STRING_TYPE, null: false, description: 'Url for the Grafana host for the Grafana integration' - field :token, GraphQL::STRING_TYPE, null: false, - description: 'API token for the Grafana integration' field :enabled, GraphQL::BOOLEAN_TYPE, null: false, description: 'Indicates whether Grafana integration is enabled' - field :created_at, Types::TimeType, null: false, description: 'Timestamp of the issue\'s creation' field :updated_at, Types::TimeType, null: false, description: 'Timestamp of the issue\'s last activity' + + field :token, GraphQL::STRING_TYPE, null: false, + deprecation_reason: 'Plain text token has been masked for security reasons', + description: 'API token for the Grafana integration. Field is permanently masked.' + + def token + object.masked_token + end end end diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index b8d7685c2cf..011871f373f 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -368,8 +368,8 @@ module ProjectsHelper @project.grafana_integration&.grafana_url end - def grafana_integration_token - @project.grafana_integration&.token + def grafana_integration_masked_token + @project.grafana_integration&.masked_token end def grafana_integration_enabled? diff --git a/app/models/generic_commit_status.rb b/app/models/generic_commit_status.rb index 8a768b3a2c0..6c8bfc35334 100644 --- a/app/models/generic_commit_status.rb +++ b/app/models/generic_commit_status.rb @@ -1,11 +1,14 @@ # frozen_string_literal: true class GenericCommitStatus < CommitStatus + EXTERNAL_STAGE_IDX = 1_000_000 + before_validation :set_default_values validates :target_url, addressable_url: true, length: { maximum: 255 }, allow_nil: true + validate :name_uniqueness_across_types, unless: :importing? # GitHub compatible API alias_attribute :context, :name @@ -13,7 +16,7 @@ class GenericCommitStatus < CommitStatus def set_default_values self.context ||= 'default' self.stage ||= 'external' - self.stage_idx ||= 1000000 + self.stage_idx ||= EXTERNAL_STAGE_IDX end def tags @@ -25,4 +28,14 @@ class GenericCommitStatus < CommitStatus .new(self, current_user) .fabricate! end + + private + + def name_uniqueness_across_types + return if !pipeline || name.blank? + + if pipeline.statuses.by_name(name).where.not(type: type).exists? + errors.add(:name, :taken) + end + end end diff --git a/app/models/grafana_integration.rb b/app/models/grafana_integration.rb index ed4c279965a..00213732fee 100644 --- a/app/models/grafana_integration.rb +++ b/app/models/grafana_integration.rb @@ -8,11 +8,13 @@ class GrafanaIntegration < ApplicationRecord algorithm: 'aes-256-gcm', key: Settings.attr_encrypted_db_key_base_32 + before_validation :check_token_changes + validates :grafana_url, length: { maximum: 1024 }, addressable_url: { enforce_sanitization: true, ascii_only: true } - validates :token, :project, presence: true + validates :encrypted_token, :project, presence: true validates :enabled, inclusion: { in: [true, false] } @@ -23,4 +25,28 @@ class GrafanaIntegration < ApplicationRecord @client ||= ::Grafana::Client.new(api_url: grafana_url.chomp('/'), token: token) end + + def masked_token + mask(encrypted_token) + end + + def masked_token_was + mask(encrypted_token_was) + end + + private + + def token + decrypt(:token, encrypted_token) + end + + def check_token_changes + return unless [encrypted_token_was, masked_token_was].include?(token) + + clear_attribute_changes [:token, :encrypted_token, :encrypted_token_iv] + end + + def mask(token) + token&.squish&.gsub(/./, '*') + end end diff --git a/app/models/note.rb b/app/models/note.rb index 0434f0963d3..8af650e27aa 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -545,7 +545,8 @@ class Note < ApplicationRecord # if they are not equal, then there are private/confidential references as well user_visible_reference_count > 0 && user_visible_reference_count == total_reference_count else - referenced_mentionables(user).any? + refs = all_references(user) + refs.all.any? && refs.stateful_not_visible_counter == 0 end end diff --git a/app/models/project.rb b/app/models/project.rb index 8cb35904d92..064c647ac59 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2366,6 +2366,10 @@ class Project < ApplicationRecord end end + def template_source? + false + end + private def closest_namespace_setting(name) diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index 3a16f7dc239..c93a19bdc3d 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -21,6 +21,14 @@ class BasePolicy < DeclarativePolicy::Base with_options scope: :user, score: 0 condition(:deactivated) { @user&.deactivated? } + desc "User email is unconfirmed or user account is locked" + with_options scope: :user, score: 0 + condition(:inactive) do + Feature.enabled?(:inactive_policy_condition, default_enabled: true) && + @user && + !@user&.active_for_authentication? + end + with_options scope: :user, score: 0 condition(:external_user) { @user.nil? || @user.external? } diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb index 764d61a9e22..2bde7bcca08 100644 --- a/app/policies/global_policy.rb +++ b/app/policies/global_policy.rb @@ -36,6 +36,13 @@ class GlobalPolicy < BasePolicy enable :use_slash_commands end + rule { inactive }.policy do + prevent :log_in + prevent :access_api + prevent :access_git + prevent :use_slash_commands + end + rule { blocked | internal }.policy do prevent :log_in prevent :access_api diff --git a/app/presenters/event_presenter.rb b/app/presenters/event_presenter.rb index f31d362d5fa..5657e0b96bc 100644 --- a/app/presenters/event_presenter.rb +++ b/app/presenters/event_presenter.rb @@ -3,6 +3,18 @@ class EventPresenter < Gitlab::View::Presenter::Delegated presents :event + def initialize(subject, **attributes) + super + + @visible_to_user_cache = ActiveSupport::Cache::MemoryStore.new + end + + # Caching `visible_to_user?` method in the presenter beause it might be called multiple times. + def visible_to_user?(user = nil) + @visible_to_user_cache.fetch(user&.id) { super(user) } + end + + # implement cache here def resource_parent_name resource_parent&.full_name || '' end diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb index 3f0aedfbfb2..569b91de73e 100644 --- a/app/services/compare_service.rb +++ b/app/services/compare_service.rb @@ -18,7 +18,7 @@ class CompareService return unless raw_compare && raw_compare.base && raw_compare.head Compare.new(raw_compare, - target_project, + start_project, base_sha: base_sha, straight: straight) end diff --git a/app/services/projects/group_links/destroy_service.rb b/app/services/projects/group_links/destroy_service.rb index c96dcaae8d5..ea7d05551fd 100644 --- a/app/services/projects/group_links/destroy_service.rb +++ b/app/services/projects/group_links/destroy_service.rb @@ -6,6 +6,12 @@ module Projects def execute(group_link) return false unless group_link + if group_link.project.private? + TodosDestroyer::ProjectPrivateWorker.perform_in(Todo::WAIT_FOR_DELETE, project.id) + else + TodosDestroyer::ConfidentialIssueWorker.perform_in(Todo::WAIT_FOR_DELETE, nil, project.id) + end + group_link.destroy end end diff --git a/app/services/projects/import_export/export_service.rb b/app/services/projects/import_export/export_service.rb index 8344397f67d..38859c1efa4 100644 --- a/app/services/projects/import_export/export_service.rb +++ b/app/services/projects/import_export/export_service.rb @@ -4,6 +4,12 @@ module Projects module ImportExport class ExportService < BaseService def execute(after_export_strategy = nil, options = {}) + unless project.template_source? || can?(current_user, :admin_project, project) + raise ::Gitlab::ImportExport::Error.new( + "User with ID: %s does not have permission to Project %s with ID: %s." % + [current_user.id, project.name, project.id]) + end + @shared = project.import_export_shared save_all! diff --git a/app/views/projects/settings/operations/_grafana_integration.html.haml b/app/views/projects/settings/operations/_grafana_integration.html.haml index cd5b5abd9ce..69e42a6c4fb 100644 --- a/app/views/projects/settings/operations/_grafana_integration.html.haml +++ b/app/views/projects/settings/operations/_grafana_integration.html.haml @@ -1,2 +1,2 @@ .js-grafana-integration{ data: { operations_settings_endpoint: project_settings_operations_path(@project), - grafana_integration: { url: grafana_integration_url, token: grafana_integration_token, enabled: grafana_integration_enabled?.to_s } } } + grafana_integration: { url: grafana_integration_url, token: grafana_integration_masked_token, enabled: grafana_integration_enabled?.to_s } } } diff --git a/changelogs/unreleased/199211-fix-user-popover-glitch.yml b/changelogs/unreleased/199211-fix-user-popover-glitch.yml new file mode 100644 index 00000000000..65b50f50bf2 --- /dev/null +++ b/changelogs/unreleased/199211-fix-user-popover-glitch.yml @@ -0,0 +1,5 @@ +--- +title: Fix user popover glitch +merge_request: 23904 +author: +type: fixed diff --git a/changelogs/unreleased/nicolasdular-broadcast-type-api.yml b/changelogs/unreleased/nicolasdular-broadcast-type-api.yml new file mode 100644 index 00000000000..2a6f1120567 --- /dev/null +++ b/changelogs/unreleased/nicolasdular-broadcast-type-api.yml @@ -0,0 +1,5 @@ +--- +title: Add broadcast type to API +merge_request: +author: +type: changed diff --git a/changelogs/unreleased/sarnold-197129-graphql-feature-flag.yml b/changelogs/unreleased/sarnold-197129-graphql-feature-flag.yml new file mode 100644 index 00000000000..053d9cbd892 --- /dev/null +++ b/changelogs/unreleased/sarnold-197129-graphql-feature-flag.yml @@ -0,0 +1,5 @@ +--- +title: Add ability to hide GraphQL fields using GitLab Feature flags +merge_request: 23563 +author: +type: added diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 62fbc642908..ed5701b6f75 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -959,9 +959,9 @@ production: &base # # Turns on AWS Server-Side Encryption with Amazon S3-Managed Keys for backups, this is optional # # encryption: 'AES256' # # Turns on AWS Server-Side Encryption with Amazon Customer-Provided Encryption Keys for backups, this is optional - # # This should be set to the 256-bit, base64-encoded encryption key for Amazon S3 to use to encrypt or decrypt your data. + # # This should be set to the 256-bit encryption key for Amazon S3 to use to encrypt or decrypt your data. # # 'encryption' must also be set in order for this to have any effect. - # # encryption_key: '<base64 key>' + # # encryption_key: '<key>' # # Specifies Amazon S3 storage class to use for backups, this is optional # # storage_class: 'STANDARD' diff --git a/doc/api/broadcast_messages.md b/doc/api/broadcast_messages.md index 9a15e3ab89d..b9f9621e1f9 100644 --- a/doc/api/broadcast_messages.md +++ b/doc/api/broadcast_messages.md @@ -35,7 +35,8 @@ Example response: "font":"#FFFFFF", "id":1, "active": false, - "target_path": "*/welcome" + "target_path": "*/welcome", + "broadcast_type": "banner" } ] ``` @@ -71,7 +72,8 @@ Example response: "font":"#FFFFFF", "id":1, "active":false, - "target_path": "*/welcome" + "target_path": "*/welcome", + "broadcast_type": "banner" } ``` @@ -92,6 +94,8 @@ Parameters: | `ends_at` | datetime | no | Ending time (defaults to one hour from current time). | | `color` | string | no | Background color hex code. | | `font` | string | no | Foreground color hex code. | +| `target_path`| string | no | Target path of the broadcast message. | +| `broadcast_type`| string | no | Appearance type (defaults to banner) | Example request: @@ -110,7 +114,8 @@ Example response: "font":"#FFFFFF", "id":1, "active": true, - "target_path": "*/welcome" + "target_path": "*/welcome", + "broadcast_type": "notification", } ``` @@ -132,6 +137,8 @@ Parameters: | `ends_at` | datetime | no | Ending time. | | `color` | string | no | Background color hex code. | | `font` | string | no | Foreground color hex code. | +| `target_path`| string | no | Target path of the broadcast message. | +| `broadcast_type`| string | no | Appearance type (defaults to banner) | Example request: @@ -150,7 +157,8 @@ Example response: "font":"#FFFFFF", "id":1, "active": true, - "target_path": "*/welcome" + "target_path": "*/welcome", + "broadcast_type": "notification", } ``` diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 3cb0690abc8..8fc13dd140b 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -2658,9 +2658,9 @@ type GrafanaIntegration { id: ID! """ - API token for the Grafana integration + API token for the Grafana integration. Field is permanently masked. """ - token: String! + token: String! @deprecated(reason: "Plain text token has been masked for security reasons") """ Timestamp of the issue's last activity diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 8e94d4b33d3..f9eda31540a 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -17380,7 +17380,7 @@ }, { "name": "token", - "description": "API token for the Grafana integration", + "description": "API token for the Grafana integration. Field is permanently masked.", "args": [ ], @@ -17393,8 +17393,8 @@ "ofType": null } }, - "isDeprecated": false, - "deprecationReason": null + "isDeprecated": true, + "deprecationReason": "Plain text token has been masked for security reasons" }, { "name": "updatedAt", diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index dc6517f7ea4..9ed47f2aea2 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -403,7 +403,7 @@ Autogenerated return type of EpicTreeReorder | `enabled` | Boolean! | Indicates whether Grafana integration is enabled | | `grafanaUrl` | String! | Url for the Grafana host for the Grafana integration | | `id` | ID! | Internal ID of the Grafana integration | -| `token` | String! | API token for the Grafana integration | +| `token` | String! | API token for the Grafana integration. Field is permanently masked. | | `updatedAt` | Time! | Timestamp of the issue's last activity | ## Group diff --git a/doc/integration/omniauth.md b/doc/integration/omniauth.md index 2e89093321a..286d06d52a8 100644 --- a/doc/integration/omniauth.md +++ b/doc/integration/omniauth.md @@ -266,8 +266,8 @@ You can enable profile syncing from selected OmniAuth providers and for all or f When authenticating using LDAP, the user's name and email are always synced. ```ruby -gitlab_rails['sync_profile_from_provider'] = ['twitter', 'google_oauth2'] -gitlab_rails['sync_profile_attributes'] = ['name', 'email', 'location'] +gitlab_rails['omniauth_sync_profile_from_provider'] = ['twitter', 'google_oauth2'] +gitlab_rails['omniauth_sync_profile_attributes'] = ['name', 'email', 'location'] ``` **For installations from source** diff --git a/doc/raketasks/backup_restore.md b/doc/raketasks/backup_restore.md index 78d634b7795..0b2b4f03167 100644 --- a/doc/raketasks/backup_restore.md +++ b/doc/raketasks/backup_restore.md @@ -355,10 +355,10 @@ For installations from source: # Turns on AWS Server-Side Encryption with Amazon S3-Managed Keys for backups, this is optional # encryption: 'AES256' # Turns on AWS Server-Side Encryption with Amazon Customer-Provided Encryption Keys for backups, this is optional - # This should be set to the base64-encoded encryption key for Amazon S3 to use to encrypt or decrypt your data. + # This should be set to the encryption key for Amazon S3 to use to encrypt or decrypt your data. # 'encryption' must also be set in order for this to have any effect. # To avoid storing the key on disk, the key can also be specified via the `GITLAB_BACKUP_ENCRYPTION_KEY` environment variable. - # encryption_key: '<base64 key>' + # encryption_key: '<key>' # Specifies Amazon S3 storage class to use for backups, this is optional # storage_class: 'STANDARD' ``` diff --git a/doc/user/asciidoc.md b/doc/user/asciidoc.md index b4d3cb58e97..da6bf287955 100644 --- a/doc/user/asciidoc.md +++ b/doc/user/asciidoc.md @@ -221,6 +221,11 @@ include::basics.adoc[] include::https://example.org/installation.adoc[] ``` +To guarantee good system performance and prevent malicious documents causing +problems, GitLab enforces a **maximum limit** on the number of include directives +processed in any one document. Currently a total of 32 documents can be +included, a number that is inclusive of transitive dependencies. + ### Blocks ```asciidoc diff --git a/lib/api/broadcast_messages.rb b/lib/api/broadcast_messages.rb index 48d4f1a0a63..af7c69f857e 100644 --- a/lib/api/broadcast_messages.rb +++ b/lib/api/broadcast_messages.rb @@ -35,6 +35,7 @@ module API optional :color, type: String, desc: 'Background color' optional :font, type: String, desc: 'Foreground color' optional :target_path, type: String, desc: 'Target path' + optional :broadcast_type, type: String, values: BroadcastMessage.broadcast_types.keys, desc: 'Broadcast type. Defaults to banner', default: -> { 'banner' } end post do authenticated_as_admin! @@ -73,6 +74,7 @@ module API optional :color, type: String, desc: 'Background color' optional :font, type: String, desc: 'Foreground color' optional :target_path, type: String, desc: 'Target path' + optional :broadcast_type, type: String, values: BroadcastMessage.broadcast_types.keys, desc: 'Broadcast Type' end put ':id' do authenticated_as_admin! diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb index 2d5253fe9f1..b4c5d7869a2 100644 --- a/lib/api/commit_statuses.rb +++ b/lib/api/commit_statuses.rb @@ -83,6 +83,8 @@ module API user: current_user, protected: user_project.protected_for?(ref)) + authorize! :update_pipeline, pipeline + status = GenericCommitStatus.running_or_pending.find_or_initialize_by( project: user_project, pipeline: pipeline, diff --git a/lib/api/commits.rb b/lib/api/commits.rb index 63a7fdfa3ab..9dcf9b015aa 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -154,7 +154,7 @@ module API not_found! 'Commit' unless commit - present commit, with: Entities::CommitDetail, stats: params[:stats] + present commit, with: Entities::CommitDetail, stats: params[:stats], current_user: current_user end desc 'Get the diff for a specific commit of a project' do diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 04e1cf6fb96..681bb98b155 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -152,8 +152,18 @@ module API class CommitDetail < Commit expose :stats, using: Entities::CommitStats, if: :stats expose :status - expose :last_pipeline, using: 'API::Entities::PipelineBasic' expose :project_id + + expose :last_pipeline do |commit, options| + pipeline = commit.last_pipeline if can_read_pipeline? + ::API::Entities::PipelineBasic.represent(pipeline, options) + end + + private + + def can_read_pipeline? + Ability.allowed?(options[:current_user], :read_pipeline, object.last_pipeline) + end end class CommitSignature < Grape::Entity @@ -932,7 +942,7 @@ module API end class BroadcastMessage < Grape::Entity - expose :message, :starts_at, :ends_at, :color, :font, :target_path + expose :message, :starts_at, :ends_at, :color, :font, :target_path, :broadcast_type end class ApplicationStatistics < Grape::Entity diff --git a/lib/api/files.rb b/lib/api/files.rb index 0b438fb5bbc..feed22d188c 100644 --- a/lib/api/files.rb +++ b/lib/api/files.rb @@ -127,6 +127,7 @@ module API get ":id/repository/files/:file_path/raw", requirements: FILE_ENDPOINT_REQUIREMENTS do assign_file_vars! + no_cache_headers set_http_headers(blob_data) send_git_blob @repo, @blob diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 688b1cc6f17..001fb92ec52 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -258,11 +258,21 @@ module API end def require_gitlab_workhorse! + verify_workhorse_api! + unless env['HTTP_GITLAB_WORKHORSE'].present? forbidden!('Request should be executed via GitLab Workhorse') end end + def verify_workhorse_api! + Gitlab::Workhorse.verify_api_request!(request.headers) + rescue => e + Gitlab::ErrorTracking.track_exception(e) + + forbidden! + end + def require_pages_enabled! not_found! unless user_project.pages_available? end diff --git a/lib/api/helpers/headers_helpers.rb b/lib/api/helpers/headers_helpers.rb index 7553af9d156..908c57bb04e 100644 --- a/lib/api/helpers/headers_helpers.rb +++ b/lib/api/helpers/headers_helpers.rb @@ -3,6 +3,8 @@ module API module Helpers module HeadersHelpers + include Gitlab::NoCacheHeaders + def set_http_headers(header_data) header_data.each do |key, value| if value.is_a?(Enumerable) @@ -12,6 +14,12 @@ module API header "X-Gitlab-#{key.to_s.split('_').collect(&:capitalize).join('-')}", value.to_s end end + + def no_cache_headers + DEFAULT_GITLAB_NO_CACHE_HEADERS.each do |k, v| + header k, v + end + end end end end diff --git a/lib/banzai/reference_parser/base_parser.rb b/lib/banzai/reference_parser/base_parser.rb index 9160c0e14cf..9ecbc3ecec2 100644 --- a/lib/banzai/reference_parser/base_parser.rb +++ b/lib/banzai/reference_parser/base_parser.rb @@ -201,12 +201,14 @@ module Banzai gather_references(nodes) end - # Gathers the references for the given HTML nodes. + # Gathers the references for the given HTML nodes. Returns visible + # references and a list of nodes which are not visible to the user def gather_references(nodes) nodes = nodes_user_can_reference(current_user, nodes) - nodes = nodes_visible_to_user(current_user, nodes) + visible = nodes_visible_to_user(current_user, nodes) + not_visible = nodes - visible - referenced_by(nodes) + { visible: referenced_by(visible), not_visible: not_visible } end # Returns a Hash containing the projects for a given list of HTML nodes. diff --git a/lib/gitlab/asciidoc.rb b/lib/gitlab/asciidoc.rb index da65caa6c9c..8d072422e17 100644 --- a/lib/gitlab/asciidoc.rb +++ b/lib/gitlab/asciidoc.rb @@ -11,6 +11,7 @@ module Gitlab # the resulting HTML through HTML pipeline filters. module Asciidoc MAX_INCLUDE_DEPTH = 5 + MAX_INCLUDES = 32 DEFAULT_ADOC_ATTRS = { 'showtitle' => true, 'sectanchors' => true, @@ -40,6 +41,7 @@ module Gitlab extensions: extensions } context[:pipeline] = :ascii_doc + context[:max_includes] = [MAX_INCLUDES, context[:max_includes]].compact.min plantuml_setup diff --git a/lib/gitlab/asciidoc/include_processor.rb b/lib/gitlab/asciidoc/include_processor.rb index 6e0b7ce60ba..53d1135a2d7 100644 --- a/lib/gitlab/asciidoc/include_processor.rb +++ b/lib/gitlab/asciidoc/include_processor.rb @@ -14,6 +14,8 @@ module Gitlab @context = context @repository = context[:repository] || context[:project].try(:repository) + @max_includes = context[:max_includes].to_i + @included = [] # Note: Asciidoctor calls #freeze on extensions, so we can't set new # instance variables after initialization. @@ -28,8 +30,11 @@ module Gitlab def include_allowed?(target, reader) doc = reader.document - return false if doc.attributes.fetch('max-include-depth').to_i < 1 + max_include_depth = doc.attributes.fetch('max-include-depth').to_i + + return false if max_include_depth < 1 return false if target_uri?(target) + return false if included.size >= max_includes true end @@ -62,7 +67,7 @@ module Gitlab private - attr_accessor :context, :repository, :cache + attr_reader :context, :repository, :cache, :max_includes, :included # Gets a Blob at a path for a specific revision. # This method will check that the Blob exists and contains readable text. @@ -77,6 +82,8 @@ module Gitlab raise 'Blob not found' unless blob raise 'File is not readable' unless blob.readable_text? + included << filename + blob end diff --git a/lib/gitlab/git/cross_repo_comparer.rb b/lib/gitlab/git/cross_repo_comparer.rb new file mode 100644 index 00000000000..3958373f7cb --- /dev/null +++ b/lib/gitlab/git/cross_repo_comparer.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +module Gitlab + module Git + class CrossRepoComparer + attr_reader :source_repo, :target_repo + + def initialize(source_repo, target_repo) + @source_repo = source_repo + @target_repo = target_repo + end + + def compare(source_ref, target_ref, straight:) + ensuring_ref_in_source(target_ref) do |target_commit_id| + Gitlab::Git::Compare.new( + source_repo, + target_commit_id, + source_ref, + straight: straight + ) + end + end + + private + + def ensuring_ref_in_source(ref, &blk) + return yield ref if source_repo == target_repo + + # If the commit doesn't exist in the target, there's nothing we can do + commit_id = target_repo.commit(ref)&.sha + return unless commit_id + + # The commit pointed to by ref may exist in the source even when they + # are different repositories. This is particularly true of close forks, + # but may also be the case if a temporary ref for this comparison has + # already been created in the past, and the result hasn't been GC'd yet. + return yield commit_id if source_repo.commit(commit_id) + + # Worst case: the commit is not in the source repo so we need to fetch + # it. Use a temporary ref and clean up afterwards + with_commit_in_source_tmp(commit_id, &blk) + end + + # Fetch the ref into source_repo from target_repo, using a temporary ref + # name that will be deleted once the method completes. This is a no-op if + # fetching the source branch fails + def with_commit_in_source_tmp(commit_id, &blk) + tmp_ref = "refs/tmp/#{SecureRandom.hex}" + + yield commit_id if source_repo.fetch_source_branch!(target_repo, commit_id, tmp_ref) + ensure + source_repo.delete_refs(tmp_ref) # best-effort + end + end + end +end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index ed3e7a1e39c..0120e3be14c 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -746,29 +746,9 @@ module Gitlab end def compare_source_branch(target_branch_name, source_repository, source_branch_name, straight:) - reachable_ref = - if source_repository == self - source_branch_name - else - # If a tmp ref was created before for a separate repo comparison (forks), - # we're able to short-circuit the tmp ref re-creation: - # 1. Take the SHA from the source repo - # 2. Read that in the current "target" repo - # 3. If that SHA is still known (readable), it means GC hasn't - # cleaned it up yet, so we can use it instead re-writing the tmp ref. - source_commit_id = source_repository.commit(source_branch_name)&.sha - commit(source_commit_id)&.sha if source_commit_id - end - - return compare(target_branch_name, reachable_ref, straight: straight) if reachable_ref - - tmp_ref = "refs/tmp/#{SecureRandom.hex}" - - return unless fetch_source_branch!(source_repository, source_branch_name, tmp_ref) - - compare(target_branch_name, tmp_ref, straight: straight) - ensure - delete_refs(tmp_ref) if tmp_ref + CrossRepoComparer + .new(source_repository, self) + .compare(source_branch_name, target_branch_name, straight: straight) end def write_ref(ref_path, ref, old_ref: nil) @@ -1046,13 +1026,6 @@ module Gitlab private - def compare(base_ref, head_ref, straight:) - Gitlab::Git::Compare.new(self, - base_ref, - head_ref, - straight: straight) - end - def empty_diff_stats Gitlab::Git::DiffStatsCollection.new([]) end diff --git a/lib/gitlab/no_cache_headers.rb b/lib/gitlab/no_cache_headers.rb new file mode 100644 index 00000000000..f80ca2c1369 --- /dev/null +++ b/lib/gitlab/no_cache_headers.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Gitlab + module NoCacheHeaders + DEFAULT_GITLAB_NO_CACHE_HEADERS = { + 'Cache-Control' => "#{ActionDispatch::Http::Cache::Response::DEFAULT_CACHE_CONTROL}, no-store, no-cache", + 'Pragma' => 'no-cache', # HTTP 1.0 compatibility + 'Expires' => 'Fri, 01 Jan 1990 00:00:00 GMT' + }.freeze + + def no_cache_headers + raise "#no_cache_headers is not implemented for this object" + end + end +end diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb index f095ac9ffd1..519eb49658a 100644 --- a/lib/gitlab/reference_extractor.rb +++ b/lib/gitlab/reference_extractor.rb @@ -6,11 +6,16 @@ module Gitlab REFERABLES = %i(user issue label milestone mentioned_user mentioned_group mentioned_project merge_request snippet commit commit_range directly_addressed_user epic).freeze attr_accessor :project, :current_user, :author + # This counter is increased by a number of references filtered out by + # banzai reference exctractor. Note that this counter is stateful and + # not idempotent and is increased whenever you call `references`. + attr_reader :stateful_not_visible_counter def initialize(project, current_user = nil) @project = project @current_user = current_user @references = {} + @stateful_not_visible_counter = 0 super() end @@ -20,11 +25,15 @@ module Gitlab end def references(type) - super(type, project, current_user) + refs = super(type, project, current_user) + @stateful_not_visible_counter += refs[:not_visible].count + + refs[:visible] end def reset_memoized_values @references = {} + @stateful_not_visible_counter = 0 super() end diff --git a/qa/qa/specs/features/api/3_create/repository/files_spec.rb b/qa/qa/specs/features/api/3_create/repository/files_spec.rb index f6f020da472..dc471128dae 100644 --- a/qa/qa/specs/features/api/3_create/repository/files_spec.rb +++ b/qa/qa/specs/features/api/3_create/repository/files_spec.rb @@ -59,5 +59,48 @@ module QA a_hash_including(message: '202 Accepted') ) end + + describe 'raw file access' do + let(:svg_file) do + <<-SVG + <?xml version="1.0" standalone="no"?> + <!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd"> + + <svg version="1.1" baseProfile="full" xmlns="http://www.w3.org/2000/svg"> + <polygon id="triangle" points="0,0 0,50 50,0" fill="#009900" stroke="#004400"/> + <script type="text/javascript"> + alert("surprise"); + </script> + </svg> + SVG + end + + it 'sets no-cache headers as expected' do + create_project_request = Runtime::API::Request.new(@api_client, '/projects') + post create_project_request.url, path: project_name, name: project_name + + create_file_request = Runtime::API::Request.new(@api_client, "/projects/#{sanitized_project_path}/repository/files/test.svg") + post create_file_request.url, branch: 'master', content: svg_file, commit_message: 'Add test.svg' + + get_file_request = Runtime::API::Request.new(@api_client, "/projects/#{sanitized_project_path}/repository/files/test.svg/raw", ref: 'master') + + 3.times do + response = get get_file_request.url + + # Subsequent responses aren't cached, so headers should match from + # request to request, especially a 200 response rather than a 304 + # (indicating a cached response.) Further, :content_disposition + # should include `attachment` for all responses. + # + expect(response.headers[:cache_control]).to include("no-store") + expect(response.headers[:cache_control]).to include("no-cache") + expect(response.headers[:pragma]).to eq("no-cache") + expect(response.headers[:expires]).to eq("Fri, 01 Jan 1990 00:00:00 GMT") + expect(response.headers[:content_disposition]).to include("attachment") + expect(response.headers[:content_disposition]).not_to include("inline") + expect(response.headers[:content_type]).to include("image/svg+xml") + end + end + end end end diff --git a/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb b/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb index 63774d1cdfa..a252b7809b8 100644 --- a/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb +++ b/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb @@ -5,9 +5,9 @@ require 'pathname' module QA context 'Configure' do let(:project) do - Resource::Project.fabricate_via_api! do |p| - p.name = Runtime::Env.auto_devops_project_name || 'autodevops-project' - p.auto_devops_enabled = true + Resource::Project.fabricate_via_api! do |project| + project.name = Runtime::Env.auto_devops_project_name || 'autodevops-project' + project.auto_devops_enabled = true end end diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb index a733c3ecaa1..305419efe96 100644 --- a/spec/controllers/dashboard_controller_spec.rb +++ b/spec/controllers/dashboard_controller_spec.rb @@ -23,6 +23,47 @@ describe DashboardController do end end + describe "GET activity as JSON" do + render_views + + let(:user) { create(:user) } + let(:project) { create(:project, :public, issues_access_level: ProjectFeature::PRIVATE) } + + before do + create(:event, :created, project: project, target: create(:issue)) + + sign_in(user) + + request.cookies[:event_filter] = 'all' + end + + context 'when user has permission to see the event' do + before do + project.add_developer(user) + end + + it 'returns count' do + get :activity, params: { format: :json } + + expect(json_response['count']).to eq(1) + end + end + + context 'when user has no permission to see the event' do + it 'filters out invisible event' do + get :activity, params: { format: :json } + + expect(json_response['html']).to include(_('No activities found')) + end + + it 'filters out invisible event when calculating the count' do + get :activity, params: { format: :json } + + expect(json_response['count']).to eq(0) + end + end + end + it_behaves_like 'authenticates sessionless user', :issues, :atom, author_id: User.first it_behaves_like 'authenticates sessionless user', :issues_calendar, :ics diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 3399ad563a1..07b82bdff04 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -47,7 +47,7 @@ describe GroupsController do it 'assigns events for all the projects in the group', :sidekiq_might_not_need_inline do subject - expect(assigns(:events)).to contain_exactly(event) + expect(assigns(:events).map(&:id)).to contain_exactly(event.id) end end end @@ -119,12 +119,12 @@ describe GroupsController do describe 'GET #activity' do render_views - before do - sign_in(user) - project - end - context 'as json' do + before do + sign_in(user) + project + end + it 'includes events from all projects in group and subgroups', :sidekiq_might_not_need_inline do 2.times do project = create(:project, group: group) @@ -141,6 +141,31 @@ describe GroupsController do expect(assigns(:projects).limit_value).to be_nil end end + + context 'when user has no permission to see the event' do + let(:user) { create(:user) } + let(:group) { create(:group) } + let(:project) { create(:project, group: group) } + + let(:project_with_restricted_access) do + create(:project, :public, issues_access_level: ProjectFeature::PRIVATE, group: group) + end + + before do + create(:event, project: project) + create(:event, :created, project: project_with_restricted_access, target: create(:issue)) + + group.add_guest(user) + + sign_in(user) + end + + it 'filters out invisible event' do + get :activity, params: { id: group.to_param }, format: :json + + expect(json_response['count']).to eq(1) + end + end end describe 'POST #create' do diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index a44218a5d2f..67e24841dee 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -64,6 +64,46 @@ describe ProjectsController do end end + describe "GET #activity as JSON" do + render_views + + let(:project) { create(:project, :public, issues_access_level: ProjectFeature::PRIVATE) } + + before do + create(:event, :created, project: project, target: create(:issue)) + + sign_in(user) + + request.cookies[:event_filter] = 'all' + end + + context 'when user has permission to see the event' do + before do + project.add_developer(user) + end + + it 'returns count' do + get :activity, params: { namespace_id: project.namespace, id: project, format: :json } + + expect(json_response['count']).to eq(1) + end + end + + context 'when user has no permission to see the event' do + it 'filters out invisible event' do + get :activity, params: { namespace_id: project.namespace, id: project, format: :json } + + expect(json_response['html']).to eq("\n") + end + + it 'filters out invisible event when calculating the count' do + get :activity, params: { namespace_id: project.namespace, id: project, format: :json } + + expect(json_response['count']).to eq(0) + end + end + end + describe "GET show" do context "user not project member" do before do diff --git a/spec/features/projects/user_sees_user_popover_spec.rb b/spec/features/projects/user_sees_user_popover_spec.rb new file mode 100644 index 00000000000..adbf9073d59 --- /dev/null +++ b/spec/features/projects/user_sees_user_popover_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'User sees user popover', :js do + set(:project) { create(:project, :repository) } + + let(:user) { project.creator } + let(:merge_request) do + create(:merge_request, source_project: project, target_project: project) + end + + before do + project.add_maintainer(user) + sign_in(user) + end + + subject { page } + + describe 'hovering over a user link in a merge request' do + before do + visit project_merge_request_path(project, merge_request) + end + + it 'displays user popover' do + popover_selector = '.user-popover' + + find('.js-user-link').hover + + expect(page).to have_css(popover_selector, visible: true) + + page.within(popover_selector) do + expect(page).to have_content(user.name) + end + end + end +end diff --git a/spec/frontend/notes/components/note_app_spec.js b/spec/frontend/notes/components/note_app_spec.js index f9b69e72619..a51c7c57f6c 100644 --- a/spec/frontend/notes/components/note_app_spec.js +++ b/spec/frontend/notes/components/note_app_spec.js @@ -12,6 +12,8 @@ import '~/behaviors/markdown/render_gfm'; import * as mockData from '../../notes/mock_data'; import * as urlUtility from '~/lib/utils/url_utility'; +jest.mock('~/user_popovers', () => jest.fn()); + setTestTimeout(1000); describe('note_app', () => { diff --git a/spec/graphql/features/authorization_spec.rb b/spec/graphql/features/authorization_spec.rb index 7ad6a622b4b..5ef1bced179 100644 --- a/spec/graphql/features/authorization_spec.rb +++ b/spec/graphql/features/authorization_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe 'Gitlab::Graphql::Authorization' do + include GraphqlHelpers + set(:user) { create(:user) } let(:permission_single) { :foo } @@ -300,37 +302,4 @@ describe 'Gitlab::Graphql::Authorization' do allow(Ability).to receive(:allowed?).with(user, permission, test_object).and_return(true) end end - - def type_factory - Class.new(Types::BaseObject) do - graphql_name 'TestType' - - field :name, GraphQL::STRING_TYPE, null: true - - yield(self) if block_given? - end - end - - def query_factory - Class.new(Types::BaseObject) do - graphql_name 'TestQuery' - - yield(self) if block_given? - end - end - - def execute_query(query_type) - schema = Class.new(GraphQL::Schema) do - use Gitlab::Graphql::Authorize - use Gitlab::Graphql::Connections - - query(query_type) - end - - schema.execute( - query_string, - context: { current_user: user }, - variables: {} - ) - end end diff --git a/spec/graphql/features/feature_flag_spec.rb b/spec/graphql/features/feature_flag_spec.rb new file mode 100644 index 00000000000..13b1e472fab --- /dev/null +++ b/spec/graphql/features/feature_flag_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Graphql Field feature flags' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + + let(:feature_flag) { 'test_feature' } + let(:test_object) { double(name: 'My name') } + let(:query_string) { '{ item() { name } }' } + let(:result) { execute_query(query_type)['data'] } + + subject { result } + + describe 'Feature flagged field' do + let(:type) { type_factory } + + let(:query_type) do + query_factory do |query| + query.field :item, type, null: true, feature_flag: feature_flag, resolve: ->(obj, args, ctx) { test_object } + end + end + + it 'returns the value when feature is enabled' do + expect(subject['item']).to eq('name' => test_object.name) + end + + it 'returns nil when the feature is disabled' do + stub_feature_flags(feature_flag => false) + + expect(subject).to be_nil + end + end +end diff --git a/spec/graphql/types/base_field_spec.rb b/spec/graphql/types/base_field_spec.rb index 77ef8933717..1f82f316aa7 100644 --- a/spec/graphql/types/base_field_spec.rb +++ b/spec/graphql/types/base_field_spec.rb @@ -111,5 +111,70 @@ describe Types::BaseField do end end end + + describe '#visible?' do + context 'and has a feature_flag' do + let(:flag) { :test_feature } + let(:field) { described_class.new(name: 'test', type: GraphQL::STRING_TYPE, feature_flag: flag, null: false) } + let(:context) { {} } + + it 'returns false if the feature is not enabled' do + stub_feature_flags(flag => false) + + expect(field.visible?(context)).to eq(false) + end + + it 'returns true if the feature is enabled' do + expect(field.visible?(context)).to eq(true) + end + + context 'falsey feature_flag values' do + using RSpec::Parameterized::TableSyntax + + where(:flag, :feature_value, :visible) do + '' | false | true + '' | true | true + nil | false | true + nil | true | true + end + + with_them do + it 'returns the correct value' do + stub_feature_flags(flag => feature_value) + + expect(field.visible?(context)).to eq(visible) + end + end + end + end + end + + describe '#description' do + context 'feature flag given' do + let(:field) { described_class.new(name: 'test', type: GraphQL::STRING_TYPE, feature_flag: flag, null: false, description: 'Test description') } + let(:flag) { :test_flag } + + it 'prepends the description' do + expect(field.description). to eq 'Test description. Available only when feature flag test_flag is enabled.' + end + + context 'falsey feature_flag values' do + using RSpec::Parameterized::TableSyntax + + where(:flag, :feature_value) do + '' | false + '' | true + nil | false + nil | true + end + + with_them do + it 'returns the correct description' do + expect(field.description).to eq('Test description') + end + end + end + end + end end end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index c7e454771bb..7fc568bb960 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -935,14 +935,14 @@ describe ProjectsHelper do helper.instance_variable_set(:@project, project) end - subject { helper.grafana_integration_token } + subject { helper.grafana_integration_masked_token } it { is_expected.to eq(nil) } context 'grafana integration exists' do let!(:grafana_integration) { create(:grafana_integration, project: project) } - it { is_expected.to eq(grafana_integration.token) } + it { is_expected.to eq(grafana_integration.masked_token) } end end diff --git a/spec/javascripts/frequent_items/utils_spec.js b/spec/javascripts/frequent_items/utils_spec.js index 2480af5b31d..fd5bd002428 100644 --- a/spec/javascripts/frequent_items/utils_spec.js +++ b/spec/javascripts/frequent_items/utils_spec.js @@ -1,5 +1,10 @@ import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils'; -import { isMobile, getTopFrequentItems, updateExistingFrequentItem } from '~/frequent_items/utils'; +import { + isMobile, + getTopFrequentItems, + updateExistingFrequentItem, + sanitizeItem, +} from '~/frequent_items/utils'; import { HOUR_IN_MS, FREQUENT_ITEMS } from '~/frequent_items/constants'; import { mockProject, unsortedFrequentItems, sortedFrequentItems } from './mock_data'; @@ -92,4 +97,16 @@ describe('Frequent Items utils spec', () => { expect(result.frequency).toBe(mockedProject.frequency); }); }); + + describe('sanitizeItem', () => { + it('strips HTML tags for name and namespace', () => { + const input = { + name: '<br><b>test</b>', + namespace: '<br>test', + id: 1, + }; + + expect(sanitizeItem(input)).toEqual({ name: 'test', namespace: 'test', id: 1 }); + }); + }); }); diff --git a/spec/javascripts/user_popovers_spec.js b/spec/javascripts/user_popovers_spec.js index e2fc359644d..3962f837a00 100644 --- a/spec/javascripts/user_popovers_spec.js +++ b/spec/javascripts/user_popovers_spec.js @@ -10,9 +10,14 @@ describe('User Popovers', () => { const dummyUser = { name: 'root' }; const dummyUserStatus = { message: 'active' }; + let popovers; + const triggerEvent = (eventName, el) => { - const event = document.createEvent('MouseEvents'); - event.initMouseEvent(eventName, true, true, window); + const event = new MouseEvent(eventName, { + bubbles: true, + cancelable: true, + view: window, + }); el.dispatchEvent(event); }; @@ -26,46 +31,54 @@ describe('User Popovers', () => { const userStatusCacheSpy = () => Promise.resolve(dummyUserStatus); spyOn(UsersCache, 'retrieveStatusById').and.callFake(userId => userStatusCacheSpy(userId)); - initUserPopovers(document.querySelectorAll('.js-user-link')); + popovers = initUserPopovers(document.querySelectorAll(selector)); }); - it('Should Show+Hide Popover on mouseenter and mouseleave', done => { - const targetLink = document.querySelector(selector); - const { userId } = targetLink.dataset; - triggerEvent('mouseenter', targetLink); + it('initializes a popover for each js-user-link element found in the document', () => { + expect(document.querySelectorAll(selector).length).toBe(popovers.length); + }); - setTimeout(() => { - const shownPopover = document.querySelector('.popover'); + describe('when user link emits mouseenter event', () => { + let userLink; - expect(shownPopover).not.toBeNull(); - expect(targetLink.getAttribute('aria-describedby')).not.toBeNull(); + beforeEach(() => { + userLink = document.querySelector(selector); - expect(shownPopover.innerHTML).toContain(dummyUser.name); - expect(UsersCache.retrieveById).toHaveBeenCalledWith(userId.toString()); + triggerEvent('mouseenter', userLink); + }); - triggerEvent('mouseleave', targetLink); + it('removes title attribute from user links', () => { + expect(userLink.getAttribute('title')).toBeFalsy(); + expect(userLink.dataset.originalTitle).toBeFalsy(); + }); - setTimeout(() => { - // After Mouse leave it should be hidden now - expect(document.querySelector('.popover')).toBeNull(); - expect(targetLink.getAttribute('aria-describedby')).toBeNull(); - done(); - }); - }, 210); // We need to wait until the 200ms mouseover delay is over, only then the popover will be visible - }); + it('populates popovers with preloaded user data', () => { + const { name, userId, username } = userLink.dataset; + const [firstPopover] = popovers; + + expect(firstPopover.$props.user).toEqual( + jasmine.objectContaining({ + name, + userId, + username, + }), + ); + }); - it('Should Not show a popover on short mouse over', done => { - const targetLink = document.querySelector(selector); - const { userId } = targetLink.dataset; - triggerEvent('mouseenter', targetLink); + it('fetches user info and status from the user cache', () => { + const { userId } = userLink.dataset; - setTimeout(() => { - expect(document.querySelector('.popover')).toBeNull(); - expect(UsersCache.retrieveById).not.toHaveBeenCalledWith(userId.toString()); + expect(UsersCache.retrieveById).toHaveBeenCalledWith(userId); + expect(UsersCache.retrieveStatusById).toHaveBeenCalledWith(userId); + }); + }); - triggerEvent('mouseleave', targetLink); + it('removes aria-describedby attribute from the user link on mouseleave', () => { + const userLink = document.querySelector(selector); - done(); - }); + userLink.setAttribute('aria-describedby', 'popover'); + triggerEvent('mouseleave', userLink); + + expect(userLink.getAttribute('aria-describedby')).toBe(null); }); }); diff --git a/spec/lib/banzai/reference_parser/mentioned_group_parser_spec.rb b/spec/lib/banzai/reference_parser/mentioned_group_parser_spec.rb index 30b99f3eda7..8346ba93f88 100644 --- a/spec/lib/banzai/reference_parser/mentioned_group_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/mentioned_group_parser_spec.rb @@ -19,7 +19,7 @@ describe Banzai::ReferenceParser::MentionedGroupParser do it 'returns empty array' do link['data-group'] = project.group.id.to_s - expect(subject.gather_references([link])).to eq([]) + expect_gathered_references(subject.gather_references([link]), [], 1) end end @@ -30,7 +30,7 @@ describe Banzai::ReferenceParser::MentionedGroupParser do end it 'returns groups' do - expect(subject.gather_references([link])).to eq([group]) + expect_gathered_references(subject.gather_references([link]), [group], 0) end end @@ -38,7 +38,7 @@ describe Banzai::ReferenceParser::MentionedGroupParser do it 'returns an empty Array' do link['data-group'] = 'test-non-existing' - expect(subject.gather_references([link])).to eq([]) + expect_gathered_references(subject.gather_references([link]), [], 1) end end end diff --git a/spec/lib/banzai/reference_parser/mentioned_project_parser_spec.rb b/spec/lib/banzai/reference_parser/mentioned_project_parser_spec.rb index 154f7c4dc36..b99c02351d0 100644 --- a/spec/lib/banzai/reference_parser/mentioned_project_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/mentioned_project_parser_spec.rb @@ -19,7 +19,7 @@ describe Banzai::ReferenceParser::MentionedProjectParser do it 'returns empty Array' do link['data-project'] = project.id.to_s - expect(subject.gather_references([link])).to eq([]) + expect_gathered_references(subject.gather_references([link]), [], 1) end end @@ -30,7 +30,7 @@ describe Banzai::ReferenceParser::MentionedProjectParser do end it 'returns an Array of referenced projects' do - expect(subject.gather_references([link])).to eq([project]) + expect_gathered_references(subject.gather_references([link]), [project], 0) end end @@ -38,7 +38,7 @@ describe Banzai::ReferenceParser::MentionedProjectParser do it 'returns an empty Array' do link['data-project'] = 'inexisting-project-id' - expect(subject.gather_references([link])).to eq([]) + expect_gathered_references(subject.gather_references([link]), [], 1) end end end diff --git a/spec/lib/banzai/reference_parser/mentioned_user_parser_spec.rb b/spec/lib/banzai/reference_parser/mentioned_user_parser_spec.rb index 1be279375bd..b10e5d19828 100644 --- a/spec/lib/banzai/reference_parser/mentioned_user_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/mentioned_user_parser_spec.rb @@ -22,7 +22,7 @@ describe Banzai::ReferenceParser::MentionedUserParser do end it 'returns empty list of users' do - expect(subject.gather_references([link])).to eq([]) + expect_gathered_references(subject.gather_references([link]), [], 0) end end end @@ -35,7 +35,7 @@ describe Banzai::ReferenceParser::MentionedUserParser do end it 'returns empty list of users' do - expect(subject.gather_references([link])).to eq([]) + expect_gathered_references(subject.gather_references([link]), [], 0) end end end @@ -44,7 +44,7 @@ describe Banzai::ReferenceParser::MentionedUserParser do it 'returns an Array of users' do link['data-user'] = user.id.to_s - expect(subject.referenced_by([link])).to eq([user]) + expect_gathered_references(subject.gather_references([link]), [user], 0) end end end diff --git a/spec/lib/banzai/reference_parser/project_parser_spec.rb b/spec/lib/banzai/reference_parser/project_parser_spec.rb index 356dde1e9c2..e87fa3e8767 100644 --- a/spec/lib/banzai/reference_parser/project_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/project_parser_spec.rb @@ -17,7 +17,7 @@ describe Banzai::ReferenceParser::ProjectParser do it 'returns an Array of projects' do link['data-project'] = project.id.to_s - expect(subject.gather_references([link])).to eq([project]) + expect_gathered_references(subject.gather_references([link]), [project], 0) end end @@ -25,7 +25,7 @@ describe Banzai::ReferenceParser::ProjectParser do it 'returns an empty Array' do link['data-project'] = '' - expect(subject.gather_references([link])).to eq([]) + expect_gathered_references(subject.gather_references([link]), [], 1) end end @@ -35,7 +35,7 @@ describe Banzai::ReferenceParser::ProjectParser do link['data-project'] = private_project.id.to_s - expect(subject.gather_references([link])).to eq([]) + expect_gathered_references(subject.gather_references([link]), [], 1) end it 'returns an Array when authorized' do @@ -43,7 +43,7 @@ describe Banzai::ReferenceParser::ProjectParser do link['data-project'] = private_project.id.to_s - expect(subject.gather_references([link])).to eq([private_project]) + expect_gathered_references(subject.gather_references([link]), [private_project], 0) end end end diff --git a/spec/lib/gitlab/asciidoc/include_processor_spec.rb b/spec/lib/gitlab/asciidoc/include_processor_spec.rb new file mode 100644 index 00000000000..72fa05939ae --- /dev/null +++ b/spec/lib/gitlab/asciidoc/include_processor_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'nokogiri' + +describe Gitlab::Asciidoc::IncludeProcessor do + let_it_be(:project) { create(:project, :repository) } + + let(:processor_context) do + { + project: project, + max_includes: max_includes, + ref: ref + } + end + let(:ref) { project.repository.root_ref } + let(:max_includes) { 10 } + + let(:reader) { Asciidoctor::PreprocessorReader.new(document, lines, 'file.adoc') } + let(:document) { Asciidoctor::Document.new(lines) } + + subject(:processor) { described_class.new(processor_context) } + + let(:a_blob) { double(:Blob, readable_text?: true, data: a_data) } + let(:a_data) { StringIO.new('include::b.adoc[]') } + + let(:lines) { [':max-include-depth: 1000'] + Array.new(10, 'include::a.adoc[]') } + + before do + allow(project.repository).to receive(:blob_at).with(ref, 'a.adoc').and_return(a_blob) + end + + describe '#include_allowed?' do + it 'allows the first include' do + expect(processor.send(:include_allowed?, 'foo.adoc', reader)).to be_truthy + end + + it 'allows the Nth + 1 include' do + (max_includes - 1).times { processor.send(:read_blob, ref, 'a.adoc') } + + expect(processor.send(:include_allowed?, 'foo.adoc', reader)).to be_truthy + end + + it 'disallows the Nth + 1 include' do + max_includes.times { processor.send(:read_blob, ref, 'a.adoc') } + + expect(processor.send(:include_allowed?, 'foo.adoc', reader)).to be_falsey + end + end +end diff --git a/spec/lib/gitlab/asciidoc_spec.rb b/spec/lib/gitlab/asciidoc_spec.rb index c8d159d1e84..c7156a500d0 100644 --- a/spec/lib/gitlab/asciidoc_spec.rb +++ b/spec/lib/gitlab/asciidoc_spec.rb @@ -425,6 +425,24 @@ module Gitlab create_file(current_file, "= AsciiDoc\n") end + def many_includes(target) + Array.new(10, "include::#{target}[]").join("\n") + end + + context 'cyclic imports' do + before do + create_file('doc/api/a.adoc', many_includes('b.adoc')) + create_file('doc/api/b.adoc', many_includes('a.adoc')) + end + + let(:include_path) { 'a.adoc' } + let(:requested_path) { 'doc/api/README.md' } + + it 'completes successfully' do + is_expected.to include('<p>Include this:</p>') + end + end + context 'with path to non-existing file' do let(:include_path) { 'not-exists.adoc' } diff --git a/spec/lib/gitlab/git/cross_repo_comparer_spec.rb b/spec/lib/gitlab/git/cross_repo_comparer_spec.rb new file mode 100644 index 00000000000..8b37b6d1667 --- /dev/null +++ b/spec/lib/gitlab/git/cross_repo_comparer_spec.rb @@ -0,0 +1,117 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Git::CrossRepoComparer do + let(:source_project) { create(:project, :repository) } + let(:target_project) { create(:project, :repository) } + + let(:source_repo) { source_project.repository.raw_repository } + let(:target_repo) { target_project.repository.raw_repository } + + let(:source_branch) { 'feature' } + let(:target_branch) { 'master' } + let(:straight) { false } + + let(:source_commit) { source_repo.commit(source_branch) } + let(:target_commit) { source_repo.commit(target_branch) } + + subject(:result) { described_class.new(source_repo, target_repo).compare(source_branch, target_branch, straight: straight) } + + describe '#compare' do + context 'within a single repository' do + let(:target_project) { source_project } + + context 'a non-straight comparison' do + it 'compares without fetching from another repo' do + expect(source_repo).not_to receive(:fetch_source_branch!) + + expect_compare(result, from: source_commit, to: target_commit) + expect(result.straight).to eq(false) + end + end + + context 'a straight comparison' do + let(:straight) { true } + + it 'compares without fetching from another repo' do + expect(source_repo).not_to receive(:fetch_source_branch!) + + expect_compare(result, from: source_commit, to: target_commit) + expect(result.straight).to eq(true) + end + end + end + + context 'across two repositories' do + context 'target ref exists in source repo' do + it 'compares without fetching from another repo' do + expect(source_repo).not_to receive(:fetch_source_branch!) + expect(source_repo).not_to receive(:delete_refs) + + expect_compare(result, from: source_commit, to: target_commit) + end + end + + context 'target ref does not exist in source repo' do + it 'compares in the source repo by fetching from the target to a temporary ref' do + new_commit_id = create_commit(target_project.owner, target_repo, target_branch) + new_commit = target_repo.commit(new_commit_id) + + # This is how the temporary ref is generated + expect(SecureRandom).to receive(:hex).at_least(:once).and_return('foo') + + expect(source_repo) + .to receive(:fetch_source_branch!) + .with(target_repo, new_commit_id, 'refs/tmp/foo') + .and_call_original + + expect(source_repo).to receive(:delete_refs).with('refs/tmp/foo').and_call_original + + expect_compare(result, from: source_commit, to: new_commit) + end + end + + context 'source ref does not exist in source repo' do + let(:source_branch) { 'does-not-exist' } + + it 'returns an empty comparison' do + expect(source_repo).not_to receive(:fetch_source_branch!) + expect(source_repo).not_to receive(:delete_refs) + + expect(result).to be_a(::Gitlab::Git::Compare) + expect(result.commits.size).to eq(0) + end + end + + context 'target ref does not exist in target repo' do + let(:target_branch) { 'does-not-exist' } + + it 'returns nil' do + expect(source_repo).not_to receive(:fetch_source_branch!) + expect(source_repo).not_to receive(:delete_refs) + + is_expected.to be_nil + end + end + end + end + + def expect_compare(of, from:, to:) + expect(of).to be_a(::Gitlab::Git::Compare) + expect(from).to be_a(::Gitlab::Git::Commit) + expect(to).to be_a(::Gitlab::Git::Commit) + + expect(of.commits).not_to be_empty + expect(of.head).to eq(from) + expect(of.base).to eq(to) + end + + def create_commit(user, repo, branch) + action = { action: :create, file_path: '/FILE', content: 'content' } + + result = repo.multi_action(user, branch_name: branch, message: 'Commit', actions: [action]) + + result.newrev + end +end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 6854d514dcc..07fef203691 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1962,66 +1962,15 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#compare_source_branch' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_GITATTRIBUTES_REPO_PATH, '', 'group/project') } - - context 'within same repository' do - it 'does not create a temp ref' do - expect(repository).not_to receive(:fetch_source_branch!) - expect(repository).not_to receive(:delete_refs) - - compare = repository.compare_source_branch('master', repository, 'feature', straight: false) - expect(compare).to be_a(Gitlab::Git::Compare) - expect(compare.commits.count).to be > 0 - end - - it 'returns empty commits when source ref does not exist' do - compare = repository.compare_source_branch('master', repository, 'non-existent-branch', straight: false) + it 'delegates to Gitlab::Git::CrossRepoComparer' do + expect_next_instance_of(::Gitlab::Git::CrossRepoComparer) do |instance| + expect(instance.source_repo).to eq(:source_repository) + expect(instance.target_repo).to eq(repository) - expect(compare.commits).to be_empty + expect(instance).to receive(:compare).with('feature', 'master', straight: :straight) end - end - context 'with different repositories' do - context 'when ref is known by source repo, but not by target' do - before do - mutable_repository.write_ref('another-branch', 'feature') - end - - it 'creates temp ref' do - expect(repository).not_to receive(:fetch_source_branch!) - expect(repository).not_to receive(:delete_refs) - - compare = repository.compare_source_branch('master', mutable_repository, 'another-branch', straight: false) - expect(compare).to be_a(Gitlab::Git::Compare) - expect(compare.commits.count).to be > 0 - end - end - - context 'when ref is known by source and target repos' do - before do - mutable_repository.write_ref('another-branch', 'feature') - repository.write_ref('another-branch', 'feature') - end - - it 'does not create a temp ref' do - expect(repository).not_to receive(:fetch_source_branch!) - expect(repository).not_to receive(:delete_refs) - - compare = repository.compare_source_branch('master', mutable_repository, 'another-branch', straight: false) - expect(compare).to be_a(Gitlab::Git::Compare) - expect(compare.commits.count).to be > 0 - end - end - - context 'when ref is unknown by source repo' do - it 'returns nil when source ref does not exist' do - expect(repository).to receive(:fetch_source_branch!).and_call_original - expect(repository).to receive(:delete_refs).and_call_original - - compare = repository.compare_source_branch('master', mutable_repository, 'non-existent-branch', straight: false) - expect(compare).to be_nil - end - end + repository.compare_source_branch('master', :source_repository, 'feature', straight: :straight) end end diff --git a/spec/lib/gitlab/no_cache_headers_spec.rb b/spec/lib/gitlab/no_cache_headers_spec.rb new file mode 100644 index 00000000000..f011b55006e --- /dev/null +++ b/spec/lib/gitlab/no_cache_headers_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::NoCacheHeaders do + class NoCacheTester + include Gitlab::NoCacheHeaders + end + + describe "#no_cache_headers" do + subject { NoCacheTester.new } + + it "raises a RuntimeError" do + expect { subject.no_cache_headers }.to raise_error(RuntimeError) + end + end +end diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index 6bc9b6365d1..0faaaa50621 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::ReferenceExtractor do - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } before do project.add_developer(project.creator) @@ -293,4 +293,34 @@ describe Gitlab::ReferenceExtractor do end end end + + describe '#references' do + let_it_be(:user) { create(:user) } + let_it_be(:issue) { create(:issue, project: project) } + let(:text) { "Ref. #{issue.to_reference}" } + + subject { described_class.new(project, user) } + + before do + subject.analyze(text) + end + + context 'when references are visible' do + before do + project.add_developer(user) + end + + it 'returns visible references of given type' do + expect(subject.references(:issue)).to eq([issue]) + end + + it 'does not increase stateful_not_visible_counter' do + expect { subject.references(:issue) }.not_to change { subject.stateful_not_visible_counter } + end + end + + it 'increases stateful_not_visible_counter' do + expect { subject.references(:issue) }.to change { subject.stateful_not_visible_counter }.by(1) + end + end end diff --git a/spec/models/generic_commit_status_spec.rb b/spec/models/generic_commit_status_spec.rb index c851810ffb3..c8ed898122b 100644 --- a/spec/models/generic_commit_status_spec.rb +++ b/spec/models/generic_commit_status_spec.rb @@ -19,6 +19,74 @@ describe GenericCommitStatus do it { is_expected.not_to allow_value('javascript:alert(1)').for(:target_url) } end + describe '#name_uniqueness_across_types' do + let(:attributes) { {} } + let(:commit_status) { described_class.new(attributes) } + let(:status_name) { 'test-job' } + + subject(:errors) { commit_status.errors[:name] } + + shared_examples 'it does not have uniqueness errors' do + it 'does not return errors' do + commit_status.valid? + + is_expected.to be_empty + end + end + + context 'without attributes' do + it_behaves_like 'it does not have uniqueness errors' + end + + context 'with only a pipeline' do + let(:attributes) { { pipeline: pipeline } } + + context 'without name' do + it_behaves_like 'it does not have uniqueness errors' + end + end + + context 'with only a name' do + let(:attributes) { { name: status_name } } + + context 'without pipeline' do + it_behaves_like 'it does not have uniqueness errors' + end + end + + context 'with pipeline and name' do + let(:attributes) do + { + pipeline: pipeline, + name: status_name + } + end + + context 'without other statuses' do + it_behaves_like 'it does not have uniqueness errors' + end + + context 'with generic statuses' do + before do + create(:generic_commit_status, pipeline: pipeline, name: status_name) + end + + it_behaves_like 'it does not have uniqueness errors' + end + + context 'with ci_build statuses' do + before do + create(:ci_build, pipeline: pipeline, name: status_name) + end + + it 'returns name error' do + expect(commit_status).to be_invalid + is_expected.to include('has already been taken') + end + end + end + end + describe '#context' do subject { generic_commit_status.context } @@ -79,6 +147,12 @@ describe GenericCommitStatus do it { is_expected.not_to be_nil } end + + describe '#stage_idx' do + subject { generic_commit_status.stage_idx } + + it { is_expected.not_to be_nil } + end end describe '#present' do diff --git a/spec/models/grafana_integration_spec.rb b/spec/models/grafana_integration_spec.rb index 615865e17b9..662e8b1dd61 100644 --- a/spec/models/grafana_integration_spec.rb +++ b/spec/models/grafana_integration_spec.rb @@ -9,7 +9,7 @@ describe GrafanaIntegration do describe 'validations' do it { is_expected.to validate_presence_of(:project) } - it { is_expected.to validate_presence_of(:token) } + it { is_expected.to validate_presence_of(:encrypted_token) } it 'disallows invalid urls for grafana_url' do unsafe_url = %{https://replaceme.com/'><script>alert(document.cookie)</script>} @@ -66,4 +66,24 @@ describe GrafanaIntegration do end end end + + describe 'attribute encryption' do + subject(:grafana_integration) { create(:grafana_integration, token: 'super-secret') } + + context 'token' do + it 'encrypts original value into encrypted_token attribute' do + expect(grafana_integration.encrypted_token).not_to be_nil + end + + it 'locks access to raw value in private method', :aggregate_failures do + expect { grafana_integration.token }.to raise_error(NoMethodError, /private method .token. called/) + expect(grafana_integration.send(:token)).to eql('super-secret') + end + + it 'prevents overriding token value with its encrypted or masked version', :aggregate_failures do + expect { grafana_integration.update(token: grafana_integration.encrypted_token) }.not_to change { grafana_integration.reload.send(:token) } + expect { grafana_integration.update(token: grafana_integration.masked_token) }.not_to change { grafana_integration.reload.send(:token) } + end + end + end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 12a74632bb8..a50608a17b6 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -350,12 +350,12 @@ describe Note do end describe "cross_reference_not_visible_for?" do - let(:private_user) { create(:user) } - let(:private_project) { create(:project, namespace: private_user.namespace) { |p| p.add_maintainer(private_user) } } - let(:private_issue) { create(:issue, project: private_project) } + let_it_be(:private_user) { create(:user) } + let_it_be(:private_project) { create(:project, namespace: private_user.namespace) { |p| p.add_maintainer(private_user) } } + let_it_be(:private_issue) { create(:issue, project: private_project) } - let(:ext_proj) { create(:project, :public) } - let(:ext_issue) { create(:issue, project: ext_proj) } + let_it_be(:ext_proj) { create(:project, :public) } + let_it_be(:ext_issue) { create(:issue, project: ext_proj) } shared_examples "checks references" do it "returns true" do @@ -393,10 +393,24 @@ describe Note do it_behaves_like "checks references" end - context "when there are two references in note" do + context "when there is a reference to a label" do + let_it_be(:private_label) { create(:label, project: private_project) } let(:note) do create :note, noteable: ext_issue, project: ext_proj, + note: "added label #{private_label.to_reference(ext_proj)}", + system: true + end + let!(:system_note_metadata) { create(:system_note_metadata, note: note, action: :label) } + + it_behaves_like "checks references" + end + + context "when there are two references in note" do + let_it_be(:ext_issue2) { create(:issue, project: ext_proj) } + let(:note) do + create :note, + noteable: ext_issue2, project: ext_proj, note: "mentioned in issue #{private_issue.to_reference(ext_proj)} and " \ "public issue #{ext_issue.to_reference(ext_proj)}", system: true diff --git a/spec/policies/global_policy_spec.rb b/spec/policies/global_policy_spec.rb index 77727c6d13b..2d261241486 100644 --- a/spec/policies/global_policy_spec.rb +++ b/spec/policies/global_policy_spec.rb @@ -141,6 +141,34 @@ describe GlobalPolicy do it { is_expected.to be_allowed(:access_api) } end end + + context 'inactive user' do + before do + current_user.update!(confirmed_at: nil, confirmation_sent_at: 5.days.ago) + end + + context 'when within the confirmation grace period' do + before do + allow(User).to receive(:allow_unconfirmed_access_for).and_return(10.days) + end + + it { is_expected.to be_allowed(:access_api) } + end + + context 'when confirmation grace period is expired' do + before do + allow(User).to receive(:allow_unconfirmed_access_for).and_return(2.days) + end + + it { is_expected.not_to be_allowed(:access_api) } + end + + it 'when `inactive_policy_condition` feature flag is turned off' do + stub_feature_flags(inactive_policy_condition: false) + + is_expected.to be_allowed(:access_api) + end + end end describe 'receive notifications' do @@ -202,6 +230,20 @@ describe GlobalPolicy do it { is_expected.not_to be_allowed(:access_git) } end + describe 'inactive user' do + before do + current_user.update!(confirmed_at: nil) + end + + it { is_expected.not_to be_allowed(:access_git) } + + it 'when `inactive_policy_condition` feature flag is turned off' do + stub_feature_flags(inactive_policy_condition: false) + + is_expected.to be_allowed(:access_git) + end + end + context 'when terms are enforced' do before do enforce_terms @@ -298,6 +340,20 @@ describe GlobalPolicy do it { is_expected.not_to be_allowed(:use_slash_commands) } end + describe 'inactive user' do + before do + current_user.update!(confirmed_at: nil) + end + + it { is_expected.not_to be_allowed(:use_slash_commands) } + + it 'when `inactive_policy_condition` feature flag is turned off' do + stub_feature_flags(inactive_policy_condition: false) + + is_expected.to be_allowed(:use_slash_commands) + end + end + context 'when access locked' do before do current_user.lock_access! diff --git a/spec/requests/api/broadcast_messages_spec.rb b/spec/requests/api/broadcast_messages_spec.rb index a502d597da1..163479226e3 100644 --- a/spec/requests/api/broadcast_messages_spec.rb +++ b/spec/requests/api/broadcast_messages_spec.rb @@ -17,7 +17,7 @@ describe API::BroadcastMessages do expect(response).to include_pagination_headers expect(json_response).to be_kind_of(Array) expect(json_response.first.keys) - .to match_array(%w(id message starts_at ends_at color font active target_path)) + .to match_array(%w(id message starts_at ends_at color font active target_path broadcast_type)) end end @@ -28,7 +28,7 @@ describe API::BroadcastMessages do expect(response).to have_gitlab_http_status(200) expect(json_response['id']).to eq message.id expect(json_response.keys) - .to match_array(%w(id message starts_at ends_at color font active target_path)) + .to match_array(%w(id message starts_at ends_at color font active target_path broadcast_type)) end end @@ -85,6 +85,32 @@ describe API::BroadcastMessages do expect(response).to have_gitlab_http_status(201) expect(json_response['target_path']).to eq attrs[:target_path] end + + it 'accepts a broadcast type' do + attrs = attributes_for(:broadcast_message, broadcast_type: 'notification') + + post api('/broadcast_messages', admin), params: attrs + + expect(response).to have_gitlab_http_status(201) + expect(json_response['broadcast_type']).to eq attrs[:broadcast_type] + end + + it 'uses default broadcast type' do + attrs = attributes_for(:broadcast_message) + + post api('/broadcast_messages', admin), params: attrs + + expect(response).to have_gitlab_http_status(201) + expect(json_response['broadcast_type']).to eq 'banner' + end + + it 'errors for invalid broadcast type' do + attrs = attributes_for(:broadcast_message, broadcast_type: 'invalid-type') + + post api('/broadcast_messages', admin), params: attrs + + expect(response).to have_gitlab_http_status(400) + end end end @@ -144,6 +170,23 @@ describe API::BroadcastMessages do expect(response).to have_gitlab_http_status(200) expect(json_response['target_path']).to eq attrs[:target_path] end + + it 'accepts a new broadcast_type' do + attrs = { broadcast_type: 'notification' } + + put api("/broadcast_messages/#{message.id}", admin), params: attrs + + expect(response).to have_gitlab_http_status(200) + expect(json_response['broadcast_type']).to eq attrs[:broadcast_type] + end + + it 'errors for invalid broadcast type' do + attrs = { broadcast_type: 'invalid-type' } + + put api("/broadcast_messages/#{message.id}", admin), params: attrs + + expect(response).to have_gitlab_http_status(400) + end end end diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index 639b8e96343..24ed836996e 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -164,6 +164,7 @@ describe API::CommitStatuses do expect(response).to have_gitlab_http_status(201) expect(job.status).to eq('pending') + expect(job.stage_idx).to eq(GenericCommitStatus::EXTERNAL_STAGE_IDX) end end @@ -331,6 +332,29 @@ describe API::CommitStatuses do end end + context 'when updating a protected ref' do + before do + create(:protected_branch, project: project, name: 'master') + post api(post_url, user), params: { state: 'running', ref: 'master' } + end + + context 'with user as developer' do + let(:user) { developer } + + it 'does not create commit status' do + expect(response).to have_gitlab_http_status(403) + end + end + + context 'with user as maintainer' do + let(:user) { create_user(:maintainer) } + + it 'creates commit status' do + expect(response).to have_gitlab_http_status(201) + end + end + end + context 'when commit SHA is invalid' do let(:sha) { 'invalid_sha' } @@ -372,6 +396,22 @@ describe API::CommitStatuses do .to include 'is blocked: Only allowed schemes are http, https' end end + + context 'when trying to update a status of a different type' do + let!(:pipeline) { create(:ci_pipeline, project: project, sha: sha, ref: 'ref') } + let!(:ci_build) { create(:ci_build, pipeline: pipeline, name: 'test-job') } + let(:params) { { state: 'pending', name: 'test-job' } } + + before do + post api(post_url, developer), params: params + end + + it 'responds with bad request status and validation errors' do + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']['name']) + .to include 'has already been taken' + end + end end context 'reporter user' do diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index d8da1c001b0..e390f3945a9 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -8,6 +8,7 @@ describe API::Commits do let(:user) { create(:user) } let(:guest) { create(:user).tap { |u| project.add_guest(u) } } + let(:developer) { create(:user).tap { |u| project.add_developer(u) } } let(:project) { create(:project, :repository, creator: user, path: 'my.project') } let(:branch_with_dot) { project.repository.find_branch('ends-with.json') } let(:branch_with_slash) { project.repository.find_branch('improve/awesome') } @@ -964,6 +965,56 @@ describe API::Commits do end end + shared_examples_for 'ref with pipeline' do + let!(:pipeline) do + project + .ci_pipelines + .create!(source: :push, ref: 'master', sha: commit.sha, protected: false) + end + + it 'includes status as "created" and a last_pipeline object' do + get api(route, current_user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/commit/detail') + expect(json_response['status']).to eq('created') + expect(json_response['last_pipeline']['id']).to eq(pipeline.id) + expect(json_response['last_pipeline']['ref']).to eq(pipeline.ref) + expect(json_response['last_pipeline']['sha']).to eq(pipeline.sha) + expect(json_response['last_pipeline']['status']).to eq(pipeline.status) + end + + context 'when pipeline succeeds' do + before do + pipeline.update!(status: 'success') + end + + it 'includes a "success" status' do + get api(route, current_user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/commit/detail') + expect(json_response['status']).to eq('success') + end + end + end + + shared_examples_for 'ref with unaccessible pipeline' do + let!(:pipeline) do + project + .ci_pipelines + .create!(source: :push, ref: 'master', sha: commit.sha, protected: false) + end + + it 'does not include last_pipeline' do + get api(route, current_user) + + expect(response).to match_response_schema('public_api/v4/commit/detail') + expect(response).to have_gitlab_http_status(200) + expect(json_response['last_pipeline']).to be_nil + end + end + context 'when stat param' do let(:route) { "/projects/#{project_id}/repository/commits/#{commit_id}" } @@ -993,6 +1044,15 @@ describe API::Commits do let(:project) { create(:project, :public, :repository) } it_behaves_like 'ref commit' + it_behaves_like 'ref with pipeline' + + context 'with private builds' do + before do + project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) + end + + it_behaves_like 'ref with unaccessible pipeline' + end end context 'when unauthenticated', 'and project is private' do @@ -1006,6 +1066,17 @@ describe API::Commits do let(:current_user) { user } it_behaves_like 'ref commit' + it_behaves_like 'ref with pipeline' + + context 'when builds are disabled' do + before do + project + .project_feature + .update!(builds_access_level: ProjectFeature::DISABLED) + end + + it_behaves_like 'ref with unaccessible pipeline' + end context 'when branch contains a dot' do let(:commit) { project.repository.commit(branch_with_dot.name) } @@ -1041,35 +1112,53 @@ describe API::Commits do it_behaves_like 'ref commit' end end + end - context 'when the ref has a pipeline' do - let!(:pipeline) { project.ci_pipelines.create(source: :push, ref: 'master', sha: commit.sha, protected: false) } + context 'when authenticated', 'as a developer' do + let(:current_user) { developer } - it 'includes a "created" status' do - get api(route, current_user) + it_behaves_like 'ref commit' + it_behaves_like 'ref with pipeline' - expect(response).to have_gitlab_http_status(200) - expect(response).to match_response_schema('public_api/v4/commit/detail') - expect(json_response['status']).to eq('created') - expect(json_response['last_pipeline']['id']).to eq(pipeline.id) - expect(json_response['last_pipeline']['ref']).to eq(pipeline.ref) - expect(json_response['last_pipeline']['sha']).to eq(pipeline.sha) - expect(json_response['last_pipeline']['status']).to eq(pipeline.status) + context 'with private builds' do + before do + project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) end - context 'when pipeline succeeds' do - before do - pipeline.update(status: 'success') - end + it_behaves_like 'ref with pipeline' + end + end - it 'includes a "success" status' do - get api(route, current_user) + context 'when authenticated', 'as a guest' do + let(:current_user) { guest } - expect(response).to have_gitlab_http_status(200) - expect(response).to match_response_schema('public_api/v4/commit/detail') - expect(json_response['status']).to eq('success') - end + it_behaves_like '403 response' do + let(:request) { get api(route, guest) } + let(:message) { '403 Forbidden' } + end + end + + context 'when authenticated', 'as a non member' do + let(:current_user) { create(:user) } + + it_behaves_like '403 response' do + let(:request) { get api(route, guest) } + let(:message) { '403 Forbidden' } + end + end + + context 'when authenticated', 'as non_member and project is public' do + let(:current_user) { create(:user) } + let(:project) { create(:project, :public, :repository) } + + it_behaves_like 'ref with pipeline' + + context 'with private builds' do + before do + project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) end + + it_behaves_like 'ref with unaccessible pipeline' end end end diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index ab915af8ab0..efad443de3f 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -447,6 +447,18 @@ describe API::Files do expect(response).to have_gitlab_http_status(200) end + it 'sets no-cache headers' do + url = route('.gitignore') + "/raw" + expect(Gitlab::Workhorse).to receive(:send_git_blob) + + get api(url, current_user), params: params + + expect(response.headers["Cache-Control"]).to include("no-store") + expect(response.headers["Cache-Control"]).to include("no-cache") + expect(response.headers["Pragma"]).to eq("no-cache") + expect(response.headers["Expires"]).to eq("Fri, 01 Jan 1990 00:00:00 GMT") + end + context 'when mandatory params are not given' do it_behaves_like '400 response' do let(:request) { get api(route("any%2Ffile"), current_user) } diff --git a/spec/requests/api/graphql/project/grafana_integration_spec.rb b/spec/requests/api/graphql/project/grafana_integration_spec.rb index 6075efb0cbd..e7155934b3a 100644 --- a/spec/requests/api/graphql/project/grafana_integration_spec.rb +++ b/spec/requests/api/graphql/project/grafana_integration_spec.rb @@ -45,7 +45,7 @@ describe 'Getting Grafana Integration' do it_behaves_like 'a working graphql query' - it { expect(integration_data['token']).to eql grafana_integration.token } + it { expect(integration_data['token']).to eql grafana_integration.masked_token } it { expect(integration_data['grafanaUrl']).to eql grafana_integration.grafana_url } it do diff --git a/spec/requests/api/oauth_tokens_spec.rb b/spec/requests/api/oauth_tokens_spec.rb index 8d7b3fa3c09..ce03756a19a 100644 --- a/spec/requests/api/oauth_tokens_spec.rb +++ b/spec/requests/api/oauth_tokens_spec.rb @@ -30,26 +30,40 @@ describe 'OAuth tokens' do end end - context "when user is blocked" do - it "does not create an access token" do - user = create(:user) + shared_examples 'does not create an access token' do + let(:user) { create(:user) } + + it { expect(response).to have_gitlab_http_status(401) } + end + + context 'when user is blocked' do + before do user.block request_oauth_token(user) - - expect(response).to have_gitlab_http_status(401) end + + include_examples 'does not create an access token' end - context "when user is ldap_blocked" do - it "does not create an access token" do - user = create(:user) + context 'when user is ldap_blocked' do + before do user.ldap_block request_oauth_token(user) + end - expect(response).to have_gitlab_http_status(401) + include_examples 'does not create an access token' + end + + context 'when user account is not confirmed' do + before do + user.update!(confirmed_at: nil) + + request_oauth_token(user) end + + include_examples 'does not create an access token' end end end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index a313f75e3ec..e3ba366dfcc 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -1545,7 +1545,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do authorize_artifacts - expect(response).to have_gitlab_http_status(500) + expect(response).to have_gitlab_http_status(:forbidden) end context 'authorization token is invalid' do @@ -1675,6 +1675,18 @@ describe API::Runner, :clean_gitlab_redis_shared_state do end end + context 'Is missing GitLab Workhorse token headers' do + let(:jwt_token) { JWT.encode({ 'iss' => 'invalid-header' }, Gitlab::Workhorse.secret, 'HS256') } + + it 'fails to post artifacts without GitLab-Workhorse' do + expect(Gitlab::ErrorTracking).to receive(:track_exception).once + + upload_artifacts(file_upload, headers_with_token) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + context 'when setting an expire date' do let(:default_artifacts_expire_in) {} let(:post_data) do diff --git a/spec/services/projects/group_links/destroy_service_spec.rb b/spec/services/projects/group_links/destroy_service_spec.rb index d78ab78c3d8..0fd1fcfe1a5 100644 --- a/spec/services/projects/group_links/destroy_service_spec.rb +++ b/spec/services/projects/group_links/destroy_service_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' describe Projects::GroupLinks::DestroyService, '#execute' do - let(:group_link) { create :project_group_link } - let(:project) { group_link.project } + let(:project) { create(:project, :private) } + let!(:group_link) { create(:project_group_link, project: project) } let(:user) { create :user } let(:subject) { described_class.new(project, user) } @@ -15,4 +15,39 @@ describe Projects::GroupLinks::DestroyService, '#execute' do it 'returns false if group_link is blank' do expect { subject.execute(nil) }.not_to change { project.project_group_links.count } end + + describe 'todos cleanup' do + context 'when project is private' do + it 'triggers todos cleanup' do + expect(TodosDestroyer::ProjectPrivateWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, project.id) + expect(project.private?).to be true + + subject.execute(group_link) + end + end + + context 'when project is public or internal' do + shared_examples_for 'removes confidential todos' do + it 'does not trigger todos cleanup' do + expect(TodosDestroyer::ProjectPrivateWorker).not_to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, project.id) + expect(TodosDestroyer::ConfidentialIssueWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, nil, project.id) + expect(project.private?).to be false + + subject.execute(group_link) + end + end + + context 'when project is public' do + let(:project) { create(:project, :public) } + + it_behaves_like 'removes confidential todos' + end + + context 'when project is internal' do + let(:project) { create(:project, :public) } + + it_behaves_like 'removes confidential todos' + end + end + end end diff --git a/spec/services/projects/import_export/export_service_spec.rb b/spec/services/projects/import_export/export_service_spec.rb index c7ac07fc524..906fef6edf5 100644 --- a/spec/services/projects/import_export/export_service_spec.rb +++ b/spec/services/projects/import_export/export_service_spec.rb @@ -10,6 +10,10 @@ describe Projects::ImportExport::ExportService do let(:service) { described_class.new(project, user) } let!(:after_export_strategy) { Gitlab::ImportExport::AfterExportStrategies::DownloadNotificationStrategy.new } + before do + project.add_maintainer(user) + end + it 'saves the version' do expect(Gitlab::ImportExport::VersionSaver).to receive(:new).and_call_original @@ -137,5 +141,18 @@ describe Projects::ImportExport::ExportService do expect(service).not_to receive(:execute_after_export_action) end end + + context 'when user does not have admin_project permission' do + let!(:another_user) { create(:user) } + + subject(:service) { described_class.new(project, another_user) } + + it 'fails' do + expected_message = + "User with ID: %s does not have permission to Project %s with ID: %s." % + [another_user.id, project.name, project.id] + expect { service.execute }.to raise_error(Gitlab::ImportExport::Error).with_message(expected_message) + end + end end end diff --git a/spec/services/projects/operations/update_service_spec.rb b/spec/services/projects/operations/update_service_spec.rb index 93cd5c43e86..d20ec0b818b 100644 --- a/spec/services/projects/operations/update_service_spec.rb +++ b/spec/services/projects/operations/update_service_spec.rb @@ -210,7 +210,7 @@ describe Projects::Operations::UpdateService do integration = project.reload.grafana_integration expect(integration.grafana_url).to eq(expected_attrs[:grafana_url]) - expect(integration.token).to eq(expected_attrs[:token]) + expect(integration.send(:token)).to eq(expected_attrs[:token]) end end @@ -226,7 +226,7 @@ describe Projects::Operations::UpdateService do integration = project.reload.grafana_integration expect(integration.grafana_url).to eq(expected_attrs[:grafana_url]) - expect(integration.token).to eq(expected_attrs[:token]) + expect(integration.send(:token)).to eq(expected_attrs[:token]) end context 'with all grafana attributes blank in params' do diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index 714256d9b08..52ec80c252b 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -5,7 +5,7 @@ require "spec_helper" describe Projects::UpdatePagesService do let_it_be(:project, refind: true) { create(:project, :repository) } let_it_be(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit('HEAD').sha) } - let_it_be(:build) { create(:ci_build, pipeline: pipeline, ref: 'HEAD') } + let(:build) { create(:ci_build, pipeline: pipeline, ref: 'HEAD') } let(:invalid_file) { fixture_file_upload('spec/fixtures/dk.png') } let(:file) { fixture_file_upload("spec/fixtures/pages.zip") } @@ -204,6 +204,32 @@ describe Projects::UpdatePagesService do end end + context 'when file size is spoofed' do + let(:metadata) { spy('metadata') } + + include_context 'pages zip with spoofed size' + + before do + file = fixture_file_upload(fake_zip_path, 'pages.zip') + metafile = fixture_file_upload('spec/fixtures/pages.zip.meta') + + create(:ci_job_artifact, :archive, file: file, job: build) + create(:ci_job_artifact, :metadata, file: metafile, job: build) + + allow(build).to receive(:artifacts_metadata_entry) + .and_return(metadata) + allow(metadata).to receive(:total_size).and_return(100) + end + + it 'raises an error' do + expect do + subject.execute + end.to raise_error(Projects::UpdatePagesService::FailedToExtractError, + 'Entry public/index.html should be 1B but is larger when inflated') + expect(deploy_status).to be_script_failure + end + end + def deploy_status GenericCommitStatus.find_by(name: 'pages:deploy') end diff --git a/spec/support/helpers/filter_spec_helper.rb b/spec/support/helpers/filter_spec_helper.rb index 279f87efb46..e27b0de2497 100644 --- a/spec/support/helpers/filter_spec_helper.rb +++ b/spec/support/helpers/filter_spec_helper.rb @@ -33,12 +33,15 @@ module FilterSpecHelper # Use this for testing instance methods, but remember to test the result of # the full pipeline by calling #call using the other methods in this helper. def filter_instance - render_context = Banzai::RenderContext.new(project, current_user) context = { project: project, current_user: current_user, render_context: render_context } described_class.new(input_text, context) end + def render_context + Banzai::RenderContext.new(project, current_user) + end + # Run text through HTML::Pipeline with the current filter and return the # result Hash # @@ -62,6 +65,9 @@ module FilterSpecHelper described_class ] + redact = context.delete(:redact) + filters.push(Banzai::Filter::ReferenceRedactorFilter) if redact + HTML::Pipeline.new(filters, context) end diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index 353c632fced..8dc99e4e042 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -349,6 +349,39 @@ module GraphqlHelpers def custom_graphql_error(path, msg) a_hash_including('path' => path, 'message' => msg) end + + def type_factory + Class.new(Types::BaseObject) do + graphql_name 'TestType' + + field :name, GraphQL::STRING_TYPE, null: true + + yield(self) if block_given? + end + end + + def query_factory + Class.new(Types::BaseObject) do + graphql_name 'TestQuery' + + yield(self) if block_given? + end + end + + def execute_query(query_type) + schema = Class.new(GraphQL::Schema) do + use Gitlab::Graphql::Authorize + use Gitlab::Graphql::Connections + + query(query_type) + end + + schema.execute( + query_string, + context: { current_user: user }, + variables: {} + ) + end end # This warms our schema, doing this as part of loading the helpers to avoid diff --git a/spec/support/helpers/reference_parser_helpers.rb b/spec/support/helpers/reference_parser_helpers.rb index f96a01d15b5..9084265b587 100644 --- a/spec/support/helpers/reference_parser_helpers.rb +++ b/spec/support/helpers/reference_parser_helpers.rb @@ -5,6 +5,11 @@ module ReferenceParserHelpers Nokogiri::HTML.fragment('<a></a>').children[0] end + def expect_gathered_references(result, visible, not_visible_count) + expect(result[:visible]).to eq(visible) + expect(result[:not_visible].count).to eq(not_visible_count) + end + shared_examples 'no project N+1 queries' do it 'avoids N+1 queries in #nodes_visible_to_user', :request_store do context = Banzai::RenderContext.new(project, user) diff --git a/spec/support/shared_contexts/pages_zip_with_spoofed_size_shared_context.rb b/spec/support/shared_contexts/pages_zip_with_spoofed_size_shared_context.rb new file mode 100644 index 00000000000..4cec5ab3b74 --- /dev/null +++ b/spec/support/shared_contexts/pages_zip_with_spoofed_size_shared_context.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +# the idea of creating zip archive with spoofed size is borrowed from +# https://github.com/rubyzip/rubyzip/pull/403/files#diff-118213fb4baa6404a40f89e1147661ebR88 +RSpec.shared_context 'pages zip with spoofed size' do + let(:real_zip_path) { Tempfile.new(['real', '.zip']).path } + let(:fake_zip_path) { Tempfile.new(['fake', '.zip']).path } + + before do + full_file_name = 'public/index.html' + true_size = 500_000 + fake_size = 1 + + ::Zip::File.open(real_zip_path, ::Zip::File::CREATE) do |zf| + zf.get_output_stream(full_file_name) do |os| + os.write 'a' * true_size + end + end + + compressed_size = nil + ::Zip::File.open(real_zip_path) do |zf| + a_entry = zf.find_entry(full_file_name) + compressed_size = a_entry.compressed_size + end + + true_size_bytes = [compressed_size, true_size, full_file_name.size].pack('LLS') + fake_size_bytes = [compressed_size, fake_size, full_file_name.size].pack('LLS') + + data = File.binread(real_zip_path) + data.gsub! true_size_bytes, fake_size_bytes + + File.open(fake_zip_path, 'wb') do |file| + file.write data + end + end + + after do + File.delete(real_zip_path) if File.exist?(real_zip_path) + File.delete(fake_zip_path) if File.exist?(fake_zip_path) + end +end |