From d0904892e884f2128e96693bbb2eb1e0a817ddd8 Mon Sep 17 00:00:00 2001 From: Pirate Praveen Date: Thu, 3 Sep 2020 11:15:55 +0530 Subject: [PATCH] New upstream version 13.2.8 --- CHANGELOG.md | 36 +++++ GITALY_SERVER_VERSION | 2 +- Gemfile.lock | 2 +- VERSION | 2 +- .../clusters/clusters_controller.rb | 5 +- .../concerns/authenticates_with_two_factor.rb | 24 +++- .../enforces_two_factor_authentication.rb | 18 +-- app/controllers/concerns/hooks_execution.rb | 12 ++ .../omniauth_callbacks_controller.rb | 11 +- .../profiles/active_sessions_controller.rb | 1 + .../profiles/two_factor_auths_controller.rb | 4 +- app/controllers/projects/hooks_controller.rb | 1 + app/controllers/projects/tags_controller.rb | 2 +- app/controllers/sessions_controller.rb | 13 +- app/finders/ci/auth_job_finder.rb | 56 ++++++++ app/finders/user_recent_events_finder.rb | 12 +- app/graphql/mutations/snippets/base.rb | 2 +- app/models/active_session.rb | 13 ++ app/models/aws/role.rb | 1 + app/models/clusters/applications/runner.rb | 2 +- app/models/member.rb | 2 + app/models/user.rb | 7 + app/services/applications/create_service.rb | 11 +- ...ntainer_registry_authentication_service.rb | 1 + app/services/ci/pipeline_trigger_service.rb | 7 +- .../clusters/aws/authorize_role_service.rb | 16 +-- app/services/members/destroy_service.rb | 33 ++++- .../projects/update_remote_mirror_service.rb | 4 + .../active_sessions/_active_session.html.haml | 2 +- config/initializers/doorkeeper.rb | 2 +- config/routes/user.rb | 1 - ...17040735_change_aws_roles_role_arn_null.rb | 15 ++ ...311_add_o_auth_paths_to_protected_paths.rb | 62 ++++++++ db/structure.sql | 6 +- .../geo/replication/datatypes.md | 18 +++ doc/api/protected_branches.md | 2 +- doc/api/scim.md | 2 +- doc/ci/environments/deployment_safety.md | 2 +- doc/user/group/saml_sso/index.md | 2 +- doc/user/profile/active_sessions.md | 5 + doc/user/profile/index.md | 8 +- .../index.md | 2 +- faraday-middleware-aws-signers-v4/.gitignore | 10 -- faraday-middleware-aws-signers-v4/.rspec | 2 - faraday-middleware-aws-signers-v4/.travis.yml | 12 -- faraday-middleware-aws-signers-v4/Gemfile | 4 - faraday-middleware-aws-signers-v4/LICENSE.txt | 21 --- faraday-middleware-aws-signers-v4/README.md | 67 --------- faraday-middleware-aws-signers-v4/Rakefile | 6 - faraday-middleware-aws-signers-v4/bin/console | 14 -- faraday-middleware-aws-signers-v4/bin/setup | 7 - .../faraday_middleware-aws-signers-v4.gemspec | 28 ---- .../lib/faraday_middleware-aws-signers-v4.rb | 1 - .../lib/faraday_middleware/aws_signers_v4.rb | 8 -- .../faraday_middleware/aws_signers_v4_ext.rb | 13 -- .../lib/faraday_middleware/ext/uri_ext.rb | 25 ---- .../request/aws_signers_v4.rb | 57 -------- lib/api/api_guard.rb | 21 ++- lib/api/badges.rb | 7 +- lib/api/conan_packages.rb | 10 +- lib/api/helpers/badges_helpers.rb | 8 +- lib/api/helpers/packages/conan/api_helpers.rb | 8 +- .../packages_manager_clients_helpers.rb | 2 +- lib/gitlab/application_rate_limiter.rb | 4 +- lib/gitlab/auth.rb | 25 +++- lib/gitlab/auth/auth_finders.rb | 16 ++- lib/gitlab/auth/two_factor_auth_verifier.rb | 36 +++++ lib/gitlab/git_access.rb | 9 +- lib/gitlab/regex.rb | 5 - locale/gitlab.pot | 5 +- package.json | 2 +- .../admin/applications_controller_spec.rb | 16 ++- .../admin/clusters_controller_spec.rb | 51 ++++--- .../application_controller_spec.rb | 9 +- .../groups/clusters_controller_spec.rb | 33 +++-- .../oauth/applications_controller_spec.rb | 23 ++- .../omniauth_callbacks_controller_spec.rb | 16 +++ .../active_sessions_controller_spec.rb | 23 +++ .../two_factor_auths_controller_spec.rb | 17 ++- .../projects/clusters_controller_spec.rb | 33 +++-- .../projects/hooks_controller_spec.rb | 22 +++ spec/controllers/sessions_controller_spec.rb | 32 +++-- .../admin/admin_manage_applications_spec.rb | 18 ++- .../user_manages_applications_spec.rb | 13 ++ spec/features/users/login_spec.rb | 12 +- spec/finders/ci/auth_job_finder_spec.rb | 64 +++++++++ .../finders/user_recent_events_finder_spec.rb | 37 ++++- spec/graphql/gitlab_schema_spec.rb | 49 +++++++ .../packages_manager_clients_helpers_spec.rb | 10 +- spec/lib/gitlab/auth/auth_finders_spec.rb | 46 +++++- .../gitlab/auth/request_authenticator_spec.rb | 11 +- .../auth/two_factor_auth_verifier_spec.rb | 112 +++++++++++++++ spec/lib/gitlab/auth_spec.rb | 72 +++++++++- spec/lib/gitlab/git_access_spec.rb | 132 ++++++++++++++++-- spec/lib/gitlab/regex_spec.rb | 9 -- ...dd_o_auth_paths_to_protected_paths_spec.rb | 52 +++++++ spec/models/active_session_spec.rb | 53 +++++++ spec/models/aws/role_spec.rb | 6 + spec/models/member_spec.rb | 18 ++- spec/models/user_spec.rb | 50 +++++++ spec/requests/api/badges_spec.rb | 28 +++- spec/requests/api/conan_packages_spec.rb | 45 +++++- spec/requests/api/go_proxy_spec.rb | 9 +- .../mutations/snippets/destroy_spec.rb | 25 ++++ spec/requests/api/helpers_spec.rb | 21 +++ spec/requests/api/maven_packages_spec.rb | 33 ++++- spec/requests/api/npm_packages_spec.rb | 11 +- spec/requests/api/releases_spec.rb | 10 +- spec/requests/api/terraform/state_spec.rb | 11 +- .../applications/create_service_spec.rb | 19 ++- ...er_registry_authentication_service_spec.rb | 13 ++ .../ci/pipeline_trigger_service_spec.rb | 18 ++- .../aws/authorize_role_service_spec.rb | 44 +++--- spec/services/members/destroy_service_spec.rb | 40 ++++++ .../update_remote_mirror_service_spec.rb | 34 +++++ .../clusters_controller_shared_examples.rb | 29 ++++ yarn.lock | 8 +- 117 files changed, 1755 insertions(+), 544 deletions(-) create mode 100644 app/finders/ci/auth_job_finder.rb create mode 100644 db/migrate/20200717040735_change_aws_roles_role_arn_null.rb create mode 100644 db/migrate/20200728182311_add_o_auth_paths_to_protected_paths.rb delete mode 100644 faraday-middleware-aws-signers-v4/.gitignore delete mode 100644 faraday-middleware-aws-signers-v4/.rspec delete mode 100644 faraday-middleware-aws-signers-v4/.travis.yml delete mode 100644 faraday-middleware-aws-signers-v4/Gemfile delete mode 100644 faraday-middleware-aws-signers-v4/LICENSE.txt delete mode 100644 faraday-middleware-aws-signers-v4/README.md delete mode 100644 faraday-middleware-aws-signers-v4/Rakefile delete mode 100755 faraday-middleware-aws-signers-v4/bin/console delete mode 100755 faraday-middleware-aws-signers-v4/bin/setup delete mode 100644 faraday-middleware-aws-signers-v4/faraday_middleware-aws-signers-v4.gemspec delete mode 100644 faraday-middleware-aws-signers-v4/lib/faraday_middleware-aws-signers-v4.rb delete mode 100644 faraday-middleware-aws-signers-v4/lib/faraday_middleware/aws_signers_v4.rb delete mode 100644 faraday-middleware-aws-signers-v4/lib/faraday_middleware/aws_signers_v4_ext.rb delete mode 100644 faraday-middleware-aws-signers-v4/lib/faraday_middleware/ext/uri_ext.rb delete mode 100644 faraday-middleware-aws-signers-v4/lib/faraday_middleware/request/aws_signers_v4.rb create mode 100644 lib/gitlab/auth/two_factor_auth_verifier.rb create mode 100644 spec/controllers/profiles/active_sessions_controller_spec.rb create mode 100644 spec/finders/ci/auth_job_finder_spec.rb create mode 100644 spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb create mode 100644 spec/migrations/20200728182311_add_o_auth_paths_to_protected_paths_spec.rb create mode 100644 spec/support/shared_examples/controllers/clusters_controller_shared_examples.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index aeb17e5bb6..037d3dd90d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,42 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 13.2.8 (2020-09-02) + +### Security (1 change) + +- Protect OAuth endpoints from brute force/password stuffing. + + +## 13.2.7 (2020-09-02) + +### Security (23 changes, 1 of them is from the community) + +- Check validity of project's import_url before mirroring repository. +- Show on two-factor authentication setup page groups that are the cause of this requirement. +- Prevent interrupted 2FA sign-in from signing-in incorrect user. +- Create new 2FA code each time user is entering 2FA setup page. +- Remove all sessions but current while enabling 2FA. +- Invalidate two factor sign-in when user password changes. +- Delete members invites created by users being deleted. +- Prevent OmniAuth from rendering arbitrary error messages. +- Prevent not-2fa authenticated users that are supposed to use it to consume api via session. +- Invalidate remember me when an active session is revoked. +- Add rate limit on webhooks testing feature. +- Add scope presence validation to OAuth Application creation. +- Allow only running job tokens for API authentication. +- Prevent Deploy Tokens to read project resources when repository is disabled. +- Change conan api to use proper workhorse validation. +- Ensure global ID is of Snippet type in GraphQL destroy mutation. +- Fix Improper Access Control on Deploy-Key. +- Set maximum limit for profile events. +- Persist EKS External ID before presenting it to the user. +- Prevent project maintainers from editing group badges. +- Upgrade jquery to v3.5. +- Update websocket-extensions gem to 0.1.5. (Vitor Meireles De Sousa) +- Update GitLab Runner Helm Chart to 0.18.3. + + ## 13.2.6 (2020-08-18) - No changes. diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 9a30e11574..f9c1cf2f57 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -13.2.6 +13.2.8 diff --git a/Gemfile.lock b/Gemfile.lock index f211012331..c6771bc4aa 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1153,7 +1153,7 @@ GEM railties (>= 3.2.0) websocket-driver (0.7.1) websocket-extensions (>= 0.1.0) - websocket-extensions (0.1.4) + websocket-extensions (0.1.5) wikicloth (0.8.1) builder expression_parser diff --git a/VERSION b/VERSION index 9a30e11574..f9c1cf2f57 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -13.2.6 +13.2.8 diff --git a/app/controllers/clusters/clusters_controller.rb b/app/controllers/clusters/clusters_controller.rb index 2e8b3d764c..cae9d09879 100644 --- a/app/controllers/clusters/clusters_controller.rb +++ b/app/controllers/clusters/clusters_controller.rb @@ -38,8 +38,7 @@ class Clusters::ClustersController < Clusters::BaseController def new if params[:provider] == 'aws' - @aws_role = current_user.aws_role || Aws::Role.new - @aws_role.ensure_role_external_id! + @aws_role = Aws::Role.create_or_find_by!(user: current_user) @instance_types = load_instance_types.to_json elsif params[:provider] == 'gcp' @@ -273,7 +272,7 @@ class Clusters::ClustersController < Clusters::BaseController end def aws_role_params - params.require(:cluster).permit(:role_arn, :role_external_id) + params.require(:cluster).permit(:role_arn) end def generate_gcp_authorize_url diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb index b885e55f90..d9e4a3a3ae 100644 --- a/app/controllers/concerns/authenticates_with_two_factor.rb +++ b/app/controllers/concerns/authenticates_with_two_factor.rb @@ -22,6 +22,8 @@ module AuthenticatesWithTwoFactor return handle_locked_user(user) unless user.can?(:log_in) session[:otp_user_id] = user.id + session[:user_updated_at] = user.updated_at + setup_u2f_authentication(user) render 'devise/sessions/two_factor' end @@ -41,6 +43,7 @@ module AuthenticatesWithTwoFactor def authenticate_with_two_factor user = self.resource = find_user return handle_locked_user(user) unless user.can?(:log_in) + return handle_changed_user(user) if user_changed?(user) if user_params[:otp_attempt].present? && session[:otp_user_id] authenticate_with_two_factor_via_otp(user) @@ -59,12 +62,14 @@ module AuthenticatesWithTwoFactor def clear_two_factor_attempt! session.delete(:otp_user_id) + session.delete(:user_updated_at) + session.delete(:challenge) end def authenticate_with_two_factor_via_otp(user) if valid_otp_attempt?(user) # Remove any lingering user data from login - session.delete(:otp_user_id) + clear_two_factor_attempt! remember_me(user) if user_params[:remember_me] == '1' user.save! @@ -81,8 +86,7 @@ module AuthenticatesWithTwoFactor def authenticate_with_two_factor_via_u2f(user) if U2fRegistration.authenticate(user, u2f_app_id, user_params[:device_response], session[:challenge]) # Remove any lingering user data from login - session.delete(:otp_user_id) - session.delete(:challenge) + clear_two_factor_attempt! remember_me(user) if user_params[:remember_me] == '1' sign_in(user, message: :two_factor_authenticated, event: :authentication) @@ -109,4 +113,18 @@ module AuthenticatesWithTwoFactor end end # rubocop: enable CodeReuse/ActiveRecord + + def handle_changed_user(user) + clear_two_factor_attempt! + + redirect_to new_user_session_path, alert: _('An error occurred. Please sign in again.') + end + + # If user has been updated since we validated the password, + # the password might have changed. + def user_changed?(user) + return false unless session[:user_updated_at] + + user.updated_at != session[:user_updated_at] + end end diff --git a/app/controllers/concerns/enforces_two_factor_authentication.rb b/app/controllers/concerns/enforces_two_factor_authentication.rb index f1dd46648f..02082f8159 100644 --- a/app/controllers/concerns/enforces_two_factor_authentication.rb +++ b/app/controllers/concerns/enforces_two_factor_authentication.rb @@ -29,12 +29,11 @@ module EnforcesTwoFactorAuthentication end def two_factor_authentication_required? - Gitlab::CurrentSettings.require_two_factor_authentication? || - current_user.try(:require_two_factor_authentication_from_group?) + two_factor_verifier.two_factor_authentication_required? end def current_user_requires_two_factor? - current_user && !current_user.temp_oauth_email? && !current_user.two_factor_enabled? && !skip_two_factor? + two_factor_verifier.current_user_needs_to_setup_two_factor? && !skip_two_factor? end # rubocop: disable CodeReuse/ActiveRecord @@ -43,7 +42,7 @@ module EnforcesTwoFactorAuthentication if Gitlab::CurrentSettings.require_two_factor_authentication? global.call else - groups = current_user.expanded_groups_requiring_two_factor_authentication.reorder(name: :asc) + groups = current_user.source_groups_of_two_factor_authentication_requirement.reorder(name: :asc) group.call(groups) end end @@ -51,14 +50,11 @@ module EnforcesTwoFactorAuthentication # rubocop: enable CodeReuse/ActiveRecord def two_factor_grace_period - periods = [Gitlab::CurrentSettings.two_factor_grace_period] - periods << current_user.two_factor_grace_period if current_user.try(:require_two_factor_authentication_from_group?) - periods.min + two_factor_verifier.two_factor_grace_period end def two_factor_grace_period_expired? - date = current_user.otp_grace_period_started_at - date && (date + two_factor_grace_period.hours) < Time.current + two_factor_verifier.two_factor_grace_period_expired? end def two_factor_skippable? @@ -70,6 +66,10 @@ module EnforcesTwoFactorAuthentication def skip_two_factor? session[:skip_two_factor] && session[:skip_two_factor] > Time.current end + + def two_factor_verifier + @two_factor_verifier ||= Gitlab::Auth::TwoFactorAuthVerifier.new(current_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables + end end EnforcesTwoFactorAuthentication.prepend_if_ee('EE::EnforcesTwoFactorAuthentication') diff --git a/app/controllers/concerns/hooks_execution.rb b/app/controllers/concerns/hooks_execution.rb index e8add1f405..ad1f834110 100644 --- a/app/controllers/concerns/hooks_execution.rb +++ b/app/controllers/concerns/hooks_execution.rb @@ -17,4 +17,16 @@ module HooksExecution flash[:alert] = "Hook execution failed: #{message}" end end + + def create_rate_limit(key, scope) + if rate_limiter.throttled?(key, scope: [scope, current_user]) + rate_limiter.log_request(request, "#{key}_request_limit".to_sym, current_user) + + render plain: _('This endpoint has been requested too many times. Try again later.'), status: :too_many_requests + end + end + + def rate_limiter + ::Gitlab::ApplicationRateLimiter + end end diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 706a484311..7a8c8cff91 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -49,12 +49,6 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController redirect_unverified_saml_initiation end - def omniauth_error - @provider = params[:provider] - @error = params[:error] - render 'errors/omniauth_error', layout: "oauth_error", status: :unprocessable_entity - end - def cas3 ticket = params['ticket'] if ticket @@ -198,9 +192,10 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController end def fail_login(user) - error_message = user.errors.full_messages.to_sentence + @provider = Gitlab::Auth::OAuth::Provider.label_for(params[:action]) + @error = user.errors.full_messages.to_sentence - redirect_to omniauth_error_path(oauth['provider'], error: error_message) + render 'errors/omniauth_error', layout: "oauth_error", status: :unprocessable_entity end def fail_auth0_login diff --git a/app/controllers/profiles/active_sessions_controller.rb b/app/controllers/profiles/active_sessions_controller.rb index f409193aef..d9ec3195fd 100644 --- a/app/controllers/profiles/active_sessions_controller.rb +++ b/app/controllers/profiles/active_sessions_controller.rb @@ -7,6 +7,7 @@ class Profiles::ActiveSessionsController < Profiles::ApplicationController def destroy ActiveSession.destroy_with_public_id(current_user, params[:id]) + current_user.forget_me! respond_to do |format| format.html { redirect_to profile_active_sessions_url, status: :found } diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb index 95b9344c55..50fbf8146e 100644 --- a/app/controllers/profiles/two_factor_auths_controller.rb +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -4,7 +4,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController skip_before_action :check_two_factor_requirement def show - unless current_user.otp_secret + unless current_user.two_factor_enabled? current_user.otp_secret = User.generate_otp_secret(32) end @@ -38,6 +38,8 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController def create if current_user.validate_and_consume_otp!(params[:pin_code]) + ActiveSession.destroy_all_but_current(current_user, session) + Users::UpdateService.new(current_user, user: current_user, otp_required_for_login: true).execute! do |user| @codes = user.generate_otp_backup_codes! end diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb index 097a357889..2f4dc1ddc3 100644 --- a/app/controllers/projects/hooks_controller.rb +++ b/app/controllers/projects/hooks_controller.rb @@ -6,6 +6,7 @@ class Projects::HooksController < Projects::ApplicationController # Authorize before_action :authorize_admin_project! before_action :hook_logs, only: :edit + before_action -> { create_rate_limit(:project_testing_hook, @project) }, only: :test respond_to :html diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb index df20daa8f7..475ca8894e 100644 --- a/app/controllers/projects/tags_controller.rb +++ b/app/controllers/projects/tags_controller.rb @@ -92,7 +92,7 @@ class Projects::TagsController < Projects::ApplicationController end format.js do - render status: :unprocessable_entity + render status: :ok end end end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 9e8075d4bc..4e1228dec3 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -8,6 +8,7 @@ class SessionsController < Devise::SessionsController include Recaptcha::Verify include RendersLdapServers include KnownSignIn + include Gitlab::Utils::StrongMemoize skip_before_action :check_two_factor_requirement, only: [:destroy] # replaced with :require_no_authentication_without_flash @@ -197,10 +198,14 @@ class SessionsController < Devise::SessionsController end def find_user - if session[:otp_user_id] - User.find(session[:otp_user_id]) - elsif user_params[:login] - User.by_login(user_params[:login]) + strong_memoize(:find_user) do + if session[:otp_user_id] && user_params[:login] + User.by_id_and_login(session[:otp_user_id], user_params[:login]).first + elsif session[:otp_user_id] + User.find(session[:otp_user_id]) + elsif user_params[:login] + User.by_login(user_params[:login]) + end end end diff --git a/app/finders/ci/auth_job_finder.rb b/app/finders/ci/auth_job_finder.rb new file mode 100644 index 0000000000..aee7dd1634 --- /dev/null +++ b/app/finders/ci/auth_job_finder.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true +module Ci + class AuthJobFinder + AuthError = Class.new(StandardError) + NotRunningJobError = Class.new(AuthError) + ErasedJobError = Class.new(AuthError) + DeletedProjectError = Class.new(AuthError) + + def initialize(token:) + @token = token + end + + def execute! + find_job_by_token.tap do |job| + next unless job + + validate_job!(job) + end + end + + def execute + execute! + rescue AuthError + end + + private + + attr_reader :token, :require_running, :raise_on_missing + + def find_job_by_token + ::Ci::Build.find_by_token(token) + end + + def validate_job!(job) + validate_running_job!(job) + validate_job_not_erased!(job) + validate_project_presence!(job) + + true + end + + def validate_running_job!(job) + raise NotRunningJobError, 'Job is not running' unless job.running? + end + + def validate_job_not_erased!(job) + raise ErasedJobError, 'Job has been erased!' if job.erased? + end + + def validate_project_presence!(job) + if job.project.nil? || job.project.pending_delete? + raise DeletedProjectError, 'Project has been deleted!' + end + end + end +end diff --git a/app/finders/user_recent_events_finder.rb b/app/finders/user_recent_events_finder.rb index 3f2e813d38..f376b26ab9 100644 --- a/app/finders/user_recent_events_finder.rb +++ b/app/finders/user_recent_events_finder.rb @@ -6,6 +6,7 @@ # WARNING: does not consider project feature visibility! # - user: The user for which to load the events # - params: +# - limit: Number of items that to be returned. Defaults to 20 and limited to 100. # - offset: The page of events to return class UserRecentEventsFinder prepend FinderWithCrossProjectAccess @@ -16,7 +17,8 @@ class UserRecentEventsFinder attr_reader :current_user, :target_user, :params - LIMIT = 20 + DEFAULT_LIMIT = 20 + MAX_LIMIT = 100 def initialize(current_user, target_user, params = {}) @current_user = current_user @@ -31,7 +33,7 @@ class UserRecentEventsFinder recent_events(params[:offset] || 0) .joins(:project) .with_associations - .limit_recent(params[:limit].presence || LIMIT, params[:offset]) + .limit_recent(limit, params[:offset]) end # rubocop: enable CodeReuse/ActiveRecord @@ -59,4 +61,10 @@ class UserRecentEventsFinder def projects target_user.project_interactions.to_sql end + + def limit + return DEFAULT_LIMIT unless params[:limit].present? + + [params[:limit].to_i, MAX_LIMIT].min + end end diff --git a/app/graphql/mutations/snippets/base.rb b/app/graphql/mutations/snippets/base.rb index c8cc721b2e..023f876d03 100644 --- a/app/graphql/mutations/snippets/base.rb +++ b/app/graphql/mutations/snippets/base.rb @@ -11,7 +11,7 @@ module Mutations private def find_object(id:) - GitlabSchema.object_from_id(id) + GitlabSchema.object_from_id(id, expected_type: ::Snippet) end def authorized_resource?(snippet) diff --git a/app/models/active_session.rb b/app/models/active_session.rb index be07c221f3..4908290e06 100644 --- a/app/models/active_session.rb +++ b/app/models/active_session.rb @@ -105,6 +105,19 @@ class ActiveSession end end + def self.destroy_all_but_current(user, current_session) + session_ids = not_impersonated(user) + session_ids.reject! { |session| session.current?(current_session) } if current_session + + Gitlab::Redis::SharedState.with do |redis| + destroy_sessions(redis, user, session_ids.map(&:session_id)) if session_ids.any? + end + end + + def self.not_impersonated(user) + list(user).reject(&:is_impersonated) + end + def self.key_name(user_id, session_id = '*') "#{Gitlab::Redis::SharedState::USER_SESSIONS_NAMESPACE}:#{user_id}:#{session_id}" end diff --git a/app/models/aws/role.rb b/app/models/aws/role.rb index 54132be749..7d34665082 100644 --- a/app/models/aws/role.rb +++ b/app/models/aws/role.rb @@ -9,6 +9,7 @@ module Aws validates :role_external_id, uniqueness: true, length: { in: 1..64 } validates :role_arn, length: 1..2048, + allow_nil: true, format: { with: Gitlab::Regex.aws_arn_regex, message: Gitlab::Regex.aws_arn_regex_message diff --git a/app/models/clusters/applications/runner.rb b/app/models/clusters/applications/runner.rb index 3f0b4edde3..5548651ebe 100644 --- a/app/models/clusters/applications/runner.rb +++ b/app/models/clusters/applications/runner.rb @@ -3,7 +3,7 @@ module Clusters module Applications class Runner < ApplicationRecord - VERSION = '0.18.2' + VERSION = '0.18.3' self.table_name = 'clusters_applications_runners' diff --git a/app/models/member.rb b/app/models/member.rb index 36f9741ce0..64453e6f24 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -76,6 +76,8 @@ class Member < ApplicationRecord scope :request, -> { where.not(requested_at: nil) } scope :non_request, -> { where(requested_at: nil) } + scope :not_accepted_invitations_by_user, -> (user) { invite.where(invite_accepted_at: nil, created_by: user) } + scope :has_access, -> { active.where('access_level > 0') } scope :guests, -> { active.where(access_level: GUEST) } diff --git a/app/models/user.rb b/app/models/user.rb index 643b759e6f..f73b3be794 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -354,6 +354,7 @@ class User < ApplicationRecord scope :order_oldest_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'ASC')) } scope :order_recent_last_activity, -> { reorder(Gitlab::Database.nulls_last_order('last_activity_on', 'DESC')) } scope :order_oldest_last_activity, -> { reorder(Gitlab::Database.nulls_first_order('last_activity_on', 'ASC')) } + scope :by_id_and_login, ->(id, login) { where(id: id).where('username = LOWER(:login) OR email = LOWER(:login)', login: login) } def active_for_authentication? super && can?(:log_in) @@ -871,6 +872,12 @@ class User < ApplicationRecord all_expanded_groups.where(require_two_factor_authentication: true) end + def source_groups_of_two_factor_authentication_requirement + Gitlab::ObjectHierarchy.new(expanded_groups_requiring_two_factor_authentication) + .all_objects + .where(id: groups) + end + # rubocop: disable CodeReuse/ServiceClass def refresh_authorized_projects Users::RefreshAuthorizedProjectsService.new(self).execute diff --git a/app/services/applications/create_service.rb b/app/services/applications/create_service.rb index 500db1e172..92500fbc25 100644 --- a/app/services/applications/create_service.rb +++ b/app/services/applications/create_service.rb @@ -11,7 +11,16 @@ module Applications # EE would override and use `request` arg def execute(request) - Doorkeeper::Application.create(params) + @application = Doorkeeper::Application.new(params) + + unless params[:scopes].present? + @application.errors.add(:base, _("Scopes can't be blank")) + + return @application + end + + @application.save + @application end end end diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 44a434f440..831a25a637 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -132,6 +132,7 @@ module Auth def can_access?(requested_project, requested_action) return false unless requested_project.container_registry_enabled? + return false if requested_project.repository_access_level == ::ProjectFeature::DISABLED case requested_action when 'pull' diff --git a/app/services/ci/pipeline_trigger_service.rb b/app/services/ci/pipeline_trigger_service.rb index 37b9b4c362..d9f41b7040 100644 --- a/app/services/ci/pipeline_trigger_service.rb +++ b/app/services/ci/pipeline_trigger_service.rb @@ -10,6 +10,9 @@ module Ci elsif job_from_token create_pipeline_from_job(job_from_token) end + + rescue Ci::AuthJobFinder::AuthError => e + error(e.message, 401) end private @@ -41,8 +44,6 @@ module Ci # this check is to not leak the presence of the project if user cannot read it return unless can?(job.user, :read_project, project) - return error("400 Job has to be running", 400) unless job.running? - pipeline = Ci::CreatePipelineService.new(project, job.user, ref: params[:ref]) .execute(:pipeline, ignore_skip_ci: true) do |pipeline| source = job.sourced_pipelines.build( @@ -64,7 +65,7 @@ module Ci def job_from_token strong_memoize(:job) do - Ci::Build.find_by_token(params[:token].to_s) + Ci::AuthJobFinder.new(token: params[:token].to_s).execute! end end diff --git a/app/services/clusters/aws/authorize_role_service.rb b/app/services/clusters/aws/authorize_role_service.rb index 6eafce0597..73e31c417e 100644 --- a/app/services/clusters/aws/authorize_role_service.rb +++ b/app/services/clusters/aws/authorize_role_service.rb @@ -9,6 +9,7 @@ module Clusters ERRORS = [ ActiveRecord::RecordInvalid, + ActiveRecord::RecordNotFound, Clusters::Aws::FetchCredentialsService::MissingRoleError, ::Aws::Errors::MissingCredentialsError, ::Aws::STS::Errors::ServiceError @@ -20,7 +21,8 @@ module Clusters end def execute - @role = create_or_update_role! + ensure_role_exists! + update_role_arn! Response.new(:ok, credentials) rescue *ERRORS @@ -31,14 +33,12 @@ module Clusters attr_reader :role, :params - def create_or_update_role! - if role = user.aws_role - role.update!(params) + def ensure_role_exists! + @role = ::Aws::Role.find_by_user_id!(user.id) + end - role - else - user.create_aws_role!(params) - end + def update_role_arn! + role.update!(params) end def credentials diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index fdd4326052..8cad065e6c 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -18,6 +18,7 @@ module Members end delete_subresources(member) unless skip_subresources + delete_project_invitations_by(member) unless skip_subresources enqueue_delete_todos(member) enqueue_unassign_issuables(member) if unassign_issuables @@ -39,24 +40,48 @@ module Members delete_project_members(member) delete_subgroup_members(member) + delete_invited_members(member) end def delete_project_members(member) groups = member.group.self_and_descendants - ProjectMember.in_namespaces(groups).with_user(member.user).each do |project_member| - self.class.new(current_user).execute(project_member, skip_authorization: @skip_auth) - end + destroy_project_members(ProjectMember.in_namespaces(groups).with_user(member.user)) end def delete_subgroup_members(member) groups = member.group.descendants - GroupMember.of_groups(groups).with_user(member.user).each do |group_member| + destroy_group_members(GroupMember.of_groups(groups).with_user(member.user)) + end + + def delete_invited_members(member) + groups = member.group.self_and_descendants + + destroy_group_members(GroupMember.of_groups(groups).not_accepted_invitations_by_user(member.user)) + + destroy_project_members(ProjectMember.in_namespaces(groups).not_accepted_invitations_by_user(member.user)) + end + + def destroy_project_members(members) + members.each do |project_member| + self.class.new(current_user).execute(project_member, skip_authorization: @skip_auth) + end + end + + def destroy_group_members(members) + members.each do |group_member| self.class.new(current_user).execute(group_member, skip_authorization: @skip_auth, skip_subresources: true) end end + def delete_project_invitations_by(member) + return unless member.is_a?(ProjectMember) && member.user && member.project + + members_to_delete = member.project.members.not_accepted_invitations_by_user(member.user) + destroy_project_members(members_to_delete) + end + def can_destroy_member?(member) can?(current_user, destroy_member_permission(member), member) end diff --git a/app/services/projects/update_remote_mirror_service.rb b/app/services/projects/update_remote_mirror_service.rb index d6c0d64746..769c65583c 100644 --- a/app/services/projects/update_remote_mirror_service.rb +++ b/app/services/projects/update_remote_mirror_service.rb @@ -7,6 +7,10 @@ module Projects def execute(remote_mirror, tries) return success unless remote_mirror.enabled? + if Gitlab::UrlBlocker.blocked_url?(CGI.unescape(Gitlab::UrlSanitizer.sanitize(remote_mirror.url))) + return error("The remote mirror URL is invalid.") + end + update_mirror(remote_mirror) success diff --git a/app/views/profiles/active_sessions/_active_session.html.haml b/app/views/profiles/active_sessions/_active_session.html.haml index 9ae75fe6b8..c4b3fa80d7 100644 --- a/app/views/profiles/active_sessions/_active_session.html.haml +++ b/app/views/profiles/active_sessions/_active_session.html.haml @@ -27,6 +27,6 @@ - unless is_current_session .float-right - = link_to profile_active_session_path(active_session.public_id), data: { confirm: _('Are you sure? The device will be signed out of GitLab.') }, method: :delete, class: "btn btn-danger gl-ml-3" do + = link_to profile_active_session_path(active_session.public_id), data: { confirm: _('Are you sure? The device will be signed out of GitLab and all remember me tokens revoked.') }, method: :delete, class: "btn btn-danger gl-ml-3" do %span.sr-only= _('Revoke') = _('Revoke') diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 76e29fb6c0..ad0b0c2008 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -17,7 +17,7 @@ Doorkeeper.configure do end resource_owner_from_credentials do |routes| - user = Gitlab::Auth.find_with_user_password(params[:username], params[:password]) + user = Gitlab::Auth.find_with_user_password(params[:username], params[:password], increment_failed_attempts: true) user unless user.try(:two_factor_enabled?) end diff --git a/config/routes/user.rb b/config/routes/user.rb index 3bf13908d3..c7a5a56d9e 100644 --- a/config/routes/user.rb +++ b/config/routes/user.rb @@ -25,7 +25,6 @@ devise_for :users, controllers: { omniauth_callbacks: :omniauth_callbacks, confirmations: :confirmations } devise_scope :user do - get '/users/auth/:provider/omniauth_error' => 'omniauth_callbacks#omniauth_error', as: :omniauth_error get '/users/almost_there' => 'confirmations#almost_there' end diff --git a/db/migrate/20200717040735_change_aws_roles_role_arn_null.rb b/db/migrate/20200717040735_change_aws_roles_role_arn_null.rb new file mode 100644 index 0000000000..707cfe96a7 --- /dev/null +++ b/db/migrate/20200717040735_change_aws_roles_role_arn_null.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class ChangeAwsRolesRoleArnNull < ActiveRecord::Migration[6.0] + DOWNTIME = false + EXAMPLE_ARN = 'arn:aws:iam::000000000000:role/example-role' + + def up + change_column_null :aws_roles, :role_arn, true + end + + def down + # Records may now exist with nulls, so we must fill them with a dummy value + change_column_null :aws_roles, :role_arn, false, EXAMPLE_ARN + end +end diff --git a/db/migrate/20200728182311_add_o_auth_paths_to_protected_paths.rb b/db/migrate/20200728182311_add_o_auth_paths_to_protected_paths.rb new file mode 100644 index 0000000000..7a5af0135f --- /dev/null +++ b/db/migrate/20200728182311_add_o_auth_paths_to_protected_paths.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +class AddOAuthPathsToProtectedPaths < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + ADD_PROTECTED_PATHS = [ + '/oauth/authorize', + '/oauth/token' + ].freeze + + EXISTING_DEFAULT_PROTECTED_PATHS = [ + '/users/password', + '/users/sign_in', + '/api/v3/session.json', + '/api/v3/session', + '/api/v4/session.json', + '/api/v4/session', + '/users', + '/users/confirmation', + '/unsubscribes/', + '/import/github/personal_access_token', + '/admin/session' + ].freeze + + NEW_DEFAULT_PROTECTED_PATHS = (EXISTING_DEFAULT_PROTECTED_PATHS + ADD_PROTECTED_PATHS).freeze + + class ApplicationSetting < ActiveRecord::Base + self.table_name = 'application_settings' + end + + def up + change_column_default :application_settings, :protected_paths, NEW_DEFAULT_PROTECTED_PATHS + + ApplicationSetting.reset_column_information + + ApplicationSetting.where.not(protected_paths: nil).each do |application_setting| + missing_paths = ADD_PROTECTED_PATHS - application_setting.protected_paths + + next if missing_paths.empty? + + updated_protected_paths = application_setting.protected_paths + missing_paths + application_setting.update!(protected_paths: updated_protected_paths) + end + end + + def down + change_column_default :application_settings, :protected_paths, EXISTING_DEFAULT_PROTECTED_PATHS + + ApplicationSetting.reset_column_information + + ApplicationSetting.where.not(protected_paths: nil).each do |application_setting| + paths_to_remove = application_setting.protected_paths - EXISTING_DEFAULT_PROTECTED_PATHS + + next if paths_to_remove.empty? + + updated_protected_paths = application_setting.protected_paths - paths_to_remove + application_setting.update!(protected_paths: updated_protected_paths) + end + end +end diff --git a/db/structure.sql b/db/structure.sql index a1b294e843..3c8a6a74c8 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9099,7 +9099,7 @@ CREATE TABLE public.application_settings ( throttle_protected_paths_enabled boolean DEFAULT false NOT NULL, throttle_protected_paths_requests_per_period integer DEFAULT 10 NOT NULL, throttle_protected_paths_period_in_seconds integer DEFAULT 60 NOT NULL, - protected_paths character varying(255)[] DEFAULT '{/users/password,/users/sign_in,/api/v3/session.json,/api/v3/session,/api/v4/session.json,/api/v4/session,/users,/users/confirmation,/unsubscribes/,/import/github/personal_access_token,/admin/session}'::character varying[], + protected_paths character varying(255)[] DEFAULT '{/users/password,/users/sign_in,/api/v3/session.json,/api/v3/session,/api/v4/session.json,/api/v4/session,/users,/users/confirmation,/unsubscribes/,/import/github/personal_access_token,/admin/session,/oauth/authorize,/oauth/token}'::character varying[], throttle_incident_management_notification_enabled boolean DEFAULT false NOT NULL, throttle_incident_management_notification_period_in_seconds integer DEFAULT 3600, throttle_incident_management_notification_per_period integer DEFAULT 3600, @@ -9433,7 +9433,7 @@ CREATE TABLE public.aws_roles ( user_id integer NOT NULL, created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, - role_arn character varying(2048) NOT NULL, + role_arn character varying(2048), role_external_id character varying(64) NOT NULL ); @@ -23867,5 +23867,7 @@ COPY "schema_migrations" (version) FROM STDIN; 20200715202659 20200716044023 20200716120419 +20200717040735 +20200728182311 \. diff --git a/doc/administration/geo/replication/datatypes.md b/doc/administration/geo/replication/datatypes.md index 5636ff7918..8fb62580bb 100644 --- a/doc/administration/geo/replication/datatypes.md +++ b/doc/administration/geo/replication/datatypes.md @@ -169,3 +169,21 @@ successfully, you must replicate their data using some other means. LFS objects from replicating](https://gitlab.com/gitlab-org/gitlab/-/issues/32696). - (*3*): If you are using Object Storage, the replication can be performed by the Object Storage provider if supported. Please see [Geo with Object Storage](object_storage.md) + +### Package file replication behind a feature flag + +Package file replication is behind a feature flag - `geo_self_service_framework_replication`, which is enabled by default. + +For GitLab self-managed instances, GitLab administrators can opt to disable them. + +To disable: + +```ruby +Feature.disable(:geo_self_service_framework_replication) +``` + +To enable: + +```ruby +Feature.enable(:geo_self_service_framework_replication) +``` diff --git a/doc/api/protected_branches.md b/doc/api/protected_branches.md index 4206fe6a56..a1f42958c3 100644 --- a/doc/api/protected_branches.md +++ b/doc/api/protected_branches.md @@ -248,7 +248,7 @@ Example response: ### Example with user / group level access **(STARTER)** Elements in the `allowed_to_push` / `allowed_to_merge` / `allowed_to_unprotect` array should take the -form `{user_id: integer}`, `{group_id: integer}` or `{access_level: integer}`. Each user must have access to the project and each group must [have this project shared](../user/project/members/share_project_with_groups.md). These access levels allow [more granular control over protected branch access](../user/project/protected_branches.md#restricting-push-and-merge-access-to-certain-users-starter) and were [added to the API in](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/3516) in GitLab 10.3 EE. +form `{user_id: integer}`, `{group_id: integer}` or `{access_level: integer}`. Each user must have access to the project and each group must [have this project shared](../user/project/members/share_project_with_groups.md). These access levels allow [more granular control over protected branch access](../user/project/protected_branches.md#restricting-push-and-merge-access-to-certain-users-starter) and were [added to the API in](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/3516) GitLab 10.3 EE. ```shell curl --request POST --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/projects/5/protected_branches?name=*-stable&allowed_to_push%5B%5D%5Buser_id%5D=1" diff --git a/doc/api/scim.md b/doc/api/scim.md index 7c8da37a94..bd86408ee6 100644 --- a/doc/api/scim.md +++ b/doc/api/scim.md @@ -2,7 +2,7 @@ > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/9388) in [GitLab Silver](https://about.gitlab.com/pricing/) 11.10. -The SCIM API implements the [the RFC7644 protocol](https://tools.ietf.org/html/rfc7644). +The SCIM API implements [the RFC7644 protocol](https://tools.ietf.org/html/rfc7644). CAUTION: **Caution:** This API is for internal system use for connecting with a SCIM provider. While it can be used directly, it is subject to change without notice. diff --git a/doc/ci/environments/deployment_safety.md b/doc/ci/environments/deployment_safety.md index a4ff164a5b..c993fd5f5d 100644 --- a/doc/ci/environments/deployment_safety.md +++ b/doc/ci/environments/deployment_safety.md @@ -26,7 +26,7 @@ Pipeline jobs in GitLab CI/CD run in parallel, so it's possible that two deploym jobs in two different pipelines attempt to deploy to the same environment at the same time. This is not desired behavior as deployments should happen sequentially. -You can ensure only one deployment job runs at a time with the [`resource_group` keyword](../yaml/README.md#resource_group) keyword in your `.gitlab-ci.yml`. +You can ensure only one deployment job runs at a time with the [`resource_group` keyword](../yaml/README.md#resource_group) in your `.gitlab-ci.yml`. For example: diff --git a/doc/user/group/saml_sso/index.md b/doc/user/group/saml_sso/index.md index afd676cf89..756eb66fd2 100644 --- a/doc/user/group/saml_sso/index.md +++ b/doc/user/group/saml_sso/index.md @@ -259,7 +259,7 @@ Group SAML SSO helps if you need to allow access via multiple SAML identity prov To proceed with configuring Group SAML SSO instead, you'll need to enable the `group_saml` OmniAuth provider. This can be done from: -- `gitlab.rb` for GitLab [Omnibus installations](#omnibus-installations). +- `gitlab.rb` for [Omnibus GitLab installations](#omnibus-installations). - `gitlab/config/gitlab.yml` for [source installations](#source-installations). ### Limitations diff --git a/doc/user/profile/active_sessions.md b/doc/user/profile/active_sessions.md index 4dbb11b581..a5b15a7880 100644 --- a/doc/user/profile/active_sessions.md +++ b/doc/user/profile/active_sessions.md @@ -29,6 +29,11 @@ exceeds 100, the oldest ones are deleted. 1. Use the previous steps to navigate to **Active Sessions**. 1. Click on **Revoke** besides a session. The current session cannot be revoked, as this would sign you out of GitLab. +NOTE: **Note:** +When any session is revoked all **Remember me** tokens for all +devices will be revoked. See ['Why do I keep getting signed out?'](index.md#why-do-i-keep-getting-signed-out) +for more information about the **Remember me** feature. +