diff options
| -rw-r--r-- | .gitlab/ci/frontend.gitlab-ci.yml | 19 | ||||
| -rw-r--r-- | .gitlab/ci/pages.gitlab-ci.yml | 3 | ||||
| -rw-r--r-- | .gitlab/ci/qa.gitlab-ci.yml | 2 | ||||
| -rw-r--r-- | .gitlab/ci/review.gitlab-ci.yml | 5 | ||||
| -rw-r--r-- | .rubocop_todo.yml | 2 | ||||
| -rw-r--r-- | app/models/merge_request_diff.rb | 6 | ||||
| -rw-r--r-- | config/routes.rb | 2 | ||||
| -rw-r--r-- | lib/api/api.rb | 2 | ||||
| -rw-r--r-- | lib/api/internal.rb | 294 | ||||
| -rw-r--r-- | lib/api/internal/base.rb | 296 | ||||
| -rw-r--r-- | lib/gitlab/git/diff_collection.rb | 6 | ||||
| -rw-r--r-- | lib/gitlab/jwt_authenticatable.rb | 42 | ||||
| -rw-r--r-- | lib/gitlab/workhorse.rb | 28 | ||||
| -rwxr-xr-x | scripts/review_apps/review-apps.sh | 7 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/diff_collection_spec.rb | 69 | ||||
| -rw-r--r-- | spec/lib/gitlab/jwt_authenticatable_spec.rb | 93 | ||||
| -rw-r--r-- | spec/lib/gitlab/workhorse_spec.rb | 51 | ||||
| -rw-r--r-- | spec/models/merge_request/metrics_spec.rb | 2 | ||||
| -rw-r--r-- | spec/models/merge_request_diff_spec.rb | 14 | ||||
| -rw-r--r-- | spec/requests/api/internal/base_spec.rb (renamed from spec/requests/api/internal_spec.rb) | 2 |
20 files changed, 540 insertions, 405 deletions
diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml index 0d73092cfba..0720ea3e056 100644 --- a/.gitlab/ci/frontend.gitlab-ci.yml +++ b/.gitlab/ci/frontend.gitlab-ci.yml @@ -233,22 +233,3 @@ qa-frontend-node:latest: extends: .qa-frontend-node image: node:latest allow_failure: true - -jsdoc: - extends: - - .default-tags - - .default-retry - - .default-cache - - .except-docs - variables: - SETUP_DB: "false" - stage: post-test - dependencies: ["compile-assets", "compile-assets pull-cache"] - script: - - date - - yarn run jsdoc || true # ignore exit code - artifacts: - name: jsdoc - expire_in: 31d - paths: - - jsdoc/ diff --git a/.gitlab/ci/pages.gitlab-ci.yml b/.gitlab/ci/pages.gitlab-ci.yml index 5d13a72e224..2de09753cca 100644 --- a/.gitlab/ci/pages.gitlab-ci.yml +++ b/.gitlab/ci/pages.gitlab-ci.yml @@ -9,7 +9,7 @@ pages: - master@gitlab-org/gitlab-ce - master@gitlab-org/gitlab-ee stage: pages - dependencies: ["coverage", "karma", "gitlab:assets:compile", "jsdoc"] + dependencies: ["coverage", "karma", "gitlab:assets:compile"] script: - mv public/ .public/ - mkdir public/ @@ -18,7 +18,6 @@ pages: - mv webpack-report/ public/webpack-report/ || true - cp .public/assets/application-*.css public/application.css || true - cp .public/assets/application-*.css.gz public/application.css.gz || true - - mv jsdoc/ public/jsdoc/ || true artifacts: paths: - public diff --git a/.gitlab/ci/qa.gitlab-ci.yml b/.gitlab/ci/qa.gitlab-ci.yml index 8628e1e0a14..9c021b23db6 100644 --- a/.gitlab/ci/qa.gitlab-ci.yml +++ b/.gitlab/ci/qa.gitlab-ci.yml @@ -19,6 +19,8 @@ package-and-qa-manual: except: refs: - master + - /(^docs[\/-].+|.+-docs$)/ + - /(^qa[\/-].*|.*-qa$)/ when: manual needs: ["build-qa-image", "gitlab:assets:compile pull-cache"] diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index c4a81a021a9..6695404653c 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -128,8 +128,9 @@ review-stop: - source utils.sh - source review-apps.sh script: - - delete - artifacts: {} + - delete_release + artifacts: + paths: [] .review-qa-base: extends: diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index be147d72f71..f1f8ff6e862 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -218,7 +218,7 @@ Lint/UriEscapeUnescape: - 'app/models/project_services/drone_ci_service.rb' - 'spec/lib/google_api/auth_spec.rb' - 'spec/requests/api/files_spec.rb' - - 'spec/requests/api/internal_spec.rb' + - 'spec/requests/api/internal/base_spec.rb' # Offense count: 1 # Configuration parameters: CheckForMethodsWithNoSideEffects. diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 2402fa8e38f..4db2b7a74e5 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -197,7 +197,7 @@ class MergeRequestDiff < ApplicationRecord def lines_count strong_memoize(:lines_count) do - diffs.diff_files.sum(&:line_count) + raw_diffs(limits: false).line_count end end @@ -222,6 +222,10 @@ class MergeRequestDiff < ApplicationRecord commits.last end + def last_commit + commits.first + end + def base_commit return unless base_commit_sha diff --git a/config/routes.rb b/config/routes.rb index c333550f758..02a405a91f8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -67,7 +67,7 @@ Rails.application.routes.draw do get 'health_check(/:checks)' => 'health_check#index', as: :health_check scope path: '-' do - # '/-/health' implemented by BasicHealthMiddleware + # '/-/health' implemented by BasicHealthCheck middleware get 'liveness' => 'health#liveness' get 'readiness' => 'health#readiness' resources :metrics, only: [:index] diff --git a/lib/api/api.rb b/lib/api/api.rb index 219ed45eff6..aa6a67d817a 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -118,7 +118,7 @@ module API mount ::API::GroupContainerRepositories mount ::API::GroupVariables mount ::API::ImportGithub - mount ::API::Internal + mount ::API::Internal::Base mount ::API::Issues mount ::API::JobArtifacts mount ::API::Jobs diff --git a/lib/api/internal.rb b/lib/api/internal.rb deleted file mode 100644 index 088ea5bd79a..00000000000 --- a/lib/api/internal.rb +++ /dev/null @@ -1,294 +0,0 @@ -# frozen_string_literal: true - -module API - # Internal access API - class Internal < Grape::API - before { authenticate_by_gitlab_shell_token! } - - helpers ::API::Helpers::InternalHelpers - helpers ::Gitlab::Identifier - - UNKNOWN_CHECK_RESULT_ERROR = 'Unknown check result'.freeze - - helpers do - def response_with_status(code: 200, success: true, message: nil, **extra_options) - status code - { status: success, message: message }.merge(extra_options).compact - end - - def lfs_authentication_url(project) - # This is a separate method so that EE can alter its behaviour more - # easily. - project.http_url_to_repo - end - end - - namespace 'internal' do - # Check if git command is allowed for project - # - # Params: - # key_id - ssh key id for Git over SSH - # user_id - user id for Git over HTTP or over SSH in keyless SSH CERT mode - # username - user name for Git over SSH in keyless SSH cert mode - # protocol - Git access protocol being used, e.g. HTTP or SSH - # project - project full_path (not path on disk) - # action - git action (git-upload-pack or git-receive-pack) - # changes - changes as "oldrev newrev ref", see Gitlab::ChangesList - # rubocop: disable CodeReuse/ActiveRecord - post "/allowed" do - # Stores some Git-specific env thread-safely - env = parse_env - Gitlab::Git::HookEnv.set(gl_repository, env) if project - - actor = - if params[:key_id] - Key.find_by(id: params[:key_id]) - elsif params[:user_id] - User.find_by(id: params[:user_id]) - elsif params[:username] - UserFinder.new(params[:username]).find_by_username - end - - protocol = params[:protocol] - - actor.update_last_used_at if actor.is_a?(Key) - user = - if actor.is_a?(Key) - actor.user - else - actor - end - - access_checker_klass = repo_type.access_checker_class - access_checker = access_checker_klass.new(actor, project, - protocol, authentication_abilities: ssh_authentication_abilities, - namespace_path: namespace_path, project_path: project_path, - redirected_path: redirected_path) - - check_result = begin - result = access_checker.check(params[:action], params[:changes]) - @project ||= access_checker.project - result - rescue Gitlab::GitAccess::UnauthorizedError => e - break response_with_status(code: 401, success: false, message: e.message) - rescue Gitlab::GitAccess::TimeoutError => e - break response_with_status(code: 503, success: false, message: e.message) - rescue Gitlab::GitAccess::NotFoundError => e - break response_with_status(code: 404, success: false, message: e.message) - end - - log_user_activity(actor) - - case check_result - when ::Gitlab::GitAccessResult::Success - payload = { - gl_repository: gl_repository, - gl_project_path: gl_project_path, - gl_id: Gitlab::GlId.gl_id(user), - gl_username: user&.username, - git_config_options: [], - gitaly: gitaly_payload(params[:action]), - gl_console_messages: check_result.console_messages - } - - # Custom option for git-receive-pack command - receive_max_input_size = Gitlab::CurrentSettings.receive_max_input_size.to_i - if receive_max_input_size > 0 - payload[:git_config_options] << "receive.maxInputSize=#{receive_max_input_size.megabytes}" - end - - response_with_status(**payload) - when ::Gitlab::GitAccessResult::CustomAction - response_with_status(code: 300, message: check_result.message, payload: check_result.payload) - else - response_with_status(code: 500, success: false, message: UNKNOWN_CHECK_RESULT_ERROR) - end - end - # rubocop: enable CodeReuse/ActiveRecord - - # rubocop: disable CodeReuse/ActiveRecord - post "/lfs_authenticate" do - status 200 - - if params[:key_id] - actor = Key.find(params[:key_id]) - actor.update_last_used_at - elsif params[:user_id] - actor = User.find_by(id: params[:user_id]) - raise ActiveRecord::RecordNotFound.new("No such user id!") unless actor - else - raise ActiveRecord::RecordNotFound.new("No key_id or user_id passed!") - end - - Gitlab::LfsToken - .new(actor) - .authentication_payload(lfs_authentication_url(project)) - end - # rubocop: enable CodeReuse/ActiveRecord - - get "/merge_request_urls" do - merge_request_urls - end - - # - # Get a ssh key using the fingerprint - # - # rubocop: disable CodeReuse/ActiveRecord - get "/authorized_keys" do - fingerprint = params.fetch(:fingerprint) do - Gitlab::InsecureKeyFingerprint.new(params.fetch(:key)).fingerprint - end - key = Key.find_by(fingerprint: fingerprint) - not_found!("Key") if key.nil? - present key, with: Entities::SSHKey - end - # rubocop: enable CodeReuse/ActiveRecord - - # - # Discover user by ssh key, user id or username - # - # rubocop: disable CodeReuse/ActiveRecord - get "/discover" do - if params[:key_id] - key = Key.find(params[:key_id]) - user = key.user - elsif params[:user_id] - user = User.find_by(id: params[:user_id]) - elsif params[:username] - user = UserFinder.new(params[:username]).find_by_username - end - - present user, with: Entities::UserSafe - end - # rubocop: enable CodeReuse/ActiveRecord - - get "/check" do - { - api_version: API.version, - gitlab_version: Gitlab::VERSION, - gitlab_rev: Gitlab.revision, - redis: redis_ping - } - end - - get "/broadcast_messages" do - if messages = BroadcastMessage.current - present messages, with: Entities::BroadcastMessage - else - [] - end - end - - get "/broadcast_message" do - if message = BroadcastMessage.current&.last - present message, with: Entities::BroadcastMessage - else - {} - end - end - - # rubocop: disable CodeReuse/ActiveRecord - post '/two_factor_recovery_codes' do - status 200 - - if params[:key_id] - key = Key.find_by(id: params[:key_id]) - - if key - key.update_last_used_at - else - break { 'success' => false, 'message' => 'Could not find the given key' } - end - - if key.is_a?(DeployKey) - break { success: false, message: 'Deploy keys cannot be used to retrieve recovery codes' } - end - - user = key.user - - unless user - break { success: false, message: 'Could not find a user for the given key' } - end - elsif params[:user_id] - user = User.find_by(id: params[:user_id]) - - unless user - break { success: false, message: 'Could not find the given user' } - end - end - - unless user.two_factor_enabled? - break { success: false, message: 'Two-factor authentication is not enabled for this user' } - end - - codes = nil - - ::Users::UpdateService.new(current_user, user: user).execute! do |user| - codes = user.generate_otp_backup_codes! - end - - { success: true, recovery_codes: codes } - end - # rubocop: enable CodeReuse/ActiveRecord - - post '/pre_receive' do - status 200 - - reference_counter_increased = Gitlab::ReferenceCounter.new(params[:gl_repository]).increase - - { reference_counter_increased: reference_counter_increased } - end - - post "/notify_post_receive" do - status 200 - - # TODO: Re-enable when Gitaly is processing the post-receive notification - # return unless Gitlab::GitalyClient.enabled? - # - # begin - # repository = wiki? ? project.wiki.repository : project.repository - # Gitlab::GitalyClient::NotificationService.new(repository.raw_repository).post_receive - # rescue GRPC::Unavailable => e - # render_api_error!(e, 500) - # end - end - - post '/post_receive' do - status 200 - - response = Gitlab::InternalPostReceive::Response.new - user = identify(params[:identifier]) - project = Gitlab::GlRepository.parse(params[:gl_repository]).first - push_options = Gitlab::PushOptions.new(params[:push_options]) - - response.reference_counter_decreased = Gitlab::ReferenceCounter.new(params[:gl_repository]).decrease - - PostReceive.perform_async(params[:gl_repository], params[:identifier], - params[:changes], push_options.as_json) - - mr_options = push_options.get(:merge_request) - if mr_options.present? - message = process_mr_push_options(mr_options, project, user, params[:changes]) - response.add_alert_message(message) - end - - broadcast_message = BroadcastMessage.current&.last&.message - response.add_alert_message(broadcast_message) - - response.add_merge_request_urls(merge_request_urls) - - # A user is not guaranteed to be returned; an orphaned write deploy - # key could be used - if user - redirect_message = Gitlab::Checks::ProjectMoved.fetch_message(user.id, project.id) - project_created_message = Gitlab::Checks::ProjectCreated.fetch_message(user.id, project.id) - - response.add_basic_message(redirect_message) - response.add_basic_message(project_created_message) - end - - present response, with: Entities::InternalPostReceive::Response - end - end - end -end diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb new file mode 100644 index 00000000000..622032b8355 --- /dev/null +++ b/lib/api/internal/base.rb @@ -0,0 +1,296 @@ +# frozen_string_literal: true + +module API + # Internal access API + module Internal + class Base < Grape::API + before { authenticate_by_gitlab_shell_token! } + + helpers ::API::Helpers::InternalHelpers + helpers ::Gitlab::Identifier + + UNKNOWN_CHECK_RESULT_ERROR = 'Unknown check result'.freeze + + helpers do + def response_with_status(code: 200, success: true, message: nil, **extra_options) + status code + { status: success, message: message }.merge(extra_options).compact + end + + def lfs_authentication_url(project) + # This is a separate method so that EE can alter its behaviour more + # easily. + project.http_url_to_repo + end + end + + namespace 'internal' do + # Check if git command is allowed for project + # + # Params: + # key_id - ssh key id for Git over SSH + # user_id - user id for Git over HTTP or over SSH in keyless SSH CERT mode + # username - user name for Git over SSH in keyless SSH cert mode + # protocol - Git access protocol being used, e.g. HTTP or SSH + # project - project full_path (not path on disk) + # action - git action (git-upload-pack or git-receive-pack) + # changes - changes as "oldrev newrev ref", see Gitlab::ChangesList + # rubocop: disable CodeReuse/ActiveRecord + post "/allowed" do + # Stores some Git-specific env thread-safely + env = parse_env + Gitlab::Git::HookEnv.set(gl_repository, env) if project + + actor = + if params[:key_id] + Key.find_by(id: params[:key_id]) + elsif params[:user_id] + User.find_by(id: params[:user_id]) + elsif params[:username] + UserFinder.new(params[:username]).find_by_username + end + + protocol = params[:protocol] + + actor.update_last_used_at if actor.is_a?(Key) + user = + if actor.is_a?(Key) + actor.user + else + actor + end + + access_checker_klass = repo_type.access_checker_class + access_checker = access_checker_klass.new(actor, project, + protocol, authentication_abilities: ssh_authentication_abilities, + namespace_path: namespace_path, project_path: project_path, + redirected_path: redirected_path) + + check_result = begin + result = access_checker.check(params[:action], params[:changes]) + @project ||= access_checker.project + result + rescue Gitlab::GitAccess::UnauthorizedError => e + break response_with_status(code: 401, success: false, message: e.message) + rescue Gitlab::GitAccess::TimeoutError => e + break response_with_status(code: 503, success: false, message: e.message) + rescue Gitlab::GitAccess::NotFoundError => e + break response_with_status(code: 404, success: false, message: e.message) + end + + log_user_activity(actor) + + case check_result + when ::Gitlab::GitAccessResult::Success + payload = { + gl_repository: gl_repository, + gl_project_path: gl_project_path, + gl_id: Gitlab::GlId.gl_id(user), + gl_username: user&.username, + git_config_options: [], + gitaly: gitaly_payload(params[:action]), + gl_console_messages: check_result.console_messages + } + + # Custom option for git-receive-pack command + receive_max_input_size = Gitlab::CurrentSettings.receive_max_input_size.to_i + if receive_max_input_size > 0 + payload[:git_config_options] << "receive.maxInputSize=#{receive_max_input_size.megabytes}" + end + + response_with_status(**payload) + when ::Gitlab::GitAccessResult::CustomAction + response_with_status(code: 300, message: check_result.message, payload: check_result.payload) + else + response_with_status(code: 500, success: false, message: UNKNOWN_CHECK_RESULT_ERROR) + end + end + # rubocop: enable CodeReuse/ActiveRecord + + # rubocop: disable CodeReuse/ActiveRecord + post "/lfs_authenticate" do + status 200 + + if params[:key_id] + actor = Key.find(params[:key_id]) + actor.update_last_used_at + elsif params[:user_id] + actor = User.find_by(id: params[:user_id]) + raise ActiveRecord::RecordNotFound.new("No such user id!") unless actor + else + raise ActiveRecord::RecordNotFound.new("No key_id or user_id passed!") + end + + Gitlab::LfsToken + .new(actor) + .authentication_payload(lfs_authentication_url(project)) + end + # rubocop: enable CodeReuse/ActiveRecord + + get "/merge_request_urls" do + merge_request_urls + end + + # + # Get a ssh key using the fingerprint + # + # rubocop: disable CodeReuse/ActiveRecord + get "/authorized_keys" do + fingerprint = params.fetch(:fingerprint) do + Gitlab::InsecureKeyFingerprint.new(params.fetch(:key)).fingerprint + end + key = Key.find_by(fingerprint: fingerprint) + not_found!("Key") if key.nil? + present key, with: Entities::SSHKey + end + # rubocop: enable CodeReuse/ActiveRecord + + # + # Discover user by ssh key, user id or username + # + # rubocop: disable CodeReuse/ActiveRecord + get "/discover" do + if params[:key_id] + key = Key.find(params[:key_id]) + user = key.user + elsif params[:user_id] + user = User.find_by(id: params[:user_id]) + elsif params[:username] + user = UserFinder.new(params[:username]).find_by_username + end + + present user, with: Entities::UserSafe + end + # rubocop: enable CodeReuse/ActiveRecord + + get "/check" do + { + api_version: API.version, + gitlab_version: Gitlab::VERSION, + gitlab_rev: Gitlab.revision, + redis: redis_ping + } + end + + get "/broadcast_messages" do + if messages = BroadcastMessage.current + present messages, with: Entities::BroadcastMessage + else + [] + end + end + + get "/broadcast_message" do + if message = BroadcastMessage.current&.last + present message, with: Entities::BroadcastMessage + else + {} + end + end + + # rubocop: disable CodeReuse/ActiveRecord + post '/two_factor_recovery_codes' do + status 200 + + if params[:key_id] + key = Key.find_by(id: params[:key_id]) + + if key + key.update_last_used_at + else + break { 'success' => false, 'message' => 'Could not find the given key' } + end + + if key.is_a?(DeployKey) + break { success: false, message: 'Deploy keys cannot be used to retrieve recovery codes' } + end + + user = key.user + + unless user + break { success: false, message: 'Could not find a user for the given key' } + end + elsif params[:user_id] + user = User.find_by(id: params[:user_id]) + + unless user + break { success: false, message: 'Could not find the given user' } + end + end + + unless user.two_factor_enabled? + break { success: false, message: 'Two-factor authentication is not enabled for this user' } + end + + codes = nil + + ::Users::UpdateService.new(current_user, user: user).execute! do |user| + codes = user.generate_otp_backup_codes! + end + + { success: true, recovery_codes: codes } + end + # rubocop: enable CodeReuse/ActiveRecord + + post '/pre_receive' do + status 200 + + reference_counter_increased = Gitlab::ReferenceCounter.new(params[:gl_repository]).increase + + { reference_counter_increased: reference_counter_increased } + end + + post "/notify_post_receive" do + status 200 + + # TODO: Re-enable when Gitaly is processing the post-receive notification + # return unless Gitlab::GitalyClient.enabled? + # + # begin + # repository = wiki? ? project.wiki.repository : project.repository + # Gitlab::GitalyClient::NotificationService.new(repository.raw_repository).post_receive + # rescue GRPC::Unavailable => e + # render_api_error!(e, 500) + # end + end + + post '/post_receive' do + status 200 + + response = Gitlab::InternalPostReceive::Response.new + user = identify(params[:identifier]) + project = Gitlab::GlRepository.parse(params[:gl_repository]).first + push_options = Gitlab::PushOptions.new(params[:push_options]) + + response.reference_counter_decreased = Gitlab::ReferenceCounter.new(params[:gl_repository]).decrease + + PostReceive.perform_async(params[:gl_repository], params[:identifier], + params[:changes], push_options.as_json) + + mr_options = push_options.get(:merge_request) + if mr_options.present? + message = process_mr_push_options(mr_options, project, user, params[:changes]) + response.add_alert_message(message) + end + + broadcast_message = BroadcastMessage.current&.last&.message + response.add_alert_message(broadcast_message) + + response.add_merge_request_urls(merge_request_urls) + + # A user is not guaranteed to be returned; an orphaned write deploy + # key could be used + if user + redirect_message = Gitlab::Checks::ProjectMoved.fetch_message(user.id, project.id) + project_created_message = Gitlab::Checks::ProjectCreated.fetch_message(user.id, project.id) + + response.add_basic_message(redirect_message) + response.add_basic_message(project_created_message) + end + + present response, with: Entities::InternalPostReceive::Response + end + end + end + end +end diff --git a/lib/gitlab/git/diff_collection.rb b/lib/gitlab/git/diff_collection.rb index 5c70cb6c66c..cb9154cb1e8 100644 --- a/lib/gitlab/git/diff_collection.rb +++ b/lib/gitlab/git/diff_collection.rb @@ -81,6 +81,12 @@ module Gitlab end end + def line_count + populate! + + @line_count + end + def decorate! collection = each_with_index do |element, i| @array[i] = yield(element) diff --git a/lib/gitlab/jwt_authenticatable.rb b/lib/gitlab/jwt_authenticatable.rb new file mode 100644 index 00000000000..1270a148e8d --- /dev/null +++ b/lib/gitlab/jwt_authenticatable.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module Gitlab + module JwtAuthenticatable + # Supposedly the effective key size for HMAC-SHA256 is 256 bits, i.e. 32 + # bytes https://tools.ietf.org/html/rfc4868#section-2.6 + SECRET_LENGTH = 32 + + def self.included(base) + base.extend(ClassMethods) + end + + module ClassMethods + include Gitlab::Utils::StrongMemoize + + def decode_jwt_for_issuer(issuer, encoded_message) + JWT.decode( + encoded_message, + secret, + true, + { iss: issuer, verify_iss: true, algorithm: 'HS256' } + ) + end + + def secret + strong_memoize(:secret) do + Base64.strict_decode64(File.read(secret_path).chomp).tap do |bytes| + raise "#{secret_path} does not contain #{SECRET_LENGTH} bytes" if bytes.length != SECRET_LENGTH + end + end + end + + def write_secret + bytes = SecureRandom.random_bytes(SECRET_LENGTH) + File.open(secret_path, 'w:BINARY', 0600) do |f| + f.chmod(0600) # If the file already existed, the '0600' passed to 'open' above was a no-op. + f.write(Base64.strict_encode64(bytes)) + end + end + end + end +end diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb index 29087d26007..139ec6e384a 100644 --- a/lib/gitlab/workhorse.rb +++ b/lib/gitlab/workhorse.rb @@ -15,9 +15,7 @@ module Gitlab ALLOWED_GIT_HTTP_ACTIONS = %w[git_receive_pack git_upload_pack info_refs].freeze DETECT_HEADER = 'Gitlab-Workhorse-Detect-Content-Type'.freeze - # Supposedly the effective key size for HMAC-SHA256 is 256 bits, i.e. 32 - # bytes https://tools.ietf.org/html/rfc4868#section-2.6 - SECRET_LENGTH = 32 + include JwtAuthenticatable class << self def git_http_ok(repository, repo_type, user, action, show_all_refs: false) @@ -187,34 +185,12 @@ module Gitlab path.readable? ? path.read.chomp : 'unknown' end - def secret - @secret ||= begin - bytes = Base64.strict_decode64(File.read(secret_path).chomp) - raise "#{secret_path} does not contain #{SECRET_LENGTH} bytes" if bytes.length != SECRET_LENGTH - - bytes - end - end - - def write_secret - bytes = SecureRandom.random_bytes(SECRET_LENGTH) - File.open(secret_path, 'w:BINARY', 0600) do |f| - f.chmod(0600) # If the file already existed, the '0600' passed to 'open' above was a no-op. - f.write(Base64.strict_encode64(bytes)) - end - end - def verify_api_request!(request_headers) decode_jwt(request_headers[INTERNAL_API_REQUEST_HEADER]) end def decode_jwt(encoded_message) - JWT.decode( - encoded_message, - secret, - true, - { iss: 'gitlab-workhorse', verify_iss: true, algorithm: 'HS256' } - ) + decode_jwt_for_issuer('gitlab-workhorse', encoded_message) end def secret_path diff --git a/scripts/review_apps/review-apps.sh b/scripts/review_apps/review-apps.sh index fc5b57451de..a9549171b54 100755 --- a/scripts/review_apps/review-apps.sh +++ b/scripts/review_apps/review-apps.sh @@ -36,7 +36,7 @@ function previous_deploy_failed() { return $status } -function delete() { +function delete_release() { if [ -z "$CI_ENVIRONMENT_SLUG" ]; then echoerr "No release given, aborting the delete!" return @@ -164,7 +164,7 @@ function create_application_secret() { function download_chart() { echoinfo "Downloading the GitLab chart..." true - curl -o gitlab.tar.bz2 "https://gitlab.com/gitlab-org/charts/gitlab/-/archive/${GITLAB_HELM_CHART_REF}/gitlab-${GITLAB_HELM_CHART_REF}.tar.bz2" + curl --location -o gitlab.tar.bz2 "https://gitlab.com/gitlab-org/charts/gitlab/-/archive/${GITLAB_HELM_CHART_REF}/gitlab-${GITLAB_HELM_CHART_REF}.tar.bz2" tar -xjf gitlab.tar.bz2 cd "gitlab-${GITLAB_HELM_CHART_REF}" @@ -193,7 +193,8 @@ function deploy() { HELM_CMD=$(cat << EOF helm upgrade --install \ - --atomic \ + --force \ + --wait \ --timeout 900 \ --set releaseOverride="$CI_ENVIRONMENT_SLUG" \ --set global.appConfig.enableUsagePing=false \ diff --git a/spec/lib/gitlab/git/diff_collection_spec.rb b/spec/lib/gitlab/git/diff_collection_spec.rb index 81658874be7..be6ab0c1200 100644 --- a/spec/lib/gitlab/git/diff_collection_spec.rb +++ b/spec/lib/gitlab/git/diff_collection_spec.rb @@ -74,6 +74,11 @@ describe Gitlab::Git::DiffCollection, :seed_helper do end end + describe '#line_count' do + subject { super().line_count } + it { is_expected.to eq file_count * line_count } + end + context 'when limiting is disabled' do let(:limits) { false } @@ -100,6 +105,11 @@ describe Gitlab::Git::DiffCollection, :seed_helper do expect(subject.size).to eq(3) end end + + describe '#line_count' do + subject { super().line_count } + it { is_expected.to eq file_count * line_count } + end end end @@ -120,6 +130,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do subject { super().real_size } it { is_expected.to eq('0+') } end + + describe '#line_count' do + subject { super().line_count } + it { is_expected.to eq 1000 } + end + it { expect(subject.size).to eq(0) } context 'when limiting is disabled' do @@ -139,6 +155,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do subject { super().real_size } it { is_expected.to eq('3') } end + + describe '#line_count' do + subject { super().line_count } + it { is_expected.to eq file_count * line_count } + end + it { expect(subject.size).to eq(3) } end end @@ -164,6 +186,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do subject { super().real_size } it { is_expected.to eq('10+') } end + + describe '#line_count' do + subject { super().line_count } + it { is_expected.to eq 10 } + end + it { expect(subject.size).to eq(10) } context 'when limiting is disabled' do @@ -183,6 +211,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do subject { super().real_size } it { is_expected.to eq('11') } end + + describe '#line_count' do + subject { super().line_count } + it { is_expected.to eq file_count * line_count } + end + it { expect(subject.size).to eq(11) } end end @@ -204,6 +238,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do subject { super().real_size } it { is_expected.to eq('3+') } end + + describe '#line_count' do + subject { super().line_count } + it { is_expected.to eq 120 } + end + it { expect(subject.size).to eq(3) } context 'when limiting is disabled' do @@ -223,6 +263,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do subject { super().real_size } it { is_expected.to eq('11') } end + + describe '#line_count' do + subject { super().line_count } + it { is_expected.to eq file_count * line_count } + end + it { expect(subject.size).to eq(11) } end end @@ -248,6 +294,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do subject { super().real_size } it { is_expected.to eq('10') } end + + describe '#line_count' do + subject { super().line_count } + it { is_expected.to eq file_count * line_count } + end + it { expect(subject.size).to eq(10) } end end @@ -270,6 +322,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do subject { super().real_size } it { is_expected.to eq('9+') } end + + describe '#line_count' do + subject { super().line_count } + it { is_expected.to eq file_count * line_count } + end + it { expect(subject.size).to eq(9) } context 'when limiting is disabled' do @@ -289,6 +347,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do subject { super().real_size } it { is_expected.to eq('10') } end + + describe '#line_count' do + subject { super().line_count } + it { is_expected.to eq file_count * line_count } + end + it { expect(subject.size).to eq(10) } end end @@ -316,6 +380,11 @@ describe Gitlab::Git::DiffCollection, :seed_helper do subject { super().real_size } it { is_expected.to eq('0')} end + + describe '#line_count' do + subject { super().line_count } + it { is_expected.to eq 0 } + end end describe '#each' do diff --git a/spec/lib/gitlab/jwt_authenticatable_spec.rb b/spec/lib/gitlab/jwt_authenticatable_spec.rb new file mode 100644 index 00000000000..0c1c491b308 --- /dev/null +++ b/spec/lib/gitlab/jwt_authenticatable_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::JwtAuthenticatable do + let(:test_class) do + Class.new do + include Gitlab::JwtAuthenticatable + + def self.secret_path + Rails.root.join('tmp', 'tests', '.jwt_shared_secret') + end + end + end + + before do + begin + File.delete(test_class.secret_path) + rescue Errno::ENOENT + end + + test_class.write_secret + end + + describe '.secret' do + subject(:secret) { test_class.secret } + + it 'returns 32 bytes' do + expect(secret).to be_a(String) + expect(secret.length).to eq(32) + expect(secret.encoding).to eq(Encoding::ASCII_8BIT) + end + + it 'accepts a trailing newline' do + File.open(test_class.secret_path, 'a') { |f| f.write "\n" } + + expect(secret.length).to eq(32) + end + + it 'raises an exception if the secret file cannot be read' do + File.delete(test_class.secret_path) + + expect { secret }.to raise_exception(Errno::ENOENT) + end + + it 'raises an exception if the secret file contains the wrong number of bytes' do + File.truncate(test_class.secret_path, 0) + + expect { secret }.to raise_exception(RuntimeError) + end + end + + describe '.write_secret' do + it 'uses mode 0600' do + expect(File.stat(test_class.secret_path).mode & 0777).to eq(0600) + end + + it 'writes base64 data' do + bytes = Base64.strict_decode64(File.read(test_class.secret_path)) + + expect(bytes).not_to be_empty + end + end + + describe '.decode_jwt_for_issuer' do + let(:payload) { { 'iss' => 'test_issuer' } } + + it 'accepts a correct header' do + encoded_message = JWT.encode(payload, test_class.secret, 'HS256') + + expect { test_class.decode_jwt_for_issuer('test_issuer', encoded_message) }.not_to raise_error + end + + it 'raises an error when the JWT is not signed' do + encoded_message = JWT.encode(payload, nil, 'none') + + expect { test_class.decode_jwt_for_issuer('test_issuer', encoded_message) }.to raise_error(JWT::DecodeError) + end + + it 'raises an error when the header is signed with the wrong secret' do + encoded_message = JWT.encode(payload, 'wrongsecret', 'HS256') + + expect { test_class.decode_jwt_for_issuer('test_issuer', encoded_message) }.to raise_error(JWT::DecodeError) + end + + it 'raises an error when the issuer is incorrect' do + payload['iss'] = 'somebody else' + encoded_message = JWT.encode(payload, test_class.secret, 'HS256') + + expect { test_class.decode_jwt_for_issuer('test_issuer', encoded_message) }.to raise_error(JWT::DecodeError) + end + end +end diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index 98421cd12d3..88bc5034da5 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -200,57 +200,6 @@ describe Gitlab::Workhorse do end end - describe ".secret" do - subject { described_class.secret } - - before do - described_class.instance_variable_set(:@secret, nil) - described_class.write_secret - end - - it 'returns 32 bytes' do - expect(subject).to be_a(String) - expect(subject.length).to eq(32) - expect(subject.encoding).to eq(Encoding::ASCII_8BIT) - end - - it 'accepts a trailing newline' do - File.open(described_class.secret_path, 'a') { |f| f.write "\n" } - expect(subject.length).to eq(32) - end - - it 'raises an exception if the secret file cannot be read' do - File.delete(described_class.secret_path) - expect { subject }.to raise_exception(Errno::ENOENT) - end - - it 'raises an exception if the secret file contains the wrong number of bytes' do - File.truncate(described_class.secret_path, 0) - expect { subject }.to raise_exception(RuntimeError) - end - end - - describe ".write_secret" do - let(:secret_path) { described_class.secret_path } - before do - begin - File.delete(secret_path) - rescue Errno::ENOENT - end - - described_class.write_secret - end - - it 'uses mode 0600' do - expect(File.stat(secret_path).mode & 0777).to eq(0600) - end - - it 'writes base64 data' do - bytes = Base64.strict_decode64(File.read(secret_path)) - expect(bytes).not_to be_empty - end - end - describe '#verify_api_request!' do let(:header_key) { described_class::INTERNAL_API_REQUEST_HEADER } let(:payload) { { 'iss' => 'gitlab-workhorse' } } diff --git a/spec/models/merge_request/metrics_spec.rb b/spec/models/merge_request/metrics_spec.rb index 49573af0fed..bd97cabc11e 100644 --- a/spec/models/merge_request/metrics_spec.rb +++ b/spec/models/merge_request/metrics_spec.rb @@ -3,8 +3,6 @@ require 'spec_helper' describe MergeRequest::Metrics do - subject { described_class.new } - describe 'associations' do it { is_expected.to belong_to(:merge_request) } it { is_expected.to belong_to(:latest_closed_by).class_name('User') } diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index e7dd7287a75..b86663fd7d9 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -400,6 +400,18 @@ describe MergeRequestDiff do end end + describe '#first_commit' do + it 'returns first commit' do + expect(diff_with_commits.first_commit.sha).to eq(diff_with_commits.merge_request_diff_commits.last.sha) + end + end + + describe '#last_commit' do + it 'returns last commit' do + expect(diff_with_commits.last_commit.sha).to eq(diff_with_commits.merge_request_diff_commits.first.sha) + end + end + describe '#commits_by_shas' do let(:commit_shas) { diff_with_commits.commit_shas } @@ -489,7 +501,7 @@ describe MergeRequestDiff do subject { diff_with_commits } it 'returns sum of all changed lines count in diff files' do - expect(subject.lines_count).to eq 109 + expect(subject.lines_count).to eq 189 end end end diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal/base_spec.rb index c94f6d22e74..a56527073c7 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe API::Internal do +describe API::Internal::Base do set(:user) { create(:user) } let(:key) { create(:key, user: user) } set(:project) { create(:project, :repository, :wiki_repo) } |