summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Release Tools Bot <delivery-team+release-tools@gitlab.com>2019-11-27 11:31:00 +0000
committerGitLab Release Tools Bot <delivery-team+release-tools@gitlab.com>2019-11-27 11:31:00 +0000
commit52b9f101a35b6eec91621d5470e6d5883bfcaf88 (patch)
tree8d9ea54cf3da9939592d14a1208cf262dd67a199
parent4c442bdda212490c660a4c0acd82d03f60d72dc9 (diff)
parentef6512ad8f7fc86ab67f210b6ca2754790e4564f (diff)
downloadgitlab-ce-52b9f101a35b6eec91621d5470e6d5883bfcaf88.tar.gz
Merge remote-tracking branch 'dev/12-5-stable' into 12-5-stable
-rw-r--r--CHANGELOG.md14
-rw-r--r--GITALY_SERVER_VERSION2
-rw-r--r--GITLAB_WORKHORSE_VERSION2
-rw-r--r--VERSION2
-rw-r--r--app/assets/javascripts/cycle_analytics/cycle_analytics_bundle.js6
-rw-r--r--app/controllers/admin/application_settings_controller.rb1
-rw-r--r--app/helpers/projects_helper.rb23
-rw-r--r--app/models/application_setting.rb40
-rw-r--r--app/models/note.rb15
-rw-r--r--app/models/project.rb6
-rw-r--r--app/models/project_feature.rb10
-rw-r--r--app/models/project_services/chat_notification_service.rb7
-rw-r--r--app/models/project_services/mattermost_service.rb2
-rw-r--r--app/models/project_services/slack_service.rb24
-rw-r--r--app/views/projects/_home_panel.html.haml9
-rw-r--r--app/views/projects/cycle_analytics/show.html.haml4
-rw-r--r--app/views/projects/edit.html.haml11
-rw-r--r--changelogs/unreleased/security-dos-issue-and-commit-comments-master.yml5
-rw-r--r--config/initializers/hangouts_chat_http_override.rb29
-rw-r--r--db/migrate/20191120084627_add_encrypted_fields_to_application_settings.rb30
-rw-r--r--db/migrate/20191120115530_encrypt_plaintext_attributes_on_application_settings.rb87
-rw-r--r--db/post_migrate/20191122135327_remove_plaintext_columns_from_application_settings.rb28
-rw-r--r--db/schema.rb20
-rw-r--r--lib/api/entities.rb4
-rw-r--r--lib/banzai/filter/label_reference_filter.rb2
-rw-r--r--lib/banzai/filter/relative_link_filter.rb2
-rw-r--r--lib/gitlab/cycle_analytics/stage_summary.rb22
-rw-r--r--lib/gitlab/import_export/attribute_cleaner.rb2
-rw-r--r--lib/microsoft_teams/notifier.rb1
-rw-r--r--locale/gitlab.pot9
-rw-r--r--spec/controllers/projects/issues_controller_spec.rb37
-rw-r--r--spec/features/cycle_analytics_spec.rb10
-rw-r--r--spec/features/projects_spec.rb4
-rw-r--r--spec/initializers/hangouts_chat_http_override_spec.rb34
-rw-r--r--spec/initializers/rest-client-hostname_override_spec.rb145
-rw-r--r--spec/lib/banzai/filter/label_reference_filter_spec.rb9
-rw-r--r--spec/lib/banzai/filter/relative_link_filter_spec.rb5
-rw-r--r--spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb23
-rw-r--r--spec/lib/gitlab/import_export/attribute_cleaner_spec.rb5
-rw-r--r--spec/migrations/encrypt_plaintext_attributes_on_application_settings_spec.rb58
-rw-r--r--spec/models/note_spec.rb64
-rw-r--r--spec/models/project_feature_spec.rb24
-rw-r--r--spec/models/project_services/chat_notification_service_spec.rb17
-rw-r--r--spec/models/project_services/slack_service_spec.rb2
-rw-r--r--spec/models/project_spec.rb78
-rw-r--r--spec/requests/api/projects_spec.rb36
-rw-r--r--spec/support/helpers/project_helpers.rb12
-rw-r--r--spec/support/helpers/search_helpers.rb6
-rw-r--r--spec/support/shared_contexts/policies/project_policy_table_shared_context.rb224
-rw-r--r--spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb54
-rw-r--r--spec/support/shared_examples/uses_gitlab_url_blocker_examples.rb155
-rw-r--r--spec/views/projects/_home_panel.html.haml_spec.rb34
-rw-r--r--spec/views/projects/edit.html.haml_spec.rb56
53 files changed, 1242 insertions, 269 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a4efa33eed7..59fb71c3245 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,20 @@
documentation](doc/development/changelog.md) for instructions on adding your own
entry.
+## 12.5.1
+
+### Security (8 changes)
+
+- Check permissions before showing a forked project's source.
+- Encrypt application setting tokens.
+- Update Workhorse and Gitaly to fix a security issue.
+- Hide commit counts from guest users in Cycle Analytics.
+- Limit potential for DNS rebind SSRF in chat notifications.
+- Ensure are cleaned by ImportExport::AttributeCleaner.
+- Remove notes regarding Related Branches from Issue activity feeds for guest users.
+- Escape namespace in label references to prevent XSS.
+
+
## 12.5.0
### Security (15 changes)
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index 0834888f558..22d6771a47d 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-1.72.0
+1.72.1
diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION
index 2a5dd0d6389..6092827e644 100644
--- a/GITLAB_WORKHORSE_VERSION
+++ b/GITLAB_WORKHORSE_VERSION
@@ -1 +1 @@
-8.14.0
+8.14.1
diff --git a/VERSION b/VERSION
index a59e16b918b..2b4b4d7cb5b 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-12.5.0-ee
+12.5.1
diff --git a/app/assets/javascripts/cycle_analytics/cycle_analytics_bundle.js b/app/assets/javascripts/cycle_analytics/cycle_analytics_bundle.js
index cd67ba5fab8..1074ce0e744 100644
--- a/app/assets/javascripts/cycle_analytics/cycle_analytics_bundle.js
+++ b/app/assets/javascripts/cycle_analytics/cycle_analytics_bundle.js
@@ -58,10 +58,16 @@ export default () => {
service: this.createCycleAnalyticsService(cycleAnalyticsEl.dataset.requestPath),
};
},
+ defaultNumberOfSummaryItems: 3,
computed: {
currentStage() {
return this.store.currentActiveStage();
},
+ summaryTableColumnClass() {
+ return this.state.summary.length === this.$options.defaultNumberOfSummaryItems
+ ? 'col-sm-3'
+ : 'col-sm-4';
+ },
},
created() {
// Conditional check placed here to prevent this method from being called on the
diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index f7e33c09928..9d81d3fad07 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -86,6 +86,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
params[:application_setting][:import_sources]&.delete("")
params[:application_setting][:restricted_visibility_levels]&.delete("")
+ params[:application_setting].delete(:elasticsearch_aws_secret_access_key) if params[:application_setting][:elasticsearch_aws_secret_access_key].blank?
# TODO Remove domain_blacklist_raw in APIv5 (See https://gitlab.com/gitlab-org/gitlab-foss/issues/67204)
params.delete(:domain_blacklist_raw) if params[:domain_blacklist_file]
params.delete(:domain_blacklist_raw) if params[:domain_blacklist]
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 47214ac4ee2..c68b6bdea0f 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -110,19 +110,26 @@ module ProjectsHelper
{ project_full_name: project.full_name }
end
- def remove_fork_project_message(project)
- _("You are going to remove the fork relationship to source project %{forked_from_project}. Are you ABSOLUTELY sure?") %
- { forked_from_project: fork_source_name(project) }
- end
+ def remove_fork_project_description_message(project)
+ source = visible_fork_source(project)
- def fork_source_name(project)
- if @project.fork_source
- @project.fork_source.full_name
+ if source
+ _('This will remove the fork relationship between this project and %{fork_source}.') %
+ { fork_source: link_to(source.full_name, project_path(source)) }
else
- @project.fork_network&.deleted_root_project_name
+ _('This will remove the fork relationship between this project and other projects in the fork network.')
end
end
+ def remove_fork_project_warning_message(project)
+ _("You are going to remove the fork relationship from %{project_full_name}. Are you ABSOLUTELY sure?") %
+ { project_full_name: project.full_name }
+ end
+
+ def visible_fork_source(project)
+ project.fork_source if project.fork_source && can?(current_user, :read_project, project.fork_source)
+ end
+
def project_nav_tabs
@nav_tabs ||= get_project_nav_tabs(@project, current_user)
end
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 4028d711fd1..72605af433f 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -313,29 +313,25 @@ class ApplicationSetting < ApplicationRecord
algorithm: 'aes-256-cbc',
insecure_mode: true
- attr_encrypted :external_auth_client_key,
- mode: :per_attribute_iv,
- key: Settings.attr_encrypted_db_key_base_truncated,
- algorithm: 'aes-256-gcm',
- encode: true
-
- attr_encrypted :external_auth_client_key_pass,
- mode: :per_attribute_iv,
- key: Settings.attr_encrypted_db_key_base_truncated,
- algorithm: 'aes-256-gcm',
- encode: true
-
- attr_encrypted :lets_encrypt_private_key,
- mode: :per_attribute_iv,
- key: Settings.attr_encrypted_db_key_base_truncated,
- algorithm: 'aes-256-gcm',
- encode: true
+ private_class_method def self.encryption_options_base_truncated_aes_256_gcm
+ {
+ mode: :per_attribute_iv,
+ key: Settings.attr_encrypted_db_key_base_truncated,
+ algorithm: 'aes-256-gcm',
+ encode: true
+ }
+ end
- attr_encrypted :eks_secret_access_key,
- mode: :per_attribute_iv,
- key: Settings.attr_encrypted_db_key_base_truncated,
- algorithm: 'aes-256-gcm',
- encode: true
+ attr_encrypted :external_auth_client_key, encryption_options_base_truncated_aes_256_gcm
+ attr_encrypted :external_auth_client_key_pass, encryption_options_base_truncated_aes_256_gcm
+ attr_encrypted :lets_encrypt_private_key, encryption_options_base_truncated_aes_256_gcm
+ attr_encrypted :eks_secret_access_key, encryption_options_base_truncated_aes_256_gcm
+ attr_encrypted :akismet_api_key, encryption_options_base_truncated_aes_256_gcm
+ attr_encrypted :elasticsearch_aws_secret_access_key, encryption_options_base_truncated_aes_256_gcm
+ attr_encrypted :recaptcha_private_key, encryption_options_base_truncated_aes_256_gcm
+ attr_encrypted :recaptcha_site_key, encryption_options_base_truncated_aes_256_gcm
+ attr_encrypted :slack_app_secret, encryption_options_base_truncated_aes_256_gcm
+ attr_encrypted :slack_app_verification_token, encryption_options_base_truncated_aes_256_gcm
before_validation :ensure_uuid!
diff --git a/app/models/note.rb b/app/models/note.rb
index ce60413b8a0..493132e30cc 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -37,6 +37,10 @@ class Note < ApplicationRecord
redact_field :note
+ TYPES_RESTRICTED_BY_ABILITY = {
+ branch: :download_code
+ }.freeze
+
# Aliases to make application_helper#edited_time_ago_with_tooltip helper work properly with notes.
# See https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/10392/diffs#note_28719102
alias_attribute :last_edited_at, :updated_at
@@ -341,7 +345,7 @@ class Note < ApplicationRecord
end
def visible_for?(user)
- !cross_reference_not_visible_for?(user)
+ !cross_reference_not_visible_for?(user) && system_note_viewable_by?(user)
end
def award_emoji?
@@ -493,6 +497,15 @@ class Note < ApplicationRecord
private
+ def system_note_viewable_by?(user)
+ return true unless system_note_metadata
+
+ restriction = TYPES_RESTRICTED_BY_ABILITY[system_note_metadata.action.to_sym]
+ return Ability.allowed?(user, restriction, project) if restriction
+
+ true
+ end
+
def keep_around_commit
project.repository.keep_around(self.commit_id)
end
diff --git a/app/models/project.rb b/app/models/project.rb
index f4aa336fbcd..7ae4e2a4cd7 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -517,7 +517,11 @@ class Project < ApplicationRecord
# This scope returns projects where user has access to both the project and the feature.
def self.filter_by_feature_visibility(feature, user)
- with_feature_available_for_user(feature, user).public_or_visible_to_user(user)
+ with_feature_available_for_user(feature, user)
+ .public_or_visible_to_user(
+ user,
+ ProjectFeature.required_minimum_access_level_for_private_project(feature)
+ )
end
scope :active, -> { joins(:issues, :notes, :merge_requests).order('issues.created_at, notes.created_at, merge_requests.created_at DESC') }
diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb
index 2013f620b5b..caa65d32c86 100644
--- a/app/models/project_feature.rb
+++ b/app/models/project_feature.rb
@@ -24,6 +24,7 @@ class ProjectFeature < ApplicationRecord
FEATURES = %i(issues merge_requests wiki snippets builds repository pages).freeze
PRIVATE_FEATURES_MIN_ACCESS_LEVEL = { merge_requests: Gitlab::Access::REPORTER }.freeze
+ PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT = { repository: Gitlab::Access::REPORTER }.freeze
STRING_OPTIONS = HashWithIndifferentAccess.new({
'disabled' => DISABLED,
'private' => PRIVATE,
@@ -51,6 +52,15 @@ class ProjectFeature < ApplicationRecord
PRIVATE_FEATURES_MIN_ACCESS_LEVEL.fetch(feature, Gitlab::Access::GUEST)
end
+ # Guest users can perform certain features on public and internal projects, but not private projects.
+ def required_minimum_access_level_for_private_project(feature)
+ feature = ensure_feature!(feature)
+
+ PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT.fetch(feature) do
+ required_minimum_access_level(feature)
+ end
+ end
+
def access_level_from_str(level)
STRING_OPTIONS.fetch(level)
end
diff --git a/app/models/project_services/chat_notification_service.rb b/app/models/project_services/chat_notification_service.rb
index ecea1a5b630..b84a79453c1 100644
--- a/app/models/project_services/chat_notification_service.rb
+++ b/app/models/project_services/chat_notification_service.rb
@@ -113,12 +113,9 @@ class ChatNotificationService < Service
private
+ # every notifier must implement this independently
def notify(message, opts)
- Slack::Notifier.new(webhook, opts).ping(
- message.pretext,
- attachments: message.attachments,
- fallback: message.fallback
- )
+ raise NotImplementedError
end
def custom_data(data)
diff --git a/app/models/project_services/mattermost_service.rb b/app/models/project_services/mattermost_service.rb
index b8bc83b870e..c1055db78e5 100644
--- a/app/models/project_services/mattermost_service.rb
+++ b/app/models/project_services/mattermost_service.rb
@@ -1,6 +1,8 @@
# frozen_string_literal: true
class MattermostService < ChatNotificationService
+ include ::SlackService::Notifier
+
def title
'Mattermost notifications'
end
diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb
index 482808255f9..7290964f442 100644
--- a/app/models/project_services/slack_service.rb
+++ b/app/models/project_services/slack_service.rb
@@ -30,4 +30,28 @@ class SlackService < ChatNotificationService
def webhook_placeholder
'https://hooks.slack.com/services/…'
end
+
+ module Notifier
+ private
+
+ def notify(message, opts)
+ # See https://github.com/stevenosloan/slack-notifier#custom-http-client
+ notifier = Slack::Notifier.new(webhook, opts.merge(http_client: HTTPClient))
+
+ notifier.ping(
+ message.pretext,
+ attachments: message.attachments,
+ fallback: message.fallback
+ )
+ end
+
+ class HTTPClient
+ def self.post(uri, params = {})
+ params.delete(:http_options) # these are internal to the client and we do not want them
+ Gitlab::HTTP.post(uri, body: params)
+ end
+ end
+ end
+
+ include Notifier
end
diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml
index b7c4114d485..daedc52f298 100644
--- a/app/views/projects/_home_panel.html.haml
+++ b/app/views/projects/_home_panel.html.haml
@@ -74,13 +74,12 @@
- if @project.forked?
%p
- - if @project.fork_source
+ - source = visible_fork_source(@project)
+ - if source
#{ s_('ForkedFromProjectPath|Forked from') }
- = link_to project_path(@project.fork_source) do
- = fork_source_name(@project)
+ = link_to source.full_name, project_path(source)
- else
- - deleted_message = s_('ForkedFromProjectPath|Forked from %{project_name} (deleted)')
- = deleted_message % { project_name: fork_source_name(@project) }
+ = s_('ForkedFromProjectPath|Forked from an inaccessible project')
= render_if_exists "projects/home_mirror"
diff --git a/app/views/projects/cycle_analytics/show.html.haml b/app/views/projects/cycle_analytics/show.html.haml
index 7fedd1ab785..1691af9dfdd 100644
--- a/app/views/projects/cycle_analytics/show.html.haml
+++ b/app/views/projects/cycle_analytics/show.html.haml
@@ -13,10 +13,10 @@
.content-block
.container-fluid
.row
- .col-sm-3.col-12.column{ "v-for" => "item in state.summary" }
+ .col-12.column{ "v-for" => "item in state.summary", ":class" => "summaryTableColumnClass" }
%h3.header {{ item.value }}
%p.text {{ item.title }}
- .col-sm-3.col-12.column
+ .col-12.column{ ":class" => "summaryTableColumnClass" }
.dropdown.inline.js-ca-dropdown
%button.dropdown-menu-toggle{ "data-toggle" => "dropdown", :type => "button" }
%span.dropdown-label {{ n__('Last %d day', 'Last %d days', 30) }}
diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml
index b5e24cbbffb..328fdd0be10 100644
--- a/app/views/projects/edit.html.haml
+++ b/app/views/projects/edit.html.haml
@@ -126,17 +126,12 @@
- if @project.forked? && can?(current_user, :remove_fork_project, @project)
.sub-section
%h4.danger-title= _('Remove fork relationship')
- %p
- = _('This will remove the fork relationship to source project')
- = succeed "." do
- - if @project.fork_source
- = link_to(fork_source_name(@project), project_path(@project.fork_source))
- - else
- = fork_source_name(@project)
+ %p= remove_fork_project_description_message(@project)
+
= form_for([@project.namespace.becomes(Namespace), @project], url: remove_fork_project_path(@project), method: :delete, remote: true, html: { class: 'transfer-project' }) do |f|
%p
%strong= _('Once removed, the fork relationship cannot be restored and you will no longer be able to send merge requests to the source.')
- = button_to _('Remove fork relationship'), '#', class: "btn btn-remove js-confirm-danger", data: { "confirm-danger-message" => remove_fork_project_message(@project) }
+ = button_to _('Remove fork relationship'), '#', class: "btn btn-remove js-confirm-danger", data: { "confirm-danger-message" => remove_fork_project_warning_message(@project) }
- if can?(current_user, :remove_project, @project)
.sub-section
diff --git a/changelogs/unreleased/security-dos-issue-and-commit-comments-master.yml b/changelogs/unreleased/security-dos-issue-and-commit-comments-master.yml
new file mode 100644
index 00000000000..c84cebdcca0
--- /dev/null
+++ b/changelogs/unreleased/security-dos-issue-and-commit-comments-master.yml
@@ -0,0 +1,5 @@
+---
+title: Fix 500 error caused by invalid byte sequences in links
+merge_request:
+author:
+type: security
diff --git a/config/initializers/hangouts_chat_http_override.rb b/config/initializers/hangouts_chat_http_override.rb
new file mode 100644
index 00000000000..4fd886697e4
--- /dev/null
+++ b/config/initializers/hangouts_chat_http_override.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+module HangoutsChat
+ class Sender
+ class HTTP
+ module GitlabHTTPOverride
+ extend ::Gitlab::Utils::Override
+
+ attr_reader :uri
+
+ # see https://github.com/enzinia/hangouts-chat/blob/6a509f61a56e757f8f417578b393b94423831ff7/lib/hangouts_chat/http.rb
+ override :post
+ def post(payload)
+ httparty_response = Gitlab::HTTP.post(
+ uri,
+ body: payload.to_json,
+ headers: { 'Content-Type' => 'application/json' },
+ parse: nil # disables automatic response parsing
+ )
+ net_http_response = httparty_response.response
+ # The rest of the integration expects a Net::HTTP response
+ net_http_response
+ end
+ end
+
+ prepend GitlabHTTPOverride
+ end
+ end
+end
diff --git a/db/migrate/20191120084627_add_encrypted_fields_to_application_settings.rb b/db/migrate/20191120084627_add_encrypted_fields_to_application_settings.rb
new file mode 100644
index 00000000000..4e0886a5121
--- /dev/null
+++ b/db/migrate/20191120084627_add_encrypted_fields_to_application_settings.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+class AddEncryptedFieldsToApplicationSettings < ActiveRecord::Migration[5.2]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ PLAINTEXT_ATTRIBUTES = %w[
+ akismet_api_key
+ elasticsearch_aws_secret_access_key
+ recaptcha_private_key
+ recaptcha_site_key
+ slack_app_secret
+ slack_app_verification_token
+ ].freeze
+
+ def up
+ PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute|
+ add_column :application_settings, "encrypted_#{plaintext_attribute}", :text
+ add_column :application_settings, "encrypted_#{plaintext_attribute}_iv", :string, limit: 255
+ end
+ end
+
+ def down
+ PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute|
+ remove_column :application_settings, "encrypted_#{plaintext_attribute}"
+ remove_column :application_settings, "encrypted_#{plaintext_attribute}_iv"
+ end
+ end
+end
diff --git a/db/migrate/20191120115530_encrypt_plaintext_attributes_on_application_settings.rb b/db/migrate/20191120115530_encrypt_plaintext_attributes_on_application_settings.rb
new file mode 100644
index 00000000000..e6b9a40ad4f
--- /dev/null
+++ b/db/migrate/20191120115530_encrypt_plaintext_attributes_on_application_settings.rb
@@ -0,0 +1,87 @@
+# frozen_string_literal: true
+
+class EncryptPlaintextAttributesOnApplicationSettings < ActiveRecord::Migration[5.2]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ PLAINTEXT_ATTRIBUTES = %w[
+ akismet_api_key
+ elasticsearch_aws_secret_access_key
+ recaptcha_private_key
+ recaptcha_site_key
+ slack_app_secret
+ slack_app_verification_token
+ ].freeze
+
+ class ApplicationSetting < ActiveRecord::Base
+ self.table_name = 'application_settings'
+
+ def self.encryption_options_base_truncated_aes_256_gcm
+ {
+ mode: :per_attribute_iv,
+ key: Gitlab::Application.secrets.db_key_base[0..31],
+ algorithm: 'aes-256-gcm',
+ encode: true
+ }
+ end
+
+ attr_encrypted :akismet_api_key, encryption_options_base_truncated_aes_256_gcm
+ attr_encrypted :elasticsearch_aws_secret_access_key, encryption_options_base_truncated_aes_256_gcm
+ attr_encrypted :recaptcha_private_key, encryption_options_base_truncated_aes_256_gcm
+ attr_encrypted :recaptcha_site_key, encryption_options_base_truncated_aes_256_gcm
+ attr_encrypted :slack_app_secret, encryption_options_base_truncated_aes_256_gcm
+ attr_encrypted :slack_app_verification_token, encryption_options_base_truncated_aes_256_gcm
+
+ def akismet_api_key
+ decrypt(:akismet_api_key, self[:encrypted_akismet_api_key]) || self[:akismet_api_key]
+ end
+
+ def elasticsearch_aws_secret_access_key
+ decrypt(:elasticsearch_aws_secret_access_key, self[:encrypted_elasticsearch_aws_secret_access_key]) || self[:elasticsearch_aws_secret_access_key]
+ end
+
+ def recaptcha_private_key
+ decrypt(:recaptcha_private_key, self[:encrypted_recaptcha_private_key]) || self[:recaptcha_private_key]
+ end
+
+ def recaptcha_site_key
+ decrypt(:recaptcha_site_key, self[:encrypted_recaptcha_site_key]) || self[:recaptcha_site_key]
+ end
+
+ def slack_app_secret
+ decrypt(:slack_app_secret, self[:encrypted_slack_app_secret]) || self[:slack_app_secret]
+ end
+
+ def slack_app_verification_token
+ decrypt(:slack_app_verification_token, self[:encrypted_slack_app_verification_token]) || self[:slack_app_verification_token]
+ end
+ end
+
+ def up
+ ApplicationSetting.find_each do |application_setting|
+ # We are using the setter from attr_encrypted gem to encrypt the data.
+ # The gem updates the two columns needed to decrypt the value:
+ # - "encrypted_#{plaintext_attribute}"
+ # - "encrypted_#{plaintext_attribute}_iv"
+ application_setting.assign_attributes(
+ PLAINTEXT_ATTRIBUTES.each_with_object({}) do |plaintext_attribute, attributes|
+ attributes[plaintext_attribute] = application_setting.send(plaintext_attribute)
+ end
+ )
+ application_setting.save(validate: false)
+ end
+ end
+
+ def down
+ ApplicationSetting.find_each do |application_setting|
+ application_setting.update_columns(
+ PLAINTEXT_ATTRIBUTES.each_with_object({}) do |plaintext_attribute, attributes|
+ attributes[plaintext_attribute] = application_setting.send(plaintext_attribute)
+ attributes["encrypted_#{plaintext_attribute}"] = nil
+ attributes["encrypted_#{plaintext_attribute}_iv"] = nil
+ end
+ )
+ end
+ end
+end
diff --git a/db/post_migrate/20191122135327_remove_plaintext_columns_from_application_settings.rb b/db/post_migrate/20191122135327_remove_plaintext_columns_from_application_settings.rb
new file mode 100644
index 00000000000..b5cd58b10a8
--- /dev/null
+++ b/db/post_migrate/20191122135327_remove_plaintext_columns_from_application_settings.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+class RemovePlaintextColumnsFromApplicationSettings < ActiveRecord::Migration[5.2]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ PLAINTEXT_ATTRIBUTES = %w[
+ akismet_api_key
+ elasticsearch_aws_secret_access_key
+ recaptcha_private_key
+ recaptcha_site_key
+ slack_app_secret
+ slack_app_verification_token
+ ].freeze
+
+ def up
+ PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute|
+ remove_column :application_settings, plaintext_attribute
+ end
+ end
+
+ def down
+ PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute|
+ add_column :application_settings, plaintext_attribute, :text
+ end
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index e3413722991..0dce19a29d7 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 2019_11_15_091425) do
+ActiveRecord::Schema.define(version: 2019_11_22_135327) do
# These are extensions that must be enabled in order to support this database
enable_extension "pg_trgm"
@@ -180,11 +180,8 @@ ActiveRecord::Schema.define(version: 2019_11_15_091425) do
t.integer "metrics_timeout", default: 10
t.integer "metrics_method_call_threshold", default: 10
t.boolean "recaptcha_enabled", default: false
- t.string "recaptcha_site_key"
- t.string "recaptcha_private_key"
t.integer "metrics_port", default: 8089
t.boolean "akismet_enabled", default: false
- t.string "akismet_api_key"
t.integer "metrics_sample_interval", default: 15
t.boolean "email_author_in_body", default: false
t.integer "default_group_visibility"
@@ -231,7 +228,6 @@ ActiveRecord::Schema.define(version: 2019_11_15_091425) do
t.boolean "elasticsearch_aws", default: false, null: false
t.string "elasticsearch_aws_region", default: "us-east-1"
t.string "elasticsearch_aws_access_key"
- t.string "elasticsearch_aws_secret_access_key"
t.integer "geo_status_timeout", default: 10
t.string "uuid"
t.decimal "polling_interval_multiplier", default: "1.0", null: false
@@ -247,8 +243,6 @@ ActiveRecord::Schema.define(version: 2019_11_15_091425) do
t.string "help_page_support_url"
t.boolean "slack_app_enabled", default: false
t.string "slack_app_id"
- t.string "slack_app_secret"
- t.string "slack_app_verification_token"
t.integer "performance_bar_allowed_group_id"
t.boolean "allow_group_owners_to_manage_ldap", default: true, null: false
t.boolean "hashed_storage_enabled", default: true, null: false
@@ -355,6 +349,18 @@ ActiveRecord::Schema.define(version: 2019_11_15_091425) do
t.boolean "sourcegraph_enabled", default: false, null: false
t.string "sourcegraph_url", limit: 255
t.boolean "sourcegraph_public_only", default: true, null: false
+ t.text "encrypted_akismet_api_key"
+ t.string "encrypted_akismet_api_key_iv", limit: 255
+ t.text "encrypted_elasticsearch_aws_secret_access_key"
+ t.string "encrypted_elasticsearch_aws_secret_access_key_iv", limit: 255
+ t.text "encrypted_recaptcha_private_key"
+ t.string "encrypted_recaptcha_private_key_iv", limit: 255
+ t.text "encrypted_recaptcha_site_key"
+ t.string "encrypted_recaptcha_site_key_iv", limit: 255
+ t.text "encrypted_slack_app_secret"
+ t.string "encrypted_slack_app_secret_iv", limit: 255
+ t.text "encrypted_slack_app_verification_token"
+ t.string "encrypted_slack_app_verification_token_iv", limit: 255
t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id"
t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id"
t.index ["instance_administration_project_id"], name: "index_applicationsettings_on_instance_administration_project_id"
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 9617f1a8acf..32a0fb9dd60 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -283,7 +283,9 @@ module API
expose :shared_runners_enabled
expose :lfs_enabled?, as: :lfs_enabled
expose :creator_id
- expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? }
+ expose :forked_from_project, using: Entities::BasicProjectDetails, if: ->(project, options) do
+ project.forked? && Ability.allowed?(options[:current_user], :read_project, project.forked_from_project)
+ end
expose :import_status
expose :import_error, if: lambda { |_project, options| options[:user_can_admin_project] } do |project|
diff --git a/lib/banzai/filter/label_reference_filter.rb b/lib/banzai/filter/label_reference_filter.rb
index db620c65237..609ea8fb5ca 100644
--- a/lib/banzai/filter/label_reference_filter.rb
+++ b/lib/banzai/filter/label_reference_filter.rb
@@ -89,7 +89,7 @@ module Banzai
parent_from_ref = from_ref_cached(project_path)
reference = parent_from_ref.to_human_reference(parent)
- label_suffix = " <i>in #{reference}</i>" if reference.present?
+ label_suffix = " <i>in #{ERB::Util.html_escape(reference)}</i>" if reference.present?
end
presenter = object.present(issuable_subject: parent)
diff --git a/lib/banzai/filter/relative_link_filter.rb b/lib/banzai/filter/relative_link_filter.rb
index c7589e69262..583b0081319 100644
--- a/lib/banzai/filter/relative_link_filter.rb
+++ b/lib/banzai/filter/relative_link_filter.rb
@@ -172,7 +172,7 @@ module Banzai
end
def cleaned_file_path(uri)
- Addressable::URI.unescape(uri.path).delete("\0").chomp("/")
+ Addressable::URI.unescape(uri.path).scrub.delete("\0").chomp("/")
end
def relative_file_path(uri)
diff --git a/lib/gitlab/cycle_analytics/stage_summary.rb b/lib/gitlab/cycle_analytics/stage_summary.rb
index ea440c441b7..9c75d4bb455 100644
--- a/lib/gitlab/cycle_analytics/stage_summary.rb
+++ b/lib/gitlab/cycle_analytics/stage_summary.rb
@@ -11,13 +11,29 @@ module Gitlab
end
def data
- [serialize(Summary::Issue.new(project: @project, from: @from, to: @to, current_user: @current_user)),
- serialize(Summary::Commit.new(project: @project, from: @from, to: @to)),
- serialize(Summary::Deploy.new(project: @project, from: @from, to: @to))]
+ summary = [issue_stats]
+ summary << commit_stats if user_has_sufficient_access?
+ summary << deploy_stats
end
private
+ def issue_stats
+ serialize(Summary::Issue.new(project: @project, from: @from, to: @to, current_user: @current_user))
+ end
+
+ def commit_stats
+ serialize(Summary::Commit.new(project: @project, from: @from, to: @to))
+ end
+
+ def deploy_stats
+ serialize(Summary::Deploy.new(project: @project, from: @from, to: @to))
+ end
+
+ def user_has_sufficient_access?
+ @project.team.member?(@current_user, Gitlab::Access::REPORTER)
+ end
+
def serialize(summary_object)
AnalyticsSummarySerializer.new.represent(summary_object)
end
diff --git a/lib/gitlab/import_export/attribute_cleaner.rb b/lib/gitlab/import_export/attribute_cleaner.rb
index b2fe9592c06..50fec9f3eb9 100644
--- a/lib/gitlab/import_export/attribute_cleaner.rb
+++ b/lib/gitlab/import_export/attribute_cleaner.rb
@@ -4,7 +4,7 @@ module Gitlab
module ImportExport
class AttributeCleaner
ALLOWED_REFERENCES = RelationFactory::PROJECT_REFERENCES + RelationFactory::USER_REFERENCES + %w[group_id commit_id]
- PROHIBITED_REFERENCES = Regexp.union(/\Acached_markdown_version\Z/, /_id\Z/, /_html\Z/).freeze
+ PROHIBITED_REFERENCES = Regexp.union(/\Acached_markdown_version\Z/, /_id\Z/, /_ids\Z/, /_html\Z/).freeze
def self.clean(*args)
new(*args).clean
diff --git a/lib/microsoft_teams/notifier.rb b/lib/microsoft_teams/notifier.rb
index a7dcd322e27..340bf709f5e 100644
--- a/lib/microsoft_teams/notifier.rb
+++ b/lib/microsoft_teams/notifier.rb
@@ -14,7 +14,6 @@ module MicrosoftTeams
response = Gitlab::HTTP.post(
@webhook.to_str,
headers: @header,
- allow_local_requests: true,
body: body(options)
)
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index f1b14d78292..a7ba66cde96 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -7635,7 +7635,7 @@ msgstr ""
msgid "ForkedFromProjectPath|Forked from"
msgstr ""
-msgid "ForkedFromProjectPath|Forked from %{project_name} (deleted)"
+msgid "ForkedFromProjectPath|Forked from an inaccessible project"
msgstr ""
msgid "Forking in progress"
@@ -17779,7 +17779,10 @@ msgstr ""
msgid "This will redirect you to an external sign in page."
msgstr ""
-msgid "This will remove the fork relationship to source project"
+msgid "This will remove the fork relationship between this project and %{fork_source}."
+msgstr ""
+
+msgid "This will remove the fork relationship between this project and other projects in the fork network."
msgstr ""
msgid "Those emails automatically become issues (with the comments becoming the email conversation) listed here."
@@ -19701,7 +19704,7 @@ msgstr ""
msgid "You are going to remove %{project_full_name}. Removed project CANNOT be restored! Are you ABSOLUTELY sure?"
msgstr ""
-msgid "You are going to remove the fork relationship to source project %{forked_from_project}. Are you ABSOLUTELY sure?"
+msgid "You are going to remove the fork relationship from %{project_full_name}. Are you ABSOLUTELY sure?"
msgstr ""
msgid "You are going to transfer %{project_full_name} to another owner. Are you ABSOLUTELY sure?"
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 8770a5ee303..29746bbd863 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -1435,6 +1435,43 @@ describe Projects::IssuesController do
expect { get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } }.not_to exceed_query_limit(control_count)
end
end
+
+ context 'private project' do
+ let!(:branch_note) { create(:discussion_note_on_issue, :system, noteable: issue, project: project) }
+ let!(:commit_note) { create(:discussion_note_on_issue, :system, noteable: issue, project: project) }
+ let!(:branch_note_meta) { create(:system_note_metadata, note: branch_note, action: "branch") }
+ let!(:commit_note_meta) { create(:system_note_metadata, note: commit_note, action: "commit") }
+
+ context 'user is allowed access' do
+ before do
+ project.add_user(user, :maintainer)
+ end
+
+ it 'displays all available notes' do
+ get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid }
+
+ expect(json_response.length).to eq(3)
+ end
+ end
+
+ context 'user is a guest' do
+ let(:json_response_note_ids) do
+ json_response.collect { |discussion| discussion["notes"] }.flatten
+ .collect { |note| note["id"].to_i }
+ end
+
+ before do
+ project.add_guest(user)
+ end
+
+ it 'does not display notes w/type listed in TYPES_RESTRICTED_BY_ACCESS_LEVEL' do
+ get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid }
+
+ expect(json_response.length).to eq(2)
+ expect(json_response_note_ids).not_to include(branch_note.id)
+ end
+ end
+ end
end
end
diff --git a/spec/features/cycle_analytics_spec.rb b/spec/features/cycle_analytics_spec.rb
index 0fc4841ee0e..e9751aa2e72 100644
--- a/spec/features/cycle_analytics_spec.rb
+++ b/spec/features/cycle_analytics_spec.rb
@@ -112,6 +112,10 @@ describe 'Cycle Analytics', :js do
wait_for_requests
end
+ it 'does not show the commit stats' do
+ expect(page).to have_no_selector(:xpath, commits_counter_selector)
+ end
+
it 'needs permissions to see restricted stages' do
expect(find('.stage-events')).to have_content(issue.title)
@@ -127,8 +131,12 @@ describe 'Cycle Analytics', :js do
find(:xpath, "//p[contains(text(),'New Issue')]/preceding-sibling::h3")
end
+ def commits_counter_selector
+ "//p[contains(text(),'Commits')]/preceding-sibling::h3"
+ end
+
def commits_counter
- find(:xpath, "//p[contains(text(),'Commits')]/preceding-sibling::h3")
+ find(:xpath, commits_counter_selector)
end
def deploys_counter
diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb
index 90e48f3c230..47f32e0113c 100644
--- a/spec/features/projects_spec.rb
+++ b/spec/features/projects_spec.rb
@@ -202,13 +202,13 @@ describe 'Project' do
expect(page).not_to have_content('Forked from')
end
- it 'shows the name of the deleted project when the source was deleted', :sidekiq_might_not_need_inline do
+ it 'does not show the name of the deleted project when the source was deleted', :sidekiq_might_not_need_inline do
forked_project
Projects::DestroyService.new(base_project, base_project.owner).execute
visit project_path(forked_project)
- expect(page).to have_content("Forked from #{base_project.full_name} (deleted)")
+ expect(page).to have_content('Forked from an inaccessible project')
end
context 'a fork of a fork' do
diff --git a/spec/initializers/hangouts_chat_http_override_spec.rb b/spec/initializers/hangouts_chat_http_override_spec.rb
new file mode 100644
index 00000000000..0eee891799f
--- /dev/null
+++ b/spec/initializers/hangouts_chat_http_override_spec.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'HangoutsChat::Sender Gitlab::HTTP override' do
+ describe 'HangoutsChat::Sender::HTTP#post' do
+ it 'calls Gitlab::HTTP.post with default protection settings' do
+ webhook_url = 'https://example.gitlab.com'
+ payload = { key: 'value' }
+ http = HangoutsChat::Sender::HTTP.new(webhook_url)
+ mock_response = double(response: 'the response')
+
+ expect(Gitlab::HTTP).to receive(:post)
+ .with(
+ URI.parse(webhook_url),
+ body: payload.to_json,
+ headers: { 'Content-Type' => 'application/json' },
+ parse: nil
+ )
+ .and_return(mock_response)
+
+ expect(http.post(payload)).to eq(mock_response.response)
+ end
+
+ it_behaves_like 'a request using Gitlab::UrlBlocker' do
+ let(:http_method) { :post }
+ let(:url_blocked_error_class) { Gitlab::HTTP::BlockedUrlError }
+
+ def make_request(uri)
+ HangoutsChat::Sender::HTTP.new(uri).post({})
+ end
+ end
+ end
+end
diff --git a/spec/initializers/rest-client-hostname_override_spec.rb b/spec/initializers/rest-client-hostname_override_spec.rb
index 90a0305c9a9..7e36656ba1c 100644
--- a/spec/initializers/rest-client-hostname_override_spec.rb
+++ b/spec/initializers/rest-client-hostname_override_spec.rb
@@ -3,147 +3,12 @@
require 'spec_helper'
describe 'rest-client dns rebinding protection' do
- include StubRequests
+ it_behaves_like 'a request using Gitlab::UrlBlocker' do
+ let(:http_method) { :get }
+ let(:url_blocked_error_class) { ArgumentError }
- context 'when local requests are not allowed' do
- it 'allows an external request with http' do
- request_stub = stub_full_request('http://example.com', ip_address: '93.184.216.34')
-
- RestClient.get('http://example.com/')
-
- expect(request_stub).to have_been_requested
- end
-
- it 'allows an external request with https' do
- request_stub = stub_full_request('https://example.com', ip_address: '93.184.216.34')
-
- RestClient.get('https://example.com/')
-
- expect(request_stub).to have_been_requested
- end
-
- it 'raises error when it is a request that resolves to a local address' do
- stub_full_request('https://example.com', ip_address: '172.16.0.0')
-
- expect { RestClient.get('https://example.com') }
- .to raise_error(ArgumentError,
- "URL 'https://example.com' is blocked: Requests to the local network are not allowed")
- end
-
- it 'raises error when it is a request that resolves to a localhost address' do
- stub_full_request('https://example.com', ip_address: '127.0.0.1')
-
- expect { RestClient.get('https://example.com') }
- .to raise_error(ArgumentError,
- "URL 'https://example.com' is blocked: Requests to localhost are not allowed")
- end
-
- it 'raises error when it is a request to local address' do
- expect { RestClient.get('http://172.16.0.0') }
- .to raise_error(ArgumentError,
- "URL 'http://172.16.0.0' is blocked: Requests to the local network are not allowed")
- end
-
- it 'raises error when it is a request to localhost address' do
- expect { RestClient.get('http://127.0.0.1') }
- .to raise_error(ArgumentError,
- "URL 'http://127.0.0.1' is blocked: Requests to localhost are not allowed")
- end
- end
-
- context 'when port different from URL scheme is used' do
- it 'allows the request' do
- request_stub = stub_full_request('https://example.com:8080', ip_address: '93.184.216.34')
-
- RestClient.get('https://example.com:8080/')
-
- expect(request_stub).to have_been_requested
- end
-
- it 'raises error when it is a request to local address' do
- expect { RestClient.get('https://172.16.0.0:8080') }
- .to raise_error(ArgumentError,
- "URL 'https://172.16.0.0:8080' is blocked: Requests to the local network are not allowed")
- end
-
- it 'raises error when it is a request to localhost address' do
- expect { RestClient.get('https://127.0.0.1:8080') }
- .to raise_error(ArgumentError,
- "URL 'https://127.0.0.1:8080' is blocked: Requests to localhost are not allowed")
- end
- end
-
- context 'when DNS rebinding protection is disabled' do
- before do
- stub_application_setting(dns_rebinding_protection_enabled: false)
- end
-
- it 'allows the request' do
- request_stub = stub_request(:get, 'https://example.com')
-
- RestClient.get('https://example.com/')
-
- expect(request_stub).to have_been_requested
- end
- end
-
- context 'when http(s) proxy environment variable is set' do
- before do
- stub_env('https_proxy' => 'https://my.proxy')
- end
-
- it 'allows the request' do
- request_stub = stub_request(:get, 'https://example.com')
-
- RestClient.get('https://example.com/')
-
- expect(request_stub).to have_been_requested
- end
- end
-
- context 'when local requests are allowed' do
- before do
- stub_application_setting(allow_local_requests_from_web_hooks_and_services: true)
- end
-
- it 'allows an external request' do
- request_stub = stub_full_request('https://example.com', ip_address: '93.184.216.34')
-
- RestClient.get('https://example.com/')
-
- expect(request_stub).to have_been_requested
- end
-
- it 'allows an external request that resolves to a local address' do
- request_stub = stub_full_request('https://example.com', ip_address: '172.16.0.0')
-
- RestClient.get('https://example.com/')
-
- expect(request_stub).to have_been_requested
- end
-
- it 'allows an external request that resolves to a localhost address' do
- request_stub = stub_full_request('https://example.com', ip_address: '127.0.0.1')
-
- RestClient.get('https://example.com/')
-
- expect(request_stub).to have_been_requested
- end
-
- it 'allows a local address request' do
- request_stub = stub_request(:get, 'http://172.16.0.0')
-
- RestClient.get('http://172.16.0.0')
-
- expect(request_stub).to have_been_requested
- end
-
- it 'allows a localhost address request' do
- request_stub = stub_request(:get, 'http://127.0.0.1')
-
- RestClient.get('http://127.0.0.1')
-
- expect(request_stub).to have_been_requested
+ def make_request(uri)
+ RestClient.get(uri)
end
end
end
diff --git a/spec/lib/banzai/filter/label_reference_filter_spec.rb b/spec/lib/banzai/filter/label_reference_filter_spec.rb
index 35e99d2586e..66af26bc51c 100644
--- a/spec/lib/banzai/filter/label_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/label_reference_filter_spec.rb
@@ -521,6 +521,15 @@ describe Banzai::Filter::LabelReferenceFilter do
expect(reference_filter(act).to_html).to eq exp
end
+
+ context 'when group name has HTML entities' do
+ let(:another_group) { create(:group, name: '<img src=x onerror=alert(1)>', path: 'another_group') }
+
+ it 'escapes the HTML entities' do
+ expect(result.text)
+ .to eq "See #{group_label.name} in #{another_project.full_name}"
+ end
+ end
end
describe 'cross-project / same-group_label complete reference' do
diff --git a/spec/lib/banzai/filter/relative_link_filter_spec.rb b/spec/lib/banzai/filter/relative_link_filter_spec.rb
index 046c346a7ac..371c7a2347c 100644
--- a/spec/lib/banzai/filter/relative_link_filter_spec.rb
+++ b/spec/lib/banzai/filter/relative_link_filter_spec.rb
@@ -119,6 +119,11 @@ describe Banzai::Filter::RelativeLinkFilter do
expect { filter(act) }.not_to raise_error
end
+ it 'does not raise an exception on URIs containing invalid utf-8 byte sequences' do
+ act = link("%FF")
+ expect { filter(act) }.not_to raise_error
+ end
+
it 'does not raise an exception with a garbled path' do
act = link("open(/var/tmp/):%20/location%0Afrom:%20/test")
expect { filter(act) }.not_to raise_error
diff --git a/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb b/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb
index 8f9dac6d281..94edef20296 100644
--- a/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb
+++ b/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb
@@ -6,6 +6,11 @@ describe Gitlab::CycleAnalytics::StageSummary do
let(:project) { create(:project, :repository) }
let(:options) { { from: 1.day.ago, current_user: user } }
let(:user) { create(:user, :admin) }
+
+ before do
+ project.add_maintainer(user)
+ end
+
let(:stage_summary) { described_class.new(project, options).data }
describe "#new_issues" do
@@ -86,6 +91,24 @@ describe Gitlab::CycleAnalytics::StageSummary do
expect(subject).to eq(2)
end
end
+
+ context 'when a guest user is signed in' do
+ let(:guest_user) { create(:user) }
+
+ before do
+ project.add_guest(guest_user)
+ options.merge!({ current_user: guest_user })
+ end
+
+ it 'does not include commit stats' do
+ data = described_class.new(project, options).data
+ expect(includes_commits?(data)).to be_falsy
+ end
+
+ def includes_commits?(data)
+ data.any? { |h| h["title"] == 'Commits' }
+ end
+ end
end
describe "#deploys" do
diff --git a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb
index 873728f9909..1b28e26a7e8 100644
--- a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb
+++ b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb
@@ -24,7 +24,10 @@ describe Gitlab::ImportExport::AttributeCleaner do
'_html' => '<p>perfectly ordinary html</p>',
'cached_markdown_version' => 12345,
'group_id' => 99,
- 'commit_id' => 99
+ 'commit_id' => 99,
+ 'issue_ids' => [1, 2, 3],
+ 'merge_request_ids' => [1, 2, 3],
+ 'note_ids' => [1, 2, 3]
}
end
diff --git a/spec/migrations/encrypt_plaintext_attributes_on_application_settings_spec.rb b/spec/migrations/encrypt_plaintext_attributes_on_application_settings_spec.rb
new file mode 100644
index 00000000000..122da7b3d72
--- /dev/null
+++ b/spec/migrations/encrypt_plaintext_attributes_on_application_settings_spec.rb
@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20191120115530_encrypt_plaintext_attributes_on_application_settings.rb')
+
+describe EncryptPlaintextAttributesOnApplicationSettings, :migration do
+ let(:migration) { described_class.new }
+ let(:application_settings) { table(:application_settings) }
+ let(:plaintext) { 'secret-token' }
+
+ PLAINTEXT_ATTRIBUTES = %w[
+ akismet_api_key
+ elasticsearch_aws_secret_access_key
+ recaptcha_private_key
+ recaptcha_site_key
+ slack_app_secret
+ slack_app_verification_token
+ ].freeze
+
+ describe '#up' do
+ it 'encrypts token and saves it' do
+ application_setting = application_settings.create
+ application_setting.update_columns(
+ PLAINTEXT_ATTRIBUTES.each_with_object({}) do |plaintext_attribute, attributes|
+ attributes[plaintext_attribute] = plaintext
+ end
+ )
+
+ migration.up
+
+ application_setting.reload
+ PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute|
+ expect(application_setting[plaintext_attribute]).not_to be_nil
+ expect(application_setting["encrypted_#{plaintext_attribute}"]).not_to be_nil
+ expect(application_setting["encrypted_#{plaintext_attribute}_iv"]).not_to be_nil
+ end
+ end
+ end
+
+ describe '#down' do
+ it 'decrypts encrypted token and saves it' do
+ application_setting = application_settings.create(
+ PLAINTEXT_ATTRIBUTES.each_with_object({}) do |plaintext_attribute, attributes|
+ attributes[plaintext_attribute] = plaintext
+ end
+ )
+
+ migration.down
+
+ application_setting.reload
+ PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute|
+ expect(application_setting[plaintext_attribute]).to eq(plaintext)
+ expect(application_setting["encrypted_#{plaintext_attribute}"]).to be_nil
+ expect(application_setting["encrypted_#{plaintext_attribute}_iv"]).to be_nil
+ end
+ end
+ end
+end
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 3ab88b52568..9c06cd049f5 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -285,6 +285,70 @@ describe Note do
end
end
+ describe "#visible_for?" do
+ using RSpec::Parameterized::TableSyntax
+
+ let_it_be(:note) { create(:note) }
+ let_it_be(:user) { create(:user) }
+
+ where(:cross_reference_visible, :system_note_viewable, :result) do
+ true | true | false
+ false | true | true
+ false | false | false
+ end
+
+ with_them do
+ it "returns expected result" do
+ expect(note).to receive(:cross_reference_not_visible_for?).and_return(cross_reference_visible)
+
+ unless cross_reference_visible
+ expect(note).to receive(:system_note_viewable_by?)
+ .with(user).and_return(system_note_viewable)
+ end
+
+ expect(note.visible_for?(user)).to eq result
+ end
+ end
+ end
+
+ describe "#system_note_viewable_by?(user)" do
+ let_it_be(:note) { create(:note) }
+ let_it_be(:user) { create(:user) }
+ let!(:metadata) { create(:system_note_metadata, note: note, action: "branch") }
+
+ context "when system_note_metadata is not present" do
+ it "returns true" do
+ expect(note).to receive(:system_note_metadata).and_return(nil)
+
+ expect(note.send(:system_note_viewable_by?, user)).to be_truthy
+ end
+ end
+
+ context "system_note_metadata isn't of type 'branch'" do
+ before do
+ metadata.action = "not_a_branch"
+ end
+
+ it "returns true" do
+ expect(note.send(:system_note_viewable_by?, user)).to be_truthy
+ end
+ end
+
+ context "user doesn't have :download_code ability" do
+ it "returns false" do
+ expect(note.send(:system_note_viewable_by?, user)).to be_falsey
+ end
+ end
+
+ context "user has the :download_code ability" do
+ it "returns true" do
+ expect(Ability).to receive(:allowed?).with(user, :download_code, note.project).and_return(true)
+
+ expect(note.send(:system_note_viewable_by?, user)).to be_truthy
+ end
+ end
+ 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) } }
diff --git a/spec/models/project_feature_spec.rb b/spec/models/project_feature_spec.rb
index 5b448c343a0..9ce1b8fd895 100644
--- a/spec/models/project_feature_spec.rb
+++ b/spec/models/project_feature_spec.rb
@@ -6,6 +6,16 @@ describe ProjectFeature do
let(:project) { create(:project) }
let(:user) { create(:user) }
+ describe 'PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT' do
+ it 'has higher level than that of PRIVATE_FEATURES_MIN_ACCESS_LEVEL' do
+ described_class::PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT.each do |feature, level|
+ if generic_level = described_class::PRIVATE_FEATURES_MIN_ACCESS_LEVEL[feature]
+ expect(level).to be >= generic_level
+ end
+ end
+ end
+ end
+
describe '.quoted_access_level_column' do
it 'returns the table name and quoted column name for a feature' do
expected = '"project_features"."issues_access_level"'
@@ -246,10 +256,24 @@ describe ProjectFeature do
expect(described_class.required_minimum_access_level('merge_requests')).to eq(Gitlab::Access::REPORTER)
end
+ it 'handles repository' do
+ expect(described_class.required_minimum_access_level(:repository)).to eq(Gitlab::Access::GUEST)
+ end
+
it 'raises error if feature is invalid' do
expect do
described_class.required_minimum_access_level(:foos)
end.to raise_error
end
end
+
+ describe '.required_minimum_access_level_for_private_project' do
+ it 'returns higher permission for repository' do
+ expect(described_class.required_minimum_access_level_for_private_project(:repository)).to eq(Gitlab::Access::REPORTER)
+ end
+
+ it 'returns normal permission for issues' do
+ expect(described_class.required_minimum_access_level_for_private_project(:issues)).to eq(Gitlab::Access::GUEST)
+ end
+ end
end
diff --git a/spec/models/project_services/chat_notification_service_spec.rb b/spec/models/project_services/chat_notification_service_spec.rb
index 6f4ddd223f6..e8c5f5d611a 100644
--- a/spec/models/project_services/chat_notification_service_spec.rb
+++ b/spec/models/project_services/chat_notification_service_spec.rb
@@ -30,7 +30,8 @@ describe ChatNotificationService do
end
describe '#execute' do
- let(:chat_service) { described_class.new }
+ subject(:chat_service) { described_class.new }
+
let(:user) { create(:user) }
let(:project) { create(:project, :repository) }
let(:webhook_url) { 'https://example.gitlab.com/' }
@@ -53,12 +54,7 @@ describe ChatNotificationService do
subject.project = project
data = Gitlab::DataBuilder::Push.build_sample(project, user)
- expect(Slack::Notifier).to receive(:new)
- .with(webhook_url, {})
- .and_return(
- double(:slack_service).as_null_object
- )
-
+ expect(chat_service).to receive(:notify).and_return(true)
expect(chat_service.execute(data)).to be true
end
end
@@ -68,12 +64,7 @@ describe ChatNotificationService do
subject.project = create(:project, :empty_repo)
data = Gitlab::DataBuilder::Push.build_sample(subject.project, user)
- expect(Slack::Notifier).to receive(:new)
- .with(webhook_url, {})
- .and_return(
- double(:slack_service).as_null_object
- )
-
+ expect(chat_service).to receive(:notify).and_return(true)
expect(chat_service.execute(data)).to be true
end
end
diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb
index f751dd6ffb9..93036ac7ec4 100644
--- a/spec/models/project_services/slack_service_spec.rb
+++ b/spec/models/project_services/slack_service_spec.rb
@@ -3,5 +3,5 @@
require 'spec_helper'
describe SlackService do
- it_behaves_like "slack or mattermost notifications", "Slack"
+ it_behaves_like "slack or mattermost notifications", 'Slack'
end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 815ab7aa166..14cb7d67e11 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -2924,7 +2924,7 @@ describe Project do
end
describe '#any_lfs_file_locks?', :request_store do
- set(:project) { create(:project) }
+ let_it_be(:project) { create(:project) }
it 'returns false when there are no LFS file locks' do
expect(project.any_lfs_file_locks?).to be_falsey
@@ -3411,6 +3411,20 @@ describe Project do
expect(projects).to eq([public_project])
end
end
+
+ context 'min_access_level' do
+ let!(:private_project) { create(:project, :private) }
+
+ before do
+ private_project.add_guest(user)
+ end
+
+ it 'excludes projects when user does not have required minimum access level' do
+ projects = described_class.all.public_or_visible_to_user(user, Gitlab::Access::REPORTER)
+
+ expect(projects).to contain_exactly(public_project)
+ end
+ end
end
describe '.ids_with_issuables_available_for' do
@@ -3539,7 +3553,7 @@ describe Project do
include ProjectHelpers
using RSpec::Parameterized::TableSyntax
- set(:group) { create(:group) }
+ let_it_be(:group) { create(:group) }
let!(:project) { create(:project, project_level, namespace: group ) }
let(:user) { create_user_from_membership(project, membership) }
@@ -3562,6 +3576,66 @@ describe Project do
end
end
end
+
+ context 'issues' do
+ let(:feature) { Issue }
+
+ where(:project_level, :feature_access_level, :membership, :expected_count) do
+ permission_table_for_guest_feature_access
+ end
+
+ with_them do
+ it "respects visibility" do
+ update_feature_access_level(project, feature_access_level)
+
+ expected_objects = expected_count == 1 ? [project] : []
+
+ expect(
+ described_class.filter_by_feature_visibility(feature, user)
+ ).to eq(expected_objects)
+ end
+ end
+ end
+
+ context 'wiki' do
+ let(:feature) { :wiki }
+
+ where(:project_level, :feature_access_level, :membership, :expected_count) do
+ permission_table_for_guest_feature_access
+ end
+
+ with_them do
+ it "respects visibility" do
+ update_feature_access_level(project, feature_access_level)
+
+ expected_objects = expected_count == 1 ? [project] : []
+
+ expect(
+ described_class.filter_by_feature_visibility(feature, user)
+ ).to eq(expected_objects)
+ end
+ end
+ end
+
+ context 'code' do
+ let(:feature) { :repository }
+
+ where(:project_level, :feature_access_level, :membership, :expected_count) do
+ permission_table_for_guest_feature_access_and_non_private_project_only
+ end
+
+ with_them do
+ it "respects visibility" do
+ update_feature_access_level(project, feature_access_level)
+
+ expected_objects = expected_count == 1 ? [project] : []
+
+ expect(
+ described_class.filter_by_feature_visibility(feature, user)
+ ).to eq(expected_objects)
+ end
+ end
+ end
end
describe '#pages_available?' do
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index f1447536e0f..cda2dd7d2f4 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -49,6 +49,8 @@ shared_examples 'languages and percentages JSON response' do
end
describe API::Projects do
+ include ProjectForksHelper
+
let(:user) { create(:user) }
let(:user2) { create(:user) }
let(:user3) { create(:user) }
@@ -1163,6 +1165,18 @@ describe API::Projects do
expect(json_response.keys).not_to include('permissions')
end
+ context 'the project is a public fork' do
+ it 'hides details of a public fork parent' do
+ public_project = create(:project, :repository, :public)
+ fork = fork_project(public_project)
+
+ get api("/projects/#{fork.id}")
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response['forked_from_project']).to be_nil
+ end
+ end
+
context 'and the project has a private repository' do
let(:project) { create(:project, :repository, :public, :repository_private) }
let(:protected_attributes) { %w(default_branch ci_config_path) }
@@ -1479,6 +1493,28 @@ describe API::Projects do
end
end
+ context 'the project is a fork' do
+ it 'shows details of a visible fork parent' do
+ fork = fork_project(project, user)
+
+ get api("/projects/#{fork.id}", user)
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response['forked_from_project']).to include('id' => project.id)
+ end
+
+ it 'hides details of a hidden fork parent' do
+ fork = fork_project(project, user)
+ fork_user = create(:user)
+ fork.team.add_developer(fork_user)
+
+ get api("/projects/#{fork.id}", fork_user)
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response['forked_from_project']).to be_nil
+ end
+ end
+
describe 'permissions' do
context 'all projects' do
before do
diff --git a/spec/support/helpers/project_helpers.rb b/spec/support/helpers/project_helpers.rb
index 61056b47aed..89f0163b4b6 100644
--- a/spec/support/helpers/project_helpers.rb
+++ b/spec/support/helpers/project_helpers.rb
@@ -10,16 +10,18 @@ module ProjectHelpers
nil
when :non_member
create(:user, name: membership)
+ when :admin
+ create(:user, :admin, name: 'admin')
else
create(:user, name: membership).tap { |u| target.add_user(u, membership) }
end
end
def update_feature_access_level(project, access_level)
- project.update!(
- repository_access_level: access_level,
- merge_requests_access_level: access_level,
- builds_access_level: access_level
- )
+ features = ProjectFeature::FEATURES.dup
+ features.delete(:pages)
+ params = features.each_with_object({}) { |feature, h| h["#{feature}_access_level"] = access_level }
+
+ project.update!(params)
end
end
diff --git a/spec/support/helpers/search_helpers.rb b/spec/support/helpers/search_helpers.rb
index d1d25fbabcd..db6e47459e9 100644
--- a/spec/support/helpers/search_helpers.rb
+++ b/spec/support/helpers/search_helpers.rb
@@ -20,6 +20,12 @@ module SearchHelpers
end
end
+ def has_search_scope?(scope)
+ page.within '.search-filter' do
+ has_link?(scope)
+ end
+ end
+
def max_limited_count
Gitlab::SearchResults::COUNT_LIMIT_MESSAGE
end
diff --git a/spec/support/shared_contexts/policies/project_policy_table_shared_context.rb b/spec/support/shared_contexts/policies/project_policy_table_shared_context.rb
index e666b346b8b..0a918ccde81 100644
--- a/spec/support/shared_contexts/policies/project_policy_table_shared_context.rb
+++ b/spec/support/shared_contexts/policies/project_policy_table_shared_context.rb
@@ -3,13 +3,28 @@
RSpec.shared_context 'ProjectPolicyTable context' do
using RSpec::Parameterized::TableSyntax
+ let(:pendings) { {} }
+ let(:pending?) do
+ pendings.include?(
+ {
+ project_level: project_level,
+ feature_access_level: feature_access_level,
+ membership: membership,
+ expected_count: expected_count
+ }
+ )
+ end
+
# rubocop:disable Metrics/AbcSize
+ # project_level, :feature_access_level, :membership, :expected_count
def permission_table_for_reporter_feature_access
+ :public | :enabled | :admin | 1
:public | :enabled | :reporter | 1
:public | :enabled | :guest | 1
:public | :enabled | :non_member | 1
:public | :enabled | :anonymous | 1
+ :public | :private | :admin | 1
:public | :private | :reporter | 1
:public | :private | :guest | 0
:public | :private | :non_member | 0
@@ -20,11 +35,13 @@ RSpec.shared_context 'ProjectPolicyTable context' do
:public | :disabled | :non_member | 0
:public | :disabled | :anonymous | 0
+ :internal | :enabled | :admin | 1
:internal | :enabled | :reporter | 1
:internal | :enabled | :guest | 1
:internal | :enabled | :non_member | 1
:internal | :enabled | :anonymous | 0
+ :internal | :private | :admin | 1
:internal | :private | :reporter | 1
:internal | :private | :guest | 0
:internal | :private | :non_member | 0
@@ -35,11 +52,7 @@ RSpec.shared_context 'ProjectPolicyTable context' do
:internal | :disabled | :non_member | 0
:internal | :disabled | :anonymous | 0
- :private | :enabled | :reporter | 1
- :private | :enabled | :guest | 1
- :private | :enabled | :non_member | 0
- :private | :enabled | :anonymous | 0
-
+ :private | :private | :admin | 1
:private | :private | :reporter | 1
:private | :private | :guest | 0
:private | :private | :non_member | 0
@@ -51,12 +64,15 @@ RSpec.shared_context 'ProjectPolicyTable context' do
:private | :disabled | :anonymous | 0
end
+ # project_level, :feature_access_level, :membership, :expected_count
def permission_table_for_guest_feature_access
+ :public | :enabled | :admin | 1
:public | :enabled | :reporter | 1
:public | :enabled | :guest | 1
:public | :enabled | :non_member | 1
:public | :enabled | :anonymous | 1
+ :public | :private | :admin | 1
:public | :private | :reporter | 1
:public | :private | :guest | 1
:public | :private | :non_member | 0
@@ -67,11 +83,13 @@ RSpec.shared_context 'ProjectPolicyTable context' do
:public | :disabled | :non_member | 0
:public | :disabled | :anonymous | 0
+ :internal | :enabled | :admin | 1
:internal | :enabled | :reporter | 1
:internal | :enabled | :guest | 1
:internal | :enabled | :non_member | 1
:internal | :enabled | :anonymous | 0
+ :internal | :private | :admin | 1
:internal | :private | :reporter | 1
:internal | :private | :guest | 1
:internal | :private | :non_member | 0
@@ -82,11 +100,7 @@ RSpec.shared_context 'ProjectPolicyTable context' do
:internal | :disabled | :non_member | 0
:internal | :disabled | :anonymous | 0
- :private | :enabled | :reporter | 1
- :private | :enabled | :guest | 1
- :private | :enabled | :non_member | 0
- :private | :enabled | :anonymous | 0
-
+ :private | :private | :admin | 1
:private | :private | :reporter | 1
:private | :private | :guest | 1
:private | :private | :non_member | 0
@@ -98,6 +112,196 @@ RSpec.shared_context 'ProjectPolicyTable context' do
:private | :disabled | :anonymous | 0
end
+ # This table is based on permission_table_for_guest_feature_access,
+ # but with a slight twist.
+ # Some features can be hidden away to GUEST, when project is private.
+ # (see ProjectFeature::PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT)
+ # This is the table for such features.
+ #
+ # e.g. `repository` feature has minimum requirement of GUEST,
+ # but a GUEST are prohibited from reading code if project is private.
+ #
+ # project_level, :feature_access_level, :membership, :expected_count
+ def permission_table_for_guest_feature_access_and_non_private_project_only
+ :public | :enabled | :admin | 1
+ :public | :enabled | :reporter | 1
+ :public | :enabled | :guest | 1
+ :public | :enabled | :non_member | 1
+ :public | :enabled | :anonymous | 1
+
+ :public | :private | :admin | 1
+ :public | :private | :reporter | 1
+ :public | :private | :guest | 1
+ :public | :private | :non_member | 0
+ :public | :private | :anonymous | 0
+
+ :public | :disabled | :reporter | 0
+ :public | :disabled | :guest | 0
+ :public | :disabled | :non_member | 0
+ :public | :disabled | :anonymous | 0
+
+ :internal | :enabled | :admin | 1
+ :internal | :enabled | :reporter | 1
+ :internal | :enabled | :guest | 1
+ :internal | :enabled | :non_member | 1
+ :internal | :enabled | :anonymous | 0
+
+ :internal | :private | :admin | 1
+ :internal | :private | :reporter | 1
+ :internal | :private | :guest | 1
+ :internal | :private | :non_member | 0
+ :internal | :private | :anonymous | 0
+
+ :internal | :disabled | :reporter | 0
+ :internal | :disabled | :guest | 0
+ :internal | :disabled | :non_member | 0
+ :internal | :disabled | :anonymous | 0
+
+ :private | :private | :admin | 1
+ :private | :private | :reporter | 1
+ :private | :private | :guest | 0
+ :private | :private | :non_member | 0
+ :private | :private | :anonymous | 0
+
+ :private | :disabled | :reporter | 0
+ :private | :disabled | :guest | 0
+ :private | :disabled | :non_member | 0
+ :private | :disabled | :anonymous | 0
+ end
+
+ # :project_level, :issues_access_level, :merge_requests_access_level, :membership, :expected_count
+ def permission_table_for_milestone_access
+ :public | :enabled | :enabled | :admin | 1
+ :public | :enabled | :enabled | :reporter | 1
+ :public | :enabled | :enabled | :guest | 1
+ :public | :enabled | :enabled | :non_member | 1
+ :public | :enabled | :enabled | :anonymous | 1
+
+ :public | :enabled | :private | :admin | 1
+ :public | :enabled | :private | :reporter | 1
+ :public | :enabled | :private | :guest | 1
+ :public | :enabled | :private | :non_member | 1
+ :public | :enabled | :private | :anonymous | 1
+
+ :public | :enabled | :disabled | :admin | 1
+ :public | :enabled | :disabled | :reporter | 1
+ :public | :enabled | :disabled | :guest | 1
+ :public | :enabled | :disabled | :non_member | 1
+ :public | :enabled | :disabled | :anonymous | 1
+
+ :public | :private | :enabled | :admin | 1
+ :public | :private | :enabled | :reporter | 1
+ :public | :private | :enabled | :guest | 1
+ :public | :private | :enabled | :non_member | 1
+ :public | :private | :enabled | :anonymous | 1
+
+ :public | :private | :private | :admin | 1
+ :public | :private | :private | :reporter | 1
+ :public | :private | :private | :guest | 1
+ :public | :private | :private | :non_member | 0
+ :public | :private | :private | :anonymous | 0
+
+ :public | :private | :disabled | :admin | 1
+ :public | :private | :disabled | :reporter | 1
+ :public | :private | :disabled | :guest | 1
+ :public | :private | :disabled | :non_member | 0
+ :public | :private | :disabled | :anonymous | 0
+
+ :public | :disabled | :enabled | :admin | 1
+ :public | :disabled | :enabled | :reporter | 1
+ :public | :disabled | :enabled | :guest | 1
+ :public | :disabled | :enabled | :non_member | 1
+ :public | :disabled | :enabled | :anonymous | 1
+
+ :public | :disabled | :private | :admin | 1
+ :public | :disabled | :private | :reporter | 1
+ :public | :disabled | :private | :guest | 0
+ :public | :disabled | :private | :non_member | 0
+ :public | :disabled | :private | :anonymous | 0
+
+ :public | :disabled | :disabled | :reporter | 0
+ :public | :disabled | :disabled | :guest | 0
+ :public | :disabled | :disabled | :non_member | 0
+ :public | :disabled | :disabled | :anonymous | 0
+
+ :internal | :enabled | :enabled | :admin | 1
+ :internal | :enabled | :enabled | :reporter | 1
+ :internal | :enabled | :enabled | :guest | 1
+ :internal | :enabled | :enabled | :non_member | 1
+ :internal | :enabled | :enabled | :anonymous | 0
+
+ :internal | :enabled | :private | :admin | 1
+ :internal | :enabled | :private | :reporter | 1
+ :internal | :enabled | :private | :guest | 1
+ :internal | :enabled | :private | :non_member | 1
+ :internal | :enabled | :private | :anonymous | 0
+
+ :internal | :enabled | :disabled | :admin | 1
+ :internal | :enabled | :disabled | :reporter | 1
+ :internal | :enabled | :disabled | :guest | 1
+ :internal | :enabled | :disabled | :non_member | 1
+ :internal | :enabled | :disabled | :anonymous | 0
+
+ :internal | :private | :enabled | :admin | 1
+ :internal | :private | :enabled | :reporter | 1
+ :internal | :private | :enabled | :guest | 1
+ :internal | :private | :enabled | :non_member | 1
+ :internal | :private | :enabled | :anonymous | 0
+
+ :internal | :private | :private | :admin | 1
+ :internal | :private | :private | :reporter | 1
+ :internal | :private | :private | :guest | 1
+ :internal | :private | :private | :non_member | 0
+ :internal | :private | :private | :anonymous | 0
+
+ :internal | :private | :disabled | :admin | 1
+ :internal | :private | :disabled | :reporter | 1
+ :internal | :private | :disabled | :guest | 1
+ :internal | :private | :disabled | :non_member | 0
+ :internal | :private | :disabled | :anonymous | 0
+
+ :internal | :disabled | :enabled | :admin | 1
+ :internal | :disabled | :enabled | :reporter | 1
+ :internal | :disabled | :enabled | :guest | 1
+ :internal | :disabled | :enabled | :non_member | 1
+ :internal | :disabled | :enabled | :anonymous | 0
+
+ :internal | :disabled | :private | :admin | 1
+ :internal | :disabled | :private | :reporter | 1
+ :internal | :disabled | :private | :guest | 0
+ :internal | :disabled | :private | :non_member | 0
+ :internal | :disabled | :private | :anonymous | 0
+
+ :internal | :disabled | :disabled | :reporter | 0
+ :internal | :disabled | :disabled | :guest | 0
+ :internal | :disabled | :disabled | :non_member | 0
+ :internal | :disabled | :disabled | :anonymous | 0
+
+ :private | :private | :private | :admin | 1
+ :private | :private | :private | :reporter | 1
+ :private | :private | :private | :guest | 1
+ :private | :private | :private | :non_member | 0
+ :private | :private | :private | :anonymous | 0
+
+ :private | :private | :disabled | :admin | 1
+ :private | :private | :disabled | :reporter | 1
+ :private | :private | :disabled | :guest | 1
+ :private | :private | :disabled | :non_member | 0
+ :private | :private | :disabled | :anonymous | 0
+
+ :private | :disabled | :private | :admin | 1
+ :private | :disabled | :private | :reporter | 1
+ :private | :disabled | :private | :guest | 0
+ :private | :disabled | :private | :non_member | 0
+ :private | :disabled | :private | :anonymous | 0
+
+ :private | :disabled | :disabled | :reporter | 0
+ :private | :disabled | :disabled | :guest | 0
+ :private | :disabled | :disabled | :non_member | 0
+ :private | :disabled | :disabled | :anonymous | 0
+ end
+
+ # :project_level, :membership, :expected_count
def permission_table_for_project_access
:public | :reporter | 1
:public | :guest | 1
diff --git a/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb b/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb
index f15128d3e13..2b68e7bfa82 100644
--- a/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb
+++ b/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb
@@ -1,11 +1,13 @@
# frozen_string_literal: true
RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
+ include StubRequests
+
let(:chat_service) { described_class.new }
- let(:webhook_url) { 'https://example.gitlab.com/' }
+ let(:webhook_url) { 'https://example.gitlab.com' }
def execute_with_options(options)
- receive(:new).with(webhook_url, options)
+ receive(:new).with(webhook_url, options.merge(http_client: SlackService::Notifier::HTTPClient))
.and_return(double(:slack_service).as_null_object)
end
@@ -38,9 +40,13 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
chat_service.branches_to_be_notified = branches_to_be_notified if branches_to_be_notified
end
+ let!(:stubbed_resolved_hostname) do
+ stub_full_request(webhook_url, method: :post).request_pattern.uri_pattern.to_s
+ end
+
it "notifies about #{event_type} events" do
chat_service.execute(data)
- expect(WebMock).to have_requested(:post, webhook_url)
+ expect(WebMock).to have_requested(:post, stubbed_resolved_hostname)
end
end
@@ -49,9 +55,13 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
chat_service.branches_to_be_notified = branches_to_be_notified if branches_to_be_notified
end
+ let!(:stubbed_resolved_hostname) do
+ stub_full_request(webhook_url, method: :post).request_pattern.uri_pattern.to_s
+ end
+
it "notifies about #{event_type} events" do
chat_service.execute(data)
- expect(WebMock).not_to have_requested(:post, webhook_url)
+ expect(WebMock).not_to have_requested(:post, stubbed_resolved_hostname)
end
end
@@ -66,6 +76,10 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
Gitlab::DataBuilder::Push.build_sample(project, user)
end
+ let!(:stubbed_resolved_hostname) do
+ stub_full_request(webhook_url, method: :post).request_pattern.uri_pattern.to_s
+ end
+
before do
allow(chat_service).to receive_messages(
project: project,
@@ -74,8 +88,6 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
webhook: webhook_url
)
- WebMock.stub_request(:post, webhook_url)
-
issue_service = Issues::CreateService.new(project, user, issue_service_options)
@issue = issue_service.execute
@issues_sample_data = issue_service.hook_data(@issue, 'open')
@@ -107,25 +119,25 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
it "calls #{service_name} API for push events" do
chat_service.execute(data)
- expect(WebMock).to have_requested(:post, webhook_url).once
+ expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once
end
it "calls #{service_name} API for issue events" do
chat_service.execute(@issues_sample_data)
- expect(WebMock).to have_requested(:post, webhook_url).once
+ expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once
end
it "calls #{service_name} API for merge requests events" do
chat_service.execute(@merge_sample_data)
- expect(WebMock).to have_requested(:post, webhook_url).once
+ expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once
end
it "calls #{service_name} API for wiki page events" do
chat_service.execute(@wiki_page_sample_data)
- expect(WebMock).to have_requested(:post, webhook_url).once
+ expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once
end
it "calls #{service_name} API for deployment events" do
@@ -133,14 +145,14 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
chat_service.execute(deployment_event_data)
- expect(WebMock).to have_requested(:post, webhook_url).once
+ expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once
end
it 'uses the username as an option for slack when configured' do
allow(chat_service).to receive(:username).and_return(username)
expect(Slack::Notifier).to receive(:new)
- .with(webhook_url, username: username)
+ .with(webhook_url, username: username, http_client: SlackService::Notifier::HTTPClient)
.and_return(
double(:slack_service).as_null_object
)
@@ -151,7 +163,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
it 'uses the channel as an option when it is configured' do
allow(chat_service).to receive(:channel).and_return(channel)
expect(Slack::Notifier).to receive(:new)
- .with(webhook_url, channel: channel)
+ .with(webhook_url, channel: channel, http_client: SlackService::Notifier::HTTPClient)
.and_return(
double(:slack_service).as_null_object
)
@@ -163,7 +175,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
chat_service.update(push_channel: "random")
expect(Slack::Notifier).to receive(:new)
- .with(webhook_url, channel: "random")
+ .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient)
.and_return(
double(:slack_service).as_null_object
)
@@ -175,7 +187,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
chat_service.update(merge_request_channel: "random")
expect(Slack::Notifier).to receive(:new)
- .with(webhook_url, channel: "random")
+ .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient)
.and_return(
double(:slack_service).as_null_object
)
@@ -187,7 +199,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
chat_service.update(issue_channel: "random")
expect(Slack::Notifier).to receive(:new)
- .with(webhook_url, channel: "random")
+ .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient)
.and_return(
double(:slack_service).as_null_object
)
@@ -219,7 +231,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
chat_service.update(wiki_page_channel: "random")
expect(Slack::Notifier).to receive(:new)
- .with(webhook_url, channel: "random")
+ .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient)
.and_return(
double(:slack_service).as_null_object
)
@@ -238,7 +250,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
note_data = Gitlab::DataBuilder::Note.build(issue_note, user)
expect(Slack::Notifier).to receive(:new)
- .with(webhook_url, channel: "random")
+ .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient)
.and_return(
double(:slack_service).as_null_object
)
@@ -286,7 +298,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
webhook: webhook_url
)
- WebMock.stub_request(:post, webhook_url)
+ stub_full_request(webhook_url, method: :post)
end
context 'on default branch' do
@@ -461,7 +473,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
webhook: webhook_url
)
- WebMock.stub_request(:post, webhook_url)
+ stub_full_request(webhook_url, method: :post)
end
context 'when commit comment event executed' do
@@ -535,7 +547,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
webhook: webhook_url
)
- WebMock.stub_request(:post, webhook_url)
+ stub_full_request(webhook_url, method: :post)
end
context 'with succeeded pipeline' do
diff --git a/spec/support/shared_examples/uses_gitlab_url_blocker_examples.rb b/spec/support/shared_examples/uses_gitlab_url_blocker_examples.rb
new file mode 100644
index 00000000000..59c119e6d96
--- /dev/null
+++ b/spec/support/shared_examples/uses_gitlab_url_blocker_examples.rb
@@ -0,0 +1,155 @@
+# frozen_string_literal: true
+
+shared_examples 'a request using Gitlab::UrlBlocker' do
+ # Written to test internal patches against 3rd party libraries
+ #
+ # Expects the following to be available in the example contexts:
+ #
+ # make_request(uri): Wraps the request we want to test goes through Gitlab::HTTP
+ # http_method: :get, :post etc
+ # url_blocked_error_class: Probably just Gitlab::HTTP::BlockedUrlError
+
+ include StubRequests
+
+ context 'when local requests are not allowed' do
+ it 'allows an external request with http' do
+ request_stub = stub_full_request('http://example.com', method: http_method, ip_address: '93.184.216.34')
+
+ make_request('http://example.com/')
+
+ expect(request_stub).to have_been_requested
+ end
+
+ it 'allows an external request with https' do
+ request_stub = stub_full_request('https://example.com', method: http_method, ip_address: '93.184.216.34')
+
+ make_request('https://example.com/')
+
+ expect(request_stub).to have_been_requested
+ end
+
+ it 'raises error when it is a request that resolves to a local address' do
+ stub_full_request('https://example.com', method: http_method, ip_address: '172.16.0.0')
+
+ expect { make_request('https://example.com') }
+ .to raise_error(url_blocked_error_class,
+ "URL 'https://example.com' is blocked: Requests to the local network are not allowed")
+ end
+
+ it 'raises error when it is a request that resolves to a localhost address' do
+ stub_full_request('https://example.com', method: http_method, ip_address: '127.0.0.1')
+
+ expect { make_request('https://example.com') }
+ .to raise_error(url_blocked_error_class,
+ "URL 'https://example.com' is blocked: Requests to localhost are not allowed")
+ end
+
+ it 'raises error when it is a request to local address' do
+ expect { make_request('http://172.16.0.0') }
+ .to raise_error(url_blocked_error_class,
+ "URL 'http://172.16.0.0' is blocked: Requests to the local network are not allowed")
+ end
+
+ it 'raises error when it is a request to localhost address' do
+ expect { make_request('http://127.0.0.1') }
+ .to raise_error(url_blocked_error_class,
+ "URL 'http://127.0.0.1' is blocked: Requests to localhost are not allowed")
+ end
+ end
+
+ context 'when port different from URL scheme is used' do
+ it 'allows the request' do
+ request_stub = stub_full_request('https://example.com:8080', method: http_method, ip_address: '93.184.216.34')
+
+ make_request('https://example.com:8080/')
+
+ expect(request_stub).to have_been_requested
+ end
+
+ it 'raises error when it is a request to local address' do
+ expect { make_request('https://172.16.0.0:8080') }
+ .to raise_error(url_blocked_error_class,
+ "URL 'https://172.16.0.0:8080' is blocked: Requests to the local network are not allowed")
+ end
+
+ it 'raises error when it is a request to localhost address' do
+ expect { make_request('https://127.0.0.1:8080') }
+ .to raise_error(url_blocked_error_class,
+ "URL 'https://127.0.0.1:8080' is blocked: Requests to localhost are not allowed")
+ end
+ end
+
+ context 'when DNS rebinding protection is disabled' do
+ before do
+ stub_application_setting(dns_rebinding_protection_enabled: false)
+ end
+
+ it 'allows the request' do
+ request_stub = stub_request(http_method, 'https://example.com')
+
+ make_request('https://example.com/')
+
+ expect(request_stub).to have_been_requested
+ end
+ end
+
+ context 'when http(s) proxy environment variable is set' do
+ before do
+ stub_env('https_proxy' => 'https://my.proxy')
+ end
+
+ it 'allows the request' do
+ request_stub = stub_request(http_method, 'https://example.com')
+
+ make_request('https://example.com/')
+
+ expect(request_stub).to have_been_requested
+ end
+ end
+
+ context 'when local requests are allowed' do
+ before do
+ stub_application_setting(allow_local_requests_from_web_hooks_and_services: true)
+ end
+
+ it 'allows an external request' do
+ request_stub = stub_full_request('https://example.com', method: http_method, ip_address: '93.184.216.34')
+
+ make_request('https://example.com/')
+
+ expect(request_stub).to have_been_requested
+ end
+
+ it 'allows an external request that resolves to a local address' do
+ request_stub = stub_full_request('https://example.com', method: http_method, ip_address: '172.16.0.0')
+
+ make_request('https://example.com/')
+
+ expect(request_stub).to have_been_requested
+ end
+
+ it 'allows an external request that resolves to a localhost address' do
+ request_stub = stub_full_request('https://example.com', method: http_method, ip_address: '127.0.0.1')
+
+ make_request('https://example.com/')
+
+ expect(request_stub).to have_been_requested
+ end
+
+ it 'allows a local address request' do
+ request_stub = stub_request(http_method, 'http://172.16.0.0')
+
+ make_request('http://172.16.0.0')
+
+ expect(request_stub).to have_been_requested
+ end
+
+ it 'allows a localhost address request' do
+ request_stub = stub_request(http_method, 'http://127.0.0.1')
+
+ make_request('http://127.0.0.1')
+
+ expect(request_stub).to have_been_requested
+ end
+ end
+end
diff --git a/spec/views/projects/_home_panel.html.haml_spec.rb b/spec/views/projects/_home_panel.html.haml_spec.rb
index 4d5b369e88e..9956144b601 100644
--- a/spec/views/projects/_home_panel.html.haml_spec.rb
+++ b/spec/views/projects/_home_panel.html.haml_spec.rb
@@ -3,6 +3,8 @@
require 'spec_helper'
describe 'projects/_home_panel' do
+ include ProjectForksHelper
+
context 'notifications' do
let(:project) { create(:project) }
@@ -144,4 +146,36 @@ describe 'projects/_home_panel' do
end
end
end
+
+ context 'forks' do
+ let(:source_project) { create(:project, :repository) }
+ let(:project) { fork_project(source_project) }
+ let(:user) { create(:user) }
+
+ before do
+ assign(:project, project)
+
+ allow(view).to receive(:current_user).and_return(user)
+ end
+
+ context 'user can read fork source' do
+ it 'shows the forked-from project' do
+ allow(view).to receive(:can?).with(user, :read_project, source_project).and_return(true)
+
+ render
+
+ expect(rendered).to have_content("Forked from #{source_project.full_name}")
+ end
+ end
+
+ context 'user cannot read fork source' do
+ it 'does not show the forked-from project' do
+ allow(view).to receive(:can?).with(user, :read_project, source_project).and_return(false)
+
+ render
+
+ expect(rendered).to have_content("Forked from an inaccessible project")
+ end
+ end
+ end
end
diff --git a/spec/views/projects/edit.html.haml_spec.rb b/spec/views/projects/edit.html.haml_spec.rb
index f576093ab45..40927a22dc4 100644
--- a/spec/views/projects/edit.html.haml_spec.rb
+++ b/spec/views/projects/edit.html.haml_spec.rb
@@ -4,6 +4,7 @@ require 'spec_helper'
describe 'projects/edit' do
include Devise::Test::ControllerHelpers
+ include ProjectForksHelper
let(:project) { create(:project) }
let(:user) { create(:admin) }
@@ -26,4 +27,59 @@ describe 'projects/edit' do
expect(rendered).not_to have_content('Export project')
end
end
+
+ context 'forking' do
+ before do
+ assign(:project, project)
+
+ allow(view).to receive(:current_user).and_return(user)
+ end
+
+ context 'project is not a fork' do
+ it 'hides the remove fork relationship settings' do
+ render
+
+ expect(rendered).not_to have_content('Remove fork relationship')
+ end
+ end
+
+ context 'project is a fork' do
+ let(:source_project) { create(:project) }
+ let(:project) { fork_project(source_project) }
+
+ it 'shows the remove fork relationship settings to an authorized user' do
+ allow(view).to receive(:can?).with(user, :remove_fork_project, project).and_return(true)
+
+ render
+
+ expect(rendered).to have_content('Remove fork relationship')
+ end
+
+ it 'hides the fork relationship settings from an unauthorized user' do
+ allow(view).to receive(:can?).with(user, :remove_fork_project, project).and_return(false)
+
+ render
+
+ expect(rendered).not_to have_content('Remove fork relationship')
+ end
+
+ it 'hides the fork source from an unauthorized user' do
+ allow(view).to receive(:can?).with(user, :read_project, source_project).and_return(false)
+
+ render
+
+ expect(rendered).to have_content('Remove fork relationship')
+ expect(rendered).not_to have_content(source_project.full_name)
+ end
+
+ it 'shows the fork source to an authorized user' do
+ allow(view).to receive(:can?).with(user, :read_project, source_project).and_return(true)
+
+ render
+
+ expect(rendered).to have_content('Remove fork relationship')
+ expect(rendered).to have_content(source_project.full_name)
+ end
+ end
+ end
end