summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Release Tools Bot <delivery-team+release-tools@gitlab.com>2020-09-02 12:39:03 +0000
committerGitLab Release Tools Bot <delivery-team+release-tools@gitlab.com>2020-09-02 12:39:03 +0000
commit90432d32acd69cf91e647fc508045659cae26b1a (patch)
tree2b2da74ca70007a0343a131ed187dcdbdbfeb7dd
parentf4a969f7f495978a7e656c69c929c9fdac111cff (diff)
parent417a126de5e49fb7c63bb3f23b22bc4a484ac359 (diff)
downloadgitlab-ce-90432d32acd69cf91e647fc508045659cae26b1a.tar.gz
Merge remote-tracking branch 'dev/13-3-stable' into 13-3-stable
-rw-r--r--CHANGELOG.md29
-rw-r--r--GITALY_SERVER_VERSION2
-rw-r--r--Gemfile.lock2
-rw-r--r--VERSION2
-rw-r--r--app/controllers/clusters/clusters_controller.rb5
-rw-r--r--app/controllers/concerns/authenticates_with_two_factor.rb24
-rw-r--r--app/controllers/concerns/enforces_two_factor_authentication.rb18
-rw-r--r--app/controllers/concerns/hooks_execution.rb12
-rw-r--r--app/controllers/omniauth_callbacks_controller.rb11
-rw-r--r--app/controllers/profiles/active_sessions_controller.rb1
-rw-r--r--app/controllers/profiles/two_factor_auths_controller.rb4
-rw-r--r--app/controllers/projects/hooks_controller.rb1
-rw-r--r--app/controllers/projects/tags_controller.rb2
-rw-r--r--app/controllers/sessions_controller.rb13
-rw-r--r--app/finders/ci/auth_job_finder.rb56
-rw-r--r--app/finders/user_recent_events_finder.rb12
-rw-r--r--app/graphql/mutations/snippets/base.rb2
-rw-r--r--app/models/active_session.rb13
-rw-r--r--app/models/aws/role.rb1
-rw-r--r--app/models/clusters/applications/runner.rb2
-rw-r--r--app/models/member.rb2
-rw-r--r--app/models/user.rb7
-rw-r--r--app/services/applications/create_service.rb11
-rw-r--r--app/services/auth/container_registry_authentication_service.rb1
-rw-r--r--app/services/ci/pipeline_trigger_service.rb7
-rw-r--r--app/services/clusters/aws/authorize_role_service.rb16
-rw-r--r--app/services/members/destroy_service.rb33
-rw-r--r--app/services/projects/update_remote_mirror_service.rb4
-rw-r--r--app/views/profiles/active_sessions/_active_session.html.haml2
-rw-r--r--config/routes/user.rb1
-rw-r--r--db/migrate/20200717040735_change_aws_roles_role_arn_null.rb15
-rw-r--r--db/schema_migrations/202007170407351
-rw-r--r--db/structure.sql2
-rw-r--r--doc/user/profile/active_sessions.md5
-rw-r--r--doc/user/profile/index.md8
-rw-r--r--lib/api/api_guard.rb21
-rw-r--r--lib/api/badges.rb7
-rw-r--r--lib/api/conan_packages.rb10
-rw-r--r--lib/api/helpers/badges_helpers.rb8
-rw-r--r--lib/api/helpers/packages/conan/api_helpers.rb8
-rw-r--r--lib/api/helpers/packages_manager_clients_helpers.rb2
-rw-r--r--lib/gitlab/application_rate_limiter.rb4
-rw-r--r--lib/gitlab/auth.rb2
-rw-r--r--lib/gitlab/auth/auth_finders.rb16
-rw-r--r--lib/gitlab/auth/two_factor_auth_verifier.rb36
-rw-r--r--lib/gitlab/git_access.rb9
-rw-r--r--lib/gitlab/regex.rb5
-rw-r--r--locale/gitlab.pot5
-rw-r--r--package.json2
-rw-r--r--spec/controllers/admin/applications_controller_spec.rb16
-rw-r--r--spec/controllers/admin/clusters_controller_spec.rb51
-rw-r--r--spec/controllers/application_controller_spec.rb9
-rw-r--r--spec/controllers/groups/clusters_controller_spec.rb33
-rw-r--r--spec/controllers/oauth/applications_controller_spec.rb23
-rw-r--r--spec/controllers/omniauth_callbacks_controller_spec.rb16
-rw-r--r--spec/controllers/profiles/active_sessions_controller_spec.rb23
-rw-r--r--spec/controllers/profiles/two_factor_auths_controller_spec.rb17
-rw-r--r--spec/controllers/projects/clusters_controller_spec.rb33
-rw-r--r--spec/controllers/projects/hooks_controller_spec.rb22
-rw-r--r--spec/controllers/sessions_controller_spec.rb32
-rw-r--r--spec/features/admin/admin_manage_applications_spec.rb18
-rw-r--r--spec/features/profiles/user_manages_applications_spec.rb13
-rw-r--r--spec/features/users/login_spec.rb12
-rw-r--r--spec/finders/ci/auth_job_finder_spec.rb64
-rw-r--r--spec/finders/user_recent_events_finder_spec.rb37
-rw-r--r--spec/graphql/gitlab_schema_spec.rb49
-rw-r--r--spec/lib/api/helpers/packages_manager_clients_helpers_spec.rb10
-rw-r--r--spec/lib/gitlab/auth/auth_finders_spec.rb46
-rw-r--r--spec/lib/gitlab/auth/request_authenticator_spec.rb11
-rw-r--r--spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb112
-rw-r--r--spec/lib/gitlab/auth_spec.rb15
-rw-r--r--spec/lib/gitlab/git_access_spec.rb132
-rw-r--r--spec/lib/gitlab/regex_spec.rb9
-rw-r--r--spec/models/active_session_spec.rb53
-rw-r--r--spec/models/aws/role_spec.rb6
-rw-r--r--spec/models/member_spec.rb13
-rw-r--r--spec/models/user_spec.rb50
-rw-r--r--spec/requests/api/badges_spec.rb26
-rw-r--r--spec/requests/api/conan_packages_spec.rb45
-rw-r--r--spec/requests/api/go_proxy_spec.rb9
-rw-r--r--spec/requests/api/graphql/mutations/snippets/destroy_spec.rb25
-rw-r--r--spec/requests/api/helpers_spec.rb21
-rw-r--r--spec/requests/api/maven_packages_spec.rb33
-rw-r--r--spec/requests/api/npm_packages_spec.rb11
-rw-r--r--spec/requests/api/nuget_packages_spec.rb2
-rw-r--r--spec/requests/api/releases_spec.rb10
-rw-r--r--spec/requests/api/terraform/state_spec.rb11
-rw-r--r--spec/services/applications/create_service_spec.rb19
-rw-r--r--spec/services/auth/container_registry_authentication_service_spec.rb13
-rw-r--r--spec/services/ci/pipeline_trigger_service_spec.rb18
-rw-r--r--spec/services/clusters/aws/authorize_role_service_spec.rb44
-rw-r--r--spec/services/members/destroy_service_spec.rb40
-rw-r--r--spec/services/projects/update_remote_mirror_service_spec.rb34
-rw-r--r--spec/support/shared_examples/controllers/clusters_controller_shared_examples.rb29
-rw-r--r--yarn.lock8
95 files changed, 1527 insertions, 258 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 60e1d7820da..89c328b93d3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,35 @@
documentation](doc/development/changelog.md) for instructions on adding your own
entry.
+## 13.3.3 (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.19.3.
+
+
## 13.3.2 (2020-08-28)
### Removed (1 change)
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index 6f292ccbb60..35f1a3e990f 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-13.3.2 \ No newline at end of file
+13.3.3 \ No newline at end of file
diff --git a/Gemfile.lock b/Gemfile.lock
index 1202f3f4cd9..640ac2558c0 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -1199,7 +1199,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 f681f251a16..4be057655b4 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-13.3.2
+13.3.3
diff --git a/app/controllers/clusters/clusters_controller.rb b/app/controllers/clusters/clusters_controller.rb
index 2e8b3d764ca..cae9d098799 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 4b4bcc8d37e..2cc51c65c26 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
@@ -39,6 +41,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)
@@ -63,12 +66,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!
@@ -85,8 +90,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)
@@ -113,4 +117,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 f1dd46648f1..02082f81598 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 e8add1f4055..ad1f8341109 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 6a393405e4d..a558b01f0c6 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -50,12 +50,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
@@ -205,9 +199,10 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
def fail_login(user)
log_failed_login(user.username, oauth['provider'])
- 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 f409193aefc..d9ec3195fd1 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 95b9344c551..50fbf8146e5 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 097a357889f..2f4dc1ddc3a 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 df20daa8f7e..475ca8894e4 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 f82212591b6..9435b9887e9 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 00000000000..aee7dd16341
--- /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 3f2e813d381..f376b26ab9c 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 c8cc721b2e0..023f876d035 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 be07c221f32..4908290e06b 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 54132be749d..7d34665082d 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 c041f605e6c..e99ed03852a 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.19.2'
+ VERSION = '0.19.3'
self.table_name = 'clusters_applications_runners'
diff --git a/app/models/member.rb b/app/models/member.rb
index 2c62ea55785..bbc5d638637 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 1a67116c1f2..f31a6823657 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -363,6 +363,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 preferred_language
read_attribute('preferred_language') ||
@@ -886,6 +887,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 500db1e172a..92500fbc254 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 44a434f4402..831a25a637e 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 37b9b4c362c..d9f41b7040e 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 fb620f77b9f..2712a4b05bb 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 => e
@@ -33,14 +35,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 fdd43260521..8cad065e6cc 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 fe2610f89fb..7961f689259 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 9ae75fe6b8e..c4b3fa80d75 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/routes/user.rb b/config/routes/user.rb
index 3bf13908d39..c7a5a56d9ed 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 00000000000..707cfe96a7c
--- /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/schema_migrations/20200717040735 b/db/schema_migrations/20200717040735
new file mode 100644
index 00000000000..6bfa1dd7261
--- /dev/null
+++ b/db/schema_migrations/20200717040735
@@ -0,0 +1 @@
+6b8fa09c9700c494eeb5151f43064f1656eaaea804742629b7bd66483e2b04cb \ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 033fc85305c..3e3014da914 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -9514,7 +9514,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
);
diff --git a/doc/user/profile/active_sessions.md b/doc/user/profile/active_sessions.md
index 4dbb11b581d..a5b15a7880c 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.
+
<!-- ## Troubleshooting
Include any troubleshooting steps that you can foresee. If you know beforehand what issues
diff --git a/doc/user/profile/index.md b/doc/user/profile/index.md
index b6ef6d7fdb7..894494da513 100644
--- a/doc/user/profile/index.md
+++ b/doc/user/profile/index.md
@@ -255,6 +255,12 @@ to get you a new `_gitlab_session` and keep you signed in through browser restar
After your `remember_user_token` expires and your `_gitlab_session` is cleared/expired,
you are asked to sign in again to verify your identity for security reasons.
+NOTE: **Note:**
+When any session is signed out, or when a session is revoked
+via [Active Sessions](active_sessions.md), all **Remember me** tokens are revoked.
+While other sessions will remain active, the **Remember me** feature will not restore
+a session if the browser is closed or the existing session expires.
+
### Increased sign-in time
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/20340) in GitLab 13.1.
@@ -264,7 +270,7 @@ The `remember_user_token` lifetime of a cookie can now extend beyond the deadlin
GitLab uses both session and persistent cookies:
- Session cookie: Session cookies are normally removed at the end of the browser session when the browser is closed. The `_gitlab_session` cookie has no expiration date.
-- Persistent cookie: The `remember_me_token` is a cookie with an expiration date of two weeks. GitLab activates this cookie if you click Remember Me when you sign in.
+- Persistent cookie: The `remember_user_token` is a cookie with an expiration date of two weeks. GitLab activates this cookie if you click Remember Me when you sign in.
By default, the server sets a time-to-live (TTL) of 1-week on any session that is used.
diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb
index 59978962b1d..46b69214877 100644
--- a/lib/api/api_guard.rb
+++ b/lib/api/api_guard.rb
@@ -69,7 +69,7 @@ module API
deploy_token_from_request ||
find_user_from_bearer_token ||
find_user_from_job_token ||
- find_user_from_warden
+ user_from_warden
end
end
@@ -103,6 +103,25 @@ module API
def user_allowed_or_deploy_token?(user)
Gitlab::UserAccess.new(user).allowed? || user.is_a?(DeployToken)
end
+
+ def user_from_warden
+ user = find_user_from_warden
+
+ return unless user
+ return if two_factor_required_but_not_setup?(user)
+
+ user
+ end
+
+ def two_factor_required_but_not_setup?(user)
+ verifier = Gitlab::Auth::TwoFactorAuthVerifier.new(user)
+
+ if verifier.two_factor_authentication_required? && verifier.current_user_needs_to_setup_two_factor?
+ verifier.two_factor_grace_period_expired?
+ else
+ false
+ end
+ end
end
class_methods do
diff --git a/lib/api/badges.rb b/lib/api/badges.rb
index f6cd3f83ff3..f9728ffc446 100644
--- a/lib/api/badges.rb
+++ b/lib/api/badges.rb
@@ -109,9 +109,10 @@ module API
end
put ":id/badges/:badge_id" do
source = find_source_if_admin(source_type)
+ badge = find_badge(source)
badge = ::Badges::UpdateService.new(declared_params(include_missing: false))
- .execute(find_badge(source))
+ .execute(badge)
if badge.valid?
present_badges(source, badge)
@@ -130,10 +131,6 @@ module API
source = find_source_if_admin(source_type)
badge = find_badge(source)
- if badge.is_a?(GroupBadge) && source.is_a?(Project)
- error!('To delete a Group badge please use the Group endpoint', 403)
- end
-
destroy_conditionally!(badge)
end
end
diff --git a/lib/api/conan_packages.rb b/lib/api/conan_packages.rb
index 6923d252fbd..7f2afea9931 100644
--- a/lib/api/conan_packages.rb
+++ b/lib/api/conan_packages.rb
@@ -26,6 +26,8 @@ module API
PACKAGE_COMPONENT_REGEX = Gitlab::Regex.conan_recipe_component_regex
CONAN_REVISION_REGEX = Gitlab::Regex.conan_revision_regex
+ CONAN_FILES = (Gitlab::Regex::Packages::CONAN_RECIPE_FILES + Gitlab::Regex::Packages::CONAN_PACKAGE_FILES).freeze
+
before do
require_packages_enabled!
@@ -259,7 +261,7 @@ module API
end
params do
- requires :file_name, type: String, desc: 'Package file name', regexp: Gitlab::Regex.conan_file_name_regex
+ requires :file_name, type: String, desc: 'Package file name', values: CONAN_FILES
end
namespace 'export/:file_name', requirements: FILE_NAME_REQUIREMENTS do
desc 'Download recipe files' do
@@ -277,7 +279,7 @@ module API
end
params do
- use :workhorse_upload_params
+ requires :file, type: ::API::Validations::Types::WorkhorseFile, desc: 'The package file to be published (generated by Multipart middleware)'
end
route_setting :authentication, job_token_allowed: true, basic_auth_personal_access_token: true
@@ -300,7 +302,7 @@ module API
params do
requires :conan_package_reference, type: String, desc: 'Conan Package ID'
requires :package_revision, type: String, desc: 'Conan Package Revision'
- requires :file_name, type: String, desc: 'Package file name', regexp: Gitlab::Regex.conan_file_name_regex
+ requires :file_name, type: String, desc: 'Package file name', values: CONAN_FILES
end
namespace 'package/:conan_package_reference/:package_revision/:file_name', requirements: FILE_NAME_REQUIREMENTS do
desc 'Download package files' do
@@ -328,7 +330,7 @@ module API
end
params do
- use :workhorse_upload_params
+ requires :file, type: ::API::Validations::Types::WorkhorseFile, desc: 'The package file to be published (generated by Multipart middleware)'
end
route_setting :authentication, job_token_allowed: true, basic_auth_personal_access_token: true
diff --git a/lib/api/helpers/badges_helpers.rb b/lib/api/helpers/badges_helpers.rb
index 46ce5b4e7b5..f402c603c87 100644
--- a/lib/api/helpers/badges_helpers.rb
+++ b/lib/api/helpers/badges_helpers.rb
@@ -6,7 +6,13 @@ module API
include ::API::Helpers::MembersHelpers
def find_badge(source)
- source.badges.find(params[:badge_id])
+ badge_id = params[:badge_id]
+
+ if source.is_a?(Project)
+ source.project_badges.find(badge_id)
+ else
+ source.badges.find(badge_id)
+ end
end
def present_badges(source, records, options = {})
diff --git a/lib/api/helpers/packages/conan/api_helpers.rb b/lib/api/helpers/packages/conan/api_helpers.rb
index a5fde1af41e..c9c2f66ef62 100644
--- a/lib/api/helpers/packages/conan/api_helpers.rb
+++ b/lib/api/helpers/packages/conan/api_helpers.rb
@@ -133,7 +133,7 @@ module API
end
def track_push_package_event
- if params[:file_name] == ::Packages::Conan::FileMetadatum::PACKAGE_BINARY && params['file.size'] > 0
+ if params[:file_name] == ::Packages::Conan::FileMetadatum::PACKAGE_BINARY && params[:file].size > 0 # rubocop: disable Style/ZeroLengthPredicate
track_event('push_package')
end
end
@@ -147,9 +147,9 @@ module API
end
def create_package_file_with_type(file_type, current_package)
- unless params['file.size'] == 0
+ unless params[:file].size == 0 # rubocop: disable Style/ZeroLengthPredicate
# conan sends two upload requests, the first has no file, so we skip record creation if file.size == 0
- ::Packages::Conan::CreatePackageFileService.new(current_package, uploaded_package_file, params.merge(conan_file_type: file_type)).execute
+ ::Packages::Conan::CreatePackageFileService.new(current_package, params[:file], params.merge(conan_file_type: file_type)).execute
end
end
@@ -220,7 +220,7 @@ module API
return unless token
- ::Ci::Build.find_by_token(token.access_token_id.to_s)
+ ::Ci::AuthJobFinder.new(token: token.access_token_id.to_s).execute
end
def decode_oauth_token_from_jwt
diff --git a/lib/api/helpers/packages_manager_clients_helpers.rb b/lib/api/helpers/packages_manager_clients_helpers.rb
index ae16b65aaa8..955d21cb44f 100644
--- a/lib/api/helpers/packages_manager_clients_helpers.rb
+++ b/lib/api/helpers/packages_manager_clients_helpers.rb
@@ -23,7 +23,7 @@ module API
return unless token
- ::Ci::Build.find_by_token(token)
+ ::Ci::AuthJobFinder.new(token: token).execute
end
def find_deploy_token_from_http_basic_auth
diff --git a/lib/gitlab/application_rate_limiter.rb b/lib/gitlab/application_rate_limiter.rb
index ed963476524..4eeec534fc0 100644
--- a/lib/gitlab/application_rate_limiter.rb
+++ b/lib/gitlab/application_rate_limiter.rb
@@ -25,11 +25,13 @@ module Gitlab
project_repositories_archive: { threshold: 5, interval: 1.minute },
project_generate_new_export: { threshold: -> { application_settings.project_export_limit }, interval: 1.minute },
project_import: { threshold: -> { application_settings.project_import_limit }, interval: 1.minute },
+ project_testing_hook: { threshold: 5, interval: 1.minute },
play_pipeline_schedule: { threshold: 1, interval: 1.minute },
show_raw_controller: { threshold: -> { application_settings.raw_blob_request_limit }, interval: 1.minute },
group_export: { threshold: -> { application_settings.group_export_limit }, interval: 1.minute },
group_download_export: { threshold: -> { application_settings.group_download_export_limit }, interval: 1.minute },
- group_import: { threshold: -> { application_settings.group_import_limit }, interval: 1.minute }
+ group_import: { threshold: -> { application_settings.group_import_limit }, interval: 1.minute },
+ group_testing_hook: { threshold: 5, interval: 1.minute }
}.freeze
end
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 332d0bc1478..0c6ed6924bf 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -222,6 +222,8 @@ module Gitlab
# Registry access (with jwt) does not have access to project
return if project && !token.has_access_to?(project)
+ # When repository is disabled, no resources are accessible via Deploy Token
+ return if project&.repository_access_level == ::ProjectFeature::DISABLED
scopes = abilities_for_scopes(token.scopes)
diff --git a/lib/gitlab/auth/auth_finders.rb b/lib/gitlab/auth/auth_finders.rb
index f3d0c053880..ccf52bae9a5 100644
--- a/lib/gitlab/auth/auth_finders.rb
+++ b/lib/gitlab/auth/auth_finders.rb
@@ -69,9 +69,7 @@ module Gitlab
current_request.env[JOB_TOKEN_HEADER].presence
return unless token
- job = ::Ci::Build.find_by_token(token)
- raise UnauthorizedError unless job
-
+ job = find_valid_running_job_by_token!(token)
@current_authenticated_job = job # rubocop:disable Gitlab/ModuleWithInstanceVariables
job.user
@@ -84,9 +82,7 @@ module Gitlab
return unless login.present? && password.present?
return unless ::Gitlab::Auth::CI_JOB_USER == login
- job = ::Ci::Build.find_by_token(password)
- raise UnauthorizedError unless job
-
+ job = find_valid_running_job_by_token!(password)
job.user
end
@@ -179,7 +175,7 @@ module Gitlab
token = parsed_oauth_token
return unless token
- job = ::Ci::Build.find_by_token(token)
+ job = ::Ci::AuthJobFinder.new(token: token).execute
return unless job
@current_authenticated_job = job # rubocop:disable Gitlab/ModuleWithInstanceVariables
@@ -304,6 +300,12 @@ module Gitlab
def blob_request?
current_request.path.include?('/raw/')
end
+
+ def find_valid_running_job_by_token!(token)
+ ::Ci::AuthJobFinder.new(token: token).execute.tap do |job|
+ raise UnauthorizedError unless job
+ end
+ end
end
end
end
diff --git a/lib/gitlab/auth/two_factor_auth_verifier.rb b/lib/gitlab/auth/two_factor_auth_verifier.rb
new file mode 100644
index 00000000000..86552ef1267
--- /dev/null
+++ b/lib/gitlab/auth/two_factor_auth_verifier.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Auth
+ class TwoFactorAuthVerifier
+ attr_reader :current_user
+
+ def initialize(current_user)
+ @current_user = current_user
+ end
+
+ def two_factor_authentication_required?
+ Gitlab::CurrentSettings.require_two_factor_authentication? ||
+ current_user&.require_two_factor_authentication_from_group?
+ end
+
+ def current_user_needs_to_setup_two_factor?
+ current_user && !current_user.temp_oauth_email? && !current_user.two_factor_enabled?
+ end
+
+ def two_factor_grace_period
+ periods = [Gitlab::CurrentSettings.two_factor_grace_period]
+ periods << current_user.two_factor_grace_period if current_user&.require_two_factor_authentication_from_group?
+ periods.min
+ end
+
+ def two_factor_grace_period_expired?
+ time = current_user&.otp_grace_period_started_at
+
+ return false unless time
+
+ two_factor_grace_period.hours.since(time) < Time.current
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index f3b53a2ba0b..b67b3a37440 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -98,6 +98,13 @@ module Gitlab
Guest.can?(download_ability, container)
end
+ def deploy_key_can_download_code?
+ authentication_abilities.include?(:download_code) &&
+ deploy_key? &&
+ deploy_key.has_access_to?(container) &&
+ (project? && project&.repository_access_level != ::Featurable::DISABLED)
+ end
+
def user_can_download_code?
authentication_abilities.include?(:download_code) &&
user_access.can_do_action?(download_ability)
@@ -257,7 +264,7 @@ module Gitlab
end
def check_download_access!
- passed = deploy_key? ||
+ passed = deploy_key_can_download_code? ||
deploy_token? ||
user_can_download_code? ||
build_can_download_code? ||
diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb
index 1e1e0d856b7..2d625737e05 100644
--- a/lib/gitlab/regex.rb
+++ b/lib/gitlab/regex.rb
@@ -6,11 +6,6 @@ module Gitlab
CONAN_RECIPE_FILES = %w[conanfile.py conanmanifest.txt conan_sources.tgz conan_export.tgz].freeze
CONAN_PACKAGE_FILES = %w[conaninfo.txt conanmanifest.txt conan_package.tgz].freeze
- def conan_file_name_regex
- @conan_file_name_regex ||=
- %r{\A#{(CONAN_RECIPE_FILES + CONAN_PACKAGE_FILES).join("|")}\z}.freeze
- end
-
def conan_package_reference_regex
@conan_package_reference_regex ||= %r{\A[A-Za-z0-9]+\z}.freeze
end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index f3bba0d4e25..e4af61a0f35 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -2831,6 +2831,9 @@ msgstr ""
msgid "An error occurred while validating username"
msgstr ""
+msgid "An error occurred. Please sign in again."
+msgstr ""
+
msgid "An error occurred. Please try again."
msgstr ""
@@ -3268,7 +3271,7 @@ msgstr ""
msgid "Are you sure? Removing this GPG key does not affect already signed commits."
msgstr ""
-msgid "Are you sure? The device will be signed out of GitLab."
+msgid "Are you sure? The device will be signed out of GitLab and all remember me tokens revoked."
msgstr ""
msgid "Are you sure? This will invalidate your registered applications and U2F devices."
diff --git a/package.json b/package.json
index 99dd3a819b7..bbed76e66a5 100644
--- a/package.json
+++ b/package.json
@@ -96,7 +96,7 @@
"ipaddr.js": "^1.9.1",
"jed": "^1.1.1",
"jest-transform-graphql": "^2.1.0",
- "jquery": "^3.4.1",
+ "jquery": "^3.5.0",
"jquery-ujs": "1.2.2",
"jquery.caret": "^0.3.1",
"jquery.waitforimages": "^2.2.0",
diff --git a/spec/controllers/admin/applications_controller_spec.rb b/spec/controllers/admin/applications_controller_spec.rb
index 732d20666cb..6c423097e70 100644
--- a/spec/controllers/admin/applications_controller_spec.rb
+++ b/spec/controllers/admin/applications_controller_spec.rb
@@ -40,7 +40,7 @@ RSpec.describe Admin::ApplicationsController do
describe 'POST #create' do
it 'creates the application' do
- create_params = attributes_for(:application, trusted: true, confidential: false)
+ create_params = attributes_for(:application, trusted: true, confidential: false, scopes: ['api'])
expect do
post :create, params: { doorkeeper_application: create_params }
@@ -63,7 +63,7 @@ RSpec.describe Admin::ApplicationsController do
context 'when the params are for a confidential application' do
it 'creates a confidential application' do
- create_params = attributes_for(:application, confidential: true)
+ create_params = attributes_for(:application, confidential: true, scopes: ['read_user'])
expect do
post :create, params: { doorkeeper_application: create_params }
@@ -75,6 +75,18 @@ RSpec.describe Admin::ApplicationsController do
expect(application).to have_attributes(create_params.except(:uid, :owner_type))
end
end
+
+ context 'when scopes are not present' do
+ it 'renders the application form on errors' do
+ create_params = attributes_for(:application, trusted: true, confidential: false)
+
+ expect do
+ post :create, params: { doorkeeper_application: create_params }
+ end.not_to change { Doorkeeper::Application.count }
+
+ expect(response).to render_template :new
+ end
+ end
end
describe 'PATCH #update' do
diff --git a/spec/controllers/admin/clusters_controller_spec.rb b/spec/controllers/admin/clusters_controller_spec.rb
index 2e0ee671d3f..d2a569a9d48 100644
--- a/spec/controllers/admin/clusters_controller_spec.rb
+++ b/spec/controllers/admin/clusters_controller_spec.rb
@@ -99,7 +99,9 @@ RSpec.describe Admin::ClustersController do
end
describe 'GET #new' do
- def get_new(provider: 'gcp')
+ let(:user) { admin }
+
+ def go(provider: 'gcp')
get :new, params: { provider: provider }
end
@@ -112,7 +114,7 @@ RSpec.describe Admin::ClustersController do
context 'when selected provider is gke and no valid gcp token exists' do
it 'redirects to gcp authorize_url' do
- get_new
+ go
expect(response).to redirect_to(assigns(:authorize_url))
end
@@ -125,7 +127,7 @@ RSpec.describe Admin::ClustersController do
end
it 'does not have authorize_url' do
- get_new
+ go
expect(assigns(:authorize_url)).to be_nil
end
@@ -137,7 +139,7 @@ RSpec.describe Admin::ClustersController do
end
it 'has new object' do
- get_new
+ go
expect(assigns(:gcp_cluster)).to be_an_instance_of(Clusters::ClusterPresenter)
end
@@ -158,16 +160,18 @@ RSpec.describe Admin::ClustersController do
describe 'functionality for existing cluster' do
it 'has new object' do
- get_new
+ go
expect(assigns(:user_cluster)).to be_an_instance_of(Clusters::ClusterPresenter)
end
end
+ include_examples 'GET new cluster shared examples'
+
describe 'security' do
- it { expect { get_new }.to be_allowed_for(:admin) }
- it { expect { get_new }.to be_denied_for(:user) }
- it { expect { get_new }.to be_denied_for(:external) }
+ it { expect { go }.to be_allowed_for(:admin) }
+ it { expect { go }.to be_denied_for(:user) }
+ it { expect { go }.to be_denied_for(:external) }
end
end
@@ -424,14 +428,13 @@ RSpec.describe Admin::ClustersController do
end
describe 'POST authorize AWS role for EKS cluster' do
- let(:role_arn) { 'arn:aws:iam::123456789012:role/role-name' }
- let(:role_external_id) { '12345' }
+ let!(:role) { create(:aws_role, user: admin) }
+ let(:role_arn) { 'arn:new-role' }
let(:params) do
{
cluster: {
- role_arn: role_arn,
- role_external_id: role_external_id
+ role_arn: role_arn
}
}
end
@@ -445,28 +448,32 @@ RSpec.describe Admin::ClustersController do
.and_return(double(execute: double))
end
- it 'creates an Aws::Role record' do
- expect { go }.to change { Aws::Role.count }
+ it 'updates the associated role with the supplied ARN' do
+ go
expect(response).to have_gitlab_http_status(:ok)
-
- role = Aws::Role.last
- expect(role.user).to eq admin
- expect(role.role_arn).to eq role_arn
- expect(role.role_external_id).to eq role_external_id
+ expect(role.reload.role_arn).to eq(role_arn)
end
- context 'role cannot be created' do
+ context 'supplied role is invalid' do
let(:role_arn) { 'invalid-role' }
- it 'does not create a record' do
- expect { go }.not_to change { Aws::Role.count }
+ it 'does not update the associated role' do
+ expect { go }.not_to change { role.role_arn }
expect(response).to have_gitlab_http_status(:unprocessable_entity)
end
end
describe 'security' do
+ before do
+ allow_next_instance_of(Clusters::Aws::AuthorizeRoleService) do |service|
+ response = double(status: :ok, body: double)
+
+ allow(service).to receive(:execute).and_return(response)
+ end
+ end
+
it { expect { go }.to be_allowed_for(:admin) }
it { expect { go }.to be_denied_for(:user) }
it { expect { go }.to be_denied_for(:external) }
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index 357044a144c..f8d4690e9ce 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/spec/controllers/application_controller_spec.rb
@@ -229,6 +229,7 @@ RSpec.describe ApplicationController do
it 'does not redirect if 2FA is not required' do
allow(controller).to receive(:two_factor_authentication_required?).and_return(false)
+ allow(controller).to receive(:current_user).and_return(create(:user))
expect(controller).not_to receive(:redirect_to)
@@ -346,13 +347,17 @@ RSpec.describe ApplicationController do
let(:user) { create :user, otp_grace_period_started_at: 2.hours.ago }
it 'returns true if the grace period has expired' do
- allow(controller).to receive(:two_factor_grace_period).and_return(1)
+ allow_next_instance_of(Gitlab::Auth::TwoFactorAuthVerifier) do |verifier|
+ allow(verifier).to receive(:two_factor_grace_period).and_return(2)
+ end
expect(subject).to be_truthy
end
it 'returns false if the grace period is still active' do
- allow(controller).to receive(:two_factor_grace_period).and_return(3)
+ allow_next_instance_of(Gitlab::Auth::TwoFactorAuthVerifier) do |verifier|
+ allow(verifier).to receive(:two_factor_grace_period).and_return(3)
+ end
expect(subject).to be_falsey
end
diff --git a/spec/controllers/groups/clusters_controller_spec.rb b/spec/controllers/groups/clusters_controller_spec.rb
index 1593e1290c4..81d5bc7770f 100644
--- a/spec/controllers/groups/clusters_controller_spec.rb
+++ b/spec/controllers/groups/clusters_controller_spec.rb
@@ -180,6 +180,8 @@ RSpec.describe Groups::ClustersController do
end
end
+ include_examples 'GET new cluster shared examples'
+
describe 'security' do
it { expect { go }.to be_allowed_for(:admin) }
it { expect { go }.to be_allowed_for(:owner).of(group) }
@@ -493,14 +495,13 @@ RSpec.describe Groups::ClustersController do
end
describe 'POST authorize AWS role for EKS cluster' do
- let(:role_arn) { 'arn:aws:iam::123456789012:role/role-name' }
- let(:role_external_id) { '12345' }
+ let!(:role) { create(:aws_role, user: user) }
+ let(:role_arn) { 'arn:new-role' }
let(:params) do
{
cluster: {
- role_arn: role_arn,
- role_external_id: role_external_id
+ role_arn: role_arn
}
}
end
@@ -514,28 +515,32 @@ RSpec.describe Groups::ClustersController do
.and_return(double(execute: double))
end
- it 'creates an Aws::Role record' do
- expect { go }.to change { Aws::Role.count }
+ it 'updates the associated role with the supplied ARN' do
+ go
expect(response).to have_gitlab_http_status(:ok)
-
- role = Aws::Role.last
- expect(role.user).to eq user
- expect(role.role_arn).to eq role_arn
- expect(role.role_external_id).to eq role_external_id
+ expect(role.reload.role_arn).to eq(role_arn)
end
- context 'role cannot be created' do
+ context 'supplied role is invalid' do
let(:role_arn) { 'invalid-role' }
- it 'does not create a record' do
- expect { go }.not_to change { Aws::Role.count }
+ it 'does not update the associated role' do
+ expect { go }.not_to change { role.role_arn }
expect(response).to have_gitlab_http_status(:unprocessable_entity)
end
end
describe 'security' do
+ before do
+ allow_next_instance_of(Clusters::Aws::AuthorizeRoleService) do |service|
+ response = double(status: :ok, body: double)
+
+ allow(service).to receive(:execute).and_return(response)
+ end
+ end
+
it { expect { go }.to be_allowed_for(:admin) }
it { expect { go }.to be_allowed_for(:owner).of(group) }
it { expect { go }.to be_allowed_for(:maintainer).of(group) }
diff --git a/spec/controllers/oauth/applications_controller_spec.rb b/spec/controllers/oauth/applications_controller_spec.rb
index 0a7975b8c1b..f21ef324884 100644
--- a/spec/controllers/oauth/applications_controller_spec.rb
+++ b/spec/controllers/oauth/applications_controller_spec.rb
@@ -123,7 +123,8 @@ RSpec.describe Oauth::ApplicationsController do
invalid_uri_params = {
doorkeeper_application: {
name: 'foo',
- redirect_uri: 'javascript://alert()'
+ redirect_uri: 'javascript://alert()',
+ scopes: ['api']
}
}
@@ -133,6 +134,23 @@ RSpec.describe Oauth::ApplicationsController do
end
end
+ context 'when scopes are not present' do
+ render_views
+
+ it 'shows an error for blank scopes' do
+ invalid_uri_params = {
+ doorkeeper_application: {
+ name: 'foo',
+ redirect_uri: 'http://example.org'
+ }
+ }
+
+ post :create, params: invalid_uri_params
+
+ expect(response.body).to include 'Scopes can&#39;t be blank'
+ end
+ end
+
it_behaves_like 'redirects to login page when the user is not signed in'
it_behaves_like 'redirects to 2fa setup page when the user requires it'
end
@@ -172,7 +190,8 @@ RSpec.describe Oauth::ApplicationsController do
{
doorkeeper_application: {
name: 'foo',
- redirect_uri: 'http://example.org'
+ redirect_uri: 'http://example.org',
+ scopes: ['api']
}
}
end
diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb
index dce996b977d..3f7f0c55f38 100644
--- a/spec/controllers/omniauth_callbacks_controller_spec.rb
+++ b/spec/controllers/omniauth_callbacks_controller_spec.rb
@@ -40,6 +40,22 @@ RSpec.describe OmniauthCallbacksController, type: :controller do
end
end
+ context 'when sign in is not valid' do
+ let(:provider) { :github }
+ let(:extern_uid) { 'my-uid' }
+
+ it 'renders omniauth error page' do
+ allow_next_instance_of(Gitlab::Auth::OAuth::User) do |instance|
+ allow(instance).to receive(:valid_sign_in?).and_return(false)
+ end
+
+ post provider
+
+ expect(response).to render_template("errors/omniauth_error")
+ expect(response).to have_gitlab_http_status(:unprocessable_entity)
+ end
+ end
+
context 'when the user is on the last sign in attempt' do
let(:extern_uid) { 'my-uid' }
diff --git a/spec/controllers/profiles/active_sessions_controller_spec.rb b/spec/controllers/profiles/active_sessions_controller_spec.rb
new file mode 100644
index 00000000000..f54f69d853d
--- /dev/null
+++ b/spec/controllers/profiles/active_sessions_controller_spec.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Profiles::ActiveSessionsController do
+ describe 'DELETE destroy' do
+ let_it_be(:user) { create(:user) }
+
+ before do
+ sign_in(user)
+ end
+
+ it 'invalidates all remember user tokens' do
+ ActiveSession.set(user, request)
+ session_id = request.session.id.public_id
+ user.remember_me!
+
+ delete :destroy, params: { id: session_id }
+
+ expect(user.reload.remember_created_at).to be_nil
+ end
+ end
+end
diff --git a/spec/controllers/profiles/two_factor_auths_controller_spec.rb b/spec/controllers/profiles/two_factor_auths_controller_spec.rb
index f645081219a..1fb0b18622b 100644
--- a/spec/controllers/profiles/two_factor_auths_controller_spec.rb
+++ b/spec/controllers/profiles/two_factor_auths_controller_spec.rb
@@ -14,10 +14,9 @@ RSpec.describe Profiles::TwoFactorAuthsController do
let(:user) { create(:user) }
it 'generates otp_secret for user' do
- expect(User).to receive(:generate_otp_secret).with(32).and_return('secret').once
+ expect(User).to receive(:generate_otp_secret).with(32).and_call_original.once
get :show
- get :show # Second hit shouldn't re-generate it
end
it 'assigns qr_code' do
@@ -27,6 +26,14 @@ RSpec.describe Profiles::TwoFactorAuthsController do
get :show
expect(assigns[:qr_code]).to eq code
end
+
+ it 'generates a unique otp_secret every time the page is loaded' do
+ expect(User).to receive(:generate_otp_secret).with(32).and_call_original.twice
+
+ 2.times do
+ get :show
+ end
+ end
end
describe 'POST create' do
@@ -57,6 +64,12 @@ RSpec.describe Profiles::TwoFactorAuthsController do
expect(assigns[:codes]).to match_array %w(a b c)
end
+ it 'calls to delete other sessions' do
+ expect(ActiveSession).to receive(:destroy_all_but_current)
+
+ go
+ end
+
it 'renders create' do
go
expect(response).to render_template(:create)
diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb
index da4faad2a39..51a451570c5 100644
--- a/spec/controllers/projects/clusters_controller_spec.rb
+++ b/spec/controllers/projects/clusters_controller_spec.rb
@@ -183,6 +183,8 @@ RSpec.describe Projects::ClustersController do
end
end
+ include_examples 'GET new cluster shared examples'
+
describe 'security' do
it 'is allowed for admin when admin mode enabled', :enable_admin_mode do
expect { go }.to be_allowed_for(:admin)
@@ -521,14 +523,13 @@ RSpec.describe Projects::ClustersController do
end
describe 'POST authorize AWS role for EKS cluster' do
- let(:role_arn) { 'arn:aws:iam::123456789012:role/role-name' }
- let(:role_external_id) { '12345' }
+ let!(:role) { create(:aws_role, user: user) }
+ let(:role_arn) { 'arn:new-role' }
let(:params) do
{
cluster: {
- role_arn: role_arn,
- role_external_id: role_external_id
+ role_arn: role_arn
}
}
end
@@ -542,28 +543,32 @@ RSpec.describe Projects::ClustersController do
.and_return(double(execute: double))
end
- it 'creates an Aws::Role record' do
- expect { go }.to change { Aws::Role.count }
+ it 'updates the associated role with the supplied ARN' do
+ go
expect(response).to have_gitlab_http_status(:ok)
-
- role = Aws::Role.last
- expect(role.user).to eq user
- expect(role.role_arn).to eq role_arn
- expect(role.role_external_id).to eq role_external_id
+ expect(role.reload.role_arn).to eq(role_arn)
end
- context 'role cannot be created' do
+ context 'supplied role is invalid' do
let(:role_arn) { 'invalid-role' }
- it 'does not create a record' do
- expect { go }.not_to change { Aws::Role.count }
+ it 'does not update the associated role' do
+ expect { go }.not_to change { role.role_arn }
expect(response).to have_gitlab_http_status(:unprocessable_entity)
end
end
describe 'security' do
+ before do
+ allow_next_instance_of(Clusters::Aws::AuthorizeRoleService) do |service|
+ response = double(status: :ok, body: double)
+
+ allow(service).to receive(:execute).and_return(response)
+ end
+ end
+
it 'is allowed for admin when admin mode enabled', :enable_admin_mode do
expect { go }.to be_allowed_for(:admin)
end
diff --git a/spec/controllers/projects/hooks_controller_spec.rb b/spec/controllers/projects/hooks_controller_spec.rb
index 85d036486ee..bd543cebeec 100644
--- a/spec/controllers/projects/hooks_controller_spec.rb
+++ b/spec/controllers/projects/hooks_controller_spec.rb
@@ -47,4 +47,26 @@ RSpec.describe Projects::HooksController do
expect(ProjectHook.first).to have_attributes(hook_params)
end
end
+
+ describe '#test' do
+ let(:hook) { create(:project_hook, project: project) }
+
+ context 'when the endpoint receives requests above the limit' do
+ before do
+ allow(Gitlab::ApplicationRateLimiter).to receive(:rate_limits)
+ .and_return(project_testing_hook: { threshold: 1, interval: 1.minute })
+ end
+
+ it 'prevents making test requests' do
+ expect_next_instance_of(TestHooks::ProjectService) do |service|
+ expect(service).to receive(:execute).and_return(http_status: 200)
+ end
+
+ 2.times { post :test, params: { namespace_id: project.namespace, project_id: project, id: hook } }
+
+ expect(response.body).to eq(_('This endpoint has been requested too many times. Try again later.'))
+ expect(response).to have_gitlab_http_status(:too_many_requests)
+ end
+ end
+ end
end
diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb
index 257dcce0899..f2e16baaccf 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -255,8 +255,8 @@ RSpec.describe SessionsController do
context 'when using two-factor authentication via OTP' do
let(:user) { create(:user, :two_factor) }
- def authenticate_2fa(user_params)
- post(:create, params: { user: user_params }, session: { otp_user_id: user.id })
+ def authenticate_2fa(user_params, otp_user_id: user.id)
+ post(:create, params: { user: user_params }, session: { otp_user_id: otp_user_id })
end
context 'remember_me field' do
@@ -293,8 +293,22 @@ RSpec.describe SessionsController do
end
end
+ # See issue gitlab-org/gitlab#20302.
+ context 'when otp_user_id is stale' do
+ render_views
+
+ it 'favors login over otp_user_id when password is present and does not authenticate the user' do
+ authenticate_2fa(
+ { login: 'random_username', password: user.password },
+ otp_user_id: user.id
+ )
+
+ expect(response).to set_flash.now[:alert].to /Invalid Login or password/
+ end
+ end
+
##
- # See #14900 issue
+ # See issue gitlab-org/gitlab-foss#14900
#
context 'when authenticating with login and OTP of another user' do
context 'when another user has 2FA enabled' do
@@ -380,18 +394,6 @@ RSpec.describe SessionsController do
end
end
end
-
- context 'when another user does not have 2FA enabled' do
- let(:another_user) { create(:user) }
-
- it 'does not leak that 2FA is disabled for another user' do
- authenticate_2fa(login: another_user.username,
- otp_attempt: 'invalid')
-
- expect(response).to set_flash.now[:alert]
- .to /Invalid two-factor code/
- end
- end
end
end
diff --git a/spec/features/admin/admin_manage_applications_spec.rb b/spec/features/admin/admin_manage_applications_spec.rb
index 3f3d71e842c..7a9a6f2ccb8 100644
--- a/spec/features/admin/admin_manage_applications_spec.rb
+++ b/spec/features/admin/admin_manage_applications_spec.rb
@@ -7,7 +7,7 @@ RSpec.describe 'admin manage applications' do
sign_in(create(:admin))
end
- it do
+ it 'creates new oauth application' do
visit admin_applications_path
click_on 'New application'
@@ -16,6 +16,7 @@ RSpec.describe 'admin manage applications' do
fill_in :doorkeeper_application_name, with: 'test'
fill_in :doorkeeper_application_redirect_uri, with: 'https://test.com'
check :doorkeeper_application_trusted
+ check :doorkeeper_application_scopes_read_user
click_on 'Submit'
expect(page).to have_content('Application: test')
expect(page).to have_content('Application ID')
@@ -43,4 +44,19 @@ RSpec.describe 'admin manage applications' do
end
expect(page.find('.oauth-applications')).not_to have_content('test_changed')
end
+
+ context 'when scopes are blank' do
+ it 'returns an error' do
+ visit admin_applications_path
+
+ click_on 'New application'
+ expect(page).to have_content('New application')
+
+ fill_in :doorkeeper_application_name, with: 'test'
+ fill_in :doorkeeper_application_redirect_uri, with: 'https://test.com'
+ click_on 'Submit'
+
+ expect(page).to have_content("Scopes can't be blank")
+ end
+ end
end
diff --git a/spec/features/profiles/user_manages_applications_spec.rb b/spec/features/profiles/user_manages_applications_spec.rb
index d65365db880..22eed748c00 100644
--- a/spec/features/profiles/user_manages_applications_spec.rb
+++ b/spec/features/profiles/user_manages_applications_spec.rb
@@ -15,6 +15,7 @@ RSpec.describe 'User manages applications' do
fill_in :doorkeeper_application_name, with: 'test'
fill_in :doorkeeper_application_redirect_uri, with: 'https://test.com'
+ check :doorkeeper_application_scopes_read_user
click_on 'Save application'
expect(page).to have_content 'Application: test'
@@ -41,4 +42,16 @@ RSpec.describe 'User manages applications' do
end
expect(page.find('.oauth-applications')).not_to have_content 'test_changed'
end
+
+ context 'when scopes are blank' do
+ it 'returns an error' do
+ expect(page).to have_content 'Add new application'
+
+ fill_in :doorkeeper_application_name, with: 'test'
+ fill_in :doorkeeper_application_redirect_uri, with: 'https://test.com'
+ click_on 'Save application'
+
+ expect(page).to have_content("Scopes can't be blank")
+ end
+ end
end
diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb
index 2d0fcfe84e6..6f6ebe34c03 100644
--- a/spec/features/users/login_spec.rb
+++ b/spec/features/users/login_spec.rb
@@ -177,6 +177,14 @@ RSpec.describe 'Login' do
expect(page).not_to have_content(I18n.t('devise.failure.already_authenticated'))
end
+ it 'does not allow sign-in if the user password is updated before entering a one-time code' do
+ user.update!(password: 'new_password')
+
+ enter_code(user.current_otp)
+
+ expect(page).to have_content('An error occurred. Please sign in again.')
+ end
+
context 'using one-time code' do
it 'allows login with valid code' do
expect(authentication_metrics)
@@ -232,7 +240,7 @@ RSpec.describe 'Login' do
expect(codes.size).to eq 10
# Ensure the generated codes get saved
- user.save
+ user.save(touch: false)
end
context 'with valid code' do
@@ -290,7 +298,7 @@ RSpec.describe 'Login' do
code = codes.sample
expect(user.invalidate_otp_backup_code!(code)).to eq true
- user.save!
+ user.save!(touch: false)
expect(user.reload.otp_backup_codes.size).to eq 9
enter_code(code)
diff --git a/spec/finders/ci/auth_job_finder_spec.rb b/spec/finders/ci/auth_job_finder_spec.rb
new file mode 100644
index 00000000000..6cd58f5cd01
--- /dev/null
+++ b/spec/finders/ci/auth_job_finder_spec.rb
@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe Ci::AuthJobFinder do
+ let_it_be(:job, reload: true) { create(:ci_build, status: :running) }
+
+ let(:token) { job.token }
+
+ subject(:finder) do
+ described_class.new(token: token)
+ end
+
+ describe '#execute!' do
+ subject(:execute) { finder.execute! }
+
+ it { is_expected.to eq(job) }
+
+ it 'raises error if the job is not running' do
+ job.success!
+
+ expect { execute }.to raise_error described_class::NotRunningJobError, 'Job is not running'
+ end
+
+ it 'raises error if the job is erased' do
+ expect(::Ci::Build).to receive(:find_by_token).with(job.token).and_return(job)
+ expect(job).to receive(:erased?).and_return(true)
+
+ expect { execute }.to raise_error described_class::ErasedJobError, 'Job has been erased!'
+ end
+
+ it 'raises error if the the project is missing' do
+ expect(::Ci::Build).to receive(:find_by_token).with(job.token).and_return(job)
+ expect(job).to receive(:project).and_return(nil)
+
+ expect { execute }.to raise_error described_class::DeletedProjectError, 'Project has been deleted!'
+ end
+
+ it 'raises error if the the project is being removed' do
+ project = double(Project)
+
+ expect(::Ci::Build).to receive(:find_by_token).with(job.token).and_return(job)
+ expect(job).to receive(:project).twice.and_return(project)
+ expect(project).to receive(:pending_delete?).and_return(true)
+
+ expect { execute }.to raise_error described_class::DeletedProjectError, 'Project has been deleted!'
+ end
+
+ context 'with wrong job token' do
+ let(:token) { 'missing' }
+
+ it { is_expected.to be_nil }
+ end
+ end
+
+ describe '#execute' do
+ subject(:execute) { finder.execute }
+
+ before do
+ job.success!
+ end
+
+ it { is_expected.to be_nil }
+ end
+end
diff --git a/spec/finders/user_recent_events_finder_spec.rb b/spec/finders/user_recent_events_finder_spec.rb
index 559d1004b4b..ddba9b595a4 100644
--- a/spec/finders/user_recent_events_finder_spec.rb
+++ b/spec/finders/user_recent_events_finder_spec.rb
@@ -11,8 +11,10 @@ RSpec.describe UserRecentEventsFinder do
let!(:private_event) { create(:event, project: private_project, author: project_owner) }
let!(:internal_event) { create(:event, project: internal_project, author: project_owner) }
let!(:public_event) { create(:event, project: public_project, author: project_owner) }
+ let(:limit) { nil }
+ let(:params) { { limit: limit } }
- subject(:finder) { described_class.new(current_user, project_owner) }
+ subject(:finder) { described_class.new(current_user, project_owner, params) }
describe '#execute' do
context 'when profile is public' do
@@ -48,5 +50,38 @@ RSpec.describe UserRecentEventsFinder do
expect(events).to include(event_b)
end
end
+
+ context 'limits' do
+ before do
+ stub_const("#{described_class}::DEFAULT_LIMIT", 1)
+ stub_const("#{described_class}::MAX_LIMIT", 3)
+ end
+
+ context 'when limit is not set' do
+ it 'returns events limited to DEFAULT_LIMIT' do
+ expect(finder.execute.size).to eq(described_class::DEFAULT_LIMIT)
+ end
+ end
+
+ context 'when limit is set' do
+ let(:limit) { 2 }
+
+ it 'returns events limited to specified limit' do
+ expect(finder.execute.size).to eq(limit)
+ end
+ end
+
+ context 'when limit is set to a number that exceeds maximum limit' do
+ let(:limit) { 4 }
+
+ before do
+ create(:event, project: public_project, author: project_owner)
+ end
+
+ it 'returns events limited to MAX_LIMIT' do
+ expect(finder.execute.size).to eq(described_class::MAX_LIMIT)
+ end
+ end
+ end
end
end
diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb
index 5d6aa863994..e52f60a2c02 100644
--- a/spec/graphql/gitlab_schema_spec.rb
+++ b/spec/graphql/gitlab_schema_spec.rb
@@ -212,6 +212,55 @@ RSpec.describe GitlabSchema do
end
end
+ describe '.parse_gid' do
+ let_it_be(:global_id) { 'gid://gitlab/TestOne/2147483647' }
+
+ before do
+ test_base = Class.new
+ test_one = Class.new(test_base)
+ test_two = Class.new(test_base)
+
+ stub_const('TestBase', test_base)
+ stub_const('TestOne', test_one)
+ stub_const('TestTwo', test_two)
+ end
+
+ it 'parses the gid' do
+ gid = described_class.parse_gid(global_id)
+
+ expect(gid.model_id).to eq '2147483647'
+ expect(gid.model_class).to eq TestOne
+ end
+
+ context 'when gid is malformed' do
+ let_it_be(:global_id) { 'malformed://gitlab/TestOne/2147483647' }
+
+ it 'raises an error' do
+ expect { described_class.parse_gid(global_id) }
+ .to raise_error(Gitlab::Graphql::Errors::ArgumentError, "#{global_id} is not a valid GitLab id.")
+ end
+ end
+
+ context 'when using expected_type' do
+ it 'accepts a single type' do
+ gid = described_class.parse_gid(global_id, expected_type: TestOne)
+
+ expect(gid.model_class).to eq TestOne
+ end
+
+ it 'accepts an ancestor type' do
+ gid = described_class.parse_gid(global_id, expected_type: TestBase)
+
+ expect(gid.model_class).to eq TestOne
+ end
+
+ it 'rejects an unknown type' do
+ expect { described_class.parse_gid(global_id, expected_type: TestTwo) }
+ .to raise_error(Gitlab::Graphql::Errors::ArgumentError, "#{global_id} is not a valid id for TestTwo.")
+ end
+ end
+ end
+
def field_instrumenters
described_class.instrumenters[:field] + described_class.instrumenters[:field_after_built_ins]
end
diff --git a/spec/lib/api/helpers/packages_manager_clients_helpers_spec.rb b/spec/lib/api/helpers/packages_manager_clients_helpers_spec.rb
index 832f4abe545..73b67f9e61c 100644
--- a/spec/lib/api/helpers/packages_manager_clients_helpers_spec.rb
+++ b/spec/lib/api/helpers/packages_manager_clients_helpers_spec.rb
@@ -11,7 +11,7 @@ RSpec.describe API::Helpers::PackagesManagerClientsHelpers do
describe '#find_job_from_http_basic_auth' do
let_it_be(:user) { personal_access_token.user }
- let(:job) { create(:ci_build, user: user) }
+ let(:job) { create(:ci_build, user: user, status: :running) }
let(:password) { job.token }
let(:headers) { { Authorization: basic_http_auth(username, password) } }
@@ -23,6 +23,14 @@ RSpec.describe API::Helpers::PackagesManagerClientsHelpers do
context 'with a valid Authorization header' do
it { is_expected.to eq job }
+
+ context 'when the job is not running' do
+ before do
+ job.update!(status: :failed)
+ end
+
+ it { is_expected.to be nil }
+ end
end
context 'with an invalid Authorization header' do
diff --git a/spec/lib/gitlab/auth/auth_finders_spec.rb b/spec/lib/gitlab/auth/auth_finders_spec.rb
index a73ac0b34af..1ac8ebe1369 100644
--- a/spec/lib/gitlab/auth/auth_finders_spec.rb
+++ b/spec/lib/gitlab/auth/auth_finders_spec.rb
@@ -37,11 +37,29 @@ RSpec.describe Gitlab::Auth::AuthFinders do
expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError)
end
- it "return user if token is valid" do
- set_token(job.token)
+ context 'with a running job' do
+ before do
+ job.update!(status: :running)
+ end
+
+ it 'return user if token is valid' do
+ set_token(job.token)
+
+ expect(subject).to eq(user)
+ expect(@current_authenticated_job).to eq job
+ end
+ end
- expect(subject).to eq(user)
- expect(@current_authenticated_job).to eq job
+ context 'with a job that is not running' do
+ before do
+ job.update!(status: :failed)
+ end
+
+ it 'returns an Unauthorized exception' do
+ set_token(job.token)
+
+ expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError)
+ end
end
end
end
@@ -557,7 +575,7 @@ RSpec.describe Gitlab::Auth::AuthFinders do
context 'with CI username' do
let(:username) { ::Gitlab::Auth::CI_JOB_USER }
let(:user) { create(:user) }
- let(:build) { create(:ci_build, user: user) }
+ let(:build) { create(:ci_build, user: user, status: :running) }
it 'returns nil without password' do
set_basic_auth_header(username, nil)
@@ -576,6 +594,13 @@ RSpec.describe Gitlab::Auth::AuthFinders do
expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError)
end
+
+ it 'returns exception if the job is not running' do
+ set_basic_auth_header(username, build.token)
+ build.success!
+
+ expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError)
+ end
end
end
@@ -586,7 +611,7 @@ RSpec.describe Gitlab::Auth::AuthFinders do
context 'with a job token' do
let(:route_authentication_setting) { { job_token_allowed: true } }
- let(:job) { create(:ci_build, user: user) }
+ let(:job) { create(:ci_build, user: user, status: :running) }
before do
env['HTTP_AUTHORIZATION'] = "Bearer #{job.token}"
@@ -641,7 +666,7 @@ RSpec.describe Gitlab::Auth::AuthFinders do
end
describe '#find_user_from_job_token' do
- let(:job) { create(:ci_build, user: user) }
+ let(:job) { create(:ci_build, user: user, status: :running) }
let(:route_authentication_setting) { { job_token_allowed: true } }
subject { find_user_from_job_token }
@@ -666,6 +691,13 @@ RSpec.describe Gitlab::Auth::AuthFinders do
expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError)
end
+ it 'returns exception if the job is not running' do
+ set_header(described_class::JOB_TOKEN_HEADER, job.token)
+ job.success!
+
+ expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError)
+ end
+
context 'when route is not allowed to be authenticated' do
let(:route_authentication_setting) { { job_token_allowed: false } }
diff --git a/spec/lib/gitlab/auth/request_authenticator_spec.rb b/spec/lib/gitlab/auth/request_authenticator_spec.rb
index ef83321cc0e..b89ceb37076 100644
--- a/spec/lib/gitlab/auth/request_authenticator_spec.rb
+++ b/spec/lib/gitlab/auth/request_authenticator_spec.rb
@@ -88,7 +88,7 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do
describe '#find_user_from_job_token' do
let!(:user) { build(:user) }
- let!(:job) { build(:ci_build, user: user) }
+ let!(:job) { build(:ci_build, user: user, status: :running) }
before do
env[Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER] = 'token'
@@ -97,13 +97,18 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do
context 'with API requests' do
before do
env['SCRIPT_NAME'] = '/api/endpoint'
+ expect(::Ci::Build).to receive(:find_by_token).with('token').and_return(job)
end
it 'tries to find the user' do
- expect(::Ci::Build).to receive(:find_by_token).and_return(job)
-
expect(subject.find_sessionless_user([:api])).to eq user
end
+
+ it 'returns nil if the job is not running' do
+ job.status = :success
+
+ expect(subject.find_sessionless_user([:api])).to be_blank
+ end
end
context 'without API requests' do
diff --git a/spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb b/spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb
new file mode 100644
index 00000000000..f906870195a
--- /dev/null
+++ b/spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb
@@ -0,0 +1,112 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Auth::TwoFactorAuthVerifier do
+ let(:user) { create(:user) }
+
+ subject { described_class.new(user) }
+
+ describe '#two_factor_authentication_required?' do
+ describe 'when it is required on application level' do
+ it 'returns true' do
+ stub_application_setting require_two_factor_authentication: true
+
+ expect(subject.two_factor_authentication_required?).to be_truthy
+ end
+ end
+
+ describe 'when it is required on group level' do
+ it 'returns true' do
+ allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(true)
+
+ expect(subject.two_factor_authentication_required?).to be_truthy
+ end
+ end
+
+ describe 'when it is not required' do
+ it 'returns false when not required on group level' do
+ allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(false)
+
+ expect(subject.two_factor_authentication_required?).to be_falsey
+ end
+ end
+ end
+
+ describe '#current_user_needs_to_setup_two_factor?' do
+ it 'returns false when current_user is nil' do
+ expect(described_class.new(nil).current_user_needs_to_setup_two_factor?).to be_falsey
+ end
+
+ it 'returns false when current_user does not have temp email' do
+ allow(user).to receive(:two_factor_enabled?).and_return(false)
+ allow(user).to receive(:temp_oauth_email?).and_return(true)
+
+ expect(subject.current_user_needs_to_setup_two_factor?).to be_falsey
+ end
+
+ it 'returns false when current_user has 2fa disabled' do
+ allow(user).to receive(:temp_oauth_email?).and_return(false)
+ allow(user).to receive(:two_factor_enabled?).and_return(true)
+
+ expect(subject.current_user_needs_to_setup_two_factor?).to be_falsey
+ end
+
+ it 'returns true when user requires 2fa authentication' do
+ allow(user).to receive(:two_factor_enabled?).and_return(false)
+ allow(user).to receive(:temp_oauth_email?).and_return(false)
+
+ expect(subject.current_user_needs_to_setup_two_factor?).to be_truthy
+ end
+ end
+
+ describe '#two_factor_grace_period' do
+ it 'returns grace period from settings if there is no period from groups' do
+ stub_application_setting two_factor_grace_period: 2
+ allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(false)
+
+ expect(subject.two_factor_grace_period).to eq(2)
+ end
+
+ it 'returns grace period from groups if there is no period from settings' do
+ allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(true)
+ allow(user).to receive(:two_factor_grace_period).and_return(3)
+
+ expect(subject.two_factor_grace_period).to eq(3)
+ end
+
+ it 'returns minimal grace period if there is grace period from settings and from group' do
+ allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(true)
+ allow(user).to receive(:two_factor_grace_period).and_return(3)
+ stub_application_setting two_factor_grace_period: 2
+
+ expect(subject.two_factor_grace_period).to eq(2)
+ end
+ end
+
+ describe '#two_factor_grace_period_expired?' do
+ before do
+ allow(user).to receive(:otp_grace_period_started_at).and_return(4.hours.ago)
+ end
+
+ it 'returns true if the grace period has expired' do
+ allow(subject).to receive(:two_factor_grace_period).and_return(2)
+
+ expect(subject.two_factor_grace_period_expired?).to be_truthy
+ end
+
+ it 'returns false if the grace period has not expired' do
+ allow(subject).to receive(:two_factor_grace_period).and_return(6)
+
+ expect(subject.two_factor_grace_period_expired?).to be_falsey
+ end
+
+ context 'when otp_grace_period_started_at is nil' do
+ it 'returns false' do
+ allow(user).to receive(:otp_grace_period_started_at).and_return(nil)
+
+ expect(subject.two_factor_grace_period_expired?).to be_falsey
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index dcaaa8d4188..3bd35fb83fd 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -441,7 +441,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
end
end
- shared_examples 'deploy token with disabled registry' do
+ shared_examples 'deploy token with disabled feature' do
context 'when registry disabled' do
before do
stub_container_registry_config(enabled: false)
@@ -452,6 +452,15 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
.to eq(auth_failure)
end
end
+
+ context 'when repository is disabled' do
+ let(:project) { create(:project, :repository_disabled) }
+
+ it 'fails when login and token are valid' do
+ expect(gl_auth.find_for_git_client(login, deploy_token.token, project: project, ip: 'ip'))
+ .to eq(auth_failure)
+ end
+ end
end
context 'when deploy token and user have the same username' do
@@ -604,7 +613,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
it_behaves_like 'registry token scope'
end
- it_behaves_like 'deploy token with disabled registry'
+ it_behaves_like 'deploy token with disabled feature'
end
context 'when the deploy token has write_registry as a scope' do
@@ -626,7 +635,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
it_behaves_like 'registry token scope'
end
- it_behaves_like 'deploy token with disabled registry'
+ it_behaves_like 'deploy token with disabled feature'
end
end
end
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index 8153886a2ab..85567ab2e55 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -472,31 +472,135 @@ RSpec.describe Gitlab::GitAccess do
let(:actor) { key }
context 'pull code' do
- context 'when project is authorized' do
- before do
- key.projects << project
+ context 'when project is public' do
+ let(:project) { create(:project, :public, :repository, *options) }
+
+ context 'when deploy key exists in the project' do
+ before do
+ key.projects << project
+ end
+
+ context 'when the repository is public' do
+ let(:options) { %i[repository_enabled] }
+
+ it { expect { pull_access_check }.not_to raise_error }
+ end
+
+ context 'when the repository is private' do
+ let(:options) { %i[repository_private] }
+
+ it { expect { pull_access_check }.not_to raise_error }
+ end
+
+ context 'when the repository is disabled' do
+ let(:options) { %i[repository_disabled] }
+
+ it { expect { pull_access_check }.to raise_error('You are not allowed to download code from this project.') }
+ end
end
- it { expect { pull_access_check }.not_to raise_error }
+ context 'when deploy key does not exist in the project' do
+ context 'when the repository is public' do
+ let(:options) { %i[repository_enabled] }
+
+ it { expect { pull_access_check }.not_to raise_error }
+ end
+
+ context 'when the repository is private' do
+ let(:options) { %i[repository_private] }
+
+ it { expect { pull_access_check }.to raise_error('You are not allowed to download code from this project.') }
+ end
+
+ context 'when the repository is disabled' do
+ let(:options) { %i[repository_disabled] }
+
+ it { expect { pull_access_check }.to raise_error('You are not allowed to download code from this project.') }
+ end
+ end
end
- context 'when unauthorized' do
- context 'from public project' do
- let(:project) { create(:project, :public, :repository) }
+ context 'when project is internal' do
+ let(:project) { create(:project, :internal, :repository, *options) }
- it { expect { pull_access_check }.not_to raise_error }
+ context 'when deploy key exists in the project' do
+ before do
+ key.projects << project
+ end
+
+ context 'when the repository is public' do
+ let(:options) { %i[repository_enabled] }
+
+ it { expect { pull_access_check }.not_to raise_error }
+ end
+
+ context 'when the repository is private' do
+ let(:options) { %i[repository_private] }
+
+ it { expect { pull_access_check }.not_to raise_error }
+ end
+
+ context 'when the repository is disabled' do
+ let(:options) { %i[repository_disabled] }
+
+ it { expect { pull_access_check }.to raise_error('You are not allowed to download code from this project.') }
+ end
end
- context 'from internal project' do
- let(:project) { create(:project, :internal, :repository) }
+ context 'when deploy key does not exist in the project' do
+ context 'when the repository is public' do
+ let(:options) { %i[repository_enabled] }
- it { expect { pull_access_check }.to raise_not_found }
+ it { expect { pull_access_check }.to raise_error('The project you were looking for could not be found.') }
+ end
+
+ context 'when the repository is private' do
+ let(:options) { %i[repository_private] }
+
+ it { expect { pull_access_check }.to raise_error('The project you were looking for could not be found.') }
+ end
+
+ context 'when the repository is disabled' do
+ let(:options) { %i[repository_disabled] }
+
+ it { expect { pull_access_check }.to raise_error('The project you were looking for could not be found.') }
+ end
end
+ end
- context 'from private project' do
- let(:project) { create(:project, :private, :repository) }
+ context 'when project is private' do
+ let(:project) { create(:project, :private, :repository, *options) }
- it { expect { pull_access_check }.to raise_not_found }
+ context 'when deploy key exists in the project' do
+ before do
+ key.projects << project
+ end
+
+ context 'when the repository is private' do
+ let(:options) { %i[repository_private] }
+
+ it { expect { pull_access_check }.not_to raise_error }
+ end
+
+ context 'when the repository is disabled' do
+ let(:options) { %i[repository_disabled] }
+
+ it { expect { pull_access_check }.to raise_error('You are not allowed to download code from this project.') }
+ end
+ end
+
+ context 'when deploy key does not exist in the project' do
+ context 'when the repository is private' do
+ let(:options) { %i[repository_private] }
+
+ it { expect { pull_access_check }.to raise_error('The project you were looking for could not be found.') }
+ end
+
+ context 'when the repository is disabled' do
+ let(:options) { %i[repository_disabled] }
+
+ it { expect { pull_access_check }.to raise_error('The project you were looking for could not be found.') }
+ end
end
end
end
diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb
index 1a6858858a7..afa930b795a 100644
--- a/spec/lib/gitlab/regex_spec.rb
+++ b/spec/lib/gitlab/regex_spec.rb
@@ -180,15 +180,6 @@ RSpec.describe Gitlab::Regex do
it { is_expected.not_to match('foo/bar') }
end
- describe '.conan_file_name_regex' do
- subject { described_class.conan_file_name_regex }
-
- it { is_expected.to match('conanfile.py') }
- it { is_expected.to match('conan_package.tgz') }
- it { is_expected.not_to match('foo.txt') }
- it { is_expected.not_to match('!!()()') }
- end
-
describe '.conan_package_reference_regex' do
subject { described_class.conan_package_reference_regex }
diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb
index 24b47be3c69..de39c8c7c5c 100644
--- a/spec/models/active_session_spec.rb
+++ b/spec/models/active_session_spec.rb
@@ -296,6 +296,59 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
end
end
+ describe '.destroy_all_but_current' do
+ it 'gracefully handles a nil session ID' do
+ expect(described_class).not_to receive(:destroy_sessions)
+
+ ActiveSession.destroy_all_but_current(user, nil)
+ end
+
+ context 'with user sessions' do
+ let(:current_session_id) { '6919a6f1bb119dd7396fadc38fd18d0d' }
+
+ before do
+ Gitlab::Redis::SharedState.with do |redis|
+ redis.set(described_class.key_name(user.id, current_session_id),
+ Marshal.dump(ActiveSession.new(session_id: Rack::Session::SessionId.new(current_session_id))))
+ redis.set(described_class.key_name(user.id, '59822c7d9fcdfa03725eff41782ad97d'),
+ Marshal.dump(ActiveSession.new(session_id: Rack::Session::SessionId.new('59822c7d9fcdfa03725eff41782ad97d'))))
+ redis.set(described_class.key_name(9999, '5c8611e4f9c69645ad1a1492f4131358'),
+ Marshal.dump(ActiveSession.new(session_id: Rack::Session::SessionId.new('5c8611e4f9c69645ad1a1492f4131358'))))
+ redis.sadd(described_class.lookup_key_name(user.id), '59822c7d9fcdfa03725eff41782ad97d')
+ redis.sadd(described_class.lookup_key_name(user.id), current_session_id)
+ redis.sadd(described_class.lookup_key_name(9999), '5c8611e4f9c69645ad1a1492f4131358')
+ end
+ end
+
+ it 'removes the entry associated with the all user sessions but current' do
+ expect { ActiveSession.destroy_all_but_current(user, request.session) }.to change { ActiveSession.session_ids_for_user(user.id).size }.from(2).to(1)
+
+ expect(ActiveSession.session_ids_for_user(9999).size).to eq(1)
+ end
+
+ it 'removes the lookup entry of deleted sessions' do
+ ActiveSession.destroy_all_but_current(user, request.session)
+
+ Gitlab::Redis::SharedState.with do |redis|
+ expect(redis.smembers(described_class.lookup_key_name(user.id))).to eq [current_session_id]
+ end
+ end
+
+ it 'does not remove impersonated sessions' do
+ impersonated_session_id = '6919a6f1bb119dd7396fadc38fd18eee'
+ Gitlab::Redis::SharedState.with do |redis|
+ redis.set(described_class.key_name(user.id, impersonated_session_id),
+ Marshal.dump(ActiveSession.new(session_id: Rack::Session::SessionId.new(impersonated_session_id), is_impersonated: true)))
+ redis.sadd(described_class.lookup_key_name(user.id), impersonated_session_id)
+ end
+
+ expect { ActiveSession.destroy_all_but_current(user, request.session) }.to change { ActiveSession.session_ids_for_user(user.id).size }.from(3).to(2)
+
+ expect(ActiveSession.session_ids_for_user(9999).size).to eq(1)
+ end
+ end
+ end
+
describe '.cleanup' do
before do
stub_const("ActiveSession::ALLOWED_NUMBER_OF_ACTIVE_SESSIONS", 5)
diff --git a/spec/models/aws/role_spec.rb b/spec/models/aws/role_spec.rb
index 612868f6eb0..ee93c9d6fad 100644
--- a/spec/models/aws/role_spec.rb
+++ b/spec/models/aws/role_spec.rb
@@ -29,6 +29,12 @@ RSpec.describe Aws::Role do
it { is_expected.to be_truthy }
end
+
+ context 'ARN is nil' do
+ let(:role_arn) { }
+
+ it { is_expected.to be_truthy }
+ end
end
end
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index a3ed39abfb3..52f47f0a211 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -195,6 +195,19 @@ RSpec.describe Member do
it { expect(described_class.non_request).to include @accepted_request_member }
end
+ describe '.not_accepted_invitations_by_user' do
+ let(:invited_by_user) { create(:project_member, :invited, project: project, created_by: @owner_user) }
+
+ before do
+ create(:project_member, :invited, invite_email: 'test@test.com', project: project, created_by: @owner_user, invite_accepted_at: Time.zone.now)
+ create(:project_member, :invited, invite_email: 'test2@test.com', project: project, created_by: @maintainer_user)
+ end
+
+ subject { described_class.not_accepted_invitations_by_user(@owner_user) }
+
+ it { is_expected.to contain_exactly(invited_by_user) }
+ end
+
describe '.search_invite_email' do
it 'returns only members the matching e-mail' do
create(:group_member, :invited)
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index f9b819e22cd..e9077ed4143 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -894,6 +894,20 @@ RSpec.describe User do
expect(described_class.without_ghosts).to match_array([user1, user2])
end
end
+
+ describe '.by_id_and_login' do
+ let_it_be(:user) { create(:user) }
+
+ it 'finds a user regardless of case' do
+ expect(described_class.by_id_and_login(user.id, user.username.upcase))
+ .to contain_exactly(user)
+ end
+
+ it 'finds a user when login is an email address regardless of case' do
+ expect(described_class.by_id_and_login(user.id, user.email.upcase))
+ .to contain_exactly(user)
+ end
+ end
end
describe "Respond to" do
@@ -3579,6 +3593,42 @@ RSpec.describe User do
end
end
+ describe '#source_groups_of_two_factor_authentication_requirement' do
+ let_it_be(:group_not_requiring_2FA) { create :group }
+ let(:user) { create :user }
+
+ before do
+ group.add_user(user, GroupMember::OWNER)
+ group_not_requiring_2FA.add_user(user, GroupMember::OWNER)
+ end
+
+ context 'when user is direct member of group requiring 2FA' do
+ let_it_be(:group) { create :group, require_two_factor_authentication: true }
+
+ it 'returns group requiring 2FA' do
+ expect(user.source_groups_of_two_factor_authentication_requirement).to contain_exactly(group)
+ end
+ end
+
+ context 'when user is member of group which parent requires 2FA' do
+ let_it_be(:parent_group) { create :group, require_two_factor_authentication: true }
+ let_it_be(:group) { create :group, parent: parent_group }
+
+ it 'returns group requiring 2FA' do
+ expect(user.source_groups_of_two_factor_authentication_requirement).to contain_exactly(group)
+ end
+ end
+
+ context 'when user is member of group which child requires 2FA' do
+ let_it_be(:group) { create :group }
+ let_it_be(:child_group) { create :group, require_two_factor_authentication: true, parent: group }
+
+ it 'returns group requiring 2FA' do
+ expect(user.source_groups_of_two_factor_authentication_requirement).to contain_exactly(group)
+ end
+ end
+ end
+
describe '.active' do
before do
described_class.ghost
diff --git a/spec/requests/api/badges_spec.rb b/spec/requests/api/badges_spec.rb
index 99d224cb8e9..d8a345a79b0 100644
--- a/spec/requests/api/badges_spec.rb
+++ b/spec/requests/api/badges_spec.rb
@@ -332,10 +332,32 @@ RSpec.describe API::Badges do
context 'when deleting a badge' do
context 'and the source is a project' do
+ let(:badge) { project.group.badges.first }
+
it 'cannot delete badges owned by the project group' do
- delete api("/projects/#{project.id}/badges/#{project_group.badges.first.id}", maintainer)
+ expect do
+ delete api("/projects/#{project.id}/badges/#{badge.id}", maintainer)
+
+ expect(response).to have_gitlab_http_status(:not_found)
+ end.not_to change { badge.reload.persisted? }
+ end
+ end
+ end
+
+ context 'when updating a badge' do
+ context 'and the source is a project' do
+ let(:badge) { project.group.badges.first }
+ let(:example_name) { 'BadgeName' }
+ let(:example_url) { 'http://www.example.com' }
+ let(:example_url2) { 'http://www.example1.com' }
+
+ it 'cannot update badges owned by the project group' do
+ expect do
+ put api("/projects/#{project.id}/badges/#{badge.id}", maintainer),
+ params: { name: example_name, link_url: example_url, image_url: example_url2 }
- expect(response).to have_gitlab_http_status(:forbidden)
+ expect(response).to have_gitlab_http_status(:not_found)
+ end.not_to change { badge.reload.updated_at }
end
end
end
diff --git a/spec/requests/api/conan_packages_spec.rb b/spec/requests/api/conan_packages_spec.rb
index 95798b060f1..7a97743ede1 100644
--- a/spec/requests/api/conan_packages_spec.rb
+++ b/spec/requests/api/conan_packages_spec.rb
@@ -13,7 +13,7 @@ RSpec.describe API::ConanPackages do
let(:base_secret) { SecureRandom.base64(64) }
let(:auth_token) { personal_access_token.token }
- let(:job) { create(:ci_build, user: user) }
+ let(:job) { create(:ci_build, user: user, status: :running) }
let(:job_token) { job.token }
let(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) }
let(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) }
@@ -93,6 +93,14 @@ RSpec.describe API::ConanPackages do
expect(response).to have_gitlab_http_status(:unauthorized)
end
+ it 'responds with 401 Unauthorized when the job is not running' do
+ job.update!(status: :failed)
+ jwt = build_jwt_from_job(job)
+ get api('/packages/conan/v1/ping'), headers: build_token_auth_header(jwt.encoded)
+
+ expect(response).to have_gitlab_http_status(:unauthorized)
+ end
+
context 'packages feature disabled' do
it 'responds with 404 Not Found' do
stub_packages_setting(enabled: false)
@@ -233,6 +241,18 @@ RSpec.describe API::ConanPackages do
end
end
+ shared_examples 'rejects invalid file_name' do |invalid_file_name|
+ let(:file_name) { invalid_file_name }
+
+ context 'with invalid file_name' do
+ it 'returns 400' do
+ subject
+
+ expect(response).to have_gitlab_http_status(:bad_request)
+ end
+ end
+ end
+
shared_examples 'rejects recipe for invalid project' do
context 'with invalid recipe path' do
let(:recipe_path) { 'aa/bb/not-existing-project/ccc' }
@@ -685,8 +705,6 @@ RSpec.describe API::ConanPackages do
context 'without a file from workhorse' do
let(:params) { { file: nil } }
- it_behaves_like 'package workhorse uploads'
-
it 'rejects the request' do
subject
@@ -694,6 +712,10 @@ RSpec.describe API::ConanPackages do
end
end
+ context 'with a file' do
+ it_behaves_like 'package workhorse uploads'
+ end
+
context 'without a token' do
it 'rejects request without a token' do
headers_with_token.delete('HTTP_AUTHORIZATION')
@@ -852,16 +874,22 @@ RSpec.describe API::ConanPackages do
end
describe 'PUT /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:file_name/authorize' do
- subject { put api("/packages/conan/v1/files/#{recipe_path}/0/export/conanfile.py/authorize"), headers: headers_with_token }
+ let(:file_name) { 'conanfile.py' }
+
+ subject { put api("/packages/conan/v1/files/#{recipe_path}/0/export/#{file_name}/authorize"), headers: headers_with_token }
it_behaves_like 'rejects invalid recipe'
+ it_behaves_like 'rejects invalid file_name', 'conanfile.py.git%2fgit-upload-pack'
it_behaves_like 'workhorse authorization'
end
describe 'PUT /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:conan_package_reference/:package_revision/:file_name/authorize' do
- subject { put api("/packages/conan/v1/files/#{recipe_path}/0/package/123456789/0/conaninfo.txt/authorize"), headers: headers_with_token }
+ let(:file_name) { 'conaninfo.txt' }
+
+ subject { put api("/packages/conan/v1/files/#{recipe_path}/0/package/123456789/0/#{file_name}/authorize"), headers: headers_with_token }
it_behaves_like 'rejects invalid recipe'
+ it_behaves_like 'rejects invalid file_name', 'conaninfo.txttest'
it_behaves_like 'workhorse authorization'
end
@@ -875,11 +903,13 @@ RSpec.describe API::ConanPackages do
method: :put,
file_key: :file,
params: params,
+ send_rewritten_field: true,
headers: headers_with_token
)
end
it_behaves_like 'rejects invalid recipe'
+ it_behaves_like 'rejects invalid file_name', 'conanfile.py.git%2fgit-upload-pack'
it_behaves_like 'uploads a package file'
end
@@ -893,12 +923,15 @@ RSpec.describe API::ConanPackages do
method: :put,
file_key: :file,
params: params,
- headers: headers_with_token
+ headers: headers_with_token,
+ send_rewritten_field: true
)
end
it_behaves_like 'rejects invalid recipe'
+ it_behaves_like 'rejects invalid file_name', 'conaninfo.txttest'
it_behaves_like 'uploads a package file'
+
context 'tracking the conan_package.tgz upload' do
let(:file_name) { ::Packages::Conan::FileMetadatum::PACKAGE_BINARY }
diff --git a/spec/requests/api/go_proxy_spec.rb b/spec/requests/api/go_proxy_spec.rb
index 2d7e319b0be..9d422ebbce2 100644
--- a/spec/requests/api/go_proxy_spec.rb
+++ b/spec/requests/api/go_proxy_spec.rb
@@ -11,7 +11,7 @@ RSpec.describe API::GoProxy do
let_it_be(:base) { "#{Settings.build_gitlab_go_url}/#{project.full_path}" }
let_it_be(:oauth) { create :oauth_access_token, scopes: 'api', resource_owner: user }
- let_it_be(:job) { create :ci_build, user: user }
+ let_it_be(:job) { create :ci_build, user: user, status: :running }
let_it_be(:pa_token) { create :personal_access_token, user: user }
let_it_be(:modules) do
@@ -393,6 +393,13 @@ RSpec.describe API::GoProxy do
expect(response).to have_gitlab_http_status(:ok)
end
+ it 'returns unauthorized with a failed job token' do
+ job.update!(status: :failed)
+ get_resource(oauth_access_token: job)
+
+ expect(response).to have_gitlab_http_status(:unauthorized)
+ end
+
it 'returns unauthorized with no authentication' do
get_resource
diff --git a/spec/requests/api/graphql/mutations/snippets/destroy_spec.rb b/spec/requests/api/graphql/mutations/snippets/destroy_spec.rb
index 8ade72635af..c861564c66b 100644
--- a/spec/requests/api/graphql/mutations/snippets/destroy_spec.rb
+++ b/spec/requests/api/graphql/mutations/snippets/destroy_spec.rb
@@ -46,6 +46,31 @@ RSpec.describe 'Destroying a Snippet' do
expect(mutation_response).to have_key('snippet')
expect(mutation_response['snippet']).to be_nil
end
+
+ context 'when a bad gid is given' do
+ let!(:project) { create(:project, :private) }
+ let!(:snippet) { create(:project_snippet, :private, project: project, author: create(:user)) }
+ let!(:snippet_gid) { project.to_gid.to_s }
+
+ it 'returns an error' do
+ post_graphql_mutation(mutation, current_user: current_user)
+
+ expect(graphql_errors)
+ .to include(a_hash_including('message' => "#{snippet_gid} is not a valid id for Snippet."))
+ end
+
+ it 'does not destroy the Snippet' do
+ expect do
+ post_graphql_mutation(mutation, current_user: current_user)
+ end.not_to change { Snippet.count }
+ end
+
+ it 'does not destroy the Project' do
+ expect do
+ post_graphql_mutation(mutation, current_user: current_user)
+ end.not_to change { Project.count }
+ end
+ end
end
end
diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb
index fefa7105327..1fa705423d2 100644
--- a/spec/requests/api/helpers_spec.rb
+++ b/spec/requests/api/helpers_spec.rb
@@ -85,6 +85,27 @@ RSpec.describe API::Helpers do
end
it { is_expected.to eq(user) }
+
+ context 'when user should have 2fa enabled' do
+ before do
+ allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(true)
+ allow_next_instance_of(Gitlab::Auth::TwoFactorAuthVerifier) do |verifier|
+ allow(verifier).to receive(:two_factor_grace_period_expired?).and_return(true)
+ end
+ end
+
+ context 'when 2fa is not enabled' do
+ it { is_expected.to be_nil }
+ end
+
+ context 'when 2fa is enabled' do
+ before do
+ allow(user).to receive(:two_factor_enabled?).and_return(true)
+ end
+
+ it { is_expected.to eq(user) }
+ end
+ end
end
context "PUT request" do
diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb
index b9351308545..b74887762a2 100644
--- a/spec/requests/api/maven_packages_spec.rb
+++ b/spec/requests/api/maven_packages_spec.rb
@@ -12,7 +12,7 @@ RSpec.describe API::MavenPackages do
let_it_be(:package_file) { package.package_files.with_file_name_like('%.xml').first }
let_it_be(:jar_file) { package.package_files.with_file_name_like('%.jar').first }
let_it_be(:personal_access_token) { create(:personal_access_token, user: user) }
- let_it_be(:job) { create(:ci_build, user: user) }
+ let_it_be(:job, reload: true) { create(:ci_build, user: user, status: :running) }
let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) }
let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) }
@@ -102,11 +102,25 @@ RSpec.describe API::MavenPackages do
end
shared_examples 'downloads with a job token' do
- it 'allows download with job token' do
- download_file(package_file.file_name, job_token: job.token)
+ context 'with a running job' do
+ it 'allows download with job token' do
+ download_file(package_file.file_name, job_token: job.token)
- expect(response).to have_gitlab_http_status(:ok)
- expect(response.media_type).to eq('application/octet-stream')
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(response.media_type).to eq('application/octet-stream')
+ end
+ end
+
+ context 'with a finished job' do
+ before do
+ job.update!(status: :failed)
+ end
+
+ it 'returns unauthorized error' do
+ download_file(package_file.file_name, job_token: job.token)
+
+ expect(response).to have_gitlab_http_status(:unauthorized)
+ end
end
end
@@ -557,13 +571,20 @@ RSpec.describe API::MavenPackages do
expect(jar_file.file_name).to eq(file_upload.original_filename)
end
- it 'allows upload with job token' do
+ it 'allows upload with running job token' do
upload_file(params.merge(job_token: job.token))
expect(response).to have_gitlab_http_status(:ok)
expect(project.reload.packages.last.build_info.pipeline).to eq job.pipeline
end
+ it 'rejects upload without running job token' do
+ job.update!(status: :failed)
+ upload_file(params.merge(job_token: job.token))
+
+ expect(response).to have_gitlab_http_status(:unauthorized)
+ end
+
it 'allows upload with deploy token' do
upload_file(params, headers_with_deploy_token)
diff --git a/spec/requests/api/npm_packages_spec.rb b/spec/requests/api/npm_packages_spec.rb
index 94647123df0..108ea84b7e6 100644
--- a/spec/requests/api/npm_packages_spec.rb
+++ b/spec/requests/api/npm_packages_spec.rb
@@ -12,7 +12,7 @@ RSpec.describe API::NpmPackages do
let_it_be(:package, reload: true) { create(:npm_package, project: project) }
let_it_be(:token) { create(:oauth_access_token, scopes: 'api', resource_owner: user) }
let_it_be(:personal_access_token) { create(:personal_access_token, user: user) }
- let_it_be(:job) { create(:ci_build, user: user) }
+ let_it_be(:job, reload: true) { create(:ci_build, user: user, status: :running) }
let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) }
let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) }
@@ -27,12 +27,19 @@ RSpec.describe API::NpmPackages do
expect_a_valid_package_response
end
- it 'returns the package info with job token' do
+ it 'returns the package info with running job token' do
get_package_with_job_token(package)
expect_a_valid_package_response
end
+ it 'denies request without running job token' do
+ job.update!(status: :success)
+ get_package_with_job_token(package)
+
+ expect(response).to have_gitlab_http_status(:unauthorized)
+ end
+
it 'denies request without oauth token' do
get_package(package)
diff --git a/spec/requests/api/nuget_packages_spec.rb b/spec/requests/api/nuget_packages_spec.rb
index ab537a61058..87c62ec41c6 100644
--- a/spec/requests/api/nuget_packages_spec.rb
+++ b/spec/requests/api/nuget_packages_spec.rb
@@ -80,7 +80,7 @@ RSpec.describe API::NugetPackages do
end
with_them do
- let(:job) { user_token ? create(:ci_build, project: project, user: user) : double(token: 'wrong') }
+ let(:job) { user_token ? create(:ci_build, project: project, user: user, status: :running) : double(token: 'wrong') }
let(:headers) { user_role == :anonymous ? {} : job_basic_auth_header(job) }
subject { get api(url), headers: headers }
diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb
index a9a92a4d3cd..779ae983886 100644
--- a/spec/requests/api/releases_spec.rb
+++ b/spec/requests/api/releases_spec.rb
@@ -671,12 +671,20 @@ RSpec.describe API::Releases do
end
context 'when a valid token is provided' do
- it 'creates the release' do
+ it 'creates the release for a running job' do
+ job.update!(status: :running)
post api("/projects/#{project.id}/releases"), params: params.merge(job_token: job.token)
expect(response).to have_gitlab_http_status(:created)
expect(project.releases.last.description).to eq('Another nice release')
end
+
+ it 'returns an :unauthorized error for a completed job' do
+ job.success!
+ post api("/projects/#{project.id}/releases"), params: params.merge(job_token: job.token)
+
+ expect(response).to have_gitlab_http_status(:unauthorized)
+ end
end
end
diff --git a/spec/requests/api/terraform/state_spec.rb b/spec/requests/api/terraform/state_spec.rb
index c6cba39314b..c47a12456c3 100644
--- a/spec/requests/api/terraform/state_spec.rb
+++ b/spec/requests/api/terraform/state_spec.rb
@@ -72,7 +72,7 @@ RSpec.describe API::Terraform::State do
let(:auth_header) { job_basic_auth_header(job) }
context 'with maintainer permissions' do
- let(:job) { create(:ci_build, project: project, user: maintainer) }
+ let(:job) { create(:ci_build, status: :running, project: project, user: maintainer) }
it 'returns terraform state belonging to a project of given state name' do
request
@@ -81,6 +81,13 @@ RSpec.describe API::Terraform::State do
expect(response.body).to eq(state.file.read)
end
+ it 'returns unauthorized if the the job is not running' do
+ job.update!(status: :failed)
+ request
+
+ expect(response).to have_gitlab_http_status(:unauthorized)
+ end
+
context 'for a project that does not exist' do
let(:project_id) { '0000' }
@@ -93,7 +100,7 @@ RSpec.describe API::Terraform::State do
end
context 'with developer permissions' do
- let(:job) { create(:ci_build, project: project, user: developer) }
+ let(:job) { create(:ci_build, status: :running, project: project, user: developer) }
it 'returns terraform state belonging to a project of given state name' do
request
diff --git a/spec/services/applications/create_service_spec.rb b/spec/services/applications/create_service_spec.rb
index 58ac723ee55..8b8beb057a9 100644
--- a/spec/services/applications/create_service_spec.rb
+++ b/spec/services/applications/create_service_spec.rb
@@ -6,9 +6,24 @@ RSpec.describe ::Applications::CreateService do
include TestRequestHelpers
let(:user) { create(:user) }
- let(:params) { attributes_for(:application) }
subject { described_class.new(user, params) }
- it { expect { subject.execute(test_request) }.to change { Doorkeeper::Application.count }.by(1) }
+ context 'when scopes are present' do
+ let(:params) { attributes_for(:application, scopes: ['read_user']) }
+
+ it { expect { subject.execute(test_request) }.to change { Doorkeeper::Application.count }.by(1) }
+ end
+
+ context 'when scopes are missing' do
+ let(:params) { attributes_for(:application) }
+
+ it { expect { subject.execute(test_request) }.not_to change { Doorkeeper::Application.count } }
+
+ it 'includes blank scopes error message' do
+ application = subject.execute(test_request)
+
+ expect(application.errors.full_messages).to include "Scopes can't be blank"
+ end
+ end
end
diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb
index 8d58c4b27e1..bc85f4f0087 100644
--- a/spec/services/auth/container_registry_authentication_service_spec.rb
+++ b/spec/services/auth/container_registry_authentication_service_spec.rb
@@ -654,6 +654,19 @@ RSpec.describe Auth::ContainerRegistryAuthenticationService do
it_behaves_like 'not a container repository factory'
end
end
+
+ context 'for project that disables repository' do
+ let(:project) { create(:project, :public, :repository_disabled) }
+
+ context 'disallow when pulling' do
+ let(:current_params) do
+ { scopes: ["repository:#{project.full_path}:pull"] }
+ end
+
+ it_behaves_like 'an inaccessible'
+ it_behaves_like 'not a container repository factory'
+ end
+ end
end
context 'registry catalog browsing authorized as admin' do
diff --git a/spec/services/ci/pipeline_trigger_service_spec.rb b/spec/services/ci/pipeline_trigger_service_spec.rb
index 18fab9623ec..ac077e3c30e 100644
--- a/spec/services/ci/pipeline_trigger_service_spec.rb
+++ b/spec/services/ci/pipeline_trigger_service_spec.rb
@@ -106,9 +106,23 @@ RSpec.describe Ci::PipelineTriggerService do
let(:params) { { token: job.token, ref: 'master', variables: nil } }
let(:job) { create(:ci_build, :success, pipeline: pipeline, user: user) }
- it 'does nothing' do
+ it 'does nothing', :aggregate_failures do
+ expect { result }.not_to change { Ci::Pipeline.count }
+ expect(result[:message]).to eq('Job is not running')
+ expect(result[:http_status]).to eq(401)
+ end
+ end
+
+ context 'when job does not have a project' do
+ let(:params) { { token: job.token, ref: 'master', variables: nil } }
+ let(:job) { create(:ci_build, status: :running, pipeline: pipeline, user: user) }
+
+ it 'does nothing', :aggregate_failures do
+ job.update!(project: nil)
+
expect { result }.not_to change { Ci::Pipeline.count }
- expect(result[:message]).to eq('400 Job has to be running')
+ expect(result[:message]).to eq('Project has been deleted!')
+ expect(result[:http_status]).to eq(401)
end
end
diff --git a/spec/services/clusters/aws/authorize_role_service_spec.rb b/spec/services/clusters/aws/authorize_role_service_spec.rb
index 3d12400a47b..5b47cf0ecde 100644
--- a/spec/services/clusters/aws/authorize_role_service_spec.rb
+++ b/spec/services/clusters/aws/authorize_role_service_spec.rb
@@ -3,47 +3,34 @@
require 'spec_helper'
RSpec.describe Clusters::Aws::AuthorizeRoleService do
- let(:user) { create(:user) }
+ subject { described_class.new(user, params: params).execute }
+
+ let(:role) { create(:aws_role) }
+ let(:user) { role.user }
let(:credentials) { instance_double(Aws::Credentials) }
let(:credentials_service) { instance_double(Clusters::Aws::FetchCredentialsService, execute: credentials) }
+ let(:role_arn) { 'arn:my-role' }
let(:params) do
params = ActionController::Parameters.new({
cluster: {
- role_arn: 'arn:my-role',
- role_external_id: 'external-id'
+ role_arn: role_arn
}
})
- params.require(:cluster).permit(:role_arn, :role_external_id)
+ params.require(:cluster).permit(:role_arn)
end
- subject { described_class.new(user, params: params).execute }
-
before do
allow(Clusters::Aws::FetchCredentialsService).to receive(:new)
.with(instance_of(Aws::Role)).and_return(credentials_service)
end
- context 'role does not exist' do
- it 'creates an Aws::Role record and returns a set of credentials' do
- expect(user).to receive(:create_aws_role!)
- .with(params).and_call_original
-
- expect(subject.status).to eq(:ok)
- expect(subject.body).to eq(credentials)
- end
- end
-
- context 'role already exists' do
- let(:role) { create(:aws_role, user: user) }
-
+ context 'role exists' do
it 'updates the existing Aws::Role record and returns a set of credentials' do
- expect(role).to receive(:update!)
- .with(params).and_call_original
-
expect(subject.status).to eq(:ok)
expect(subject.body).to eq(credentials)
+ expect(role.reload.role_arn).to eq(role_arn)
end
end
@@ -61,11 +48,14 @@ RSpec.describe Clusters::Aws::AuthorizeRoleService do
end
end
- context 'cannot create role' do
- before do
- allow(user).to receive(:create_aws_role!)
- .and_raise(ActiveRecord::RecordInvalid.new(user))
- end
+ context 'role does not exist' do
+ let(:user) { create(:user) }
+
+ include_examples 'bad request'
+ end
+
+ context 'supplied ARN is invalid' do
+ let(:role_arn) { 'invalid' }
include_examples 'bad request'
end
diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb
index 13e7b4c1006..5c90f1f54ea 100644
--- a/spec/services/members/destroy_service_spec.rb
+++ b/spec/services/members/destroy_service_spec.rb
@@ -292,6 +292,10 @@ RSpec.describe Members::DestroyService do
before do
create(:group_member, :developer, group: subsubgroup, user: member_user)
+ create(:project_member, :invited, project: group_project, created_by: member_user)
+ create(:group_member, :invited, group: group, created_by: member_user)
+ create(:project_member, :invited, project: subsubproject, created_by: member_user)
+ create(:group_member, :invited, group: subgroup, created_by: member_user)
subsubproject.add_developer(member_user)
control_project.add_maintainer(user)
@@ -325,5 +329,41 @@ RSpec.describe Members::DestroyService do
it 'does not remove the user from the control project' do
expect(control_project.members.map(&:user)).to include(user)
end
+
+ it 'removes group members invited by deleted user' do
+ expect(group.members.not_accepted_invitations_by_user(member_user)).to be_empty
+ end
+
+ it 'removes project members invited by deleted user' do
+ expect(group_project.members.not_accepted_invitations_by_user(member_user)).to be_empty
+ end
+
+ it 'removes subgroup members invited by deleted user' do
+ expect(subgroup.members.not_accepted_invitations_by_user(member_user)).to be_empty
+ end
+
+ it 'removes subproject members invited by deleted user' do
+ expect(subsubproject.members.not_accepted_invitations_by_user(member_user)).to be_empty
+ end
+ end
+
+ context 'deletion of invitations created by deleted project member' do
+ let(:user) { project.owner }
+ let(:member_user) { create(:user) }
+ let(:opts) { {} }
+
+ let(:project) { create(:project) }
+
+ before do
+ create(:project_member, :invited, project: project, created_by: member_user)
+
+ project_member = create(:project_member, :maintainer, user: member_user, project: project)
+
+ described_class.new(user).execute(project_member, opts)
+ end
+
+ it 'removes project members invited by deleted user' do
+ expect(project.members.not_accepted_invitations_by_user(member_user)).to be_empty
+ end
end
end
diff --git a/spec/services/projects/update_remote_mirror_service_spec.rb b/spec/services/projects/update_remote_mirror_service_spec.rb
index 09244db8010..6785b71fcc0 100644
--- a/spec/services/projects/update_remote_mirror_service_spec.rb
+++ b/spec/services/projects/update_remote_mirror_service_spec.rb
@@ -56,6 +56,40 @@ RSpec.describe Projects::UpdateRemoteMirrorService do
expect(remote_mirror.last_error).to include('Badly broken')
end
+ context 'when the URL is blocked' do
+ before do
+ allow(Gitlab::UrlBlocker).to receive(:blocked_url?).and_return(true)
+ end
+
+ it 'fails and returns error status' do
+ expect(execute!).to eq(status: :error, message: 'The remote mirror URL is invalid.')
+ end
+ end
+
+ context "when given URLs containing escaped elements" do
+ using RSpec::Parameterized::TableSyntax
+
+ where(:url, :result_status) do
+ "https://user:0a%23@test.example.com/project.git" | :success
+ "https://git.example.com:1%2F%2F@source.developers.google.com/project.git" | :success
+ CGI.escape("git://localhost:1234/some-path?some-query=some-val\#@example.com/") | :error
+ CGI.escape(CGI.escape("https://user:0a%23@test.example.com/project.git")) | :error
+ end
+
+ with_them do
+ before do
+ allow(remote_mirror).to receive(:url).and_return(url)
+ allow(service).to receive(:update_mirror).with(remote_mirror).and_return(true)
+ end
+
+ it "returns expected status" do
+ result = execute!
+
+ expect(result[:status]).to eq(result_status)
+ end
+ end
+ end
+
context 'when the update fails because of a `Gitlab::Git::CommandError`' do
before do
allow(remote_mirror).to receive(:update_repository)
diff --git a/spec/support/shared_examples/controllers/clusters_controller_shared_examples.rb b/spec/support/shared_examples/controllers/clusters_controller_shared_examples.rb
new file mode 100644
index 00000000000..aa17e72d08e
--- /dev/null
+++ b/spec/support/shared_examples/controllers/clusters_controller_shared_examples.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+RSpec.shared_examples 'GET new cluster shared examples' do
+ describe 'EKS cluster' do
+ context 'user already has an associated AWS role' do
+ let!(:role) { create(:aws_role, user: user) }
+
+ it 'does not create an Aws::Role record' do
+ expect { go(provider: 'aws') }.not_to change { Aws::Role.count }
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(assigns(:aws_role)).to eq(role)
+ end
+ end
+
+ context 'user does not have an associated AWS role' do
+ it 'creates an Aws::Role record' do
+ expect { go(provider: 'aws') }.to change { Aws::Role.count }
+
+ expect(response).to have_gitlab_http_status(:ok)
+
+ role = assigns(:aws_role)
+ expect(role.user).to eq(user)
+ expect(role.role_arn).to be_nil
+ expect(role.role_external_id).to be_present
+ end
+ end
+ end
+end
diff --git a/yarn.lock b/yarn.lock
index 068513341c2..decf652a973 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -7064,10 +7064,10 @@ jquery.waitforimages@^2.2.0:
resolved "https://registry.yarnpkg.com/jquery.waitforimages/-/jquery.waitforimages-2.2.0.tgz#63f23131055a1b060dc913e6d874bcc9b9e6b16b"
integrity sha1-Y/IxMQVaGwYNyRPm2HS8ybnmsWs=
-"jquery@>= 1.9.1", jquery@>=1.8.0, jquery@^3.4.1:
- version "3.4.1"
- resolved "https://registry.yarnpkg.com/jquery/-/jquery-3.4.1.tgz#714f1f8d9dde4bdfa55764ba37ef214630d80ef2"
- integrity sha512-36+AdBzCL+y6qjw5Tx7HgzeGCzC81MDDgaUP8ld2zhx58HdqXGoBd+tHdrBMiyjGQs0Hxs/MLZTu/eHNJJuWPw==
+"jquery@>= 1.9.1", jquery@>=1.8.0, jquery@^3.5.0:
+ version "3.5.1"
+ resolved "https://registry.yarnpkg.com/jquery/-/jquery-3.5.1.tgz#d7b4d08e1bfdb86ad2f1a3d039ea17304717abb5"
+ integrity sha512-XwIBPqcMn57FxfT+Go5pzySnm4KWkT1Tv7gjrpT1srtf8Weynl6R273VJ5GjkRb51IzMp5nbaPjJXMWeju2MKg==
js-base64@^2.1.8:
version "2.5.1"