Commit a986819a authored by GitLab Bot's avatar GitLab Bot

Add latest changes from gitlab-org/security/gitlab@13-3-stable-ee

parent 92d5172a
...@@ -22,6 +22,8 @@ module AuthenticatesWithTwoFactor ...@@ -22,6 +22,8 @@ module AuthenticatesWithTwoFactor
return handle_locked_user(user) unless user.can?(:log_in) return handle_locked_user(user) unless user.can?(:log_in)
session[:otp_user_id] = user.id session[:otp_user_id] = user.id
session[:user_updated_at] = user.updated_at
setup_u2f_authentication(user) setup_u2f_authentication(user)
render 'devise/sessions/two_factor' render 'devise/sessions/two_factor'
end end
...@@ -39,6 +41,7 @@ module AuthenticatesWithTwoFactor ...@@ -39,6 +41,7 @@ module AuthenticatesWithTwoFactor
def authenticate_with_two_factor def authenticate_with_two_factor
user = self.resource = find_user user = self.resource = find_user
return handle_locked_user(user) unless user.can?(:log_in) 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] if user_params[:otp_attempt].present? && session[:otp_user_id]
authenticate_with_two_factor_via_otp(user) authenticate_with_two_factor_via_otp(user)
...@@ -63,12 +66,14 @@ module AuthenticatesWithTwoFactor ...@@ -63,12 +66,14 @@ module AuthenticatesWithTwoFactor
def clear_two_factor_attempt! def clear_two_factor_attempt!
session.delete(:otp_user_id) session.delete(:otp_user_id)
session.delete(:user_updated_at)
session.delete(:challenge)
end end
def authenticate_with_two_factor_via_otp(user) def authenticate_with_two_factor_via_otp(user)
if valid_otp_attempt?(user) if valid_otp_attempt?(user)
# Remove any lingering user data from login # 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' remember_me(user) if user_params[:remember_me] == '1'
user.save! user.save!
...@@ -85,8 +90,7 @@ module AuthenticatesWithTwoFactor ...@@ -85,8 +90,7 @@ module AuthenticatesWithTwoFactor
def authenticate_with_two_factor_via_u2f(user) def authenticate_with_two_factor_via_u2f(user)
if U2fRegistration.authenticate(user, u2f_app_id, user_params[:device_response], session[:challenge]) if U2fRegistration.authenticate(user, u2f_app_id, user_params[:device_response], session[:challenge])
# Remove any lingering user data from login # Remove any lingering user data from login
session.delete(:otp_user_id) clear_two_factor_attempt!
session.delete(:challenge)
remember_me(user) if user_params[:remember_me] == '1' remember_me(user) if user_params[:remember_me] == '1'
sign_in(user, message: :two_factor_authenticated, event: :authentication) sign_in(user, message: :two_factor_authenticated, event: :authentication)
...@@ -113,4 +117,18 @@ module AuthenticatesWithTwoFactor ...@@ -113,4 +117,18 @@ module AuthenticatesWithTwoFactor
end end
end end
# rubocop: enable CodeReuse/ActiveRecord # 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 end
...@@ -29,12 +29,11 @@ module EnforcesTwoFactorAuthentication ...@@ -29,12 +29,11 @@ module EnforcesTwoFactorAuthentication
end end
def two_factor_authentication_required? def two_factor_authentication_required?
Gitlab::CurrentSettings.require_two_factor_authentication? || two_factor_verifier.two_factor_authentication_required?
current_user.try(:require_two_factor_authentication_from_group?)
end end
def current_user_requires_two_factor? 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 end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
...@@ -43,7 +42,7 @@ module EnforcesTwoFactorAuthentication ...@@ -43,7 +42,7 @@ module EnforcesTwoFactorAuthentication
if Gitlab::CurrentSettings.require_two_factor_authentication? if Gitlab::CurrentSettings.require_two_factor_authentication?
global.call global.call
else 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) group.call(groups)
end end
end end
...@@ -51,14 +50,11 @@ module EnforcesTwoFactorAuthentication ...@@ -51,14 +50,11 @@ module EnforcesTwoFactorAuthentication
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def two_factor_grace_period def two_factor_grace_period
periods = [Gitlab::CurrentSettings.two_factor_grace_period] two_factor_verifier.two_factor_grace_period
periods << current_user.two_factor_grace_period if current_user.try(:require_two_factor_authentication_from_group?)
periods.min
end end
def two_factor_grace_period_expired? def two_factor_grace_period_expired?
date = current_user.otp_grace_period_started_at two_factor_verifier.two_factor_grace_period_expired?
date && (date + two_factor_grace_period.hours) < Time.current
end end
def two_factor_skippable? def two_factor_skippable?
...@@ -70,6 +66,10 @@ module EnforcesTwoFactorAuthentication ...@@ -70,6 +66,10 @@ module EnforcesTwoFactorAuthentication
def skip_two_factor? def skip_two_factor?
session[:skip_two_factor] && session[:skip_two_factor] > Time.current session[:skip_two_factor] && session[:skip_two_factor] > Time.current
end end
def two_factor_verifier
@two_factor_verifier ||= Gitlab::Auth::TwoFactorAuthVerifier.new(current_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end
end end
EnforcesTwoFactorAuthentication.prepend_if_ee('EE::EnforcesTwoFactorAuthentication') EnforcesTwoFactorAuthentication.prepend_if_ee('EE::EnforcesTwoFactorAuthentication')
...@@ -17,4 +17,16 @@ module HooksExecution ...@@ -17,4 +17,16 @@ module HooksExecution
flash[:alert] = "Hook execution failed: #{message}" flash[:alert] = "Hook execution failed: #{message}"
end end
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 end
...@@ -50,12 +50,6 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -50,12 +50,6 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
redirect_unverified_saml_initiation redirect_unverified_saml_initiation
end end
def omniauth_error
@provider = params[:provider]
@error = params[:error]
render 'errors/omniauth_error', layout: "oauth_error", status: :unprocessable_entity
end
def cas3 def cas3
ticket = params['ticket'] ticket = params['ticket']
if ticket if ticket
...@@ -205,9 +199,10 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -205,9 +199,10 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
def fail_login(user) def fail_login(user)
log_failed_login(user.username, oauth['provider']) 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 end
def fail_auth0_login def fail_auth0_login
......
...@@ -7,6 +7,7 @@ class Profiles::ActiveSessionsController < Profiles::ApplicationController ...@@ -7,6 +7,7 @@ class Profiles::ActiveSessionsController < Profiles::ApplicationController
def destroy def destroy
ActiveSession.destroy_with_public_id(current_user, params[:id]) ActiveSession.destroy_with_public_id(current_user, params[:id])
current_user.forget_me!
respond_to do |format| respond_to do |format|
format.html { redirect_to profile_active_sessions_url, status: :found } format.html { redirect_to profile_active_sessions_url, status: :found }
......
...@@ -4,7 +4,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController ...@@ -4,7 +4,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
skip_before_action :check_two_factor_requirement skip_before_action :check_two_factor_requirement
def show def show
unless current_user.otp_secret unless current_user.two_factor_enabled?
current_user.otp_secret = User.generate_otp_secret(32) current_user.otp_secret = User.generate_otp_secret(32)
end end
...@@ -38,6 +38,8 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController ...@@ -38,6 +38,8 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
def create def create
if current_user.validate_and_consume_otp!(params[:pin_code]) 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| Users::UpdateService.new(current_user, user: current_user, otp_required_for_login: true).execute! do |user|
@codes = user.generate_otp_backup_codes! @codes = user.generate_otp_backup_codes!
end end
......
...@@ -6,6 +6,7 @@ class Projects::HooksController < Projects::ApplicationController ...@@ -6,6 +6,7 @@ class Projects::HooksController < Projects::ApplicationController
# Authorize # Authorize
before_action :authorize_admin_project! before_action :authorize_admin_project!
before_action :hook_logs, only: :edit before_action :hook_logs, only: :edit
before_action -> { create_rate_limit(:project_testing_hook, @project) }, only: :test
respond_to :html respond_to :html
......
...@@ -92,7 +92,7 @@ class Projects::TagsController < Projects::ApplicationController ...@@ -92,7 +92,7 @@ class Projects::TagsController < Projects::ApplicationController
end end
format.js do format.js do
render status: :unprocessable_entity render status: :ok
end end
end end
end end
......
...@@ -8,6 +8,7 @@ class SessionsController < Devise::SessionsController ...@@ -8,6 +8,7 @@ class SessionsController < Devise::SessionsController
include Recaptcha::Verify include Recaptcha::Verify
include RendersLdapServers include RendersLdapServers
include KnownSignIn include KnownSignIn
include Gitlab::Utils::StrongMemoize
skip_before_action :check_two_factor_requirement, only: [:destroy] skip_before_action :check_two_factor_requirement, only: [:destroy]
# replaced with :require_no_authentication_without_flash # replaced with :require_no_authentication_without_flash
...@@ -197,10 +198,14 @@ class SessionsController < Devise::SessionsController ...@@ -197,10 +198,14 @@ class SessionsController < Devise::SessionsController
end end
def find_user def find_user
if session[:otp_user_id] strong_memoize(:find_user) do
User.find(session[:otp_user_id]) if session[:otp_user_id] && user_params[:login]
elsif user_params[:login] User.by_id_and_login(session[:otp_user_id], user_params[:login]).first
User.by_login(user_params[:login]) elsif session[:otp_user_id]
User.find(session[:otp_user_id])
elsif user_params[:login]
User.by_login(user_params[:login])
end
end end
end end
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
# WARNING: does not consider project feature visibility! # WARNING: does not consider project feature visibility!
# - user: The user for which to load the events # - user: The user for which to load the events
# - params: # - params:
# - limit: Number of items that to be returned. Defaults to 20 and limited to 100.
# - offset: The page of events to return # - offset: The page of events to return
class UserRecentEventsFinder class UserRecentEventsFinder
prepend FinderWithCrossProjectAccess prepend FinderWithCrossProjectAccess
...@@ -16,7 +17,8 @@ class UserRecentEventsFinder ...@@ -16,7 +17,8 @@ class UserRecentEventsFinder
attr_reader :current_user, :target_user, :params attr_reader :current_user, :target_user, :params
LIMIT = 20 DEFAULT_LIMIT = 20
MAX_LIMIT = 100
def initialize(current_user, target_user, params = {}) def initialize(current_user, target_user, params = {})
@current_user = current_user @current_user = current_user
...@@ -31,7 +33,7 @@ class UserRecentEventsFinder ...@@ -31,7 +33,7 @@ class UserRecentEventsFinder
recent_events(params[:offset] || 0) recent_events(params[:offset] || 0)
.joins(:project) .joins(:project)
.with_associations .with_associations
.limit_recent(params[:limit].presence || LIMIT, params[:offset]) .limit_recent(limit, params[:offset])
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
...@@ -59,4 +61,10 @@ class UserRecentEventsFinder ...@@ -59,4 +61,10 @@ class UserRecentEventsFinder
def projects def projects
target_user.project_interactions.to_sql target_user.project_interactions.to_sql
end end
def limit
return DEFAULT_LIMIT unless params[:limit].present?
[params[:limit].to_i, MAX_LIMIT].min
end
end end
...@@ -11,7 +11,7 @@ module Mutations ...@@ -11,7 +11,7 @@ module Mutations
private private
def find_object(id:) def find_object(id:)
GitlabSchema.object_from_id(id) GitlabSchema.object_from_id(id, expected_type: ::Snippet)
end end
def authorized_resource?(snippet) def authorized_resource?(snippet)
......
...@@ -105,6 +105,19 @@ class ActiveSession ...@@ -105,6 +105,19 @@ class ActiveSession
end end
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 = '*') def self.key_name(user_id, session_id = '*')
"#{Gitlab::Redis::SharedState::USER_SESSIONS_NAMESPACE}:#{user_id}:#{session_id}" "#{Gitlab::Redis::SharedState::USER_SESSIONS_NAMESPACE}:#{user_id}:#{session_id}"
end end
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
module Clusters module Clusters
module Applications module Applications
class Runner < ApplicationRecord class Runner < ApplicationRecord
VERSION = '0.19.2' VERSION = '0.19.3'
self.table_name = 'clusters_applications_runners' self.table_name = 'clusters_applications_runners'
......
...@@ -76,6 +76,8 @@ class Member < ApplicationRecord ...@@ -76,6 +76,8 @@ class Member < ApplicationRecord
scope :request, -> { where.not(requested_at: nil) } scope :request, -> { where.not(requested_at: nil) }
scope :non_request, -> { where(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 :has_access, -> { active.where('access_level > 0') }
scope :guests, -> { active.where(access_level: GUEST) } scope :guests, -> { active.where(access_level: GUEST) }
......
...@@ -363,6 +363,7 @@ class User < ApplicationRecord ...@@ -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_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_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 :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 def preferred_language
read_attribute('preferred_language') || read_attribute('preferred_language') ||
...@@ -886,6 +887,12 @@ class User < ApplicationRecord ...@@ -886,6 +887,12 @@ class User < ApplicationRecord
all_expanded_groups.where(require_two_factor_authentication: true) all_expanded_groups.where(require_two_factor_authentication: true)
end 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 # rubocop: disable CodeReuse/ServiceClass
def refresh_authorized_projects def refresh_authorized_projects
Users::RefreshAuthorizedProjectsService.new(self).execute Users::RefreshAuthorizedProjectsService.new(self).execute
......
...@@ -132,6 +132,7 @@ module Auth ...@@ -132,6 +132,7 @@ module Auth
def can_access?(requested_project, requested_action) def can_access?(requested_project, requested_action)
return false unless requested_project.container_registry_enabled? return false unless requested_project.container_registry_enabled?
return false if requested_project.repository_access_level == ::ProjectFeature::DISABLED
case requested_action case requested_action
when 'pull' when 'pull'
......
...@@ -18,6 +18,7 @@ module Members ...@@ -18,6 +18,7 @@ module Members
end end
delete_subresources(member) unless skip_subresources delete_subresources(member) unless skip_subresources
delete_project_invitations_by(member) unless skip_subresources
enqueue_delete_todos(member) enqueue_delete_todos(member)
enqueue_unassign_issuables(member) if unassign_issuables enqueue_unassign_issuables(member) if unassign_issuables
...@@ -39,24 +40,48 @@ module Members ...@@ -39,24 +40,48 @@ module Members
delete_project_members(member) delete_project_members(member)
delete_subgroup_members(member) delete_subgroup_members(member)
delete_invited_members(member)
end end
def delete_project_members(member) def delete_project_members(member)
groups = member.group.self_and_descendants groups = member.group.self_and_descendants
ProjectMember.in_namespaces(groups).with_user(member.user).each do |project_member| destroy_project_members(ProjectMember.in_namespaces(groups).with_user(member.user))
self.class.new(current_user).execute(project_member, skip_authorization: @skip_auth)
end
end end
def delete_subgroup_members(member) def delete_subgroup_members(member)
groups = member.group.descendants 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) self.class.new(current_user).execute(group_member, skip_authorization: @skip_auth, skip_subresources: true)
end end
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) def can_destroy_member?(member)
can?(current_user, destroy_member_permission(member), member) can?(current_user, destroy_member_permission(member), member)
end end
......
...@@ -27,6 +27,6 @@ ...@@ -27,6 +27,6 @@
- unless is_current_session - unless is_current_session
.float-right .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') %span.sr-only= _('Revoke')
= _('Revoke') = _('Revoke')
---
title: Show on two-factor authentication setup page groups that are the cause of this
requirement
merge_request:
author:
type: security
---
title: Prevent interrupted 2FA sign-in from signing-in incorrect user
merge_request:
author:
type: security
---
title: Create new 2FA code each time user is entering 2FA setup page
merge_request:
author:
type: security
---
title: Remove all sessions but current while enabling 2FA
merge_request:
author:
type: security
---
title: Invalidate two factor sign-in when user password changes
merge_request:
author:
type: security
---
title: Delete members invites created by users being deleted
merge_request:
author:
type: security
---
title: Prevent OmniAuth from rendering arbitrary error messages
merge_request:
author:
type: security
---
title: Prevent not-2fa authenticated users that are supposed to use it to consume
api via session
merge_request:
author:
type: security
---
title: Invalidate remember me when an active session is revoked
merge_request:
author:
type: security
---
title: Add rate limit on webhooks testing feature
merge_request:
author:
type: security
---
title: Prevent Deploy Tokens to read project resources when repository is disabled
merge_request:
author:
type: security
---
title: Ensure global ID is of Snippet type in GraphQL destroy mutation
merge_request:
author:
type: security
---
title: Fix Improper Access Control on Deploy-Key
merge_request:
author:
type: security
---
title: Set maximum limit for profile events
merge_request:
author:
type: security
---
title: Upgrade jquery to v3.5
merge_request:
author:
type: security
---
title: Update GitLab Runner Helm Chart to 0.19.3
merge_request:
author:
type: security
...@@ -25,7 +25,6 @@ devise_for :users, controllers: { omniauth_callbacks: :omniauth_callbacks, ...@@ -25,7 +25,6 @@ devise_for :users, controllers: { omniauth_callbacks: :omniauth_callbacks,
confirmations: :confirmations } confirmations: :confirmations }
devise_scope :user do devise_scope :user do
get '/users/auth/:provider/omniauth_error' => 'omniauth_callbacks#omniauth_error', as: :omniauth_error
get '/users/almost_there' => 'confirmations#almost_there' get '/users/almost_there' => 'confirmations#almost_there'
end end
......
...@@ -29,6 +29,11 @@ exceeds 100, the oldest ones are deleted. ...@@ -29,6 +29,11 @@ exceeds 100, the oldest ones are deleted.
1. Use the previous steps to navigate to **Active Sessions**. 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. 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 <!-- ## Troubleshooting
Include any troubleshooting steps that you can foresee. If you know beforehand what issues Include any troubleshooting steps that you can foresee. If you know beforehand what issues
......
...@@ -255,6 +255,12 @@ to get you a new `_gitlab_session` and keep you signed in through browser restar ...@@ -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, 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. 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 ### Increased sign-in time
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/20340) in GitLab 13.1. > [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 ...@@ -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: 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. - 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. By default, the server sets a time-to-live (TTL) of 1-week on any session that is used.
......
...@@ -69,7 +69,7 @@ module API ...@@ -69,7 +69,7 @@ module API
deploy_token_from_request || deploy_token_from_request ||
find_user_from_bearer_token || find_user_from_bearer_token ||
find_user_from_job_token || find_user_from_job_token ||
find_user_from_warden user_from_warden
end end
end end
...@@ -103,6 +103,25 @@ module API ...@@ -103,6 +103,25 @@ module API
def user_allowed_or_deploy_token?(user) def user_allowed_or_deploy_token?(user)
Gitlab::UserAccess.new(user).allowed? || user.is_a?(DeployToken) Gitlab::UserAccess.new(user).allowed? || user.is_a?(DeployToken)
end 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 end
class_methods do class_methods do
......
...@@ -25,11 +25,13 @@ module Gitlab ...@@ -25,11 +25,13 @@ module Gitlab
project_repositories_archive: { threshold: 5, interval: 1.minute }, project_repositories_archive: { threshold: 5, interval: 1.minute },
project_generate_new_export: { threshold: -> { application_settings.project_export_limit }, 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_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 }, play_pipeline_schedule: { threshold: 1, interval: 1.minute },
show_raw_controller: { threshold: -> { application_settings.raw_blob_request_limit }, 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_export: { threshold: -> { application_settings.group_export_limit }, interval: 1.minute },
group_download_export: { threshold: -> { application_settings.group_download_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 }.freeze
end end
......
...@@ -222,6 +222,8 @@ module Gitlab ...@@ -222,6 +222,8 @@ module Gitlab
# Registry access (with jwt) does not have access to project # Registry access (with jwt) does not have access to project
return if project && !token.has_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) scopes = abilities_for_scopes(token.scopes)
......
# 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
...@@ -98,6 +98,13 @@ module Gitlab ...@@ -98,6 +98,13 @@ module Gitlab
Guest.can?(download_ability, container) Guest.can?(download_ability, container)
end 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? def user_can_download_code?
authentication_abilities.include?(:download_code) && authentication_abilities.include?(:download_code) &&
user_access.can_do_action?(download_ability) user_access.can_do_action?(download_ability)
...@@ -257,7 +264,7 @@ module Gitlab ...@@ -257,7 +264,7 @@ module Gitlab
end end
def check_download_access! def check_download_access!
passed = deploy_key? || passed = deploy_key_can_download_code? ||
deploy_token? || deploy_token? ||
user_can_download_code? || user_can_download_code? ||
build_can_download_code? || build_can_download_code? ||
......
...@@ -2831,6 +2831,9 @@ msgstr "" ...@@ -2831,6 +2831,9 @@ msgstr ""
msgid "An error occurred while validating username" msgid "An error occurred while validating username"
msgstr "" msgstr ""
msgid "An error occurred. Please sign in again."
msgstr ""
msgid "An error occurred. Please try again." msgid "An error occurred. Please try again."
msgstr "" msgstr ""
...@@ -3268,7 +3271,7 @@ msgstr "" ...@@ -3268,7 +3271,7 @@ msgstr ""
msgid "Are you sure? Removing this GPG key does not affect already signed commits." msgid "Are you sure? Removing this GPG key does not affect already signed commits."
msgstr "" 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 "" msgstr ""
msgid "Are you sure? This will invalidate your registered applications and U2F devices." msgid "Are you sure? This will invalidate your registered applications and U2F devices."
......
...@@ -229,6 +229,7 @@ RSpec.describe ApplicationController do ...@@ -229,6 +229,7 @@ RSpec.describe ApplicationController do
it 'does not redirect if 2FA is not required' 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(:two_factor_authentication_required?).and_return(false)
allow(controller).to receive(:current_user).and_return(create(:user))
expect(controller).not_to receive(:redirect_to) expect(controller).not_to receive(:redirect_to)
...@@ -346,13 +347,17 @@ RSpec.describe ApplicationController do ...@@ -346,13 +347,17 @@ RSpec.describe ApplicationController do
let(:user) { create :user, otp_grace_period_started_at: 2.hours.ago } let(:user) { create :user, otp_grace_period_started_at: 2.hours.ago }
it 'returns true if the grace period has expired' do 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 expect(subject).to be_truthy
end end
it 'returns false if the grace period is still active' do 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 expect(subject).to be_falsey
end end
......
...@@ -40,6 +40,22 @@ RSpec.describe OmniauthCallbacksController, type: :controller do ...@@ -40,6 +40,22 @@ RSpec.describe OmniauthCallbacksController, type: :controller do
end end
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 context 'when the user is on the last sign in attempt' do
let(:extern_uid) { 'my-uid' } let(:extern_uid) { 'my-uid' }
......
# 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
...@@ -14,10 +14,9 @@ RSpec.describe Profiles::TwoFactorAuthsController do ...@@ -14,10 +14,9 @@ RSpec.describe Profiles::TwoFactorAuthsController do
let(:user) { create(:user) } let(:user) { create(:user) }
it 'generates otp_secret for user' do 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
get :show # Second hit shouldn't re-generate it
end end
it 'assigns qr_code' do it 'assigns qr_code' do
...@@ -27,6 +26,14 @@ RSpec.describe Profiles::TwoFactorAuthsController do ...@@ -27,6 +26,14 @@ RSpec.describe Profiles::TwoFactorAuthsController do
get :show get :show
expect(assigns[:qr_code]).to eq code expect(assigns[:qr_code]).to eq code
end 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 end
describe 'POST create' do describe 'POST create' do
...@@ -57,6 +64,12 @@ RSpec.describe Profiles::TwoFactorAuthsController do ...@@ -57,6 +64,12 @@ RSpec.describe Profiles::TwoFactorAuthsController do
expect(assigns[:codes]).to match_array %w(a b c) expect(assigns[:codes]).to match_array %w(a b c)
end end
it 'calls to delete other sessions' do
expect(ActiveSession).to receive(:destroy_all_but_current)
go
end
it 'renders create' do it 'renders create' do
go go
expect(response).to render_template(:create) expect(response).to render_template(:create)
......
...@@ -47,4 +47,26 @@ RSpec.describe Projects::HooksController do ...@@ -47,4 +47,26 @@ RSpec.describe Projects::HooksController do
expect(ProjectHook.first).to have_attributes(hook_params) expect(ProjectHook.first).to have_attributes(hook_params)
end end
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 end
...@@ -255,8 +255,8 @@ RSpec.describe SessionsController do ...@@ -255,8 +255,8 @@ RSpec.describe SessionsController do
context 'when using two-factor authentication via OTP' do context 'when using two-factor authentication via OTP' do
let(:user) { create(:user, :two_factor) } let(:user) { create(:user, :two_factor) }
def authenticate_2fa(user_params) def authenticate_2fa(user_params, otp_user_id: user.id)
post(:create, params: { user: user_params }, session: { otp_user_id: user.id }) post(:create, params: { user: user_params }, session: { otp_user_id: otp_user_id })
end end
context 'remember_me field' do context 'remember_me field' do
...@@ -293,8 +293,22 @@ RSpec.describe SessionsController do ...@@ -293,8 +293,22 @@ RSpec.describe SessionsController do
end end
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 authenticating with login and OTP of another user' do
context 'when another user has 2FA enabled' do context 'when another user has 2FA enabled' do
...@@ -380,18 +394,6 @@ RSpec.describe SessionsController do ...@@ -380,18 +394,6 @@ RSpec.describe SessionsController do
end end
end 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
end end
......
...@@ -177,6 +177,14 @@ RSpec.describe 'Login' do ...@@ -177,6 +177,14 @@ RSpec.describe 'Login' do
expect(page).not_to have_content(I18n.t('devise.failure.already_authenticated')) expect(page).not_to have_content(I18n.t('devise.failure.already_authenticated'))
end 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 context 'using one-time code' do
it 'allows login with valid code' do it 'allows login with valid code' do
expect(authentication_metrics) expect(authentication_metrics)
...@@ -232,7 +240,7 @@ RSpec.describe 'Login' do ...@@ -232,7 +240,7 @@ RSpec.describe 'Login' do
expect(codes.size).to eq 10 expect(codes.size).to eq 10
# Ensure the generated codes get saved # Ensure the generated codes get saved
user.save user.save(touch: false)
end end
context 'with valid code' do context 'with valid code' do
...@@ -290,7 +298,7 @@ RSpec.describe 'Login' do ...@@ -290,7 +298,7 @@ RSpec.describe 'Login' do
code = codes.sample code = codes.sample
expect(user.invalidate_otp_backup_code!(code)).to eq true 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 expect(user.reload.otp_backup_codes.size).to eq 9
enter_code(code) enter_code(code)
......
...@@ -11,8 +11,10 @@ RSpec.describe UserRecentEventsFinder do ...@@ -11,8 +11,10 @@ RSpec.describe UserRecentEventsFinder do
let!(:private_event) { create(:event, project: private_project, author: project_owner) } let!(:private_event) { create(:event, project: private_project, author: project_owner) }
let!(:internal_event) { create(:event, project: internal_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!(: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 describe '#execute' do
context 'when profile is public' do context 'when profile is public' do
...@@ -48,5 +50,38 @@ RSpec.describe UserRecentEventsFinder do ...@@ -48,5 +50,38 @@ RSpec.describe UserRecentEventsFinder do
expect(events).to include(event_b) expect(events).to include(event_b)
end end
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
end end
...@@ -212,6 +212,55 @@ RSpec.describe GitlabSchema do ...@@ -212,6 +212,55 @@ RSpec.describe GitlabSchema do
end end
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 def field_instrumenters
described_class.instrumenters[:field] + described_class.instrumenters[:field_after_built_ins] described_class.instrumenters[:field] + described_class.instrumenters[:field_after_built_ins]
end end
......
# 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
...@@ -441,7 +441,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do ...@@ -441,7 +441,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
end end
end end
shared_examples 'deploy token with disabled registry' do shared_examples 'deploy token with disabled feature' do
context 'when registry disabled' do context 'when registry disabled' do
before do before do
stub_container_registry_config(enabled: false) stub_container_registry_config(enabled: false)
...@@ -452,6 +452,15 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do ...@@ -452,6 +452,15 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
.to eq(auth_failure) .to eq(auth_failure)
end end
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 end
context 'when deploy token and user have the same username' do 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 ...@@ -604,7 +613,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
it_behaves_like 'registry token scope' it_behaves_like 'registry token scope'
end end
it_behaves_like 'deploy token with disabled registry' it_behaves_like 'deploy token with disabled feature'
end end
context 'when the deploy token has write_registry as a scope' do 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 ...@@ -626,7 +635,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
it_behaves_like 'registry token scope' it_behaves_like 'registry token scope'
end end
it_behaves_like 'deploy token with disabled registry' it_behaves_like 'deploy token with disabled feature'
end end
end end
end end
......
...@@ -472,31 +472,135 @@ RSpec.describe Gitlab::GitAccess do ...@@ -472,31 +472,135 @@ RSpec.describe Gitlab::GitAccess do
let(:actor) { key } let(:actor) { key }
context 'pull code' do context 'pull code' do
context 'when project is authorized' do context 'when project is public' do
before do let(:project) { create(:project, :public, :repository, *options) }
key.projects << project
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 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 end
context 'when unauthorized' do context 'when project is internal' do
context 'from public project' do let(:project) { create(:project, :internal, :repository, *options) }
let(:project) { create(:project, :public, :repository) }
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 end
context 'from internal project' do context 'when deploy key does not exist in the project' do
let(:project) { create(:project, :internal, :repository) } 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
end
context 'from private project' do context 'when project is private' do
let(:project) { create(:project, :private, :repository) } 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 end
end end
......
...@@ -296,6 +296,59 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do ...@@ -296,6 +296,59 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
end end
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 describe '.cleanup' do
before do before do
stub_const("ActiveSession::ALLOWED_NUMBER_OF_ACTIVE_SESSIONS", 5) stub_const("ActiveSession::ALLOWED_NUMBER_OF_ACTIVE_SESSIONS", 5)
......
...@@ -195,6 +195,19 @@ RSpec.describe Member do ...@@ -195,6 +195,19 @@ RSpec.describe Member do
it { expect(described_class.non_request).to include @accepted_request_member } it { expect(described_class.non_request).to include @accepted_request_member }
end 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 describe '.search_invite_email' do
it 'returns only members the matching e-mail' do it 'returns only members the matching e-mail' do
create(:group_member, :invited) create(:group_member, :invited)
......
...@@ -894,6 +894,20 @@ RSpec.describe User do ...@@ -894,6 +894,20 @@ RSpec.describe User do
expect(described_class.without_ghosts).to match_array([user1, user2]) expect(described_class.without_ghosts).to match_array([user1, user2])
end end
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 end
describe "Respond to" do describe "Respond to" do
...@@ -3579,6 +3593,42 @@ RSpec.describe User do ...@@ -3579,6 +3593,42 @@ RSpec.describe User do
end end
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 describe '.active' do
before do before do
described_class.ghost described_class.ghost
......
...@@ -46,6 +46,31 @@ RSpec.describe 'Destroying a Snippet' do ...@@ -46,6 +46,31 @@ RSpec.describe 'Destroying a Snippet' do
expect(mutation_response).to have_key('snippet') expect(mutation_response).to have_key('snippet')
expect(mutation_response['snippet']).to be_nil expect(mutation_response['snippet']).to be_nil
end 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
end end
......
...@@ -85,6 +85,27 @@ RSpec.describe API::Helpers do ...@@ -85,6 +85,27 @@ RSpec.describe API::Helpers do
end end
it { is_expected.to eq(user) } 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 end
context "PUT request" do context "PUT request" do
......
...@@ -654,6 +654,19 @@ RSpec.describe Auth::ContainerRegistryAuthenticationService do ...@@ -654,6 +654,19 @@ RSpec.describe Auth::ContainerRegistryAuthenticationService do
it_behaves_like 'not a container repository factory' it_behaves_like 'not a container repository factory'
end end
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 end
context 'registry catalog browsing authorized as admin' do context 'registry catalog browsing authorized as admin' do
......
...@@ -292,6 +292,10 @@ RSpec.describe Members::DestroyService do ...@@ -292,6 +292,10 @@ RSpec.describe Members::DestroyService do
before do before do
create(:group_member, :developer, group: subsubgroup, user: member_user) 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) subsubproject.add_developer(member_user)
control_project.add_maintainer(user) control_project.add_maintainer(user)
...@@ -325,5 +329,41 @@ RSpec.describe Members::DestroyService do ...@@ -325,5 +329,41 @@ RSpec.describe Members::DestroyService do
it 'does not remove the user from the control project' do it 'does not remove the user from the control project' do
expect(control_project.members.map(&:user)).to include(user) expect(control_project.members.map(&:user)).to include(user)
end 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
end end
File mode changed from 100644 to 100755
File mode changed from 100644 to 100755
...@@ -7064,10 +7064,10 @@ jquery.waitforimages@^2.2.0: ...@@ -7064,10 +7064,10 @@ jquery.waitforimages@^2.2.0:
resolved "https://registry.yarnpkg.com/jquery.waitforimages/-/jquery.waitforimages-2.2.0.tgz#63f23131055a1b060dc913e6d874bcc9b9e6b16b" resolved "https://registry.yarnpkg.com/jquery.waitforimages/-/jquery.waitforimages-2.2.0.tgz#63f23131055a1b060dc913e6d874bcc9b9e6b16b"
integrity sha1-Y/IxMQVaGwYNyRPm2HS8ybnmsWs= integrity sha1-Y/IxMQVaGwYNyRPm2HS8ybnmsWs=
"jquery@>= 1.9.1", jquery@>=1.8.0, jquery@^3.4.1: "jquery@>= 1.9.1", jquery@>=1.8.0, jquery@^3.5.0:
version "3.4.1" version "3.5.1"
resolved "https://registry.yarnpkg.com/jquery/-/jquery-3.4.1.tgz#714f1f8d9dde4bdfa55764ba37ef214630d80ef2" resolved "https://registry.yarnpkg.com/jquery/-/jquery-3.5.1.tgz#d7b4d08e1bfdb86ad2f1a3d039ea17304717abb5"
integrity sha512-36+AdBzCL+y6qjw5Tx7HgzeGCzC81MDDgaUP8ld2zhx58HdqXGoBd+tHdrBMiyjGQs0Hxs/MLZTu/eHNJJuWPw== integrity sha512-XwIBPqcMn57FxfT+Go5pzySnm4KWkT1Tv7gjrpT1srtf8Weynl6R273VJ5GjkRb51IzMp5nbaPjJXMWeju2MKg==
js-base64@^2.1.8: js-base64@^2.1.8:
version "2.5.1" version "2.5.1"
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment