diff options
91 files changed, 899 insertions, 370 deletions
diff --git a/.prettierignore b/.prettierignore new file mode 100644 index 00000000000..62fe6a45b95 --- /dev/null +++ b/.prettierignore @@ -0,0 +1,6 @@ +/config/ +/node_modules/ +/public/ +/vendor/ +karma.config.js +webpack.config.js diff --git a/.rubocop.yml b/.rubocop.yml index 14840ddd262..0582bfe8d70 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -118,6 +118,9 @@ Gitlab/ModuleWithInstanceVariables: - spec/support/**/*.rb - features/steps/**/*.rb +Gitlab/HTTParty: + Enabled: true + GitlabSecurity/PublicSend: Enabled: true Exclude: @@ -38,7 +38,7 @@ gem 'devise', '~> 4.2' gem 'doorkeeper', '~> 4.3' gem 'doorkeeper-openid_connect', '~> 1.3' gem 'omniauth', '~> 1.8' -gem 'omniauth-auth0', '~> 1.4.1' +gem 'omniauth-auth0', '~> 2.0.0' gem 'omniauth-azure-oauth2', '~> 0.0.9' gem 'omniauth-cas3', '~> 1.1.4' gem 'omniauth-facebook', '~> 4.0.0' diff --git a/Gemfile.lock b/Gemfile.lock index 9eff80cd4d7..aed9f1d6b30 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -527,8 +527,8 @@ GEM omniauth (1.8.1) hashie (>= 3.4.6, < 3.6.0) rack (>= 1.6.2, < 3) - omniauth-auth0 (1.4.1) - omniauth-oauth2 (~> 1.1) + omniauth-auth0 (2.0.0) + omniauth-oauth2 (~> 1.4) omniauth-authentiq (0.3.1) omniauth-oauth2 (~> 1.3, >= 1.3.1) omniauth-azure-oauth2 (0.0.9) @@ -1105,7 +1105,7 @@ DEPENDENCIES oauth2 (~> 1.4) octokit (~> 4.8) omniauth (~> 1.8) - omniauth-auth0 (~> 1.4.1) + omniauth-auth0 (~> 2.0.0) omniauth-authentiq (~> 0.3.1) omniauth-azure-oauth2 (~> 0.0.9) omniauth-cas3 (~> 1.1.4) diff --git a/app/assets/javascripts/boards/components/issue_card_inner.js b/app/assets/javascripts/boards/components/issue_card_inner.js index 7e882a57202..8aee5b23c76 100644 --- a/app/assets/javascripts/boards/components/issue_card_inner.js +++ b/app/assets/javascripts/boards/components/issue_card_inner.js @@ -1,6 +1,6 @@ import $ from 'jquery'; import Vue from 'vue'; -import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; +import UserAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; import eventHub from '../eventhub'; const Store = gl.issueBoards.BoardsStore; @@ -45,7 +45,7 @@ gl.issueBoards.IssueCardInner = Vue.extend({ }; }, components: { - userAvatarLink, + UserAvatarLink, }, computed: { numberOverLimit() { diff --git a/app/assets/javascripts/performance_bar/components/detailed_metric.vue b/app/assets/javascripts/performance_bar/components/detailed_metric.vue index d4881f07972..db8a0055acd 100644 --- a/app/assets/javascripts/performance_bar/components/detailed_metric.vue +++ b/app/assets/javascripts/performance_bar/components/detailed_metric.vue @@ -70,6 +70,7 @@ export default { <td v-for="key in keys" :key="key" + class="break-word" > {{ item[key] }} </td> diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 37d33320445..d0dda50a835 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -446,6 +446,10 @@ img.emoji { opacity: .5; } +.break-word { + word-wrap: break-word; +} + /** COMMON CLASSES **/ .prepend-top-0 { margin-top: 0; } .prepend-top-5 { margin-top: 5px; } diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index 6397757bf88..cc74cb72795 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -622,7 +622,7 @@ } .dropdown-content { - max-height: $dropdown-max-height; + max-height: 252px; overflow-y: auto; } @@ -699,6 +699,31 @@ border-radius: $border-radius-base; } +.git-revision-dropdown { + .dropdown-content { + max-height: 215px; + } +} + +.sidebar-move-issue-dropdown { + .dropdown-content { + max-height: 160px; + } +} + +.dropdown-menu-author { + .dropdown-content { + max-height: 215px; + } +} + +.dropdown-menu-labels { + .dropdown-content { + max-height: 128px; + } +} + + .dropdown-menu-due-date { .dropdown-content { max-height: 230px; diff --git a/app/assets/stylesheets/framework/sidebar.scss b/app/assets/stylesheets/framework/sidebar.scss index d1d98270ad9..3dd4a613789 100644 --- a/app/assets/stylesheets/framework/sidebar.scss +++ b/app/assets/stylesheets/framework/sidebar.scss @@ -152,3 +152,4 @@ .sidebar-collapsed-icon .sidebar-collapsed-value { font-size: 12px; } + diff --git a/app/assets/stylesheets/pages/boards.scss b/app/assets/stylesheets/pages/boards.scss index c03d4c2eebf..318d3ddaece 100644 --- a/app/assets/stylesheets/pages/boards.scss +++ b/app/assets/stylesheets/pages/boards.scss @@ -31,8 +31,12 @@ .dropdown-menu-issues-board-new { width: 320px; + .open & { + max-height: 400px; + } + .dropdown-content { - max-height: 150px; + max-height: 162px; } } diff --git a/app/assets/stylesheets/pages/branches.scss b/app/assets/stylesheets/pages/branches.scss index 3e2fa8ca88d..49fe50977f5 100644 --- a/app/assets/stylesheets/pages/branches.scss +++ b/app/assets/stylesheets/pages/branches.scss @@ -1,6 +1,17 @@ +.content-list > .branch-item, +.branch-title { + display: flex; + align-items: center; +} + +.branch-info { + flex: auto; + min-width: 0; + overflow: hidden; +} + .divergence-graph { - padding: 12px 12px 0 0; - float: right; + padding: 0 6px; .graph-side { position: relative; @@ -53,3 +64,9 @@ background-color: $divergence-graph-separator-bg; } } + +.divergence-graph, +.branch-item .controls { + flex: 0 0 auto; + white-space: nowrap; +} diff --git a/app/assets/stylesheets/pages/events.scss b/app/assets/stylesheets/pages/events.scss index 8871a069d5d..d9267f5cdf3 100644 --- a/app/assets/stylesheets/pages/events.scss +++ b/app/assets/stylesheets/pages/events.scss @@ -162,17 +162,14 @@ * Last push widget */ .event-last-push { - overflow: auto; width: 100%; + display: flex; + align-items: center; .event-last-push-text { @include str-truncated(100%); - padding: 4px 0; font-size: 13px; - float: left; - margin-right: -150px; - padding-right: 150px; - line-height: 20px; + margin-right: $gl-padding; } } diff --git a/app/assets/stylesheets/pages/labels.scss b/app/assets/stylesheets/pages/labels.scss index 0f49d15203b..b0852adb459 100644 --- a/app/assets/stylesheets/pages/labels.scss +++ b/app/assets/stylesheets/pages/labels.scss @@ -26,9 +26,15 @@ } } +.dropdown-menu-labels { + .dropdown-content { + max-height: 135px; + } +} + .dropdown-new-label { .dropdown-content { - max-height: 260px; + max-height: 136px; } } diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 85de0d8e70f..584b0579b72 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -9,7 +9,6 @@ .new_project, .edit-project, .import-project { - .help-block { margin-bottom: 10px; } @@ -18,18 +17,25 @@ border-radius: $border-radius-base; } - .input-group > div { + .input-group { + display: flex; - &:last-child { - padding-right: 0; + .select2-container { + display: unset; + max-width: unset; + width: unset !important; + flex-grow: 1; + } + + > div { + &:last-child { + padding-right: 0; + } } } @media (max-width: $screen-xs-max) { .input-group > div { - - margin-bottom: 14px; - &:last-child { margin-bottom: 0; } @@ -41,17 +47,24 @@ } .input-group-addon { + overflow: hidden; + text-overflow: ellipsis; + line-height: unset; + width: unset; + max-width: 50%; + text-align: left; &.static-namespace { height: 35px; border-radius: 3px; border: 1px solid $border-color; + max-width: 100%; + flex-grow: 1; } + .select2 a, + .btn-default { - border-top-left-radius: 0; - border-bottom-left-radius: 0; + border-radius: 0 $border-radius-base $border-radius-base 0; } } } @@ -290,7 +303,7 @@ font-size: 13px; font-weight: $gl-font-weight-bold; line-height: 13px; - letter-spacing: .4px; + letter-spacing: 0.4px; padding: 6px 14px; text-align: center; vertical-align: middle; @@ -443,7 +456,7 @@ a.deploy-project-label { text-decoration: none; &.disabled { - opacity: .3; + opacity: 0.3; cursor: not-allowed; } } @@ -600,26 +613,26 @@ a.deploy-project-label { } .first-column { - @media(min-width: $screen-xs-min) { + @media (min-width: $screen-xs-min) { max-width: 50%; padding-right: 30px; } - @media(max-width: $screen-xs-max) { + @media (max-width: $screen-xs-max) { max-width: 100%; width: 100%; } } .second-column { - @media(min-width: $screen-xs-min) { + @media (min-width: $screen-xs-min) { width: 50%; flex: 1; padding-left: 30px; position: relative; } - @media(max-width: $screen-xs-max) { + @media (max-width: $screen-xs-max) { max-width: 100%; width: 100%; padding-left: 0; @@ -632,7 +645,7 @@ a.deploy-project-label { } &::before { - content: "OR"; + content: 'OR'; position: absolute; left: -10px; top: 50%; @@ -656,7 +669,7 @@ a.deploy-project-label { } &::after { - content: ""; + content: ''; position: absolute; background-color: $border-color; bottom: 0; @@ -921,10 +934,7 @@ pre.light-well { border-right: solid 1px transparent; } } -} -.protected-tags-list, -.protected-branches-list { .dropdown-menu-toggle { width: 100%; max-width: 300px; diff --git a/app/assets/stylesheets/pages/search.scss b/app/assets/stylesheets/pages/search.scss index c9363188505..dbde0720993 100644 --- a/app/assets/stylesheets/pages/search.scss +++ b/app/assets/stylesheets/pages/search.scss @@ -112,7 +112,7 @@ input[type="checkbox"]:hover { } .dropdown-content { - max-height: 350px; + max-height: 302px; } } diff --git a/app/assets/stylesheets/performance_bar.scss b/app/assets/stylesheets/performance_bar.scss index 5d1a9489aad..d69b390ac27 100644 --- a/app/assets/stylesheets/performance_bar.scss +++ b/app/assets/stylesheets/performance_bar.scss @@ -102,8 +102,14 @@ } } - .performance-bar-modal .modal-footer { - display: none; + .performance-bar-modal { + .modal-footer { + display: none; + } + + .modal-dialog { + width: 860px; + } } } diff --git a/app/controllers/admin/application_controller.rb b/app/controllers/admin/application_controller.rb index c27f2ee3c09..a4648b33cfa 100644 --- a/app/controllers/admin/application_controller.rb +++ b/app/controllers/admin/application_controller.rb @@ -3,23 +3,9 @@ # Automatically sets the layout and ensures an administrator is logged in class Admin::ApplicationController < ApplicationController before_action :authenticate_admin! - before_action :display_read_only_information layout 'admin' def authenticate_admin! render_404 unless current_user.admin? end - - def display_read_only_information - return unless Gitlab::Database.read_only? - - flash.now[:notice] = read_only_message - end - - private - - # Overridden in EE - def read_only_message - _('You are on a read-only GitLab instance.') - end end diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 8440945ab43..fff249577a2 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -95,6 +95,14 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController handle_omniauth end + def auth0 + if oauth['uid'].blank? + fail_auth0_login + else + handle_omniauth + end + end + private def handle_omniauth @@ -170,6 +178,12 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController redirect_to new_user_session_path end + def fail_auth0_login + flash[:alert] = 'Wrong extern UID provided. Make sure Auth0 is configured correctly.' + + redirect_to new_user_session_path + end + def handle_disabled_provider label = Gitlab::Auth::OAuth::Provider.label_for(oauth['provider']) flash[:alert] = "Signing in using #{label} has been disabled" diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 3ddf8eb3369..701be97ee96 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -323,4 +323,11 @@ module ApplicationHelper def locale_path asset_path("locale/#{Gitlab::I18n.locale}/app.js") end + + # Overridden in EE + def read_only_message + return unless Gitlab::Database.read_only? + + _('You are on a read-only GitLab instance.') + end end diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 4c4d7cca8a5..63e4a5dc45c 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -245,7 +245,8 @@ module ApplicationSettingsHelper :usage_ping_enabled, :user_default_external, :user_oauth_applications, - :version_check_enabled + :version_check_enabled, + :allow_local_requests_from_hooks_and_services ] end end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 3cbbf8b5dfa..862933bf127 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -330,7 +330,8 @@ class ApplicationSetting < ActiveRecord::Base usage_ping_enabled: Settings.gitlab['usage_ping_enabled'], gitaly_timeout_fast: 10, gitaly_timeout_medium: 30, - gitaly_timeout_default: 55 + gitaly_timeout_default: 55, + allow_local_requests_from_hooks_and_services: false } end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index f2edcdd61fd..44f9bdf111e 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -14,7 +14,7 @@ module Ci has_many :stages has_many :statuses, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline - has_many :builds, foreign_key: :commit_id + has_many :builds, foreign_key: :commit_id, inverse_of: :pipeline has_many :trigger_requests, dependent: :destroy, foreign_key: :commit_id # rubocop:disable Cop/ActiveRecordDependent has_many :variables, class_name: 'Ci::PipelineVariable' diff --git a/app/models/project.rb b/app/models/project.rb index e5ede967668..250680e2a2c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -38,6 +38,9 @@ class Project < ActiveRecord::Base attachments: 2 }.freeze + # Valids ports to import from + VALID_IMPORT_PORTS = [22, 80, 443].freeze + cache_markdown_field :description, pipeline: :description delegate :feature_available?, :builds_enabled?, :wiki_enabled?, diff --git a/app/models/project_services/assembla_service.rb b/app/models/project_services/assembla_service.rb index ae6af732ed4..4234b8044e5 100644 --- a/app/models/project_services/assembla_service.rb +++ b/app/models/project_services/assembla_service.rb @@ -1,6 +1,4 @@ class AssemblaService < Service - include HTTParty - prop_accessor :token, :subdomain validates :token, presence: true, if: :activated? @@ -31,6 +29,6 @@ class AssemblaService < Service return unless supported_events.include?(data[:object_kind]) url = "https://atlas.assembla.com/spaces/#{subdomain}/github_tool?secret_key=#{token}" - AssemblaService.post(url, body: { payload: data }.to_json, headers: { 'Content-Type' => 'application/json' }) + Gitlab::HTTP.post(url, body: { payload: data }.to_json, headers: { 'Content-Type' => 'application/json' }) end end diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb index 42939ea0ec8..54e4b3278db 100644 --- a/app/models/project_services/bamboo_service.rb +++ b/app/models/project_services/bamboo_service.rb @@ -117,14 +117,14 @@ class BambooService < CiService url = build_url(path) if username.blank? && password.blank? - HTTParty.get(url, verify: false) + Gitlab::HTTP.get(url, verify: false) else url << '&os_authType=basic' - HTTParty.get(url, verify: false, - basic_auth: { - username: username, - password: password - }) + Gitlab::HTTP.get(url, verify: false, + basic_auth: { + username: username, + password: password + }) end end end diff --git a/app/models/project_services/buildkite_service.rb b/app/models/project_services/buildkite_service.rb index fc30f6e3365..d2aaff8817a 100644 --- a/app/models/project_services/buildkite_service.rb +++ b/app/models/project_services/buildkite_service.rb @@ -71,7 +71,7 @@ class BuildkiteService < CiService end def calculate_reactive_cache(sha, ref) - response = HTTParty.get(commit_status_path(sha), verify: false) + response = Gitlab::HTTP.get(commit_status_path(sha), verify: false) status = if response.code == 200 && response['status'] diff --git a/app/models/project_services/campfire_service.rb b/app/models/project_services/campfire_service.rb index 8d7a4fceb08..cb4af73807b 100644 --- a/app/models/project_services/campfire_service.rb +++ b/app/models/project_services/campfire_service.rb @@ -1,6 +1,4 @@ class CampfireService < Service - include HTTParty - prop_accessor :token, :subdomain, :room validates :token, presence: true, if: :activated? @@ -31,7 +29,6 @@ class CampfireService < Service def execute(data) return unless supported_events.include?(data[:object_kind]) - self.class.base_uri base_uri message = build_message(data) speak(self.room, message, auth) end @@ -69,14 +66,14 @@ class CampfireService < Service } } } - res = self.class.post(path, auth.merge(body)) + res = Gitlab::HTTP.post(path, base_uri: base_uri, **auth.merge(body)) res.code == 201 ? res : nil end # Returns a list of rooms, or []. # https://github.com/basecamp/campfire-api/blob/master/sections/rooms.md#get-rooms def rooms(auth) - res = self.class.get("/rooms.json", auth) + res = Gitlab::HTTP.get("/rooms.json", base_uri: base_uri, **auth) res.code == 200 ? res["rooms"] : [] end diff --git a/app/models/project_services/drone_ci_service.rb b/app/models/project_services/drone_ci_service.rb index c93f1632652..71b10fc6bc1 100644 --- a/app/models/project_services/drone_ci_service.rb +++ b/app/models/project_services/drone_ci_service.rb @@ -49,7 +49,7 @@ class DroneCiService < CiService end def calculate_reactive_cache(sha, ref) - response = HTTParty.get(commit_status_path(sha, ref), verify: enable_ssl_verification) + response = Gitlab::HTTP.get(commit_status_path(sha, ref), verify: enable_ssl_verification) status = if response.code == 200 && response['status'] diff --git a/app/models/project_services/external_wiki_service.rb b/app/models/project_services/external_wiki_service.rb index 720ad61162e..1553f169827 100644 --- a/app/models/project_services/external_wiki_service.rb +++ b/app/models/project_services/external_wiki_service.rb @@ -1,6 +1,4 @@ class ExternalWikiService < Service - include HTTParty - prop_accessor :external_wiki_url validates :external_wiki_url, presence: true, url: true, if: :activated? @@ -24,7 +22,7 @@ class ExternalWikiService < Service end def execute(_data) - @response = HTTParty.get(properties['external_wiki_url'], verify: true) rescue nil + @response = Gitlab::HTTP.get(properties['external_wiki_url'], verify: true) rescue nil if @response != 200 nil end diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb index 5fb15c383ca..df6dcd90985 100644 --- a/app/models/project_services/issue_tracker_service.rb +++ b/app/models/project_services/issue_tracker_service.rb @@ -77,13 +77,13 @@ class IssueTrackerService < Service result = false begin - response = HTTParty.head(self.project_url, verify: true) + response = Gitlab::HTTP.head(self.project_url, verify: true) if response message = "#{self.type} received response #{response.code} when attempting to connect to #{self.project_url}" result = true end - rescue HTTParty::Error, Timeout::Error, SocketError, Errno::ECONNRESET, Errno::ECONNREFUSED, OpenSSL::SSL::SSLError => error + rescue Gitlab::HTTP::Error, Timeout::Error, SocketError, Errno::ECONNRESET, Errno::ECONNREFUSED, OpenSSL::SSL::SSLError => error message = "#{self.type} had an error when trying to connect to #{self.project_url}: #{error.message}" end Rails.logger.info(message) diff --git a/app/models/project_services/mock_ci_service.rb b/app/models/project_services/mock_ci_service.rb index 72ddf9a4be3..2221459c90b 100644 --- a/app/models/project_services/mock_ci_service.rb +++ b/app/models/project_services/mock_ci_service.rb @@ -52,7 +52,7 @@ class MockCiService < CiService # # def commit_status(sha, ref) - response = HTTParty.get(commit_status_path(sha), verify: false) + response = Gitlab::HTTP.get(commit_status_path(sha), verify: false) read_commit_status(response) rescue Errno::ECONNREFUSED :error diff --git a/app/models/project_services/packagist_service.rb b/app/models/project_services/packagist_service.rb index f68a0c1a3c3..ba62a5b7ac0 100644 --- a/app/models/project_services/packagist_service.rb +++ b/app/models/project_services/packagist_service.rb @@ -1,6 +1,4 @@ class PackagistService < Service - include HTTParty - prop_accessor :username, :token, :server validates :username, presence: true, if: :activated? diff --git a/app/models/project_services/pivotaltracker_service.rb b/app/models/project_services/pivotaltracker_service.rb index f9dfa2e91c3..3476e7d2283 100644 --- a/app/models/project_services/pivotaltracker_service.rb +++ b/app/models/project_services/pivotaltracker_service.rb @@ -1,6 +1,4 @@ class PivotaltrackerService < Service - include HTTParty - API_ENDPOINT = 'https://www.pivotaltracker.com/services/v5/source_commits'.freeze prop_accessor :token, :restrict_to_branch @@ -52,7 +50,7 @@ class PivotaltrackerService < Service 'message' => commit[:message] } } - PivotaltrackerService.post( + Gitlab::HTTP.post( API_ENDPOINT, body: message.to_json, headers: { diff --git a/app/models/project_services/pushover_service.rb b/app/models/project_services/pushover_service.rb index e3a1ca2d45f..8777a44b72f 100644 --- a/app/models/project_services/pushover_service.rb +++ b/app/models/project_services/pushover_service.rb @@ -1,6 +1,5 @@ class PushoverService < Service - include HTTParty - base_uri 'https://api.pushover.net/1' + BASE_URI = 'https://api.pushover.net/1'.freeze prop_accessor :api_key, :user_key, :device, :priority, :sound validates :api_key, :user_key, :priority, presence: true, if: :activated? @@ -99,6 +98,6 @@ class PushoverService < Service pushover_data[:sound] = sound end - PushoverService.post('/messages.json', body: pushover_data) + Gitlab::HTTP.post('/messages.json', base_uri: BASE_URI, body: pushover_data) end end diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb index cbe137452bd..145313b8e71 100644 --- a/app/models/project_services/teamcity_service.rb +++ b/app/models/project_services/teamcity_service.rb @@ -83,7 +83,7 @@ class TeamcityService < CiService branch = Gitlab::Git.ref_name(data[:ref]) - HTTParty.post( + Gitlab::HTTP.post( build_url('httpAuth/app/rest/buildQueue'), body: "<build branchName=\"#{branch}\">"\ "<buildType id=\"#{build_type}\"/>"\ @@ -134,10 +134,10 @@ class TeamcityService < CiService end def get_path(path) - HTTParty.get(build_url(path), verify: false, - basic_auth: { - username: username, - password: password - }) + Gitlab::HTTP.get(build_url(path), verify: false, + basic_auth: { + username: username, + password: password + }) end end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 3b3d9239086..ad27f320853 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -65,7 +65,7 @@ module Ci project.pipelines .where(ref: pipeline.ref) .where.not(id: pipeline.id) - .where.not(sha: project.repository.sha_from_ref(pipeline.ref)) + .where.not(sha: project.commit(pipeline.ref).try(:id)) .created_or_pending end diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index f2d676af5c3..a34024f4f80 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -28,7 +28,7 @@ module Projects def add_repository_to_project if project.external_import? && !unknown_url? - raise Error, 'Blocked import URL.' if Gitlab::UrlBlocker.blocked_url?(project.import_url) + raise Error, 'Blocked import URL.' if Gitlab::UrlBlocker.blocked_url?(project.import_url, valid_ports: Project::VALID_IMPORT_PORTS) end # We should skip the repository for a GitHub import or GitLab project import, diff --git a/app/services/submit_usage_ping_service.rb b/app/services/submit_usage_ping_service.rb index 2623f253d98..ac029fad7ea 100644 --- a/app/services/submit_usage_ping_service.rb +++ b/app/services/submit_usage_ping_service.rb @@ -14,16 +14,17 @@ class SubmitUsagePingService def execute return false unless Gitlab::CurrentSettings.usage_ping_enabled? - response = HTTParty.post( + response = Gitlab::HTTP.post( URL, body: Gitlab::UsageData.to_json(force_refresh: true), + allow_local_requests: true, headers: { 'Content-type' => 'application/json' } ) store_metrics(response) true - rescue HTTParty::Error => e + rescue Gitlab::HTTP::Error => e Rails.logger.info "Unable to contact GitLab, Inc.: #{e}" false diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 36e589d5aa8..809ce1303d8 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -3,23 +3,20 @@ class WebHookService attr_reader :body, :headers, :code def initialize - @headers = HTTParty::Response::Headers.new({}) + @headers = Gitlab::HTTP::Response::Headers.new({}) @body = '' @code = 'internal error' end end - include HTTParty - - # HTTParty timeout - default_timeout Gitlab.config.gitlab.webhook_timeout - - attr_accessor :hook, :data, :hook_name + attr_accessor :hook, :data, :hook_name, :request_options def initialize(hook, data, hook_name) @hook = hook @data = data @hook_name = hook_name.to_s + @request_options = { timeout: Gitlab.config.gitlab.webhook_timeout } + @request_options.merge!(allow_local_requests: true) if @hook.is_a?(SystemHook) end def execute @@ -73,11 +70,12 @@ class WebHookService end def make_request(url, basic_auth = false) - self.class.post(url, + Gitlab::HTTP.post(url, body: data.to_json, headers: build_headers(hook_name), verify: hook.enable_ssl_verification, - basic_auth: basic_auth) + basic_auth: basic_auth, + **request_options) end def make_request_with_auth diff --git a/app/validators/importable_url_validator.rb b/app/validators/importable_url_validator.rb index 37a314adee6..3ec1594e202 100644 --- a/app/validators/importable_url_validator.rb +++ b/app/validators/importable_url_validator.rb @@ -4,7 +4,7 @@ # protect against Server-side Request Forgery (SSRF). class ImportableUrlValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) - if Gitlab::UrlBlocker.blocked_url?(value) + if Gitlab::UrlBlocker.blocked_url?(value, valid_ports: Project::VALID_IMPORT_PORTS) record.errors.add(attribute, "imports are not allowed from that URL") end end diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 81d7db04a3c..54b39df8cf3 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -860,5 +860,14 @@ .col-sm-10 = f.number_field :throttle_authenticated_web_period_in_seconds, class: 'form-control' + %fieldset + %legend Outbound requests + .form-group + .col-sm-offset-2.col-sm-10 + .checkbox + = f.label :allow_local_requests_from_hooks_and_services do + = f.check_box :allow_local_requests_from_hooks_and_services + Allow requests to the local network from hooks and services + .form-actions = f.submit 'Save', class: 'btn btn-save' diff --git a/app/views/import/gitlab_projects/new.html.haml b/app/views/import/gitlab_projects/new.html.haml index df5841d1911..dec85368d10 100644 --- a/app/views/import/gitlab_projects/new.html.haml +++ b/app/views/import/gitlab_projects/new.html.haml @@ -13,13 +13,13 @@ .form-group .input-group - if current_user.can_select_namespace? - .input-group-addon + .input-group-addon.has-tooltip{ title: root_url } = root_url = select_tag :namespace_id, namespaces_options(namespace_id_from(params) || :current_user, display_path: true, extra_group: namespace_id_from(params)), class: 'select2 js-select-namespace', tabindex: 1 - else - .input-group-addon.static-namespace - #{root_url}#{current_user.username}/ + .input-group-addon.static-namespace.has-tooltip{ title: user_url(current_user.username) + '/' } + #{user_url(current_user.username)}/ = hidden_field_tag :namespace_id, value: current_user.namespace_id .form-group.col-xs-12.col-sm-6.project-path = label_tag :path, 'Project name', class: 'label-light' diff --git a/app/views/layouts/_mailer.html.haml b/app/views/layouts/_mailer.html.haml index b50537438a9..ddc1cdb24b5 100644 --- a/app/views/layouts/_mailer.html.haml +++ b/app/views/layouts/_mailer.html.haml @@ -67,12 +67,8 @@ %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:25px 0;font-size:13px;line-height:1.6;color:#5c5c5c;" } %img{ alt: "GitLab", height: "33", src: image_url('mailers/gitlab_footer_logo.gif'), style: "display:block;margin:0 auto 1em;", width: "90" }/ %div - %a{ href: profile_notifications_url, style: "color:#3777b0;text-decoration:none;" } Manage all notifications - · - %a{ href: help_url, style: "color:#3777b0;text-decoration:none;" } Help - %div - You're receiving this email because of your account on - = succeed "." do - %a{ href: root_url, style: "color:#3777b0;text-decoration:none;" }= Gitlab.config.gitlab.host + - manage_notifications_link = link_to(_("Manage all notifications"), profile_notifications_url, style: "color:#3777b0;text-decoration:none;") + - help_link = link_to(_("Help"), help_url, style: "color:#3777b0;text-decoration:none;") + = _("You're receiving this email because of your account on %{host}. %{manage_notifications_link} · %{help_link}").html_safe % { host: Gitlab.config.gitlab.host, manage_notifications_link: manage_notifications_link, help_link: help_link } = yield :additional_footer diff --git a/app/views/layouts/_page.html.haml b/app/views/layouts/_page.html.haml index f0963cf9da8..f67a8878c80 100644 --- a/app/views/layouts/_page.html.haml +++ b/app/views/layouts/_page.html.haml @@ -6,6 +6,7 @@ .mobile-overlay .alert-wrapper = render "layouts/broadcast" + = render 'layouts/header/read_only_banner' = yield :flash_message - unless @hide_breadcrumbs = render "layouts/nav/breadcrumbs" diff --git a/app/views/layouts/header/_read_only_banner.html.haml b/app/views/layouts/header/_read_only_banner.html.haml new file mode 100644 index 00000000000..f3d563c362f --- /dev/null +++ b/app/views/layouts/header/_read_only_banner.html.haml @@ -0,0 +1,7 @@ +- message = read_only_message +- if message + .flash-container.flash-container-page + .flash-notice + %div{ class: (container_class) } + %span + = message diff --git a/app/views/projects/_last_push.html.haml b/app/views/projects/_last_push.html.haml index 6f5eb828902..6a1035d2dc7 100644 --- a/app/views/projects/_last_push.html.haml +++ b/app/views/projects/_last_push.html.haml @@ -13,6 +13,6 @@ #{time_ago_with_tooltip(event.created_at)} - .pull-right + .flex-right = link_to new_mr_path_from_push_event(event), title: _("New merge request"), class: "btn btn-info btn-sm qa-create-merge-request" do #{ _('Create merge request') } diff --git a/app/views/projects/_new_project_fields.html.haml b/app/views/projects/_new_project_fields.html.haml index f4b5ef1555e..241bc3dbca0 100644 --- a/app/views/projects/_new_project_fields.html.haml +++ b/app/views/projects/_new_project_fields.html.haml @@ -9,12 +9,12 @@ Project path .input-group - if current_user.can_select_namespace? - .input-group-addon + .input-group-addon.has-tooltip{ title: root_url } = root_url = f.select :namespace_id, namespaces_options(namespace_id_from(params) || :current_user, display_path: true, extra_group: namespace_id_from(params)), {}, { class: 'select2 js-select-namespace qa-project-namespace-select', tabindex: 1} - else - .input-group-addon.static-namespace + .input-group-addon.static-namespace.has-tooltip{ title: user_url(current_user.username) + '/' } #{user_url(current_user.username)}/ = f.hidden_field :namespace_id, value: current_user.namespace_id .form-group.project-path.col-sm-6 diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index 1da0e865a41..883dfb3e6c8 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -5,81 +5,82 @@ - number_commits_behind = diverging_commit_counts[:behind] - number_commits_ahead = diverging_commit_counts[:ahead] - merge_project = can?(current_user, :create_merge_request, @project) ? @project : (current_user && current_user.fork_of(@project)) -%li{ class: "js-branch-#{branch.name}" } - %div - = link_to project_tree_path(@project, branch.name), class: 'item-title str-truncated ref-name' do - = sprite_icon('fork', size: 12) - = branch.name - - - if branch.name == @repository.root_ref - %span.label.label-primary default - - elsif merged - %span.label.label-info.has-tooltip{ title: s_('Branches|Merged into %{default_branch}') % { default_branch: @repository.root_ref } } - = s_('Branches|merged') +%li{ class: "branch-item js-branch-#{branch.name}" } + .branch-info + .branch-title + = link_to project_tree_path(@project, branch.name), class: 'item-title str-truncated-100 ref-name' do + = sprite_icon('fork', size: 12) + = branch.name + + - if branch.name == @repository.root_ref + %span.label.label-primary default + - elsif merged + %span.label.label-info.has-tooltip{ title: s_('Branches|Merged into %{default_branch}') % { default_branch: @repository.root_ref } } + = s_('Branches|merged') - - if protected_branch?(@project, branch) - %span.label.label-success - = s_('Branches|protected') - .controls.hidden-xs< - - if merge_project && create_mr_button?(@repository.root_ref, branch.name) - = link_to create_mr_path(@repository.root_ref, branch.name), class: 'btn btn-default' do - = _('Merge request') + - if protected_branch?(@project, branch) + %span.label.label-success + = s_('Branches|protected') - - if branch.name != @repository.root_ref - = link_to project_compare_index_path(@project, from: @repository.root_ref, to: branch.name), - class: "btn btn-default #{'prepend-left-10' unless merge_project}", - method: :post, - title: s_('Branches|Compare') do - = s_('Branches|Compare') + .block-truncated + - if commit + = render 'projects/branches/commit', commit: commit, project: @project + - else + = s_('Branches|Cant find HEAD commit for this branch') - = render 'projects/buttons/download', project: @project, ref: branch.name, pipeline: @refs_pipelines[branch.name] + - if branch.name != @repository.root_ref + .divergence-graph{ title: s_('%{number_commits_behind} commits behind %{default_branch}, %{number_commits_ahead} commits ahead') % { number_commits_behind: diverging_count_label(number_commits_behind), + default_branch: @repository.root_ref, + number_commits_ahead: diverging_count_label(number_commits_ahead) } } + .graph-side + .bar.bar-behind{ style: "width: #{number_commits_behind * bar_graph_width_factor}%" } + %span.count.count-behind= diverging_count_label(number_commits_behind) + .graph-separator + .graph-side + .bar.bar-ahead{ style: "width: #{number_commits_ahead * bar_graph_width_factor}%" } + %span.count.count-ahead= diverging_count_label(number_commits_ahead) - - if can?(current_user, :push_code, @project) - - if branch.name == @project.repository.root_ref - %button{ class: "btn btn-remove remove-row js-ajax-loading-spinner has-tooltip disabled", - disabled: true, - title: s_('Branches|The default branch cannot be deleted') } - = icon("trash-o") - - elsif protected_branch?(@project, branch) - - if can?(current_user, :delete_protected_branch, @project) - %button{ class: "btn btn-remove remove-row js-ajax-loading-spinner has-tooltip", - title: s_('Branches|Delete protected branch'), - data: { toggle: "modal", - target: "#modal-delete-branch", - delete_path: project_branch_path(@project, branch.name), - branch_name: branch.name, - is_merged: ("true" if merged) } } - = icon("trash-o") - - else - %button{ class: "btn btn-remove remove-row js-ajax-loading-spinner has-tooltip disabled", - disabled: true, - title: s_('Branches|Only a project master or owner can delete a protected branch') } - = icon("trash-o") - - else - = link_to project_branch_path(@project, branch.name), - class: "btn btn-remove remove-row js-ajax-loading-spinner has-tooltip", - title: s_('Branches|Delete branch'), - method: :delete, - data: { confirm: s_("Branches|Deleting the '%{branch_name}' branch cannot be undone. Are you sure?") % { branch_name: branch.name } }, - remote: true, - 'aria-label' => s_('Branches|Delete branch') do - = icon("trash-o") + .controls.hidden-xs< + - if merge_project && create_mr_button?(@repository.root_ref, branch.name) + = link_to create_mr_path(@repository.root_ref, branch.name), class: 'btn btn-default' do + = _('Merge request') - if branch.name != @repository.root_ref - .divergence-graph{ title: s_('%{number_commits_behind} commits behind %{default_branch}, %{number_commits_ahead} commits ahead') % { number_commits_behind: diverging_count_label(number_commits_behind), - default_branch: @repository.root_ref, - number_commits_ahead: diverging_count_label(number_commits_ahead) } } - .graph-side - .bar.bar-behind{ style: "width: #{number_commits_behind * bar_graph_width_factor}%" } - %span.count.count-behind= diverging_count_label(number_commits_behind) - .graph-separator - .graph-side - .bar.bar-ahead{ style: "width: #{number_commits_ahead * bar_graph_width_factor}%" } - %span.count.count-ahead= diverging_count_label(number_commits_ahead) + = link_to project_compare_index_path(@project, from: @repository.root_ref, to: branch.name), + class: "btn btn-default #{'prepend-left-10' unless merge_project}", + method: :post, + title: s_('Branches|Compare') do + = s_('Branches|Compare') + = render 'projects/buttons/download', project: @project, ref: branch.name, pipeline: @refs_pipelines[branch.name] - - if commit - = render 'projects/branches/commit', commit: commit, project: @project - - else - %p - = s_('Branches|Cant find HEAD commit for this branch') + - if can?(current_user, :push_code, @project) + - if branch.name == @project.repository.root_ref + %button{ class: "btn btn-remove remove-row js-ajax-loading-spinner has-tooltip disabled", + disabled: true, + title: s_('Branches|The default branch cannot be deleted') } + = icon("trash-o") + - elsif protected_branch?(@project, branch) + - if can?(current_user, :delete_protected_branch, @project) + %button{ class: "btn btn-remove remove-row js-ajax-loading-spinner has-tooltip", + title: s_('Branches|Delete protected branch'), + data: { toggle: "modal", + target: "#modal-delete-branch", + delete_path: project_branch_path(@project, branch.name), + branch_name: branch.name, + is_merged: ("true" if merged) } } + = icon("trash-o") + - else + %button{ class: "btn btn-remove remove-row js-ajax-loading-spinner has-tooltip disabled", + disabled: true, + title: s_('Branches|Only a project master or owner can delete a protected branch') } + = icon("trash-o") + - else + = link_to project_branch_path(@project, branch.name), + class: "btn btn-remove remove-row js-ajax-loading-spinner has-tooltip", + title: s_('Branches|Delete branch'), + method: :delete, + data: { confirm: s_("Branches|Deleting the '%{branch_name}' branch cannot be undone. Are you sure?") % { branch_name: branch.name } }, + remote: true, + 'aria-label' => s_('Branches|Delete branch') do + = icon("trash-o") diff --git a/changelogs/unreleased/42037-long-instance-names-group-names-covers-namespace-dropdown.yml b/changelogs/unreleased/42037-long-instance-names-group-names-covers-namespace-dropdown.yml new file mode 100644 index 00000000000..f7758734a6f --- /dev/null +++ b/changelogs/unreleased/42037-long-instance-names-group-names-covers-namespace-dropdown.yml @@ -0,0 +1,5 @@ +--- +title: Long instance urls do not overflow anymore during project creation +merge_request: 17717 +author: +type: fixed diff --git a/changelogs/unreleased/44382-ui-breakdown-for-create-merge-request.yml b/changelogs/unreleased/44382-ui-breakdown-for-create-merge-request.yml new file mode 100644 index 00000000000..dd8c0b19d5f --- /dev/null +++ b/changelogs/unreleased/44382-ui-breakdown-for-create-merge-request.yml @@ -0,0 +1,5 @@ +--- +title: Fix UI breakdown for Create merge request button +merge_request: 17821 +author: Takuya Noguchi +type: fixed diff --git a/changelogs/unreleased/44386-better-ux-for-long-name-branches.yml b/changelogs/unreleased/44386-better-ux-for-long-name-branches.yml new file mode 100644 index 00000000000..16712486f0f --- /dev/null +++ b/changelogs/unreleased/44386-better-ux-for-long-name-branches.yml @@ -0,0 +1,5 @@ +--- +title: UX re-design branch items with flexbox +merge_request: 17832 +author: Takuya Noguchi +type: fixed diff --git a/changelogs/unreleased/ci-pipeline-commit-lookup.yml b/changelogs/unreleased/ci-pipeline-commit-lookup.yml new file mode 100644 index 00000000000..b2a1e4c2163 --- /dev/null +++ b/changelogs/unreleased/ci-pipeline-commit-lookup.yml @@ -0,0 +1,5 @@ +--- +title: Use porcelain commit lookup method on CI::CreatePipelineService +merge_request: 17911 +author: +type: fixed diff --git a/changelogs/unreleased/fix-auth0-unsafe-login.yml b/changelogs/unreleased/fix-auth0-unsafe-login.yml new file mode 100644 index 00000000000..01c6ea69dcc --- /dev/null +++ b/changelogs/unreleased/fix-auth0-unsafe-login.yml @@ -0,0 +1,5 @@ +--- +title: Fix GitLab Auth0 integration signing in the wrong user +merge_request: +author: +type: security diff --git a/changelogs/unreleased/fj-15329-services-callbacks-ssrf.yml b/changelogs/unreleased/fj-15329-services-callbacks-ssrf.yml new file mode 100644 index 00000000000..7fa6f6a5874 --- /dev/null +++ b/changelogs/unreleased/fj-15329-services-callbacks-ssrf.yml @@ -0,0 +1,5 @@ +--- +title: Fixed some SSRF vulnerabilities in services, hooks and integrations +merge_request: 2337 +author: +type: security diff --git a/changelogs/unreleased/increase-unicorn-memory-killer-limits.yml b/changelogs/unreleased/increase-unicorn-memory-killer-limits.yml new file mode 100644 index 00000000000..6d7d2df4f4a --- /dev/null +++ b/changelogs/unreleased/increase-unicorn-memory-killer-limits.yml @@ -0,0 +1,5 @@ +--- +title: Increase the memory limits used in the unicorn killer +merge_request: 17948 +author: +type: other diff --git a/changelogs/unreleased/move-email-footer-info-to-single-line.yml b/changelogs/unreleased/move-email-footer-info-to-single-line.yml new file mode 100644 index 00000000000..87ed5638056 --- /dev/null +++ b/changelogs/unreleased/move-email-footer-info-to-single-line.yml @@ -0,0 +1,5 @@ +--- +title: Move email footer info to a single line +merge_request: 17916 +author: +type: changed diff --git a/changelogs/unreleased/remove-unnecessary-validate-project.yml b/changelogs/unreleased/remove-unnecessary-validate-project.yml deleted file mode 100644 index ebc8da03dd8..00000000000 --- a/changelogs/unreleased/remove-unnecessary-validate-project.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: 'Remove unecessary validate: true from belongs_to :project' -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/tc-re-add-read-only-banner.yml b/changelogs/unreleased/tc-re-add-read-only-banner.yml new file mode 100644 index 00000000000..35bcd7e184e --- /dev/null +++ b/changelogs/unreleased/tc-re-add-read-only-banner.yml @@ -0,0 +1,5 @@ +--- +title: Add read-only banner to all pages +merge_request: 17798 +author: +type: fixed diff --git a/config.ru b/config.ru index 7b15939c6ff..405d01863ac 100644 --- a/config.ru +++ b/config.ru @@ -7,8 +7,8 @@ if defined?(Unicorn) # Unicorn self-process killer require 'unicorn/worker_killer' - min = (ENV['GITLAB_UNICORN_MEMORY_MIN'] || 300 * 1 << 20).to_i - max = (ENV['GITLAB_UNICORN_MEMORY_MAX'] || 350 * 1 << 20).to_i + min = (ENV['GITLAB_UNICORN_MEMORY_MIN'] || 400 * 1 << 20).to_i + max = (ENV['GITLAB_UNICORN_MEMORY_MAX'] || 650 * 1 << 20).to_i # Max memory size (RSS) per worker use Unicorn::WorkerKiller::Oom, min, max diff --git a/config/application.rb b/config/application.rb index 0ff95e33a9c..13501d4bdb5 100644 --- a/config/application.rb +++ b/config/application.rb @@ -170,7 +170,7 @@ module Gitlab ENV['GIT_TERMINAL_PROMPT'] = '0' # Gitlab Read-only middleware support - config.middleware.insert_after ActionDispatch::Flash, 'Gitlab::Middleware::ReadOnly' + config.middleware.insert_after ActionDispatch::Flash, '::Gitlab::Middleware::ReadOnly' config.generators do |g| g.factory_bot false diff --git a/config/webpack.config.js b/config/webpack.config.js index f5fb7de6176..42fe4b345e1 100644 --- a/config/webpack.config.js +++ b/config/webpack.config.js @@ -9,14 +9,12 @@ const StatsWriterPlugin = require('webpack-stats-plugin').StatsWriterPlugin; const CopyWebpackPlugin = require('copy-webpack-plugin'); const CompressionPlugin = require('compression-webpack-plugin'); const NameAllModulesPlugin = require('name-all-modules-plugin'); -const BundleAnalyzerPlugin = require('webpack-bundle-analyzer') - .BundleAnalyzerPlugin; +const BundleAnalyzerPlugin = require('webpack-bundle-analyzer').BundleAnalyzerPlugin; const WatchMissingNodeModulesPlugin = require('react-dev-utils/WatchMissingNodeModulesPlugin'); const ROOT_PATH = path.resolve(__dirname, '..'); const IS_PRODUCTION = process.env.NODE_ENV === 'production'; -const IS_DEV_SERVER = - process.argv.join(' ').indexOf('webpack-dev-server') !== -1; +const IS_DEV_SERVER = process.argv.join(' ').indexOf('webpack-dev-server') !== -1; const DEV_SERVER_HOST = process.env.DEV_SERVER_HOST || 'localhost'; const DEV_SERVER_PORT = parseInt(process.env.DEV_SERVER_PORT, 10) || 3808; const DEV_SERVER_LIVERELOAD = process.env.DEV_SERVER_LIVERELOAD !== 'false'; @@ -29,10 +27,10 @@ let watchAutoEntries = []; function generateEntries() { // generate automatic entry points const autoEntries = {}; - const pageEntries = glob.sync('pages/**/index.js', { - cwd: path.join(ROOT_PATH, 'app/assets/javascripts'), - }); - watchAutoEntries = [path.join(ROOT_PATH, 'app/assets/javascripts/pages/')]; + const pageEntries = glob.sync('pages/**/index.js', { cwd: path.join(ROOT_PATH, 'app/assets/javascripts') }); + watchAutoEntries = [ + path.join(ROOT_PATH, 'app/assets/javascripts/pages/'), + ]; function generateAutoEntries(path, prefix = '.') { const chunkPath = path.replace(/\/index\.js$/, ''); @@ -40,16 +38,16 @@ function generateEntries() { autoEntries[chunkName] = `${prefix}/${path}`; } - pageEntries.forEach(path => generateAutoEntries(path)); + pageEntries.forEach(( path ) => generateAutoEntries(path)); autoEntriesCount = Object.keys(autoEntries).length; const manualEntries = { - common: './commons/index.js', - main: './main.js', - raven: './raven/index.js', - webpack_runtime: './webpack.js', - ide: './ide/index.js', + common: './commons/index.js', + main: './main.js', + raven: './raven/index.js', + webpack_runtime: './webpack.js', + ide: './ide/index.js', }; return Object.assign(manualEntries, autoEntries); @@ -63,12 +61,8 @@ const config = { output: { path: path.join(ROOT_PATH, 'public/assets/webpack'), publicPath: '/assets/webpack/', - filename: IS_PRODUCTION - ? '[name].[chunkhash].bundle.js' - : '[name].bundle.js', - chunkFilename: IS_PRODUCTION - ? '[name].[chunkhash].chunk.js' - : '[name].chunk.js', + filename: IS_PRODUCTION ? '[name].[chunkhash].bundle.js' : '[name].bundle.js', + chunkFilename: IS_PRODUCTION ? '[name].[chunkhash].chunk.js' : '[name].chunk.js', }, module: { @@ -97,8 +91,8 @@ const config = { { loader: 'worker-loader', options: { - inline: true, - }, + inline: true + } }, { loader: 'babel-loader' }, ], @@ -109,7 +103,7 @@ const config = { loader: 'file-loader', options: { name: '[name].[hash].[ext]', - }, + } }, { test: /katex.css$/, @@ -119,8 +113,8 @@ const config = { { loader: 'css-loader', options: { - name: '[name].[hash].[ext]', - }, + name: '[name].[hash].[ext]' + } }, ], }, @@ -130,18 +124,15 @@ const config = { loader: 'file-loader', options: { name: '[name].[hash].[ext]', - }, + } }, { test: /monaco-editor\/\w+\/vs\/loader\.js$/, use: [ { loader: 'exports-loader', options: 'l.global' }, - { - loader: 'imports-loader', - options: 'l=>{},this=>l,AMDLoader=>this,module=>undefined', - }, + { loader: 'imports-loader', options: 'l=>{},this=>l,AMDLoader=>this,module=>undefined' }, ], - }, + } ], noParse: [/monaco-editor\/\w+\/vs\//], @@ -159,10 +150,10 @@ const config = { source: false, chunks: false, modules: false, - assets: true, + assets: true }); return JSON.stringify(stats, null, 2); - }, + } }), // prevent pikaday from including moment.js @@ -179,7 +170,7 @@ const config = { new NameAllModulesPlugin(), // assign deterministic chunk ids - new webpack.NamedChunksPlugin(chunk => { + new webpack.NamedChunksPlugin((chunk) => { if (chunk.name) { return chunk.name; } @@ -196,12 +187,9 @@ const config = { const pagesBase = path.join(ROOT_PATH, 'app/assets/javascripts/pages'); if (m.resource.indexOf(pagesBase) === 0) { - moduleNames.push( - path - .relative(pagesBase, m.resource) - .replace(/\/index\.[a-z]+$/, '') - .replace(/\//g, '__'), - ); + moduleNames.push(path.relative(pagesBase, m.resource) + .replace(/\/index\.[a-z]+$/, '') + .replace(/\//g, '__')); } else { moduleNames.push(path.relative(m.context, m.resource)); } @@ -209,8 +197,7 @@ const config = { chunk.forEachModule(collectModuleNames); - const hash = crypto - .createHash('sha256') + const hash = crypto.createHash('sha256') .update(moduleNames.join('_')) .digest('hex'); @@ -222,23 +209,13 @@ const config = { names: ['main', 'common', 'webpack_runtime'], }), - // enable scope hoisting - new webpack.optimize.ModuleConcatenationPlugin(), - // copy pre-compiled vendor libraries verbatim new CopyWebpackPlugin([ { - from: path.join( - ROOT_PATH, - `node_modules/monaco-editor/${IS_PRODUCTION ? 'min' : 'dev'}/vs`, - ), + from: path.join(ROOT_PATH, `node_modules/monaco-editor/${IS_PRODUCTION ? 'min' : 'dev'}/vs`), to: 'monaco-editor/vs', transform: function(content, path) { - if ( - /\.js$/.test(path) && - !/worker/i.test(path) && - !/typescript/i.test(path) - ) { + if (/\.js$/.test(path) && !/worker/i.test(path) && !/typescript/i.test(path)) { return ( '(function(){\n' + 'var define = this.define, require = this.require;\n' + @@ -248,23 +225,23 @@ const config = { ); } return content; - }, - }, + } + } ]), ], resolve: { extensions: ['.js'], alias: { - '~': path.join(ROOT_PATH, 'app/assets/javascripts'), - emojis: path.join(ROOT_PATH, 'fixtures/emojis'), - empty_states: path.join(ROOT_PATH, 'app/views/shared/empty_states'), - icons: path.join(ROOT_PATH, 'app/views/shared/icons'), - images: path.join(ROOT_PATH, 'app/assets/images'), - vendor: path.join(ROOT_PATH, 'vendor/assets/javascripts'), - vue$: 'vue/dist/vue.esm.js', - spec: path.join(ROOT_PATH, 'spec/javascripts'), - }, + '~': path.join(ROOT_PATH, 'app/assets/javascripts'), + 'emojis': path.join(ROOT_PATH, 'fixtures/emojis'), + 'empty_states': path.join(ROOT_PATH, 'app/views/shared/empty_states'), + 'icons': path.join(ROOT_PATH, 'app/views/shared/icons'), + 'images': path.join(ROOT_PATH, 'app/assets/images'), + 'vendor': path.join(ROOT_PATH, 'vendor/assets/javascripts'), + 'vue$': 'vue/dist/vue.esm.js', + 'spec': path.join(ROOT_PATH, 'spec/javascripts'), + } }, // sqljs requires fs @@ -279,14 +256,15 @@ if (IS_PRODUCTION) { new webpack.NoEmitOnErrorsPlugin(), new webpack.LoaderOptionsPlugin({ minimize: true, - debug: false, + debug: false }), + new webpack.optimize.ModuleConcatenationPlugin(), new webpack.optimize.UglifyJsPlugin({ - sourceMap: true, + sourceMap: true }), new webpack.DefinePlugin({ - 'process.env': { NODE_ENV: JSON.stringify('production') }, - }), + 'process.env': { NODE_ENV: JSON.stringify('production') } + }) ); // compression can require a lot of compute time and is disabled in CI @@ -304,7 +282,7 @@ if (IS_DEV_SERVER) { headers: { 'Access-Control-Allow-Origin': '*' }, stats: 'errors-only', hot: DEV_SERVER_LIVERELOAD, - inline: DEV_SERVER_LIVERELOAD, + inline: DEV_SERVER_LIVERELOAD }; config.plugins.push( // watch node_modules for changes if we encounter a missing module compile error @@ -320,14 +298,12 @@ if (IS_DEV_SERVER) { ]; // report our auto-generated bundle count - console.log( - `${autoEntriesCount} entries from '/pages' automatically added to webpack output.`, - ); + console.log(`${autoEntriesCount} entries from '/pages' automatically added to webpack output.`); callback(); - }); + }) }, - }, + } ); if (DEV_SERVER_LIVERELOAD) { config.plugins.push(new webpack.HotModuleReplacementPlugin()); @@ -342,7 +318,7 @@ if (WEBPACK_REPORT) { openAnalyzer: false, reportFilename: path.join(ROOT_PATH, 'webpack-report/index.html'), statsFilename: path.join(ROOT_PATH, 'webpack-report/stats.json'), - }), + }) ); } diff --git a/db/migrate/20180223144945_add_allow_local_requests_from_hooks_and_services_to_application_settings.rb b/db/migrate/20180223144945_add_allow_local_requests_from_hooks_and_services_to_application_settings.rb new file mode 100644 index 00000000000..c994a54698b --- /dev/null +++ b/db/migrate/20180223144945_add_allow_local_requests_from_hooks_and_services_to_application_settings.rb @@ -0,0 +1,18 @@ +class AddAllowLocalRequestsFromHooksAndServicesToApplicationSettings < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default(:application_settings, :allow_local_requests_from_hooks_and_services, + :boolean, + default: false, + allow_null: false) + end + + def down + remove_column(:application_settings, :allow_local_requests_from_hooks_and_services) + end +end diff --git a/db/post_migrate/20180220150310_remove_empty_extern_uid_auth0_identities.rb b/db/post_migrate/20180220150310_remove_empty_extern_uid_auth0_identities.rb new file mode 100644 index 00000000000..2d5a8617169 --- /dev/null +++ b/db/post_migrate/20180220150310_remove_empty_extern_uid_auth0_identities.rb @@ -0,0 +1,25 @@ +class RemoveEmptyExternUidAuth0Identities < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + class Identity < ActiveRecord::Base + self.table_name = 'identities' + include EachBatch + end + + def up + broken_auth0_identities.each_batch do |identity| + identity.delete_all + end + end + + def broken_auth0_identities + Identity.where(provider: 'auth0', extern_uid: [nil, '']) + end + + def down + end +end diff --git a/db/schema.rb b/db/schema.rb index e441ca2a1f0..56116a2d241 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -157,6 +157,7 @@ ActiveRecord::Schema.define(version: 20180320182229) do t.boolean "authorized_keys_enabled", default: true, null: false t.string "auto_devops_domain" t.boolean "pages_domain_verification_enabled", default: true, null: false + t.boolean "allow_local_requests_from_hooks_and_services", default: false, null: false end create_table "audit_events", force: :cascade do |t| diff --git a/doc/ci/caching/img/clear_runners_cache.png b/doc/ci/caching/img/clear_runners_cache.png Binary files differnew file mode 100644 index 00000000000..e5db4a47b3e --- /dev/null +++ b/doc/ci/caching/img/clear_runners_cache.png diff --git a/doc/ci/caching/index.md b/doc/ci/caching/index.md index c4b2a25d4a8..c159198d16b 100644 --- a/doc/ci/caching/index.md +++ b/doc/ci/caching/index.md @@ -454,17 +454,19 @@ next run of the pipeline, the cache will be stored in a different location. > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/41249) in GitLab 10.4. If you want to avoid editing `.gitlab-ci.yml`, you can easily clear the cache -via GitLab's UI. This will have an impact on all caches of your project as -name of the cache directory will be renamed by appending an integer to it -(`-1`, `-2`, etc.): +via GitLab's UI: -1. Navigate to your project's **CI/CD > Pipelines** page. -1. Click on the **Clear Runner caches** to clean up the cache. -1. On the next push, your CI/CD job will use a new cache. +1. Navigate to your project's **CI/CD > Pipelines** page +1. Click on the **Clear Runner caches** button to clean up the cache + +  + +1. On the next push, your CI/CD job will use a new cache Behind the scenes, this works by increasing a counter in the database, and the -value of that counter is used to create the key for the cache. After a push, a -new key is generated and the old cache is not valid anymore. +value of that counter is used to create the key for the cache by appending an +integer to it: `-1`, `-2`, etc. After a push, a new key is generated and the +old cache is not valid anymore. ## Cache vs artifacts diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 3b9f9be2f6e..7184f3367be 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -1132,7 +1132,7 @@ job1: ## `retry` -> [Introduced][ce-3442] in GitLab 9.5. +> [Introduced][ce-12909] in GitLab 9.5. `retry` allows you to configure how many times a job is going to be retried in case of a failure. @@ -1547,5 +1547,5 @@ CI with various languages. [variables]: ../variables/README.md [ce-7983]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7983 [ce-7447]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7447 -[ce-3442]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3442 +[ce-12909]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12909 [schedules]: ../../user/project/pipelines/schedules.md diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index 1e060ffd941..a211effdfa7 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -23,10 +23,6 @@ When downtime is necessary the migration has to be approved by: An up-to-date list of people holding these titles can be found at <https://about.gitlab.com/team/>. -The document ["What Requires Downtime?"](what_requires_downtime.md) specifies -various database operations, whether they require downtime and how to -work around that whenever possible. - When writing your migrations, also consider that databases might have stale data or inconsistencies and guard for that. Try to make as few assumptions as possible about the state of the database. @@ -41,6 +37,18 @@ Migrations that make changes to the database schema (e.g. adding a column) can only be added in the monthly release, patch releases may only contain data migrations _unless_ schema changes are absolutely required to solve a problem. +## What Requires Downtime? + +The document ["What Requires Downtime?"](what_requires_downtime.md) specifies +various database operations, such as + +- [adding, dropping, and renaming columns](what_requires_downtime.md#adding-columns) +- [changing column constraints and types](what_requires_downtime.md#changing-column-constraints) +- [adding and dropping indexes, tables, and foreign keys](what_requires_downtime.md#adding-indexes) + +and whether they require downtime and how to work around that whenever possible. + + ## Downtime Tagging Every migration must specify if it requires downtime or not, and if it should diff --git a/doc/integration/auth0.md b/doc/integration/auth0.md index c39d7ab57c6..a75836a915a 100644 --- a/doc/integration/auth0.md +++ b/doc/integration/auth0.md @@ -56,7 +56,8 @@ for initial settings. "name" => "auth0", "args" => { client_id: 'YOUR_AUTH0_CLIENT_ID', client_secret: 'YOUR_AUTH0_CLIENT_SECRET', - namespace: 'YOUR_AUTH0_DOMAIN' + domain: 'YOUR_AUTH0_DOMAIN', + scope: 'openid profile email' } } ] @@ -69,8 +70,8 @@ for initial settings. args: { client_id: 'YOUR_AUTH0_CLIENT_ID', client_secret: 'YOUR_AUTH0_CLIENT_SECRET', - namespace: 'YOUR_AUTH0_DOMAIN' - } + domain: 'YOUR_AUTH0_DOMAIN', + scope: 'openid profile email' } } ``` diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 208710b0935..c15c128507d 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -516,10 +516,6 @@ module Gitlab end end - def sha_from_ref(ref) - rev_parse_target(ref).oid - end - # Return the object that +revspec+ points to. If +revspec+ is an # annotated tag, then return the tag's target instead. def rev_parse_target(revspec) @@ -2409,6 +2405,10 @@ module Gitlab def rev_list_param(spec) spec == :all ? ['--all'] : spec end + + def sha_from_ref(ref) + rev_parse_target(ref).oid + end end end end diff --git a/lib/gitlab/git/wiki.rb b/lib/gitlab/git/wiki.rb index 52b44b9b3c5..8d82820915d 100644 --- a/lib/gitlab/git/wiki.rb +++ b/lib/gitlab/git/wiki.rb @@ -29,7 +29,6 @@ module Gitlab @repository.gitaly_migrate(:wiki_write_page) do |is_enabled| if is_enabled gitaly_write_page(name, format, content, commit_details) - gollum_wiki.clear_cache else gollum_write_page(name, format, content, commit_details) end @@ -40,7 +39,6 @@ module Gitlab @repository.gitaly_migrate(:wiki_delete_page) do |is_enabled| if is_enabled gitaly_delete_page(page_path, commit_details) - gollum_wiki.clear_cache else gollum_delete_page(page_path, commit_details) end @@ -51,7 +49,6 @@ module Gitlab @repository.gitaly_migrate(:wiki_update_page) do |is_enabled| if is_enabled gitaly_update_page(page_path, title, format, content, commit_details) - gollum_wiki.clear_cache else gollum_update_page(page_path, title, format, content, commit_details) end diff --git a/lib/gitlab/http.rb b/lib/gitlab/http.rb new file mode 100644 index 00000000000..96558872a37 --- /dev/null +++ b/lib/gitlab/http.rb @@ -0,0 +1,11 @@ +# This class is used as a proxy for all outbounding http connection +# coming from callbacks, services and hooks. The direct use of the HTTParty +# is discouraged because it can lead to several security problems, like SSRF +# calling internal IP or services. +module Gitlab + class HTTP + include HTTParty # rubocop:disable Gitlab/HTTParty + + connection_adapter ProxyHTTPConnectionAdapter + end +end diff --git a/lib/gitlab/proxy_http_connection_adapter.rb b/lib/gitlab/proxy_http_connection_adapter.rb new file mode 100644 index 00000000000..c70d6f4cd84 --- /dev/null +++ b/lib/gitlab/proxy_http_connection_adapter.rb @@ -0,0 +1,34 @@ +# This class is part of the Gitlab::HTTP wrapper. Depending on the value +# of the global setting allow_local_requests_from_hooks_and_services this adapter +# will allow/block connection to internal IPs and/or urls. +# +# This functionality can be overriden by providing the setting the option +# allow_local_requests = true in the request. For example: +# Gitlab::HTTP.get('http://www.gitlab.com', allow_local_requests: true) +# +# This option will take precedence over the global setting. +module Gitlab + class ProxyHTTPConnectionAdapter < HTTParty::ConnectionAdapter + def connection + if !allow_local_requests? && blocked_url? + raise URI::InvalidURIError + end + + super + end + + private + + def blocked_url? + Gitlab::UrlBlocker.blocked_url?(uri, allow_private_networks: false) + end + + def allow_local_requests? + options.fetch(:allow_local_requests, allow_settings_local_requests?) + end + + def allow_settings_local_requests? + Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services? + end + end +end diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb index 13150ddab67..0f9f939e204 100644 --- a/lib/gitlab/url_blocker.rb +++ b/lib/gitlab/url_blocker.rb @@ -3,11 +3,7 @@ require 'resolv' module Gitlab class UrlBlocker class << self - # Used to specify what hosts and port numbers should be prohibited for project - # imports. - VALID_PORTS = [22, 80, 443].freeze - - def blocked_url?(url) + def blocked_url?(url, allow_private_networks: true, valid_ports: []) return false if url.nil? blocked_ips = ["127.0.0.1", "::1", "0.0.0.0"] @@ -18,12 +14,15 @@ module Gitlab # Allow imports from the GitLab instance itself but only from the configured ports return false if internal?(uri) - return true if blocked_port?(uri.port) + return true if blocked_port?(uri.port, valid_ports) return true if blocked_user_or_hostname?(uri.user) return true if blocked_user_or_hostname?(uri.hostname) - server_ips = Addrinfo.getaddrinfo(uri.hostname, 80, nil, :STREAM).map(&:ip_address) + addrs_info = Addrinfo.getaddrinfo(uri.hostname, 80, nil, :STREAM) + server_ips = addrs_info.map(&:ip_address) + return true if (blocked_ips & server_ips).any? + return true if !allow_private_networks && private_network?(addrs_info) rescue Addressable::URI::InvalidURIError return true rescue SocketError @@ -35,10 +34,10 @@ module Gitlab private - def blocked_port?(port) - return false if port.blank? + def blocked_port?(port, valid_ports) + return false if port.blank? || valid_ports.blank? - port < 1024 && !VALID_PORTS.include?(port) + port < 1024 && !valid_ports.include?(port) end def blocked_user_or_hostname?(value) @@ -61,6 +60,10 @@ module Gitlab (uri.port.blank? || uri.port == config.gitlab_shell.ssh_port) end + def private_network?(addrs_info) + addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? } + end + def config Gitlab.config end diff --git a/lib/mattermost/session.rb b/lib/mattermost/session.rb index 65ccdb3c347..85f78e44f32 100644 --- a/lib/mattermost/session.rb +++ b/lib/mattermost/session.rb @@ -22,16 +22,14 @@ module Mattermost # going. class Session include Doorkeeper::Helpers::Controller - include HTTParty LEASE_TIMEOUT = 60 - base_uri Settings.mattermost.host - - attr_accessor :current_resource_owner, :token + attr_accessor :current_resource_owner, :token, :base_uri def initialize(current_user) @current_resource_owner = current_user + @base_uri = Settings.mattermost.host end def with_session @@ -73,24 +71,32 @@ module Mattermost def get(path, options = {}) handle_exceptions do - self.class.get(path, options.merge(headers: @headers)) + Gitlab::HTTP.get(path, build_options(options)) end end def post(path, options = {}) handle_exceptions do - self.class.post(path, options.merge(headers: @headers)) + Gitlab::HTTP.post(path, build_options(options)) end end def delete(path, options = {}) handle_exceptions do - self.class.delete(path, options.merge(headers: @headers)) + Gitlab::HTTP.delete(path, build_options(options)) end end private + def build_options(options) + options.tap do |hash| + hash[:headers] = @headers + hash[:allow_local_requests] = true + hash[:base_uri] = base_uri if base_uri.presence + end + end + def create raise Mattermost::NoSessionError unless oauth_uri raise Mattermost::NoSessionError unless token_uri @@ -165,14 +171,14 @@ module Mattermost def handle_exceptions yield - rescue HTTParty::Error => e + rescue Gitlab::HTTP::Error => e raise Mattermost::ConnectionError.new(e.message) rescue Errno::ECONNREFUSED => e raise Mattermost::ConnectionError.new(e.message) end def parse_cookie(response) - cookie_hash = CookieHash.new + cookie_hash = Gitlab::HTTP::CookieHash.new response.get_fields('Set-Cookie').each { |c| cookie_hash.add_cookies(c) } cookie_hash end diff --git a/lib/microsoft_teams/notifier.rb b/lib/microsoft_teams/notifier.rb index 3bef68a1bcb..c08d3e933a8 100644 --- a/lib/microsoft_teams/notifier.rb +++ b/lib/microsoft_teams/notifier.rb @@ -9,14 +9,15 @@ module MicrosoftTeams result = false begin - response = HTTParty.post( + response = Gitlab::HTTP.post( @webhook.to_str, headers: @header, + allow_local_requests: true, body: body(options) ) result = true if response - rescue HTTParty::Error, StandardError => error + rescue Gitlab::HTTP::Error, StandardError => error Rails.logger.info("#{self.class.name}: Error while connecting to #{@webhook}: #{error.message}") end diff --git a/public/robots.txt b/public/robots.txt index 123272a9834..1f9d42f4adc 100644 --- a/public/robots.txt +++ b/public/robots.txt @@ -20,6 +20,7 @@ Disallow: /projects/new Disallow: /groups/new Disallow: /groups/*/edit Disallow: /users +Disallow: /help # Global snippets User-Agent: * diff --git a/rubocop/cop/gitlab/httparty.rb b/rubocop/cop/gitlab/httparty.rb new file mode 100644 index 00000000000..215f18b6993 --- /dev/null +++ b/rubocop/cop/gitlab/httparty.rb @@ -0,0 +1,62 @@ +require_relative '../../spec_helpers' + +module RuboCop + module Cop + module Gitlab + class HTTParty < RuboCop::Cop::Cop + include SpecHelpers + + MSG_SEND = <<~EOL.freeze + Avoid calling `HTTParty` directly. Instead, use the Gitlab::HTTP + wrapper. To allow request to localhost or the private network set + the option :allow_local_requests in the request call. + EOL + + MSG_INCLUDE = <<~EOL.freeze + Avoid including `HTTParty` directly. Instead, use the Gitlab::HTTP + wrapper. To allow request to localhost or the private network set + the option :allow_local_requests in the request call. + EOL + + def_node_matcher :includes_httparty?, <<~PATTERN + (send nil? :include (const nil? :HTTParty)) + PATTERN + + def_node_matcher :httparty_node?, <<~PATTERN + (send (const nil? :HTTParty)...) + PATTERN + + def on_send(node) + return if in_spec?(node) + + add_offense(node, location: :expression, message: MSG_SEND) if httparty_node?(node) + add_offense(node, location: :expression, message: MSG_INCLUDE) if includes_httparty?(node) + end + + def autocorrect(node) + if includes_httparty?(node) + autocorrect_includes_httparty(node) + else + autocorrect_httparty_node(node) + end + end + + def autocorrect_includes_httparty(node) + lambda do |corrector| + corrector.remove(node.source_range) + end + end + + def autocorrect_httparty_node(node) + _, method_name, *arg_nodes = *node + + replacement = "Gitlab::HTTP.#{method_name}(#{arg_nodes.map(&:source).join(', ')})" + + lambda do |corrector| + corrector.replace(node.source_range, replacement) + end + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index b36a3f9c8a0..0b4c7d31442 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -1,6 +1,7 @@ # rubocop:disable Naming/FileName require_relative 'cop/gitlab/module_with_instance_variables' require_relative 'cop/gitlab/predicate_memoization' +require_relative 'cop/gitlab/httparty' require_relative 'cop/include_sidekiq_worker' require_relative 'cop/line_break_around_conditional_block' require_relative 'cop/migration/add_column' diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb index c639ad32ec6..9fd129e4ee9 100644 --- a/spec/controllers/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/omniauth_callbacks_controller_spec.rb @@ -3,73 +3,90 @@ require 'spec_helper' describe OmniauthCallbacksController do include LoginHelpers - let(:user) { create(:omniauth_user, extern_uid: 'my-uid', provider: provider) } - let(:provider) { :github } + let(:user) { create(:omniauth_user, extern_uid: extern_uid, provider: provider) } before do - mock_auth_hash(provider.to_s, 'my-uid', user.email) + mock_auth_hash(provider.to_s, extern_uid, user.email) stub_omniauth_provider(provider, context: request) end - it 'allows sign in' do - post provider + context 'github' do + let(:extern_uid) { 'my-uid' } + let(:provider) { :github } - expect(request.env['warden']).to be_authenticated - end + it 'allows sign in' do + post provider + + expect(request.env['warden']).to be_authenticated + end - shared_context 'sign_up' do - let(:user) { double(email: 'new@example.com') } + shared_context 'sign_up' do + let(:user) { double(email: 'new@example.com') } - before do - stub_omniauth_setting(block_auto_created_users: false) + before do + stub_omniauth_setting(block_auto_created_users: false) + end end - end - context 'sign up' do - include_context 'sign_up' + context 'sign up' do + include_context 'sign_up' - it 'is allowed' do - post provider + it 'is allowed' do + post provider - expect(request.env['warden']).to be_authenticated + expect(request.env['warden']).to be_authenticated + end end - end - context 'when OAuth is disabled' do - before do - stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') - settings = Gitlab::CurrentSettings.current_application_settings - settings.update(disabled_oauth_sign_in_sources: [provider.to_s]) - end + context 'when OAuth is disabled' do + before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + settings = Gitlab::CurrentSettings.current_application_settings + settings.update(disabled_oauth_sign_in_sources: [provider.to_s]) + end - it 'prevents login via POST' do - post provider + it 'prevents login via POST' do + post provider - expect(request.env['warden']).not_to be_authenticated - end + expect(request.env['warden']).not_to be_authenticated + end - it 'shows warning when attempting login' do - post provider + it 'shows warning when attempting login' do + post provider - expect(response).to redirect_to new_user_session_path - expect(flash[:alert]).to eq('Signing in using GitHub has been disabled') - end + expect(response).to redirect_to new_user_session_path + expect(flash[:alert]).to eq('Signing in using GitHub has been disabled') + end - it 'allows linking the disabled provider' do - user.identities.destroy_all - sign_in(user) + it 'allows linking the disabled provider' do + user.identities.destroy_all + sign_in(user) - expect { post provider }.to change { user.reload.identities.count }.by(1) - end + expect { post provider }.to change { user.reload.identities.count }.by(1) + end - context 'sign up' do - include_context 'sign_up' + context 'sign up' do + include_context 'sign_up' - it 'is prevented' do - post provider + it 'is prevented' do + post provider - expect(request.env['warden']).not_to be_authenticated + expect(request.env['warden']).not_to be_authenticated + end end end end + + context 'auth0' do + let(:extern_uid) { '' } + let(:provider) { :auth0 } + + it 'does not allow sign in without extern_uid' do + post 'auth0' + + expect(request.env['warden']).not_to be_authenticated + expect(response.status).to eq(302) + expect(controller).to set_flash[:alert].to('Wrong extern UID provided. Make sure Auth0 is configured correctly.') + end + end end diff --git a/spec/features/read_only_spec.rb b/spec/features/read_only_spec.rb new file mode 100644 index 00000000000..8bfaf558466 --- /dev/null +++ b/spec/features/read_only_spec.rb @@ -0,0 +1,25 @@ +require 'rails_helper' + +describe 'read-only message' do + set(:user) { create(:user) } + + before do + sign_in(user) + end + + it 'shows read-only banner when database is read-only' do + allow(Gitlab::Database).to receive(:read_only?).and_return(true) + + visit root_dashboard_path + + expect(page).to have_content('You are on a read-only GitLab instance.') + end + + it 'does not show read-only banner when database is able to read-write' do + allow(Gitlab::Database).to receive(:read_only?).and_return(false) + + visit root_dashboard_path + + expect(page).not_to have_content('You are on a read-only GitLab instance.') + end +end diff --git a/spec/lib/gitlab/http_spec.rb b/spec/lib/gitlab/http_spec.rb new file mode 100644 index 00000000000..b0bc081a3c8 --- /dev/null +++ b/spec/lib/gitlab/http_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe Gitlab::HTTP do + describe 'allow_local_requests_from_hooks_and_services is' do + before do + WebMock.stub_request(:get, /.*/).to_return(status: 200, body: 'Success') + end + + context 'disabled' do + before do + allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_hooks_and_services?).and_return(false) + end + + it 'deny requests to localhost' do + expect { described_class.get('http://localhost:3003') }.to raise_error(URI::InvalidURIError) + end + + it 'deny requests to private network' do + expect { described_class.get('http://192.168.1.2:3003') }.to raise_error(URI::InvalidURIError) + end + + context 'if allow_local_requests set to true' do + it 'override the global value and allow requests to localhost or private network' do + expect { described_class.get('http://localhost:3003', allow_local_requests: true) }.not_to raise_error + end + end + end + + context 'enabled' do + before do + allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_hooks_and_services?).and_return(true) + end + + it 'allow requests to localhost' do + expect { described_class.get('http://localhost:3003') }.not_to raise_error + end + + it 'allow requests to private network' do + expect { described_class.get('http://192.168.1.2:3003') }.not_to raise_error + end + + context 'if allow_local_requests set to false' do + it 'override the global value and ban requests to localhost or private network' do + expect { described_class.get('http://localhost:3003', allow_local_requests: false) }.to raise_error(URI::InvalidURIError) + end + end + end + end +end diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index d9b3c2350b1..2d35b026485 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' describe Gitlab::UrlBlocker do describe '#blocked_url?' do + let(:valid_ports) { Project::VALID_IMPORT_PORTS } + it 'allows imports from configured web host and port' do import_url = "http://#{Gitlab.config.gitlab.host}:#{Gitlab.config.gitlab.port}/t.git" expect(described_class.blocked_url?(import_url)).to be false @@ -17,7 +19,7 @@ describe Gitlab::UrlBlocker do end it 'returns true for bad port' do - expect(described_class.blocked_url?('https://gitlab.com:25/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://gitlab.com:25/foo/foo.git', valid_ports: valid_ports)).to be true end it 'returns true for alternative version of 127.0.0.1 (0177.1)' do @@ -71,6 +73,47 @@ describe Gitlab::UrlBlocker do it 'returns false for legitimate URL' do expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git')).to be false end + + context 'when allow_private_networks is' do + let(:private_networks) { ['192.168.1.2', '10.0.0.2', '172.16.0.2'] } + let(:fake_domain) { 'www.fakedomain.fake' } + + context 'true (default)' do + it 'does not block urls from private networks' do + private_networks.each do |ip| + stub_domain_resolv(fake_domain, ip) + + expect(described_class).not_to be_blocked_url("http://#{fake_domain}") + + unstub_domain_resolv + + expect(described_class).not_to be_blocked_url("http://#{ip}") + end + end + end + + context 'false' do + it 'blocks urls from private networks' do + private_networks.each do |ip| + stub_domain_resolv(fake_domain, ip) + + expect(described_class).to be_blocked_url("http://#{fake_domain}", allow_private_networks: false) + + unstub_domain_resolv + + expect(described_class).to be_blocked_url("http://#{ip}", allow_private_networks: false) + end + end + end + + def stub_domain_resolv(domain, ip) + allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([double(ip_address: ip, ipv4_private?: true)]) + end + + def unstub_domain_resolv + allow(Addrinfo).to receive(:getaddrinfo).and_call_original + end + end end # Resolv does not support resolving UTF-8 domain names diff --git a/spec/lib/mattermost/command_spec.rb b/spec/lib/mattermost/command_spec.rb index 369e7b181b9..8ba15ae0f38 100644 --- a/spec/lib/mattermost/command_spec.rb +++ b/spec/lib/mattermost/command_spec.rb @@ -4,10 +4,11 @@ describe Mattermost::Command do let(:params) { { 'token' => 'token', team_id: 'abc' } } before do - Mattermost::Session.base_uri('http://mattermost.example.com') + session = Mattermost::Session.new(nil) + session.base_uri = 'http://mattermost.example.com' allow_any_instance_of(Mattermost::Client).to receive(:with_session) - .and_yield(Mattermost::Session.new(nil)) + .and_yield(session) end describe '#create' do diff --git a/spec/lib/mattermost/session_spec.rb b/spec/lib/mattermost/session_spec.rb index 3db19d06305..c855643c4d8 100644 --- a/spec/lib/mattermost/session_spec.rb +++ b/spec/lib/mattermost/session_spec.rb @@ -15,7 +15,7 @@ describe Mattermost::Session, type: :request do it { is_expected.to respond_to(:strategy) } before do - described_class.base_uri(mattermost_url) + subject.base_uri = mattermost_url end describe '#with session' do diff --git a/spec/lib/mattermost/team_spec.rb b/spec/lib/mattermost/team_spec.rb index 3c8206031cf..2cfa6802612 100644 --- a/spec/lib/mattermost/team_spec.rb +++ b/spec/lib/mattermost/team_spec.rb @@ -2,10 +2,11 @@ require 'spec_helper' describe Mattermost::Team do before do - Mattermost::Session.base_uri('http://mattermost.example.com') + session = Mattermost::Session.new(nil) + session.base_uri = 'http://mattermost.example.com' allow_any_instance_of(Mattermost::Client).to receive(:with_session) - .and_yield(Mattermost::Session.new(nil)) + .and_yield(session) end describe '#all' do diff --git a/spec/migrations/remove_empty_extern_uid_auth0_identities_spec.rb b/spec/migrations/remove_empty_extern_uid_auth0_identities_spec.rb new file mode 100644 index 00000000000..441c4295a40 --- /dev/null +++ b/spec/migrations/remove_empty_extern_uid_auth0_identities_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20180220150310_remove_empty_extern_uid_auth0_identities.rb') + +describe RemoveEmptyExternUidAuth0Identities, :migration do + let(:identities) { table(:identities) } + + before do + identities.create(provider: 'auth0', extern_uid: '') + identities.create(provider: 'auth0', extern_uid: 'valid') + identities.create(provider: 'github', extern_uid: '') + + migrate! + end + + it 'leaves the correct auth0 identity' do + expect(identities.where(provider: 'auth0').pluck(:extern_uid)).to eq(['valid']) + end + + it 'leaves the correct github identity' do + expect(identities.where(provider: 'github').count).to eq(1) + end +end diff --git a/spec/models/project_services/mattermost_slash_commands_service_spec.rb b/spec/models/project_services/mattermost_slash_commands_service_spec.rb index a5bdf9a9337..05d33cd3874 100644 --- a/spec/models/project_services/mattermost_slash_commands_service_spec.rb +++ b/spec/models/project_services/mattermost_slash_commands_service_spec.rb @@ -9,10 +9,11 @@ describe MattermostSlashCommandsService do let(:user) { create(:user) } before do - Mattermost::Session.base_uri("http://mattermost.example.com") + session = Mattermost::Session.new(nil) + session.base_uri = 'http://mattermost.example.com' allow_any_instance_of(Mattermost::Client).to receive(:with_session) - .and_yield(Mattermost::Session.new(nil)) + .and_yield(session) end describe '#configure' do diff --git a/spec/rubocop/cop/gitlab/httparty_spec.rb b/spec/rubocop/cop/gitlab/httparty_spec.rb new file mode 100644 index 00000000000..510839a21d7 --- /dev/null +++ b/spec/rubocop/cop/gitlab/httparty_spec.rb @@ -0,0 +1,74 @@ +require 'spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../../rubocop/cop/gitlab/httparty' + +describe RuboCop::Cop::Gitlab::HTTParty do # rubocop:disable RSpec/FilePath + include CopHelper + + subject(:cop) { described_class.new } + + shared_examples('registering include offense') do |options| + let(:offending_lines) { options[:offending_lines] } + + it 'registers an offense when the class includes HTTParty' do + inspect_source(source) + + aggregate_failures do + expect(cop.offenses.size).to eq(offending_lines.size) + expect(cop.offenses.map(&:line)).to eq(offending_lines) + end + end + end + + shared_examples('registering call offense') do |options| + let(:offending_lines) { options[:offending_lines] } + + it 'registers an offense when the class calls HTTParty' do + inspect_source(source) + + aggregate_failures do + expect(cop.offenses.size).to eq(offending_lines.size) + expect(cop.offenses.map(&:line)).to eq(offending_lines) + end + end + end + + context 'when source is a regular module' do + it_behaves_like 'registering include offense', offending_lines: [2] do + let(:source) do + <<~RUBY + module M + include HTTParty + end + RUBY + end + end + end + + context 'when source is a regular class' do + it_behaves_like 'registering include offense', offending_lines: [2] do + let(:source) do + <<~RUBY + class Foo + include HTTParty + end + RUBY + end + end + end + + context 'when HTTParty is called' do + it_behaves_like 'registering call offense', offending_lines: [3] do + let(:source) do + <<~RUBY + class Foo + def bar + HTTParty.get('http://example.com') + end + end + RUBY + end + end + end +end diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb index 0ce41e7c7ee..feb5120bc68 100644 --- a/spec/services/ci/process_pipeline_service_spec.rb +++ b/spec/services/ci/process_pipeline_service_spec.rb @@ -9,6 +9,8 @@ describe Ci::ProcessPipelineService, '#execute' do end before do + stub_ci_pipeline_to_return_yaml_file + stub_not_protect_default_branch project.add_developer(user) diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index 21910e69d2e..2ef2e61babc 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -14,6 +14,20 @@ describe WebHookService do end let(:service_instance) { described_class.new(project_hook, data, :push_hooks) } + describe '#initialize' do + it 'allow_local_requests is true if hook is a SystemHook' do + instance = described_class.new(build(:system_hook), data, :system_hook) + expect(instance.request_options[:allow_local_requests]).to be_truthy + end + + it 'allow_local_requests is false if hook is not a SystemHook' do + %i(project_hook service_hook web_hook_log).each do |hook| + instance = described_class.new(build(hook), data, hook) + expect(instance.request_options[:allow_local_requests]).to be_falsey + end + end + end + describe '#execute' do before do project.hooks << [project_hook] |
