summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Release Tools Bot <delivery-team+release-tools@gitlab.com>2020-10-01 17:30:33 +0000
committerGitLab Release Tools Bot <delivery-team+release-tools@gitlab.com>2020-10-01 17:30:33 +0000
commitb4a90a651b273552f77b01273684a7baaf9a89a0 (patch)
tree0e47d67e645117da5034114e4f9f556890e56d8e
parent22088debca3b88ed45a7767e915511712e968919 (diff)
parent94a627fba65f14e04f876b321544fb82aacdfadc (diff)
downloadgitlab-ce-13-2-stable.tar.gz
Merge remote-tracking branch 'dev/13-2-stable' into 13-2-stable13-2-stable
-rw-r--r--CHANGELOG.md20
-rw-r--r--GITALY_SERVER_VERSION2
-rw-r--r--VERSION2
-rw-r--r--app/channels/application_cable/connection.rb2
-rw-r--r--app/controllers/admin/users_controller.rb21
-rw-r--r--app/controllers/profiles/active_sessions_controller.rb4
-rw-r--r--app/controllers/profiles/emails_controller.rb12
-rw-r--r--app/controllers/projects/raw_controller.rb1
-rw-r--r--app/controllers/registrations_controller.rb10
-rw-r--r--app/graphql/mutations/metrics/dashboard/annotations/base.rb2
-rw-r--r--app/helpers/active_sessions_helper.rb9
-rw-r--r--app/helpers/safe_params_helper.rb2
-rw-r--r--app/models/active_session.rb114
-rw-r--r--app/models/clusters/applications/runner.rb2
-rw-r--r--app/models/member.rb2
-rw-r--r--app/models/releases/link.rb4
-rw-r--r--app/services/issuable_base_service.rb1
-rw-r--r--app/services/issues/update_service.rb12
-rw-r--r--app/services/members/base_service.rb5
-rw-r--r--app/services/todos/destroy/entity_leave_service.rb45
-rw-r--r--app/validators/future_date_validator.rb17
-rw-r--r--app/views/profiles/active_sessions/_active_session.html.haml5
-rw-r--r--app/workers/all_queues.yml8
-rw-r--r--app/workers/remove_unaccepted_member_invites_worker.rb33
-rw-r--r--config/initializers/1_settings.rb3
-rw-r--r--config/initializers/warden.rb3
-rw-r--r--db/migrate/20200831204646_add_project_feature_flags_to_plan_limits.rb9
-rw-r--r--db/migrate/20200831222347_insert_project_feature_flags_plan_limits.rb25
-rw-r--r--db/migrate/20200916120837_add_index_to_members_for_unaccepted_invitations.rb20
-rw-r--r--db/schema_migrations/202009161208371
-rw-r--r--db/structure.sql8
-rw-r--r--doc/user/admin_area/custom_project_templates.md3
-rw-r--r--doc/user/group/custom_project_templates.md5
-rw-r--r--doc/user/project/members/index.md3
-rw-r--r--lib/api/api_guard.rb6
-rw-r--r--lib/gitlab/application_rate_limiter.rb4
-rw-r--r--locale/gitlab.pot12
-rw-r--r--spec/controllers/admin/users_controller_spec.rb37
-rw-r--r--spec/controllers/groups/group_members_controller_spec.rb89
-rw-r--r--spec/controllers/profiles/emails_controller_spec.rb68
-rw-r--r--spec/controllers/projects/project_members_controller_spec.rb95
-rw-r--r--spec/controllers/projects/raw_controller_spec.rb5
-rw-r--r--spec/controllers/registrations_controller_spec.rb18
-rw-r--r--spec/factories/projects.rb8
-rw-r--r--spec/graphql/mutations/issues/set_confidential_spec.rb21
-rw-r--r--spec/graphql/mutations/merge_requests/set_milestone_spec.rb48
-rw-r--r--spec/migrations/insert_project_feature_flags_plan_limits_spec.rb80
-rw-r--r--spec/models/active_session_spec.rb218
-rw-r--r--spec/models/concerns/expirable_spec.rb10
-rw-r--r--spec/models/member_spec.rb7
-rw-r--r--spec/models/members/project_member_spec.rb5
-rw-r--r--spec/requests/api/api_spec.rb25
-rw-r--r--spec/requests/api/files_spec.rb15
-rw-r--r--spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb18
-rw-r--r--spec/requests/api/members_spec.rb69
-rw-r--r--spec/services/issues/create_service_spec.rb7
-rw-r--r--spec/services/issues/update_service_spec.rb19
-rw-r--r--spec/services/members/update_service_spec.rb32
-rw-r--r--spec/services/merge_requests/update_service_spec.rb66
-rw-r--r--spec/services/todos/destroy/entity_leave_service_spec.rb328
-rw-r--r--spec/support/shared_examples/cached_response_shared_examples.rb12
-rw-r--r--spec/validators/future_date_validator_spec.rb36
-rw-r--r--spec/workers/remove_expired_members_worker_spec.rb26
-rw-r--r--spec/workers/remove_unaccepted_member_invites_worker_spec.rb76
-rwxr-xr-x[-rw-r--r--]vendor/gitignore/C++.gitignore0
-rwxr-xr-x[-rw-r--r--]vendor/gitignore/Java.gitignore0
66 files changed, 1509 insertions, 366 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7ccc40f0355..e7a48576423 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,26 @@
documentation](doc/development/changelog.md) for instructions on adding your own
entry.
+## 13.2.10 (2020-10-01)
+
+### Security (14 changes)
+
+- Do not store session id in Redis.
+- Fix permission checks when updating confidentiality and milestone on issues or merge requests.
+- Purge unaccepted member invitations older than 90 days.
+- Adds feature flags plan limits.
+- Prevent SVG XSS via Web IDE.
+- Ensure user has no solo owned groups before triggering account deletion.
+- Security fix safe params helper.
+- Do not bypass admin mode when authenticated with deploy token.
+- Fixes release asset link filepath ReDoS.
+- Ensure global ID is of Annotation type in GraphQL destroy mutation.
+- Validate that membership expiry dates are not in the past.
+- Rate limit adding new email and re-sending email confirmation.
+- Fix redaction of confidential Todos.
+- Update GitLab Runner Helm Chart to 0.19.4.
+
+
## 13.2.9 (2020-09-04)
### Fixed (2 changes)
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index 3aa3e92b435..b9b445425c3 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-13.2.9
+13.2.10
diff --git a/VERSION b/VERSION
index 3aa3e92b435..b9b445425c3 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-13.2.9
+13.2.10
diff --git a/app/channels/application_cable/connection.rb b/app/channels/application_cable/connection.rb
index 1361269f2a2..b2d98d243f9 100644
--- a/app/channels/application_cable/connection.rb
+++ b/app/channels/application_cable/connection.rb
@@ -15,7 +15,7 @@ module ApplicationCable
private
def find_user_from_session_store
- session = ActiveSession.sessions_from_ids([session_id]).first
+ session = ActiveSession.sessions_from_ids([session_id.private_id]).first
Warden::SessionSerializer.new('rack.session' => session).fetch(:user)
end
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index fc0acd8f99a..9cb1d16fbd8 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -5,6 +5,7 @@ class Admin::UsersController < Admin::ApplicationController
before_action :user, except: [:index, :new, :create]
before_action :check_impersonation_availability, only: :impersonate
+ before_action :ensure_destroy_prerequisites_met, only: [:destroy]
def index
@users = User.filter_items(params[:filter]).order_name_asc
@@ -168,7 +169,7 @@ class Admin::UsersController < Admin::ApplicationController
end
def destroy
- user.delete_async(deleted_by: current_user, params: params.permit(:hard_delete))
+ user.delete_async(deleted_by: current_user, params: destroy_params)
respond_to do |format|
format.html { redirect_to admin_users_path, status: :found, notice: _("The user is being deleted.") }
@@ -197,6 +198,24 @@ class Admin::UsersController < Admin::ApplicationController
user == current_user
end
+ def destroy_params
+ params.permit(:hard_delete)
+ end
+
+ def ensure_destroy_prerequisites_met
+ return if hard_delete?
+
+ if user.solo_owned_groups.present?
+ message = s_('AdminUsers|You must transfer ownership or delete the groups owned by this user before you can delete their account')
+
+ redirect_to admin_user_path(user), status: :see_other, alert: message
+ end
+ end
+
+ def hard_delete?
+ destroy_params[:hard_delete]
+ end
+
def user
@user ||= find_routable!(User, params[:id])
end
diff --git a/app/controllers/profiles/active_sessions_controller.rb b/app/controllers/profiles/active_sessions_controller.rb
index d9ec3195fd1..e4cd5d65e1a 100644
--- a/app/controllers/profiles/active_sessions_controller.rb
+++ b/app/controllers/profiles/active_sessions_controller.rb
@@ -6,7 +6,9 @@ class Profiles::ActiveSessionsController < Profiles::ApplicationController
end
def destroy
- ActiveSession.destroy_with_public_id(current_user, params[:id])
+ # params[:id] can be either an Rack::Session::SessionId#private_id
+ # or an encrypted Rack::Session::SessionId#public_id
+ ActiveSession.destroy_with_deprecated_encryption(current_user, params[:id])
current_user.forget_me!
respond_to do |format|
diff --git a/app/controllers/profiles/emails_controller.rb b/app/controllers/profiles/emails_controller.rb
index f666a1150a6..da553e34ef6 100644
--- a/app/controllers/profiles/emails_controller.rb
+++ b/app/controllers/profiles/emails_controller.rb
@@ -2,6 +2,8 @@
class Profiles::EmailsController < Profiles::ApplicationController
before_action :find_email, only: [:destroy, :resend_confirmation_instructions]
+ before_action -> { rate_limit!(:profile_add_new_email) }, only: [:create]
+ before_action -> { rate_limit!(:profile_resend_email_confirmation) }, only: [:resend_confirmation_instructions]
def index
@primary_email = current_user.email
@@ -38,6 +40,16 @@ class Profiles::EmailsController < Profiles::ApplicationController
private
+ def rate_limit!(action)
+ rate_limiter = ::Gitlab::ApplicationRateLimiter
+
+ if rate_limiter.throttled?(action, scope: current_user)
+ rate_limiter.log_request(request, action, current_user)
+
+ redirect_back_or_default(options: { alert: _('This action has been performed too many times. Try again later.') })
+ end
+ end
+
def email_params
params.require(:email).permit(:email)
end
diff --git a/app/controllers/projects/raw_controller.rb b/app/controllers/projects/raw_controller.rb
index 69a3898af55..29f1e4bfd44 100644
--- a/app/controllers/projects/raw_controller.rb
+++ b/app/controllers/projects/raw_controller.rb
@@ -12,6 +12,7 @@ class Projects::RawController < Projects::ApplicationController
before_action :authorize_download_code!
before_action :show_rate_limit, only: [:show], unless: :external_storage_request?
before_action :assign_ref_vars
+ before_action :no_cache_headers, only: [:show]
before_action :redirect_to_external_storage, only: :show, if: :static_objects_external_storage_enabled?
def show
diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index b1c1fe3ba74..f054be50e20 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -10,7 +10,7 @@ class RegistrationsController < Devise::RegistrationsController
skip_before_action :required_signup_info, :check_two_factor_requirement, only: [:welcome, :update_registration]
prepend_before_action :check_captcha, only: :create
- before_action :whitelist_query_limiting, only: [:destroy]
+ before_action :whitelist_query_limiting, :ensure_destroy_prerequisites_met, only: [:destroy]
before_action :ensure_terms_accepted,
if: -> { action_name == 'create' && Gitlab::CurrentSettings.current_application_settings.enforce_terms? }
before_action :load_recaptcha, only: :new
@@ -125,6 +125,14 @@ class RegistrationsController < Devise::RegistrationsController
private
+ def ensure_destroy_prerequisites_met
+ if current_user.solo_owned_groups.present?
+ redirect_to profile_account_path,
+ status: :see_other,
+ alert: s_('Profiles|You must transfer ownership or delete groups you are an owner of before you can delete your account')
+ end
+ end
+
def user_created_message(confirmed: false)
"User Created: username=#{resource.username} email=#{resource.email} ip=#{request.remote_ip} confirmed:#{confirmed}"
end
diff --git a/app/graphql/mutations/metrics/dashboard/annotations/base.rb b/app/graphql/mutations/metrics/dashboard/annotations/base.rb
index 3126267da64..ad52f84378d 100644
--- a/app/graphql/mutations/metrics/dashboard/annotations/base.rb
+++ b/app/graphql/mutations/metrics/dashboard/annotations/base.rb
@@ -9,7 +9,7 @@ module Mutations
# This method is defined here in order to be used by `authorized_find!` in the subclasses.
def find_object(id:)
- GitlabSchema.object_from_id(id)
+ GitlabSchema.object_from_id(id, expected_type: ::Metrics::Dashboard::Annotation)
end
end
end
diff --git a/app/helpers/active_sessions_helper.rb b/app/helpers/active_sessions_helper.rb
index 8fb23f99cb3..2256bd006ea 100644
--- a/app/helpers/active_sessions_helper.rb
+++ b/app/helpers/active_sessions_helper.rb
@@ -22,4 +22,13 @@ module ActiveSessionsHelper
sprite_icon(icon_name, size: 16, css_class: 'gl-mt-2')
end
+
+ def revoke_session_path(active_session)
+ if active_session.session_private_id
+ profile_active_session_path(active_session.session_private_id)
+ else
+ # TODO: remove in 13.7
+ profile_active_session_path(active_session.public_id)
+ end
+ end
end
diff --git a/app/helpers/safe_params_helper.rb b/app/helpers/safe_params_helper.rb
index 72bf1377b02..e9f0d82bb27 100644
--- a/app/helpers/safe_params_helper.rb
+++ b/app/helpers/safe_params_helper.rb
@@ -5,7 +5,7 @@ module SafeParamsHelper
# Use this helper when generating links with `params.merge(...)`
def safe_params
if params.respond_to?(:permit!)
- params.except(:host, :port, :protocol).permit!
+ params.except(*ActionDispatch::Routing::RouteSet::RESERVED_OPTIONS).permit!
else
params
end
diff --git a/app/models/active_session.rb b/app/models/active_session.rb
index 4908290e06b..dded0eb1dc3 100644
--- a/app/models/active_session.rb
+++ b/app/models/active_session.rb
@@ -9,14 +9,14 @@ class ActiveSession
attr_accessor :created_at, :updated_at,
:ip_address, :browser, :os,
:device_name, :device_type,
- :is_impersonated, :session_id
+ :is_impersonated, :session_id, :session_private_id
- def current?(session)
- return false if session_id.nil? || session.id.nil?
+ def current?(rack_session)
+ return false if session_private_id.nil? || rack_session.id.nil?
# Rack v2.0.8+ added private_id, which uses the hash of the
# public_id to avoid timing attacks.
- session_id.private_id == session.id.private_id
+ session_private_id == rack_session.id.private_id
end
def human_device_type
@@ -25,13 +25,14 @@ class ActiveSession
# This is not the same as Rack::Session::SessionId#public_id, but we
# need to preserve this for backwards compatibility.
+ # TODO: remove in 13.7
def public_id
- Gitlab::CryptoHelper.aes256_gcm_encrypt(session_id.public_id)
+ Gitlab::CryptoHelper.aes256_gcm_encrypt(session_id)
end
def self.set(user, request)
Gitlab::Redis::SharedState.with do |redis|
- session_id = request.session.id.public_id
+ session_private_id = request.session.id.private_id
client = DeviceDetector.new(request.user_agent)
timestamp = Time.current
@@ -43,25 +44,37 @@ class ActiveSession
device_type: client.device_type,
created_at: user.current_sign_in_at || timestamp,
updated_at: timestamp,
- session_id: session_id,
+ # TODO: remove in 13.7
+ session_id: request.session.id.public_id,
+ session_private_id: session_private_id,
is_impersonated: request.session[:impersonator_id].present?
)
redis.pipelined do
redis.setex(
- key_name(user.id, session_id),
+ key_name(user.id, session_private_id),
Settings.gitlab['session_expire_delay'] * 60,
Marshal.dump(active_user_session)
)
redis.sadd(
lookup_key_name(user.id),
- session_id
+ session_private_id
)
+
+ # We remove the ActiveSession stored by using public_id to avoid
+ # duplicate entries
+ remove_deprecated_active_sessions_with_public_id(redis, user.id, request.session.id.public_id)
end
end
end
+ # TODO: remove in 13.7
+ private_class_method def self.remove_deprecated_active_sessions_with_public_id(redis, user_id, rack_session_public_id)
+ redis.srem(lookup_key_name(user_id), rack_session_public_id)
+ redis.del(key_name(user_id, rack_session_public_id))
+ end
+
def self.list(user)
Gitlab::Redis::SharedState.with do |redis|
cleaned_up_lookup_entries(redis, user).map do |raw_session|
@@ -70,27 +83,29 @@ class ActiveSession
end
end
- def self.destroy(user, session_id)
- return unless session_id
-
+ def self.cleanup(user)
Gitlab::Redis::SharedState.with do |redis|
- destroy_sessions(redis, user, [session_id])
+ clean_up_old_sessions(redis, user)
+ cleaned_up_lookup_entries(redis, user)
end
end
- def self.destroy_with_public_id(user, public_id)
- decrypted_id = decrypt_public_id(public_id)
+ # TODO: remove in 13.7
+ # After upgrade there might be a duplicate ActiveSessions:
+ # - one with the public_id stored in #session_id
+ # - another with private_id stored in #session_private_id
+ def self.destroy_with_rack_session_id(user, rack_session_id)
+ return unless rack_session_id
- return if decrypted_id.nil?
-
- session_id = Rack::Session::SessionId.new(decrypted_id)
- destroy(user, session_id)
+ Gitlab::Redis::SharedState.with do |redis|
+ destroy_sessions(redis, user, [rack_session_id.public_id, rack_session_id.private_id])
+ end
end
def self.destroy_sessions(redis, user, session_ids)
- key_names = session_ids.map { |session_id| key_name(user.id, session_id.public_id) }
+ key_names = session_ids.map { |session_id| key_name(user.id, session_id) }
- redis.srem(lookup_key_name(user.id), session_ids.map(&:public_id))
+ redis.srem(lookup_key_name(user.id), session_ids)
Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do
redis.del(key_names)
@@ -98,19 +113,29 @@ class ActiveSession
end
end
- def self.cleanup(user)
+ # TODO: remove in 13.7
+ # After upgrade, .destroy might be called with the session id encrypted
+ # by .public_id.
+ def self.destroy_with_deprecated_encryption(user, session_id)
+ return unless session_id
+
+ decrypted_session_id = decrypt_public_id(session_id)
+ rack_session_private_id = if decrypted_session_id
+ Rack::Session::SessionId.new(decrypted_session_id).private_id
+ end
+
Gitlab::Redis::SharedState.with do |redis|
- clean_up_old_sessions(redis, user)
- cleaned_up_lookup_entries(redis, user)
+ destroy_sessions(redis, user, [session_id, decrypted_session_id, rack_session_private_id].compact)
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
+ def self.destroy_all_but_current(user, current_rack_session)
+ sessions = not_impersonated(user)
+ sessions.reject! { |session| session.current?(current_rack_session) } if current_rack_session
Gitlab::Redis::SharedState.with do |redis|
- destroy_sessions(redis, user, session_ids.map(&:session_id)) if session_ids.any?
+ session_ids = (sessions.map(&:session_id) | sessions.map(&:session_private_id)).compact
+ destroy_sessions(redis, user, session_ids) if session_ids.any?
end
end
@@ -132,17 +157,16 @@ class ActiveSession
# Lists the relevant session IDs for the user.
#
- # Returns an array of Rack::Session::SessionId objects
+ # Returns an array of strings
def self.session_ids_for_user(user_id)
Gitlab::Redis::SharedState.with do |redis|
- session_ids = redis.smembers(lookup_key_name(user_id))
- session_ids.map { |id| Rack::Session::SessionId.new(id) }
+ redis.smembers(lookup_key_name(user_id))
end
end
# Lists the session Hash objects for the given session IDs.
#
- # session_ids - An array of Rack::Session::SessionId objects
+ # session_ids - An array of strings
#
# Returns an array of ActiveSession objects
def self.sessions_from_ids(session_ids)
@@ -168,27 +192,12 @@ class ActiveSession
# Returns an ActiveSession object
def self.load_raw_session(raw_session)
# rubocop:disable Security/MarshalLoad
- session = Marshal.load(raw_session)
+ Marshal.load(raw_session)
# rubocop:enable Security/MarshalLoad
+ end
- # Older ActiveSession models serialize `session_id` as strings, To
- # avoid breaking older sessions, we keep backwards compatibility
- # with older Redis keys and initiate Rack::Session::SessionId here.
- session.session_id = Rack::Session::SessionId.new(session.session_id) if session.try(:session_id).is_a?(String)
- session
- end
-
- def self.rack_session_keys(session_ids)
- session_ids.each_with_object([]) do |session_id, arr|
- # This is a redis-rack implementation detail
- # (https://github.com/redis-store/redis-rack/blob/master/lib/rack/session/redis.rb#L88)
- #
- # We need to delete session keys based on the legacy public key name
- # and the newer private ID keys, but there's no well-defined interface
- # so we have to do it directly.
- arr << "#{Gitlab::Redis::SharedState::SESSION_NAMESPACE}:#{session_id.public_id}"
- arr << "#{Gitlab::Redis::SharedState::SESSION_NAMESPACE}:#{session_id.private_id}"
- end
+ def self.rack_session_keys(rack_session_ids)
+ rack_session_ids.map { |session_id| "#{Gitlab::Redis::SharedState::SESSION_NAMESPACE}:#{session_id}" }
end
def self.raw_active_session_entries(redis, session_ids, user_id)
@@ -220,7 +229,7 @@ class ActiveSession
sessions = active_session_entries(session_ids, user.id, redis)
sessions.sort_by! {|session| session.updated_at }.reverse!
destroyable_sessions = sessions.drop(ALLOWED_NUMBER_OF_ACTIVE_SESSIONS)
- destroyable_session_ids = destroyable_sessions.map { |session| session.session_id }
+ destroyable_session_ids = destroyable_sessions.flat_map { |session| [session.session_id, session.session_private_id] }.compact
destroy_sessions(redis, user, destroyable_session_ids) if destroyable_session_ids.any?
end
@@ -244,6 +253,7 @@ class ActiveSession
entries.compact
end
+ # TODO: remove in 13.7
private_class_method def self.decrypt_public_id(public_id)
Gitlab::CryptoHelper.aes256_gcm_decrypt(public_id)
rescue
diff --git a/app/models/clusters/applications/runner.rb b/app/models/clusters/applications/runner.rb
index 5548651ebe0..71d007ba28c 100644
--- a/app/models/clusters/applications/runner.rb
+++ b/app/models/clusters/applications/runner.rb
@@ -3,7 +3,7 @@
module Clusters
module Applications
class Runner < ApplicationRecord
- VERSION = '0.18.3'
+ VERSION = '0.19.4'
self.table_name = 'clusters_applications_runners'
diff --git a/app/models/member.rb b/app/models/member.rb
index 64453e6f24c..34360fc62f7 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -5,6 +5,7 @@ class Member < ApplicationRecord
include AfterCommitQueue
include Sortable
include Importable
+ include CreatedAtFilterable
include Expirable
include Gitlab::Access
include Presentable
@@ -20,6 +21,7 @@ class Member < ApplicationRecord
delegate :name, :username, :email, to: :user, prefix: true
+ validates :expires_at, allow_blank: true, future_date: true
validates :user, presence: true, unless: :invite?
validates :source, presence: true
validates :user_id, uniqueness: { scope: [:source_type, :source_id],
diff --git a/app/models/releases/link.rb b/app/models/releases/link.rb
index dc7e78a85a9..82272f4857a 100644
--- a/app/models/releases/link.rb
+++ b/app/models/releases/link.rb
@@ -6,7 +6,9 @@ module Releases
belongs_to :release
- FILEPATH_REGEX = /\A\/([\-\.\w]+\/?)*[\da-zA-Z]+\z/.freeze
+ # See https://gitlab.com/gitlab-org/gitlab/-/issues/218753
+ # Regex modified to prevent catastrophic backtracking
+ FILEPATH_REGEX = %r{\A\/[^\/](?!.*\/\/.*)[\-\.\w\/]+[\da-zA-Z]+\z}.freeze
validates :url, presence: true, addressable_url: { schemes: %w(http https ftp) }, uniqueness: { scope: :release }
validates :name, presence: true, uniqueness: { scope: :release }
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index 65a73dadc2e..e309ea7d678 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -19,6 +19,7 @@ class IssuableBaseService < BaseService
def filter_params(issuable)
unless can_admin_issuable?(issuable)
+ params.delete(:milestone)
params.delete(:milestone_id)
params.delete(:labels)
params.delete(:add_label_ids)
diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb
index 8d22f0edcdd..a8cd516acdb 100644
--- a/app/services/issues/update_service.rb
+++ b/app/services/issues/update_service.rb
@@ -3,6 +3,7 @@
module Issues
class UpdateService < Issues::BaseService
include SpamCheckMethods
+ extend ::Gitlab::Utils::Override
def execute(issue)
handle_move_between_ids(issue)
@@ -17,6 +18,17 @@ module Issues
super
end
+ override :filter_params
+ def filter_params(issue)
+ super
+
+ # filter confidential in `Issues::UpdateService` and not in `IssuableBaseService#filtr_params`
+ # because we do allow users that cannot admin issues to set confidential flag when creating an issue
+ unless can_admin_issuable?(issue)
+ params.delete(:confidential)
+ end
+ end
+
def before_update(issue, skip_spam_check: false)
spam_check(issue, current_user, action: :update) unless skip_spam_check
end
diff --git a/app/services/members/base_service.rb b/app/services/members/base_service.rb
index 5d69418fb7d..3f55f661d9b 100644
--- a/app/services/members/base_service.rb
+++ b/app/services/members/base_service.rb
@@ -7,6 +7,11 @@ module Members
def initialize(current_user = nil, params = {})
@current_user = current_user
@params = params
+
+ # could be a string, force to an integer, part of fix
+ # https://gitlab.com/gitlab-org/gitlab/-/issues/219496
+ # Allow the ArgumentError to be raised if it can't be converted to an integer.
+ @params[:access_level] = Integer(@params[:access_level]) if @params[:access_level]
end
def after_execute(args)
diff --git a/app/services/todos/destroy/entity_leave_service.rb b/app/services/todos/destroy/entity_leave_service.rb
index 4743e9b02ce..0c0548a17a1 100644
--- a/app/services/todos/destroy/entity_leave_service.rb
+++ b/app/services/todos/destroy/entity_leave_service.rb
@@ -52,7 +52,14 @@ module Todos
# rubocop: disable CodeReuse/ActiveRecord
def remove_project_todos
- Todo.where(project_id: non_authorized_projects, user_id: user.id).delete_all
+ # Issues are viewable by guests (even in private projects), so remove those todos
+ # from projects without guest access
+ Todo.where(project_id: non_authorized_guest_projects, user_id: user.id)
+ .delete_all
+
+ # MRs require reporter access, so remove those todos that are not authorized
+ Todo.where(project_id: non_authorized_reporter_projects, target_type: MergeRequest.name, user_id: user.id)
+ .delete_all
end
# rubocop: enable CodeReuse/ActiveRecord
@@ -68,7 +75,7 @@ module Todos
when Project
{ id: entity.id }
when Namespace
- { namespace_id: non_member_groups }
+ { namespace_id: non_authorized_reporter_groups }
end
Project.where(condition)
@@ -76,8 +83,32 @@ module Todos
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
- def non_authorized_projects
- projects.where('id NOT IN (?)', user.authorized_projects.select(:id))
+ def authorized_reporter_projects
+ user.authorized_projects(Gitlab::Access::REPORTER).select(:id)
+ end
+ # rubocop: enable CodeReuse/ActiveRecord
+
+ # rubocop: disable CodeReuse/ActiveRecord
+ def authorized_guest_projects
+ user.authorized_projects(Gitlab::Access::GUEST).select(:id)
+ end
+ # rubocop: enable CodeReuse/ActiveRecord
+
+ # rubocop: disable CodeReuse/ActiveRecord
+ def non_authorized_reporter_projects
+ projects.where('id NOT IN (?)', authorized_reporter_projects)
+ end
+ # rubocop: enable CodeReuse/ActiveRecord
+
+ # rubocop: disable CodeReuse/ActiveRecord
+ def non_authorized_guest_projects
+ projects.where('id NOT IN (?)', authorized_guest_projects)
+ end
+ # rubocop: enable CodeReuse/ActiveRecord
+
+ # rubocop: disable CodeReuse/ActiveRecord
+ def authorized_reporter_groups
+ GroupsFinder.new(user, min_access_level: Gitlab::Access::REPORTER).execute.select(:id)
end
# rubocop: enable CodeReuse/ActiveRecord
@@ -91,9 +122,9 @@ module Todos
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
- def non_member_groups
+ def non_authorized_reporter_groups
entity.self_and_descendants.select(:id)
- .where('id NOT IN (?)', user.membership_groups.select(:id))
+ .where('id NOT IN (?)', authorized_reporter_groups)
end
# rubocop: enable CodeReuse/ActiveRecord
@@ -106,8 +137,6 @@ module Todos
# rubocop: disable CodeReuse/ActiveRecord
def confidential_issues
assigned_ids = IssueAssignee.select(:issue_id).where(user_id: user.id)
- authorized_reporter_projects = user
- .authorized_projects(Gitlab::Access::REPORTER).select(:id)
Issue.where(project_id: projects, confidential: true)
.where('project_id NOT IN(?)', authorized_reporter_projects)
diff --git a/app/validators/future_date_validator.rb b/app/validators/future_date_validator.rb
new file mode 100644
index 00000000000..1ba4a1e273d
--- /dev/null
+++ b/app/validators/future_date_validator.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+# FutureDateValidator
+
+# Validates that a date is in the future.
+#
+# Example:
+#
+# class Member < ActiveRecord::Base
+# validates :expires_at, allow_blank: true, future_date: true
+# end
+
+class FutureDateValidator < ActiveModel::EachValidator
+ def validate_each(record, attribute, value)
+ record.errors.add(attribute, _('cannot be a date in the past')) if value < Date.current
+ end
+end
diff --git a/app/views/profiles/active_sessions/_active_session.html.haml b/app/views/profiles/active_sessions/_active_session.html.haml
index c4b3fa80d75..97f13a55dea 100644
--- a/app/views/profiles/active_sessions/_active_session.html.haml
+++ b/app/views/profiles/active_sessions/_active_session.html.haml
@@ -27,6 +27,9 @@
- 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 and all remember me tokens revoked.') }, method: :delete, class: "btn btn-danger gl-ml-3" do
+ = link_to(revoke_session_path(active_session),
+ { 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/app/workers/all_queues.yml b/app/workers/all_queues.yml
index 5148772c881..59a949fadef 100644
--- a/app/workers/all_queues.yml
+++ b/app/workers/all_queues.yml
@@ -291,6 +291,14 @@
:weight: 1
:idempotent:
:tags: []
+- :name: cronjob:remove_unaccepted_member_invites
+ :feature_category: :authentication_and_authorization
+ :has_external_dependencies:
+ :urgency: :low
+ :resource_boundary: :unknown
+ :weight: 1
+ :idempotent: true
+ :tags: []
- :name: cronjob:remove_unreferenced_lfs_objects
:feature_category: :git_lfs
:has_external_dependencies:
diff --git a/app/workers/remove_unaccepted_member_invites_worker.rb b/app/workers/remove_unaccepted_member_invites_worker.rb
new file mode 100644
index 00000000000..4b75b43791e
--- /dev/null
+++ b/app/workers/remove_unaccepted_member_invites_worker.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+class RemoveUnacceptedMemberInvitesWorker # rubocop:disable Scalability/IdempotentWorker
+ include ApplicationWorker
+ include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
+
+ feature_category :authentication_and_authorization
+ urgency :low
+ idempotent!
+
+ EXPIRATION_THRESHOLD = 90.days
+ BATCH_SIZE = 10_000
+
+ def perform
+ # We need to check for user_id IS NULL because we have accepted invitations
+ # in the database where we did not clear the invite_token. We do not
+ # want to accidentally delete those members.
+ loop do
+ # rubocop: disable CodeReuse/ActiveRecord
+ inner_query = Member
+ .select(:id)
+ .invite
+ .created_before(EXPIRATION_THRESHOLD.ago)
+ .where(user_id: nil)
+ .limit(BATCH_SIZE)
+
+ records_deleted = Member.where(id: inner_query).delete_all
+ # rubocop: enable CodeReuse/ActiveRecord
+
+ break if records_deleted == 0
+ end
+ end
+end
diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb
index b7432c4cbe6..1bba665c918 100644
--- a/config/initializers/1_settings.rb
+++ b/config/initializers/1_settings.rb
@@ -440,6 +440,9 @@ Settings.cron_jobs['remove_expired_members_worker']['job_class'] = 'RemoveExpire
Settings.cron_jobs['remove_expired_group_links_worker'] ||= Settingslogic.new({})
Settings.cron_jobs['remove_expired_group_links_worker']['cron'] ||= '10 0 * * *'
Settings.cron_jobs['remove_expired_group_links_worker']['job_class'] = 'RemoveExpiredGroupLinksWorker'
+Settings.cron_jobs['remove_unaccepted_member_invites_worker'] ||= Settingslogic.new({})
+Settings.cron_jobs['remove_unaccepted_member_invites_worker']['cron'] ||= '10 15 * * *'
+Settings.cron_jobs['remove_unaccepted_member_invites_worker']['job_class'] = 'RemoveUnacceptedMemberInvitesWorker'
Settings.cron_jobs['prune_old_events_worker'] ||= Settingslogic.new({})
Settings.cron_jobs['prune_old_events_worker']['cron'] ||= '0 */6 * * *'
Settings.cron_jobs['prune_old_events_worker']['job_class'] = 'PruneOldEventsWorker'
diff --git a/config/initializers/warden.rb b/config/initializers/warden.rb
index d8a4da8cdf9..a83d83d9fb5 100644
--- a/config/initializers/warden.rb
+++ b/config/initializers/warden.rb
@@ -40,7 +40,8 @@ Rails.application.configure do |config|
activity = Gitlab::Auth::Activity.new(opts)
tracker = Gitlab::Auth::BlockedUserTracker.new(user, auth)
- ActiveSession.destroy(user, auth.request.session.id)
+ # TODO: switch to `auth.request.session.id.private_id` in 13.7
+ ActiveSession.destroy_with_rack_session_id(user, auth.request.session.id)
activity.user_session_destroyed!
##
diff --git a/db/migrate/20200831204646_add_project_feature_flags_to_plan_limits.rb b/db/migrate/20200831204646_add_project_feature_flags_to_plan_limits.rb
new file mode 100644
index 00000000000..d4bd2431d9c
--- /dev/null
+++ b/db/migrate/20200831204646_add_project_feature_flags_to_plan_limits.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class AddProjectFeatureFlagsToPlanLimits < ActiveRecord::Migration[6.0]
+ DOWNTIME = false
+
+ def change
+ add_column(:plan_limits, :project_feature_flags, :integer, default: 200, null: false)
+ end
+end
diff --git a/db/migrate/20200831222347_insert_project_feature_flags_plan_limits.rb b/db/migrate/20200831222347_insert_project_feature_flags_plan_limits.rb
new file mode 100644
index 00000000000..2c91016d9dd
--- /dev/null
+++ b/db/migrate/20200831222347_insert_project_feature_flags_plan_limits.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+class InsertProjectFeatureFlagsPlanLimits < ActiveRecord::Migration[6.0]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ def up
+ return unless Gitlab.com?
+
+ create_or_update_plan_limit('project_feature_flags', 'free', 50)
+ create_or_update_plan_limit('project_feature_flags', 'bronze', 100)
+ create_or_update_plan_limit('project_feature_flags', 'silver', 150)
+ create_or_update_plan_limit('project_feature_flags', 'gold', 200)
+ end
+
+ def down
+ return unless Gitlab.com?
+
+ create_or_update_plan_limit('project_feature_flags', 'free', 0)
+ create_or_update_plan_limit('project_feature_flags', 'bronze', 0)
+ create_or_update_plan_limit('project_feature_flags', 'silver', 0)
+ create_or_update_plan_limit('project_feature_flags', 'gold', 0)
+ end
+end
diff --git a/db/migrate/20200916120837_add_index_to_members_for_unaccepted_invitations.rb b/db/migrate/20200916120837_add_index_to_members_for_unaccepted_invitations.rb
new file mode 100644
index 00000000000..3b0e4b99eb5
--- /dev/null
+++ b/db/migrate/20200916120837_add_index_to_members_for_unaccepted_invitations.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+class AddIndexToMembersForUnacceptedInvitations < ActiveRecord::Migration[6.0]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ INDEX_NAME = 'idx_members_created_at_user_id_invite_token'
+ INDEX_SCOPE = 'invite_token IS NOT NULL AND user_id IS NULL'
+
+ disable_ddl_transaction!
+
+ def up
+ add_concurrent_index(:members, :created_at, where: INDEX_SCOPE, name: INDEX_NAME)
+ end
+
+ def down
+ remove_concurrent_index(:members, :created_at, where: INDEX_SCOPE, name: INDEX_NAME)
+ end
+end
diff --git a/db/schema_migrations/20200916120837 b/db/schema_migrations/20200916120837
new file mode 100644
index 00000000000..41ce56533b2
--- /dev/null
+++ b/db/schema_migrations/20200916120837
@@ -0,0 +1 @@
+ff246eb2761c4504b67b7d7b197990a671626038e50f1b82d6b3e4739a1ec3d4 \ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 3c8a6a74c8e..ecc4ec7b955 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -13891,7 +13891,8 @@ CREATE TABLE public.plan_limits (
ci_max_artifact_size_requirements integer DEFAULT 0 NOT NULL,
ci_max_artifact_size_coverage_fuzzing integer DEFAULT 0 NOT NULL,
ci_max_artifact_size_browser_performance integer DEFAULT 0 NOT NULL,
- ci_max_artifact_size_load_performance integer DEFAULT 0 NOT NULL
+ ci_max_artifact_size_load_performance integer DEFAULT 0 NOT NULL,
+ project_feature_flags integer DEFAULT 200 NOT NULL
);
CREATE SEQUENCE public.plan_limits_id_seq
@@ -18580,6 +18581,8 @@ CREATE INDEX idx_jira_connect_subscriptions_on_installation_id ON public.jira_co
CREATE UNIQUE INDEX idx_jira_connect_subscriptions_on_installation_id_namespace_id ON public.jira_connect_subscriptions USING btree (jira_connect_installation_id, namespace_id);
+CREATE INDEX idx_members_created_at_user_id_invite_token ON public.members USING btree (created_at) WHERE ((invite_token IS NOT NULL) AND (user_id IS NULL));
+
CREATE INDEX idx_merge_requests_on_id_and_merge_jid ON public.merge_requests USING btree (id, merge_jid) WHERE ((merge_jid IS NOT NULL) AND (state_id = 4));
CREATE INDEX idx_merge_requests_on_source_project_and_branch_state_opened ON public.merge_requests USING btree (source_project_id, source_branch) WHERE (state_id = 1);
@@ -23869,5 +23872,8 @@ COPY "schema_migrations" (version) FROM STDIN;
20200716120419
20200717040735
20200728182311
+20200831204646
+20200831222347
+20200916120837
\.
diff --git a/doc/user/admin_area/custom_project_templates.md b/doc/user/admin_area/custom_project_templates.md
index d194ca42215..8d044b94fbd 100644
--- a/doc/user/admin_area/custom_project_templates.md
+++ b/doc/user/admin_area/custom_project_templates.md
@@ -15,7 +15,8 @@ templates are sourced.
Every project directly under the group namespace will be
available to the user if they have access to them. For example:
-- Public project in the group will be available to every logged in user.
+- Public projects, in the group will be available to every signed-in user, if all enabled [project features](../project/settings/index.md#sharing-and-permissions)
+ are set to **Everyone With Access**.
- Private projects will be available only if the user is a member of the project.
Repository and database information that are copied over to each new project are
diff --git a/doc/user/group/custom_project_templates.md b/doc/user/group/custom_project_templates.md
index ebeacda24c6..7c02d80d08d 100644
--- a/doc/user/group/custom_project_templates.md
+++ b/doc/user/group/custom_project_templates.md
@@ -20,9 +20,8 @@ GitLab administrators can
[set project templates for an entire GitLab instance](../admin_area/custom_project_templates.md).
Within this section, you can configure the group where all the custom project
-templates are sourced. Every project directly under the group namespace will be
-available to the user if they have access to them. For example, every public
-project in the group will be available to every logged in user.
+templates are sourced. Every project _template_ directly under the group namespace is
+available to every signed-in user, if all enabled [project features](../project/settings/index.md#sharing-and-permissions) are set to **Everyone With Access**.
However, private projects will be available only if the user is a member of the project.
diff --git a/doc/user/project/members/index.md b/doc/user/project/members/index.md
index 3eb411aef18..d8579c4cd8e 100644
--- a/doc/user/project/members/index.md
+++ b/doc/user/project/members/index.md
@@ -93,6 +93,9 @@ invitation, change their access level, or even delete them.
Once the user accepts the invitation, they will be prompted to create a new
GitLab account using the same e-mail address the invitation was sent to.
+Note: **Note:**
+Unaccepted invites are automatically deleted after 90 days.
+
## Project membership and requesting access
Project owners can :
diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb
index 8c4c466cd75..8633df4cb70 100644
--- a/lib/api/api_guard.rb
+++ b/lib/api/api_guard.rb
@@ -53,8 +53,10 @@ module API
user = find_user_from_sources
return unless user
- # Sessions are enforced to be unavailable for API calls, so ignore them for admin mode
- Gitlab::Auth::CurrentUserMode.bypass_session!(user.id) if Feature.enabled?(:user_mode_in_session)
+ if user.is_a?(User) && Feature.enabled?(:user_mode_in_session)
+ # Sessions are enforced to be unavailable for API calls, so ignore them for admin mode
+ Gitlab::Auth::CurrentUserMode.bypass_session!(user.id)
+ end
unless api_access_allowed?(user)
forbidden!(api_access_denied_message(user))
diff --git a/lib/gitlab/application_rate_limiter.rb b/lib/gitlab/application_rate_limiter.rb
index 4eeec534fc0..6173918b453 100644
--- a/lib/gitlab/application_rate_limiter.rb
+++ b/lib/gitlab/application_rate_limiter.rb
@@ -31,7 +31,9 @@ module Gitlab
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_testing_hook: { threshold: 5, interval: 1.minute }
+ group_testing_hook: { threshold: 5, interval: 1.minute },
+ profile_add_new_email: { threshold: 5, interval: 1.minute },
+ profile_resend_email_confirmation: { threshold: 5, interval: 1.minute }
}.freeze
end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 81f7b67b2ad..fa20e5373fc 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -1942,6 +1942,9 @@ msgstr ""
msgid "AdminUsers|You cannot remove your own admin rights."
msgstr ""
+msgid "AdminUsers|You must transfer ownership or delete the groups owned by this user before you can delete their account"
+msgstr ""
+
msgid "Administration"
msgstr ""
@@ -17980,6 +17983,9 @@ msgstr ""
msgid "Profiles|You don't have access to delete this user."
msgstr ""
+msgid "Profiles|You must transfer ownership or delete groups you are an owner of before you can delete your account"
+msgstr ""
+
msgid "Profiles|You must transfer ownership or delete these groups before you can delete your account."
msgstr ""
@@ -23972,6 +23978,9 @@ msgstr ""
msgid "This action can lead to data loss. To prevent accidental actions we ask you to confirm your intention."
msgstr ""
+msgid "This action has been performed too many times. Try again later."
+msgstr ""
+
msgid "This also resolves all related threads"
msgstr ""
@@ -27623,6 +27632,9 @@ msgstr ""
msgid "by %{user}"
msgstr ""
+msgid "cannot be a date in the past"
+msgstr ""
+
msgid "cannot be changed if a personal project has container registry tags."
msgstr ""
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb
index 08a1d7c9fa9..9e7f24ff7e3 100644
--- a/spec/controllers/admin/users_controller_spec.rb
+++ b/spec/controllers/admin/users_controller_spec.rb
@@ -36,7 +36,7 @@ RSpec.describe Admin::UsersController do
end
end
- describe 'DELETE #user with projects', :sidekiq_might_not_need_inline do
+ describe 'DELETE #destroy', :sidekiq_might_not_need_inline do
let(:project) { create(:project, namespace: user.namespace) }
let!(:issue) { create(:issue, author: user) }
@@ -59,6 +59,41 @@ RSpec.describe Admin::UsersController do
expect(User.exists?(user.id)).to be_falsy
expect(Issue.exists?(issue.id)).to be_falsy
end
+
+ context 'prerequisites for account deletion' do
+ context 'solo-owned groups' do
+ let(:group) { create(:group) }
+
+ context 'if the user is the sole owner of at least one group' do
+ before do
+ create(:group_member, :owner, group: group, user: user)
+ end
+
+ context 'soft-delete' do
+ it 'fails' do
+ delete :destroy, params: { id: user.username }
+
+ message = s_('AdminUsers|You must transfer ownership or delete the groups owned by this user before you can delete their account')
+
+ expect(flash[:alert]).to eq(message)
+ expect(response).to have_gitlab_http_status(:see_other)
+ expect(response).to redirect_to admin_user_path(user)
+ expect(User.exists?(user.id)).to be_truthy
+ end
+ end
+
+ context 'hard-delete' do
+ it 'succeeds' do
+ delete :destroy, params: { id: user.username, hard_delete: true }
+
+ expect(response).to redirect_to(admin_users_path)
+ expect(flash[:notice]).to eq(_('The user is being deleted.'))
+ expect(User.exists?(user.id)).to be_falsy
+ end
+ end
+ end
+ end
+ end
end
describe 'PUT #activate' do
diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb
index 85f1b247ee9..4b9dd3629f1 100644
--- a/spec/controllers/groups/group_members_controller_spec.rb
+++ b/spec/controllers/groups/group_members_controller_spec.rb
@@ -139,6 +139,45 @@ RSpec.describe Groups::GroupMembersController do
expect(group.users).not_to include group_user
end
end
+
+ context 'access expiry date' do
+ before do
+ group.add_owner(user)
+ end
+
+ subject do
+ post :create, params: {
+ group_id: group,
+ user_ids: group_user.id,
+ access_level: Gitlab::Access::GUEST,
+ expires_at: expires_at
+ }
+ end
+
+ context 'when set to a date in the past' do
+ let(:expires_at) { 2.days.ago }
+
+ it 'does not add user to members' do
+ subject
+
+ expect(flash[:alert]).to include('Expires at cannot be a date in the past')
+ expect(response).to redirect_to(group_group_members_path(group))
+ expect(group.users).not_to include group_user
+ end
+ end
+
+ context 'when set to a date in the future' do
+ let(:expires_at) { 5.days.from_now }
+
+ it 'adds user to members' do
+ subject
+
+ expect(response).to set_flash.to 'Users were successfully added.'
+ expect(response).to redirect_to(group_group_members_path(group))
+ expect(group.users).to include group_user
+ end
+ end
+ end
end
describe 'PUT update' do
@@ -149,15 +188,49 @@ RSpec.describe Groups::GroupMembersController do
sign_in(user)
end
- Gitlab::Access.options.each do |label, value|
- it "can change the access level to #{label}" do
- put :update, params: {
- group_member: { access_level: value },
- group_id: group,
- id: requester
- }, xhr: true
+ context 'access level' do
+ Gitlab::Access.options.each do |label, value|
+ it "can change the access level to #{label}" do
+ put :update, params: {
+ group_member: { access_level: value },
+ group_id: group,
+ id: requester
+ }, xhr: true
- expect(requester.reload.human_access).to eq(label)
+ expect(requester.reload.human_access).to eq(label)
+ end
+ end
+ end
+
+ context 'access expiry date' do
+ subject do
+ put :update, xhr: true, params: {
+ group_member: {
+ expires_at: expires_at
+ },
+ group_id: group,
+ id: requester
+ }
+ end
+
+ context 'when set to a date in the past' do
+ let(:expires_at) { 2.days.ago }
+
+ it 'does not update the member' do
+ subject
+
+ expect(requester.reload.expires_at).not_to eq(expires_at.to_date)
+ end
+ end
+
+ context 'when set to a date in the future' do
+ let(:expires_at) { 5.days.from_now }
+
+ it 'updates the member' do
+ subject
+
+ expect(requester.reload.expires_at).to eq(expires_at.to_date)
+ end
end
end
end
diff --git a/spec/controllers/profiles/emails_controller_spec.rb b/spec/controllers/profiles/emails_controller_spec.rb
index 246f8a6cd76..950120ae564 100644
--- a/spec/controllers/profiles/emails_controller_spec.rb
+++ b/spec/controllers/profiles/emails_controller_spec.rb
@@ -3,7 +3,7 @@
require 'spec_helper'
RSpec.describe Profiles::EmailsController do
- let(:user) { create(:user) }
+ let_it_be(:user) { create(:user) }
before do
sign_in(user)
@@ -15,37 +15,75 @@ RSpec.describe Profiles::EmailsController do
end
end
- describe '#create' do
- context 'when email address is valid' do
- let(:email_params) { { email: "add_email@example.com" } }
+ shared_examples_for 'respects the rate limit' do
+ context 'after the rate limit is exceeded' do
+ before do
+ allowed_threshold = Gitlab::ApplicationRateLimiter.rate_limits[action][:threshold]
+
+ allow(Gitlab::ApplicationRateLimiter)
+ .to receive(:increment)
+ .and_return(allowed_threshold + 1)
+ end
+
+ it 'does not send any email' do
+ expect { subject }.not_to change { ActionMailer::Base.deliveries.size }
+ end
- it 'sends an email confirmation' do
- expect { post(:create, params: { email: email_params }) }.to change { ActionMailer::Base.deliveries.size }
+ it 'displays an alert' do
+ subject
+
+ expect(response).to have_gitlab_http_status(:redirect)
+ expect(flash[:alert]).to eq(_('This action has been performed too many times. Try again later.'))
end
end
+ end
+
+ describe '#create' do
+ let(:email) { 'add_email@example.com' }
+ let(:params) { { email: { email: email } } }
+
+ subject { post(:create, params: params) }
+
+ it 'sends an email confirmation' do
+ expect { subject }.to change { ActionMailer::Base.deliveries.size }
+ end
context 'when email address is invalid' do
- let(:email_params) { { email: "test.@example.com" } }
+ let(:email) { 'invalid.@example.com' }
it 'does not send an email confirmation' do
- expect { post(:create, params: { email: email_params }) }.not_to change { ActionMailer::Base.deliveries.size }
+ expect { subject }.not_to change { ActionMailer::Base.deliveries.size }
end
end
+
+ it_behaves_like 'respects the rate limit' do
+ let(:action) { :profile_add_new_email }
+ end
end
describe '#resend_confirmation_instructions' do
- let(:email_params) { { email: "add_email@example.com" } }
+ let_it_be(:email) { create(:email, user: user) }
+ let(:params) { { id: email.id } }
+
+ subject { put(:resend_confirmation_instructions, params: params) }
it 'resends an email confirmation' do
- email = user.emails.create(email: 'add_email@example.com')
+ expect { subject }.to change { ActionMailer::Base.deliveries.size }
- expect { put(:resend_confirmation_instructions, params: { id: email }) }.to change { ActionMailer::Base.deliveries.size }
- expect(ActionMailer::Base.deliveries.last.to).to eq [email_params[:email]]
- expect(ActionMailer::Base.deliveries.last.subject).to match "Confirmation instructions"
+ expect(ActionMailer::Base.deliveries.last.to).to eq [email.email]
+ expect(ActionMailer::Base.deliveries.last.subject).to match 'Confirmation instructions'
+ end
+
+ context 'email does not exist' do
+ let(:params) { { id: non_existing_record_id } }
+
+ it 'does not send an email confirmation' do
+ expect { subject }.not_to change { ActionMailer::Base.deliveries.size }
+ end
end
- it 'unable to resend an email confirmation' do
- expect { put(:resend_confirmation_instructions, params: { id: 1 }) }.not_to change { ActionMailer::Base.deliveries.size }
+ it_behaves_like 'respects the rate limit' do
+ let(:action) { :profile_resend_email_confirmation }
end
end
end
diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb
index 40a220d57a7..ae05e2d2631 100644
--- a/spec/controllers/projects/project_members_controller_spec.rb
+++ b/spec/controllers/projects/project_members_controller_spec.rb
@@ -129,6 +129,46 @@ RSpec.describe Projects::ProjectMembersController do
expect(response).to redirect_to(project_project_members_path(project))
end
end
+
+ context 'access expiry date' do
+ before do
+ project.add_maintainer(user)
+ end
+
+ subject do
+ post :create, params: {
+ namespace_id: project.namespace,
+ project_id: project,
+ user_ids: project_user.id,
+ access_level: Gitlab::Access::GUEST,
+ expires_at: expires_at
+ }
+ end
+
+ context 'when set to a date in the past' do
+ let(:expires_at) { 2.days.ago }
+
+ it 'does not add user to members' do
+ subject
+
+ expect(flash[:alert]).to include('Expires at cannot be a date in the past')
+ expect(response).to redirect_to(project_project_members_path(project))
+ expect(project.users).not_to include project_user
+ end
+ end
+
+ context 'when set to a date in the future' do
+ let(:expires_at) { 5.days.from_now }
+
+ it 'adds user to members' do
+ subject
+
+ expect(response).to set_flash.to 'Users were successfully added.'
+ expect(response).to redirect_to(project_project_members_path(project))
+ expect(project.users).to include project_user
+ end
+ end
+ end
end
describe 'PUT update' do
@@ -139,16 +179,53 @@ RSpec.describe Projects::ProjectMembersController do
sign_in(user)
end
- Gitlab::Access.options.each do |label, value|
- it "can change the access level to #{label}" do
- put :update, params: {
- project_member: { access_level: value },
- namespace_id: project.namespace,
- project_id: project,
- id: requester
- }, xhr: true
+ context 'access level' do
+ Gitlab::Access.options.each do |label, value|
+ it "can change the access level to #{label}" do
+ params = {
+ project_member: { access_level: value },
+ namespace_id: project.namespace,
+ project_id: project,
+ id: requester
+ }
+
+ put :update, params: params, xhr: true
+
+ expect(requester.reload.human_access).to eq(label)
+ end
+ end
+ end
+
+ context 'access expiry date' do
+ subject do
+ put :update, xhr: true, params: {
+ project_member: {
+ expires_at: expires_at
+ },
+ namespace_id: project.namespace,
+ project_id: project,
+ id: requester
+ }
+ end
- expect(requester.reload.human_access).to eq(label)
+ context 'when set to a date in the past' do
+ let(:expires_at) { 2.days.ago }
+
+ it 'does not update the member' do
+ subject
+
+ expect(requester.reload.expires_at).not_to eq(expires_at.to_date)
+ end
+ end
+
+ context 'when set to a date in the future' do
+ let(:expires_at) { 5.days.from_now }
+
+ it 'updates the member' do
+ subject
+
+ expect(requester.reload.expires_at).to eq(expires_at.to_date)
+ end
end
end
end
diff --git a/spec/controllers/projects/raw_controller_spec.rb b/spec/controllers/projects/raw_controller_spec.rb
index 5f10343eb76..b3921164c81 100644
--- a/spec/controllers/projects/raw_controller_spec.rb
+++ b/spec/controllers/projects/raw_controller_spec.rb
@@ -33,6 +33,11 @@ RSpec.describe Projects::RawController do
it_behaves_like 'project cache control headers'
it_behaves_like 'content disposition headers'
+ it_behaves_like 'uncached response' do
+ before do
+ subject
+ end
+ end
end
context 'image header' do
diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb
index 66caa58666f..c1afc75a6fd 100644
--- a/spec/controllers/registrations_controller_spec.rb
+++ b/spec/controllers/registrations_controller_spec.rb
@@ -384,6 +384,24 @@ RSpec.describe RegistrationsController do
expect_success
end
end
+
+ context 'prerequisites for account deletion' do
+ context 'solo-owned groups' do
+ let(:group) { create(:group) }
+
+ context 'if the user is the sole owner of at least one group' do
+ before do
+ create(:group_member, :owner, group: group, user: user)
+ end
+
+ it 'fails' do
+ delete :destroy, params: { password: '12345678' }
+
+ expect_failure(s_('Profiles|You must transfer ownership or delete groups you are an owner of before you can delete your account'))
+ end
+ end
+ end
+ end
end
describe '#update_registration' do
diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index e4b53186ea8..216991c56c5 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -31,6 +31,7 @@ FactoryBot.define do
pages_access_level do
visibility_level == Gitlab::VisibilityLevel::PUBLIC ? ProjectFeature::ENABLED : ProjectFeature::PRIVATE
end
+ metrics_dashboard_access_level { ProjectFeature::PRIVATE }
# we can't assign the delegated `#ci_cd_settings` attributes directly, as the
# `#ci_cd_settings` relation needs to be created first
@@ -54,7 +55,9 @@ FactoryBot.define do
issues_access_level: evaluator.issues_access_level,
forking_access_level: evaluator.forking_access_level,
merge_requests_access_level: merge_requests_access_level,
- repository_access_level: evaluator.repository_access_level
+ repository_access_level: evaluator.repository_access_level,
+ pages_access_level: evaluator.pages_access_level,
+ metrics_dashboard_access_level: evaluator.metrics_dashboard_access_level
}
if ActiveRecord::Migrator.current_version >= PAGES_ACCESS_LEVEL_SCHEMA_VERSION
@@ -300,6 +303,9 @@ FactoryBot.define do
trait(:pages_enabled) { pages_access_level { ProjectFeature::ENABLED } }
trait(:pages_disabled) { pages_access_level { ProjectFeature::DISABLED } }
trait(:pages_private) { pages_access_level { ProjectFeature::PRIVATE } }
+ trait(:metrics_dashboard_enabled) { metrics_dashboard_access_level { ProjectFeature::ENABLED } }
+ trait(:metrics_dashboard_disabled) { metrics_dashboard_access_level { ProjectFeature::DISABLED } }
+ trait(:metrics_dashboard_private) { metrics_dashboard_access_level { ProjectFeature::PRIVATE } }
trait :auto_devops do
association :auto_devops, factory: :project_auto_devops
diff --git a/spec/graphql/mutations/issues/set_confidential_spec.rb b/spec/graphql/mutations/issues/set_confidential_spec.rb
index 820f9aa5e17..0b2fc0ecb93 100644
--- a/spec/graphql/mutations/issues/set_confidential_spec.rb
+++ b/spec/graphql/mutations/issues/set_confidential_spec.rb
@@ -3,7 +3,8 @@
require 'spec_helper'
RSpec.describe Mutations::Issues::SetConfidential do
- let(:issue) { create(:issue) }
+ let(:project) { create(:project, :private) }
+ let(:issue) { create(:issue, project: project, assignees: [user]) }
let(:user) { create(:user) }
subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
@@ -14,7 +15,7 @@ RSpec.describe Mutations::Issues::SetConfidential do
let(:confidential) { true }
let(:mutated_issue) { subject[:issue] }
- subject { mutation.resolve(project_path: issue.project.full_path, iid: issue.iid, confidential: confidential) }
+ subject { mutation.resolve(project_path: project.full_path, iid: issue.iid, confidential: confidential) }
it 'raises an error if the resource is not accessible to the user' do
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
@@ -22,7 +23,7 @@ RSpec.describe Mutations::Issues::SetConfidential do
context 'when the user can update the issue' do
before do
- issue.project.add_developer(user)
+ project.add_developer(user)
end
it 'returns the issue as confidential' do
@@ -39,5 +40,19 @@ RSpec.describe Mutations::Issues::SetConfidential do
end
end
end
+
+ context 'when guest user is an assignee' do
+ let(:project) { create(:project, :public) }
+
+ before do
+ project.add_guest(user)
+ end
+
+ it 'does not change issue confidentiality' do
+ expect(mutated_issue).to eq(issue)
+ expect(mutated_issue.confidential).to be_falsey
+ expect(subject[:errors]).to be_empty
+ end
+ end
end
end
diff --git a/spec/graphql/mutations/merge_requests/set_milestone_spec.rb b/spec/graphql/mutations/merge_requests/set_milestone_spec.rb
index 1c0d655ee83..ccb2d9bd132 100644
--- a/spec/graphql/mutations/merge_requests/set_milestone_spec.rb
+++ b/spec/graphql/mutations/merge_requests/set_milestone_spec.rb
@@ -3,31 +3,29 @@
require 'spec_helper'
RSpec.describe Mutations::MergeRequests::SetMilestone do
- let(:merge_request) { create(:merge_request) }
let(:user) { create(:user) }
+ let(:project) { create(:project, :private) }
+ let(:merge_request) { create(:merge_request, source_project: project, target_project: project, assignees: [user]) }
+ let(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
+ let(:milestone) { create(:milestone, project: project) }
- subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
+ subject { mutation.resolve(project_path: project.full_path, iid: merge_request.iid, milestone: milestone) }
specify { expect(described_class).to require_graphql_authorizations(:update_merge_request) }
describe '#resolve' do
- let(:milestone) { create(:milestone, project: merge_request.project) }
- let(:mutated_merge_request) { subject[:merge_request] }
-
- subject { mutation.resolve(project_path: merge_request.project.full_path, iid: merge_request.iid, milestone: milestone) }
-
it 'raises an error if the resource is not accessible to the user' do
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
context 'when the user can update the merge request' do
before do
- merge_request.project.add_developer(user)
+ project.add_developer(user)
end
it 'returns the merge request with the milestone' do
- expect(mutated_merge_request).to eq(merge_request)
- expect(mutated_merge_request.milestone).to eq(milestone)
+ expect(subject[:merge_request]).to eq(merge_request)
+ expect(subject[:merge_request].milestone).to eq(milestone)
expect(subject[:errors]).to be_empty
end
@@ -43,13 +41,37 @@ RSpec.describe Mutations::MergeRequests::SetMilestone do
let(:milestone) { nil }
it 'removes the milestone' do
- merge_request.update!(milestone: create(:milestone, project: merge_request.project))
+ merge_request.update!(milestone: create(:milestone, project: project))
- expect(mutated_merge_request.milestone).to eq(nil)
+ expect(subject[:merge_request].milestone).to be_nil
end
it 'does not do anything if the MR already does not have a milestone' do
- expect(mutated_merge_request.milestone).to eq(nil)
+ expect(subject[:merge_request].milestone).to be_nil
+ end
+ end
+ end
+
+ context 'when issue assignee is a guest' do
+ let(:project) { create(:project, :public) }
+
+ before do
+ project.add_guest(user)
+ end
+
+ it 'does not update the milestone' do
+ expect(subject[:merge_request]).to eq(merge_request)
+ expect(subject[:merge_request].milestone).to be_nil
+ expect(subject[:errors]).to be_empty
+ end
+
+ context 'when passing milestone_id as nil' do
+ let(:milestone) { nil }
+
+ it 'does not remove the milestone' do
+ merge_request.update!(milestone: create(:milestone, project: project))
+
+ expect(subject[:merge_request].milestone).not_to be_nil
end
end
end
diff --git a/spec/migrations/insert_project_feature_flags_plan_limits_spec.rb b/spec/migrations/insert_project_feature_flags_plan_limits_spec.rb
new file mode 100644
index 00000000000..1ad070de1ea
--- /dev/null
+++ b/spec/migrations/insert_project_feature_flags_plan_limits_spec.rb
@@ -0,0 +1,80 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join(
+ 'db',
+ 'migrate',
+ '20200831222347_insert_project_feature_flags_plan_limits.rb'
+)
+
+RSpec.describe InsertProjectFeatureFlagsPlanLimits do
+ let(:migration) { described_class.new }
+ let(:plans) { table(:plans) }
+ let(:plan_limits) { table(:plan_limits) }
+ let!(:default_plan) { plans.create!(name: 'default') }
+ let!(:free_plan) { plans.create!(name: 'free') }
+ let!(:bronze_plan) { plans.create!(name: 'bronze') }
+ let!(:silver_plan) { plans.create!(name: 'silver') }
+ let!(:gold_plan) { plans.create!(name: 'gold') }
+ let!(:default_plan_limits) do
+ plan_limits.create!(plan_id: default_plan.id, project_feature_flags: 200)
+ end
+
+ context 'when on Gitlab.com' do
+ before do
+ expect(Gitlab).to receive(:com?).at_most(:twice).and_return(true)
+ end
+
+ describe '#up' do
+ it 'updates the project_feature_flags plan limits' do
+ migration.up
+
+ expect(plan_limits.pluck(:plan_id, :project_feature_flags)).to contain_exactly(
+ [default_plan.id, 200],
+ [free_plan.id, 50],
+ [bronze_plan.id, 100],
+ [silver_plan.id, 150],
+ [gold_plan.id, 200]
+ )
+ end
+ end
+
+ describe '#down' do
+ it 'removes the project_feature_flags plan limits' do
+ migration.up
+ migration.down
+
+ expect(plan_limits.pluck(:plan_id, :project_feature_flags)).to contain_exactly(
+ [default_plan.id, 200],
+ [free_plan.id, 0],
+ [bronze_plan.id, 0],
+ [silver_plan.id, 0],
+ [gold_plan.id, 0]
+ )
+ end
+ end
+ end
+
+ context 'when on self-hosted' do
+ before do
+ expect(Gitlab).to receive(:com?).at_most(:twice).and_return(false)
+ end
+
+ describe '#up' do
+ it 'does not change the plan limits' do
+ migration.up
+
+ expect(plan_limits.pluck(:project_feature_flags)).to contain_exactly(200)
+ end
+ end
+
+ describe '#down' do
+ it 'does not change the plan limits' do
+ migration.up
+ migration.down
+
+ expect(plan_limits.pluck(:project_feature_flags)).to contain_exactly(200)
+ end
+ end
+ end
+end
diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb
index de39c8c7c5c..f0bae3f29c0 100644
--- a/spec/models/active_session_spec.rb
+++ b/spec/models/active_session_spec.rb
@@ -23,7 +23,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
describe '#current?' do
it 'returns true if the active session matches the current session' do
- active_session = ActiveSession.new(session_id: rack_session)
+ active_session = ActiveSession.new(session_private_id: rack_session.private_id)
expect(active_session.current?(session)).to be true
end
@@ -45,7 +45,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
describe '#public_id' do
it 'returns an encrypted, url-encoded session id' do
original_session_id = Rack::Session::SessionId.new("!*'();:@&\n=+$,/?%abcd#123[4567]8")
- active_session = ActiveSession.new(session_id: original_session_id)
+ active_session = ActiveSession.new(session_id: original_session_id.public_id)
encrypted_id = active_session.public_id
derived_session_id = Gitlab::CryptoHelper.aes256_gcm_decrypt(encrypted_id)
@@ -106,8 +106,8 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
redis.sadd(
"session:lookup:user:gitlab:#{user.id}",
%w[
- 6919a6f1bb119dd7396fadc38fd18d0d
- 59822c7d9fcdfa03725eff41782ad97d
+ 2::418729c72310bbf349a032f0bb6e3fce9f5a69df8f000d8ae0ac5d159d8f21ae
+ 2::d2ee6f70d6ef0e8701efa3f6b281cbe8e6bf3d109ef052a8b5ce88bfc7e71c26
]
)
end
@@ -135,7 +135,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
redis.set("session:gitlab:#{rack_session.private_id}", Marshal.dump({ _csrf_token: 'abcd' }))
end
- expect(ActiveSession.sessions_from_ids([rack_session])).to eq [{ _csrf_token: 'abcd' }]
+ expect(ActiveSession.sessions_from_ids([rack_session.private_id])).to eq [{ _csrf_token: 'abcd' }]
end
it 'avoids a redis lookup for an empty array' do
@@ -150,12 +150,11 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
redis = double(:redis)
expect(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis)
- sessions = %w[session-a session-b session-c session-d]
+ sessions = %w[session-a session-b]
mget_responses = sessions.map { |session| [Marshal.dump(session)]}
- expect(redis).to receive(:mget).exactly(4).times.and_return(*mget_responses)
+ expect(redis).to receive(:mget).twice.times.and_return(*mget_responses)
- session_ids = [1, 2].map { |id| Rack::Session::SessionId.new(id.to_s) }
- expect(ActiveSession.sessions_from_ids(session_ids).map(&:to_s)).to eql(sessions)
+ expect(ActiveSession.sessions_from_ids([1, 2])).to eql(sessions)
end
end
@@ -165,7 +164,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
Gitlab::Redis::SharedState.with do |redis|
expect(redis.scan_each.to_a).to include(
- "session:user:gitlab:#{user.id}:6919a6f1bb119dd7396fadc38fd18d0d",
+ "session:user:gitlab:#{user.id}:2::418729c72310bbf349a032f0bb6e3fce9f5a69df8f000d8ae0ac5d159d8f21ae",
"session:lookup:user:gitlab:#{user.id}"
)
end
@@ -208,13 +207,41 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
end
end
end
+
+ context 'ActiveSession stored by deprecated rack_session_public_key' do
+ let(:active_session) { ActiveSession.new(session_id: rack_session.public_id) }
+ let(:deprecated_active_session_lookup_key) { rack_session.public_id }
+
+ before do
+ Gitlab::Redis::SharedState.with do |redis|
+ redis.set("session:user:gitlab:#{user.id}:#{deprecated_active_session_lookup_key}",
+ '')
+ redis.sadd(described_class.lookup_key_name(user.id),
+ deprecated_active_session_lookup_key)
+ end
+ end
+
+ it 'removes deprecated key and stores only new one' do
+ expected_session_keys = ["session:user:gitlab:#{user.id}:#{rack_session.private_id}",
+ "session:lookup:user:gitlab:#{user.id}"]
+
+ ActiveSession.set(user, request)
+
+ Gitlab::Redis::SharedState.with do |redis|
+ actual_session_keys = redis.scan_each(match: 'session:*').to_a
+ expect(actual_session_keys).to(match_array(expected_session_keys))
+
+ expect(redis.smembers("session:lookup:user:gitlab:#{user.id}")).to eq [rack_session.private_id]
+ end
+ end
+ end
end
- describe '.destroy' do
+ describe '.destroy_with_rack_session_id' do
it 'gracefully handles a nil session ID' do
expect(described_class).not_to receive(:destroy_sessions)
- ActiveSession.destroy(user, nil)
+ ActiveSession.destroy_with_rack_session_id(user, nil)
end
it 'removes the entry associated with the currently killed user session' do
@@ -224,7 +251,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
redis.set("session:user:gitlab:9999:5c8611e4f9c69645ad1a1492f4131358", '')
end
- ActiveSession.destroy(user, request.session.id)
+ ActiveSession.destroy_with_rack_session_id(user, request.session.id)
Gitlab::Redis::SharedState.with do |redis|
expect(redis.scan_each(match: "session:user:gitlab:*")).to match_array [
@@ -240,7 +267,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
redis.sadd("session:lookup:user:gitlab:#{user.id}", '6919a6f1bb119dd7396fadc38fd18d0d')
end
- ActiveSession.destroy(user, request.session.id)
+ ActiveSession.destroy_with_rack_session_id(user, request.session.id)
Gitlab::Redis::SharedState.with do |redis|
expect(redis.scan_each(match: "session:lookup:user:gitlab:#{user.id}").to_a).to be_empty
@@ -249,12 +276,12 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
it 'removes the devise session' do
Gitlab::Redis::SharedState.with do |redis|
- redis.set("session:user:gitlab:#{user.id}:#{rack_session.public_id}", '')
+ redis.set("session:user:gitlab:#{user.id}:#{rack_session.private_id}", '')
# Emulate redis-rack: https://github.com/redis-store/redis-rack/blob/c75f7f1a6016ee224e2615017fbfee964f23a837/lib/rack/session/redis.rb#L88
redis.set("session:gitlab:#{rack_session.private_id}", '')
end
- ActiveSession.destroy(user, request.session.id)
+ ActiveSession.destroy_with_rack_session_id(user, request.session.id)
Gitlab::Redis::SharedState.with do |redis|
expect(redis.scan_each(match: "session:gitlab:*").to_a).to be_empty
@@ -262,37 +289,83 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
end
end
- describe '.destroy_with_public_id' do
- it 'receives a user and public id and destroys the associated session' do
- ActiveSession.set(user, request)
- session = ActiveSession.list(user).first
+ describe '.destroy_with_deprecated_encryption' do
+ shared_examples 'removes all session data' do
+ before do
+ Gitlab::Redis::SharedState.with do |redis|
+ redis.set("session:user:gitlab:#{user.id}:#{active_session_lookup_key}", '')
+ # Emulate redis-rack: https://github.com/redis-store/redis-rack/blob/c75f7f1a6016ee224e2615017fbfee964f23a837/lib/rack/session/redis.rb#L88
+ redis.set("session:gitlab:#{rack_session.private_id}", '')
+
+ redis.set(described_class.key_name(user.id, active_session_lookup_key),
+ Marshal.dump(active_session))
+ redis.sadd(described_class.lookup_key_name(user.id),
+ active_session_lookup_key)
+ end
+ end
+
+ it 'removes the devise session' do
+ subject
+
+ Gitlab::Redis::SharedState.with do |redis|
+ expect(redis.scan_each(match: "session:gitlab:*").to_a).to be_empty
+ end
+ end
- ActiveSession.destroy_with_public_id(user, session.public_id)
+ it 'removes the lookup entry' do
+ subject
- total_sessions = ActiveSession.list(user).count
- expect(total_sessions).to eq 0
+ Gitlab::Redis::SharedState.with do |redis|
+ expect(redis.scan_each(match: "session:lookup:user:gitlab:#{user.id}").to_a).to be_empty
+ end
+ end
+
+ it 'removes the ActiveSession' do
+ subject
+
+ Gitlab::Redis::SharedState.with do |redis|
+ expect(redis.scan_each(match: "session:user:gitlab:*").to_a).to be_empty
+ end
+ end
end
- it 'handles invalid input for public id' do
- expect do
- ActiveSession.destroy_with_public_id(user, nil)
- end.not_to raise_error
+ context 'destroy called with Rack::Session::SessionId#private_id' do
+ subject { ActiveSession.destroy_with_deprecated_encryption(user, rack_session.private_id) }
+
+ it 'calls .destroy_sessions' do
+ expect(ActiveSession).to(
+ receive(:destroy_sessions)
+ .with(anything, user, [rack_session.private_id]))
+
+ subject
+ end
- expect do
- ActiveSession.destroy_with_public_id(user, "")
- end.not_to raise_error
+ context 'ActiveSession with session_private_id' do
+ let(:active_session) { ActiveSession.new(session_private_id: rack_session.private_id) }
+ let(:active_session_lookup_key) { rack_session.private_id }
- expect do
- ActiveSession.destroy_with_public_id(user, "aaaaaaaa")
- end.not_to raise_error
+ include_examples 'removes all session data'
+ end
end
- it 'does not attempt to destroy session when given invalid input for public id' do
- expect(ActiveSession).not_to receive(:destroy)
+ context 'destroy called with ActiveSession#public_id (deprecated)' do
+ let(:active_session) { ActiveSession.new(session_id: rack_session.public_id) }
+ let(:encrypted_active_session_id) { active_session.public_id }
+ let(:active_session_lookup_key) { rack_session.public_id }
+
+ subject { ActiveSession.destroy_with_deprecated_encryption(user, encrypted_active_session_id) }
+
+ it 'calls .destroy_sessions' do
+ expect(ActiveSession).to(
+ receive(:destroy_sessions)
+ .with(anything, user, [active_session.public_id, rack_session.public_id, rack_session.private_id]))
+
+ subject
+ end
- ActiveSession.destroy_with_public_id(user, nil)
- ActiveSession.destroy_with_public_id(user, "")
- ActiveSession.destroy_with_public_id(user, "aaaaaaaa")
+ context 'ActiveSession with session_id (deprecated)' do
+ include_examples 'removes all session data'
+ end
end
end
@@ -308,29 +381,43 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
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')
+ # setup for current user
+ [current_session_id, '59822c7d9fcdfa03725eff41782ad97d'].each do |session_public_id|
+ session_private_id = Rack::Session::SessionId.new(session_public_id).private_id
+ active_session = ActiveSession.new(session_private_id: session_private_id)
+ redis.set(described_class.key_name(user.id, session_private_id),
+ Marshal.dump(active_session))
+ redis.sadd(described_class.lookup_key_name(user.id),
+ session_private_id)
+ end
+
+ # setup for unrelated user
+ unrelated_user_id = 9999
+ session_private_id = Rack::Session::SessionId.new('5c8611e4f9c69645ad1a1492f4131358').private_id
+ active_session = ActiveSession.new(session_private_id: session_private_id)
+
+ redis.set(described_class.key_name(unrelated_user_id, session_private_id),
+ Marshal.dump(active_session))
+ redis.sadd(described_class.lookup_key_name(unrelated_user_id),
+ session_private_id)
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.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
+ session_private_id = Rack::Session::SessionId.new(current_session_id).private_id
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]
+ expect(
+ redis.smembers(described_class.lookup_key_name(user.id))
+ ).to eq([session_private_id])
end
end
@@ -464,5 +551,38 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
end
end
end
+
+ context 'cleaning up old sessions stored by Rack::Session::SessionId#private_id' do
+ let(:max_number_of_sessions_plus_one) { ActiveSession::ALLOWED_NUMBER_OF_ACTIVE_SESSIONS + 1 }
+ let(:max_number_of_sessions_plus_two) { ActiveSession::ALLOWED_NUMBER_OF_ACTIVE_SESSIONS + 2 }
+
+ before do
+ Gitlab::Redis::SharedState.with do |redis|
+ (1..max_number_of_sessions_plus_two).each do |number|
+ redis.set(
+ "session:user:gitlab:#{user.id}:#{number}",
+ Marshal.dump(ActiveSession.new(session_private_id: number.to_s, updated_at: number.days.ago))
+ )
+ redis.sadd(
+ "session:lookup:user:gitlab:#{user.id}",
+ "#{number}"
+ )
+ end
+ end
+ end
+
+ it 'removes obsolete active sessions entries' do
+ ActiveSession.cleanup(user)
+
+ Gitlab::Redis::SharedState.with do |redis|
+ sessions = redis.scan_each(match: "session:user:gitlab:#{user.id}:*").to_a
+
+ expect(sessions.count).to eq(ActiveSession::ALLOWED_NUMBER_OF_ACTIVE_SESSIONS)
+ expect(sessions).not_to(
+ include("session:user:gitlab:#{user.id}:#{max_number_of_sessions_plus_one}",
+ "session:user:gitlab:#{user.id}:#{max_number_of_sessions_plus_two}"))
+ end
+ end
+ end
end
end
diff --git a/spec/models/concerns/expirable_spec.rb b/spec/models/concerns/expirable_spec.rb
index b20d759fc3f..5eb6530881e 100644
--- a/spec/models/concerns/expirable_spec.rb
+++ b/spec/models/concerns/expirable_spec.rb
@@ -4,9 +4,13 @@ require 'spec_helper'
RSpec.describe Expirable do
describe 'ProjectMember' do
- let(:no_expire) { create(:project_member) }
- let(:expire_later) { create(:project_member, expires_at: Time.current + 6.days) }
- let(:expired) { create(:project_member, expires_at: Time.current - 6.days) }
+ let_it_be(:no_expire) { create(:project_member) }
+ let_it_be(:expire_later) { create(:project_member, expires_at: 8.days.from_now) }
+ let_it_be(:expired) { create(:project_member, expires_at: 1.day.from_now) }
+
+ before do
+ travel_to(3.days.from_now)
+ end
describe '.expired' do
it { expect(ProjectMember.expired).to match_array([expired]) }
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index 4323263855e..d1b81d99867 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -18,6 +18,13 @@ RSpec.describe Member do
it { is_expected.to validate_presence_of(:source) }
it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.all_values) }
+ context 'expires_at' do
+ it { is_expected.not_to allow_value(Date.yesterday).for(:expires_at) }
+ it { is_expected.to allow_value(Date.tomorrow).for(:expires_at) }
+ it { is_expected.to allow_value(Date.today).for(:expires_at) }
+ it { is_expected.to allow_value(nil).for(:expires_at) }
+ end
+
it_behaves_like 'an object with email-formated attributes', :invite_email do
subject { build(:project_member) }
end
diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb
index f25f8933184..388d04c8012 100644
--- a/spec/models/members/project_member_spec.rb
+++ b/spec/models/members/project_member_spec.rb
@@ -44,8 +44,9 @@ RSpec.describe ProjectMember do
let(:maintainer) { create(:project_member, project: project) }
it "creates an expired event when left due to expiry" do
- expired = create(:project_member, project: project, expires_at: Time.current - 6.days)
- expired.destroy
+ expired = create(:project_member, project: project, expires_at: 1.day.from_now)
+ travel_to(2.days.from_now) { expired.destroy }
+
expect(Event.recent.first).to be_expired_action
end
diff --git a/spec/requests/api/api_spec.rb b/spec/requests/api/api_spec.rb
index bd0426601db..7d637757f38 100644
--- a/spec/requests/api/api_spec.rb
+++ b/spec/requests/api/api_spec.rb
@@ -67,4 +67,29 @@ RSpec.describe API::API do
end
end
end
+
+ describe 'authentication with deploy token' do
+ context 'admin mode' do
+ let_it_be(:project) { create(:project, :public) }
+ let_it_be(:package) { create(:maven_package, project: project, name: project.full_path) }
+ let_it_be(:maven_metadatum) { package.maven_metadatum }
+ let_it_be(:package_file) { package.package_files.first }
+ let_it_be(:deploy_token) { create(:deploy_token) }
+ let(:headers_with_deploy_token) do
+ {
+ Gitlab::Auth::AuthFinders::DEPLOY_TOKEN_HEADER => deploy_token.token
+ }
+ end
+
+ it 'does not bypass the session' do
+ expect(Gitlab::Auth::CurrentUserMode).not_to receive(:bypass_session!)
+
+ get(api("/packages/maven/#{maven_metadatum.path}/#{package_file.file_name}"),
+ headers: headers_with_deploy_token)
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(response.media_type).to eq('application/octet-stream')
+ end
+ end
+ end
end
diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb
index b50f63ed67c..91877776190 100644
--- a/spec/requests/api/files_spec.rb
+++ b/spec/requests/api/files_spec.rb
@@ -502,16 +502,13 @@ RSpec.describe API::Files do
expect(response).to have_gitlab_http_status(:ok)
end
- it 'sets no-cache headers' do
- url = route('.gitignore') + "/raw"
- expect(Gitlab::Workhorse).to receive(:send_git_blob)
-
- get api(url, current_user), params: params
+ it_behaves_like 'uncached response' do
+ before do
+ url = route('.gitignore') + "/raw"
+ expect(Gitlab::Workhorse).to receive(:send_git_blob)
- expect(response.headers["Cache-Control"]).to include("no-store")
- expect(response.headers["Cache-Control"]).to include("no-cache")
- expect(response.headers["Pragma"]).to eq("no-cache")
- expect(response.headers["Expires"]).to eq("Fri, 01 Jan 1990 00:00:00 GMT")
+ get api(url, current_user), params: params
+ end
end
context 'when mandatory params are not given' do
diff --git a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb
index 2459a6f3828..2d4ed3ff060 100644
--- a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb
+++ b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb
@@ -9,13 +9,9 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Delete do
let_it_be(:project) { create(:project, :private, :repository) }
let_it_be(:environment) { create(:environment, project: project) }
let_it_be(:annotation) { create(:metrics_dashboard_annotation, environment: environment) }
- let(:mutation) do
- variables = {
- id: GitlabSchema.id_from_object(annotation).to_s
- }
- graphql_mutation(:delete_annotation, variables)
- end
+ let(:variables) { { id: GitlabSchema.id_from_object(annotation).to_s } }
+ let(:mutation) { graphql_mutation(:delete_annotation, variables) }
def mutation_response
graphql_mutation_response(:delete_annotation)
@@ -37,15 +33,11 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Delete do
end
context 'with invalid params' do
- let(:mutation) do
- variables = {
- id: 'invalid_id'
- }
+ let(:variables) { { id: GitlabSchema.id_from_object(project).to_s } }
- graphql_mutation(:delete_annotation, variables)
+ it_behaves_like 'a mutation that returns top-level errors' do
+ let(:match_errors) { eq(["#{variables[:id]} is not a valid id for #{annotation.class}."]) }
end
-
- it_behaves_like 'a mutation that returns top-level errors', errors: ['invalid_id is not a valid GitLab id.']
end
context 'when the delete fails' do
diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb
index 23889912d7a..549853db88c 100644
--- a/spec/requests/api/members_spec.rb
+++ b/spec/requests/api/members_spec.rb
@@ -244,13 +244,12 @@ RSpec.describe API::Members do
it 'creates a new member' do
expect do
post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
- params: { user_id: stranger.id, access_level: Member::DEVELOPER, expires_at: '2016-08-05' }
+ params: { user_id: stranger.id, access_level: Member::DEVELOPER }
expect(response).to have_gitlab_http_status(:created)
end.to change { source.members.count }.by(1)
expect(json_response['id']).to eq(stranger.id)
expect(json_response['access_level']).to eq(Member::DEVELOPER)
- expect(json_response['expires_at']).to eq('2016-08-05')
end
end
@@ -285,6 +284,40 @@ RSpec.describe API::Members do
end
end
+ context 'access expiry date' do
+ subject do
+ post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
+ params: { user_id: stranger.id, access_level: Member::DEVELOPER, expires_at: expires_at }
+ end
+
+ context 'when set to a date in the past' do
+ let(:expires_at) { 2.days.ago.to_date }
+
+ it 'does not create a member' do
+ expect do
+ subject
+ end.not_to change { source.members.count }
+
+ expect(response).to have_gitlab_http_status(:bad_request)
+ expect(json_response['message']).to eq({ 'expires_at' => ['cannot be a date in the past'] })
+ end
+ end
+
+ context 'when set to a date in the future' do
+ let(:expires_at) { 2.days.from_now.to_date }
+
+ it 'creates a member' do
+ expect do
+ subject
+ end.to change { source.members.count }.by(1)
+
+ expect(response).to have_gitlab_http_status(:created)
+ expect(json_response['id']).to eq(stranger.id)
+ expect(json_response['expires_at']).to eq(expires_at.to_s)
+ end
+ end
+ end
+
it "returns 409 if member already exists" do
post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
params: { user_id: maintainer.id, access_level: Member::MAINTAINER }
@@ -369,12 +402,40 @@ RSpec.describe API::Members do
context 'when authenticated as a maintainer/owner' do
it 'updates the member' do
put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer),
- params: { access_level: Member::MAINTAINER, expires_at: '2016-08-05' }
+ params: { access_level: Member::MAINTAINER }
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['id']).to eq(developer.id)
expect(json_response['access_level']).to eq(Member::MAINTAINER)
- expect(json_response['expires_at']).to eq('2016-08-05')
+ end
+ end
+
+ context 'access expiry date' do
+ subject do
+ put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer),
+ params: { expires_at: expires_at, access_level: Member::MAINTAINER }
+ end
+
+ context 'when set to a date in the past' do
+ let(:expires_at) { 2.days.ago.to_date }
+
+ it 'does not update the member' do
+ subject
+
+ expect(response).to have_gitlab_http_status(:bad_request)
+ expect(json_response['message']).to eq({ 'expires_at' => ['cannot be a date in the past'] })
+ end
+ end
+
+ context 'when set to a date in the future' do
+ let(:expires_at) { 2.days.from_now.to_date }
+
+ it 'updates the member' do
+ subject
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(json_response['expires_at']).to eq(expires_at.to_s)
+ end
end
end
diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb
index fdf2326b75e..0a9e2fd1bcb 100644
--- a/spec/services/issues/create_service_spec.rb
+++ b/spec/services/issues/create_service_spec.rb
@@ -25,6 +25,7 @@ RSpec.describe Issues::CreateService do
assignee_ids: [assignee.id],
label_ids: labels.map(&:id),
milestone_id: milestone.id,
+ milestone: milestone,
due_date: Date.tomorrow }
end
@@ -59,6 +60,12 @@ RSpec.describe Issues::CreateService do
expect(issue.milestone).to be_nil
expect(issue.due_date).to be_nil
end
+
+ it 'creates confidential issues' do
+ issue = described_class.new(project, guest, confidential: true).execute
+
+ expect(issue.confidential).to be_truthy
+ end
end
it 'creates a pending todo for new assignee' do
diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index 77bd540e22f..b115879a729 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -10,6 +10,7 @@ RSpec.describe Issues::UpdateService, :mailer do
let_it_be(:project, reload: true) { create(:project, :repository, group: group) }
let_it_be(:label) { create(:label, project: project) }
let_it_be(:label2) { create(:label, project: project) }
+ let_it_be(:milestone) { create(:milestone, project: project) }
let(:issue) do
create(:issue, title: 'Old title',
@@ -52,7 +53,8 @@ RSpec.describe Issues::UpdateService, :mailer do
state_event: 'close',
label_ids: [label.id],
due_date: Date.tomorrow,
- discussion_locked: true
+ discussion_locked: true,
+ milestone_id: milestone.id
}
end
@@ -69,6 +71,14 @@ RSpec.describe Issues::UpdateService, :mailer do
expect(issue.labels).to match_array [label]
expect(issue.due_date).to eq Date.tomorrow
expect(issue.discussion_locked).to be_truthy
+ expect(issue.confidential).to be_falsey
+ expect(issue.milestone).to eq milestone
+ end
+
+ it 'updates issue milestone when passing `milestone` param' do
+ update_issue(milestone: milestone)
+
+ expect(issue.milestone).to eq milestone
end
it 'refreshes the number of open issues when the issue is made confidential', :use_clean_rails_memory_store_caching do
@@ -82,6 +92,8 @@ RSpec.describe Issues::UpdateService, :mailer do
expect(TodosDestroyer::ConfidentialIssueWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, issue.id)
update_issue(confidential: true)
+
+ expect(issue.confidential).to be_truthy
end
it 'does not enqueue ConfidentialIssueWorker when an issue is made non confidential' do
@@ -91,6 +103,8 @@ RSpec.describe Issues::UpdateService, :mailer do
expect(TodosDestroyer::ConfidentialIssueWorker).not_to receive(:perform_in)
update_issue(confidential: false)
+
+ expect(issue.confidential).to be_falsey
end
it 'updates open issue counter for assignees when issue is reassigned' do
@@ -157,7 +171,7 @@ RSpec.describe Issues::UpdateService, :mailer do
end
it 'filters out params that cannot be set without the :admin_issue permission' do
- described_class.new(project, guest, opts).execute(issue)
+ described_class.new(project, guest, opts.merge(confidential: true)).execute(issue)
expect(issue).to be_valid
expect(issue.title).to eq 'New title'
@@ -167,6 +181,7 @@ RSpec.describe Issues::UpdateService, :mailer do
expect(issue.milestone).to be_nil
expect(issue.due_date).to be_nil
expect(issue.discussion_locked).to be_falsey
+ expect(issue.confidential).to be_falsey
end
end
diff --git a/spec/services/members/update_service_spec.rb b/spec/services/members/update_service_spec.rb
index f510916558b..2d4457f3f62 100644
--- a/spec/services/members/update_service_spec.rb
+++ b/spec/services/members/update_service_spec.rb
@@ -31,17 +31,35 @@ RSpec.describe Members::UpdateService do
end
context 'when member is downgraded to guest' do
- let(:params) do
- { access_level: Gitlab::Access::GUEST }
+ shared_examples 'schedules to delete confidential todos' do
+ it do
+ expect(TodosDestroyer::EntityLeaveWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, source.class.name).once
+
+ updated_member = described_class.new(current_user, params).execute(member, permission: permission)
+
+ expect(updated_member).to be_valid
+ expect(updated_member.access_level).to eq(Gitlab::Access::GUEST)
+ end
+ end
+
+ context 'with Gitlab::Access::GUEST level as a string' do
+ let(:params) { { access_level: Gitlab::Access::GUEST.to_s } }
+
+ it_behaves_like 'schedules to delete confidential todos'
end
- it 'schedules to delete confidential todos' do
- expect(TodosDestroyer::EntityLeaveWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, source.class.name).once
+ context 'with Gitlab::Access::GUEST level as an integer' do
+ let(:params) { { access_level: Gitlab::Access::GUEST } }
+
+ it_behaves_like 'schedules to delete confidential todos'
+ end
+ end
- updated_member = described_class.new(current_user, params).execute(member, permission: permission)
+ context 'when access_level is invalid' do
+ let(:params) { { access_level: 'invalid' } }
- expect(updated_member).to be_valid
- expect(updated_member.access_level).to eq(Gitlab::Access::GUEST)
+ it 'raises an error' do
+ expect { described_class.new(current_user, params) }.to raise_error(ArgumentError, 'invalid value for Integer(): "invalid"')
end
end
end
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index c3433c8c9d2..5cbfb92263f 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -6,12 +6,13 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
include ProjectForksHelper
let(:group) { create(:group, :public) }
- let(:project) { create(:project, :repository, group: group) }
+ let(:project) { create(:project, :private, :repository, group: group) }
let(:user) { create(:user) }
let(:user2) { create(:user) }
let(:user3) { create(:user) }
let(:label) { create(:label, project: project) }
let(:label2) { create(:label) }
+ let(:milestone) { create(:milestone, project: project) }
let(:merge_request) do
create(:merge_request, :simple, title: 'Old title',
@@ -60,7 +61,8 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
}
end
- let(:service) { described_class.new(project, user, opts) }
+ let(:service) { described_class.new(project, current_user, opts) }
+ let(:current_user) { user }
before do
allow(service).to receive(:execute_hooks)
@@ -83,6 +85,26 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
expect(@merge_request.discussion_locked).to be_truthy
end
+ context 'updating milestone' do
+ RSpec.shared_examples 'updates milestone' do
+ it 'sets milestone' do
+ expect(@merge_request.milestone).to eq milestone
+ end
+ end
+
+ context 'when milestone_id param' do
+ let(:opts) { { milestone_id: milestone.id } }
+
+ it_behaves_like 'updates milestone'
+ end
+
+ context 'when milestone param' do
+ let(:opts) { { milestone: milestone } }
+
+ it_behaves_like 'updates milestone'
+ end
+ end
+
it 'executes hooks with update action' do
expect(service).to have_received(:execute_hooks)
.with(
@@ -150,6 +172,46 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
expect(note.note).to eq 'locked this merge request'
end
+ context 'when current user cannot admin issues in the project' do
+ let(:guest) { create(:user) }
+ let(:current_user) { guest }
+
+ before do
+ project.add_guest(guest)
+ end
+
+ it 'filters out params that cannot be set without the :admin_merge_request permission' do
+ expect(@merge_request).to be_valid
+ expect(@merge_request.title).to eq('New title')
+ expect(@merge_request.assignees).to match_array([user3])
+ expect(@merge_request).to be_opened
+ expect(@merge_request.labels.count).to eq(0)
+ expect(@merge_request.target_branch).to eq('target')
+ expect(@merge_request.discussion_locked).to be_falsey
+ expect(@merge_request.milestone).to be_nil
+ end
+
+ context 'updating milestone' do
+ RSpec.shared_examples 'does not update milestone' do
+ it 'sets milestone' do
+ expect(@merge_request.milestone).to be_nil
+ end
+ end
+
+ context 'when milestone_id param' do
+ let(:opts) { { milestone_id: milestone.id } }
+
+ it_behaves_like 'does not update milestone'
+ end
+
+ context 'when milestone param' do
+ let(:opts) { { milestone: milestone } }
+
+ it_behaves_like 'does not update milestone'
+ end
+ end
+ end
+
context 'when not including source branch removal options' do
before do
opts.delete(:force_remove_source_branch)
diff --git a/spec/services/todos/destroy/entity_leave_service_spec.rb b/spec/services/todos/destroy/entity_leave_service_spec.rb
index ccafe3bb7a8..921037bd5db 100644
--- a/spec/services/todos/destroy/entity_leave_service_spec.rb
+++ b/spec/services/todos/destroy/entity_leave_service_spec.rb
@@ -3,77 +3,145 @@
require 'spec_helper'
RSpec.describe Todos::Destroy::EntityLeaveService do
+ let_it_be(:user, reload: true) { create(:user) }
+ let_it_be(:user2, reload: true) { create(:user) }
+
let(:group) { create(:group, :private) }
- let(:project) { create(:project, group: group) }
- let(:user) { create(:user) }
- let(:user2) { create(:user) }
- let(:issue) { create(:issue, project: project, confidential: true) }
- let(:mr) { create(:merge_request, source_project: project) }
-
- let!(:todo_mr_user) { create(:todo, user: user, target: mr, project: project) }
- let!(:todo_issue_user) { create(:todo, user: user, target: issue, project: project) }
- let!(:todo_group_user) { create(:todo, user: user, group: group) }
- let!(:todo_issue_user2) { create(:todo, user: user2, target: issue, project: project) }
- let!(:todo_group_user2) { create(:todo, user: user2, group: group) }
+ let(:project) { create(:project, :private, group: group) }
+ let(:issue) { create(:issue, project: project) }
+ let(:issue_c) { create(:issue, project: project, confidential: true) }
+ let!(:todo_group_user) { create(:todo, user: user, group: group) }
+ let!(:todo_group_user2) { create(:todo, user: user2, group: group) }
+
+ let(:mr) { create(:merge_request, source_project: project) }
+ let!(:todo_mr_user) { create(:todo, user: user, target: mr, project: project) }
+ let!(:todo_issue_user) { create(:todo, user: user, target: issue, project: project) }
+ let!(:todo_issue_c_user) { create(:todo, user: user, target: issue_c, project: project) }
+ let!(:todo_issue_c_user2) { create(:todo, user: user2, target: issue_c, project: project) }
+
+ shared_examples 'using different access permissions' do |access_table|
+ using RSpec::Parameterized::TableSyntax
+
+ where(:group_access, :project_access, :c_todos, :mr_todos, :method, &access_table)
+
+ with_them do
+ before do
+ set_access(project, user, project_access) if project_access
+ set_access(group, user, group_access) if group_access
+ end
+
+ it "#{params[:method].to_s.humanize(capitalize: false)}" do
+ send(method)
+ end
+ end
+ end
+
+ shared_examples 'does not remove any todos' do
+ it { does_not_remove_any_todos }
+ end
+
+ shared_examples 'removes only confidential issues todos' do
+ it { removes_only_confidential_issues_todos }
+ end
+
+ def does_not_remove_any_todos
+ expect { subject }.not_to change { Todo.count }
+ end
+
+ def removes_only_confidential_issues_todos
+ expect { subject }.to change { Todo.count }.from(6).to(5)
+ end
+
+ def removes_confidential_issues_and_merge_request_todos
+ expect { subject }.to change { Todo.count }.from(6).to(4)
+ expect(user.todos).to match_array([todo_issue_user, todo_group_user])
+ end
+
+ def set_access(object, user, access_name)
+ case access_name
+ when :developer
+ object.add_developer(user)
+ when :reporter
+ object.add_reporter(user)
+ when :guest
+ object.add_guest(user)
+ end
+ end
describe '#execute' do
- context 'when a user leaves a project' do
+ describe 'updating a Project' do
subject { described_class.new(user.id, project.id, 'Project').execute }
+ # a private project in a private group is valid
context 'when project is private' do
- it 'removes project todos for the provided user' do
- expect { subject }.to change { Todo.count }.from(5).to(3)
-
- expect(user.todos).to match_array([todo_group_user])
- expect(user2.todos).to match_array([todo_issue_user2, todo_group_user2])
- end
+ context 'when user is not a member of the project' do
+ it 'removes project todos for the provided user' do
+ expect { subject }.to change { Todo.count }.from(6).to(3)
- context 'when the user is member of the project' do
- before do
- project.add_developer(user)
- end
-
- it 'does not remove any todos' do
- expect { subject }.not_to change { Todo.count }
+ expect(user.todos).to match_array([todo_group_user])
+ expect(user2.todos).to match_array([todo_issue_c_user2, todo_group_user2])
end
end
- context 'when the user is a project guest' do
- before do
- project.add_guest(user)
- end
+ context 'access permissions' do
+ # rubocop:disable RSpec/LeakyConstantDeclaration
+ PRIVATE_PROJECT_PRIVATE_GROUP_ACCESS_TABLE =
+ lambda do |_|
+ [
+ # :group_access, :project_access, :c_todos, :mr_todos, :method
+ [nil, :reporter, :keep, :keep, :does_not_remove_any_todos],
+ [nil, :guest, :delete, :delete, :removes_confidential_issues_and_merge_request_todos],
+ [:reporter, nil, :keep, :keep, :does_not_remove_any_todos],
+ [:guest, nil, :delete, :delete, :removes_confidential_issues_and_merge_request_todos],
+ [:guest, :reporter, :keep, :keep, :does_not_remove_any_todos],
+ [:guest, :guest, :delete, :delete, :removes_confidential_issues_and_merge_request_todos]
+ ]
+ end
+ # rubocop:enable RSpec/LeakyConstantDeclaration
- it 'removes only confidential issues todos' do
- expect { subject }.to change { Todo.count }.from(5).to(4)
- end
+ it_behaves_like 'using different access permissions', PRIVATE_PROJECT_PRIVATE_GROUP_ACCESS_TABLE
end
+ end
- context 'when the user is member of a parent group' do
- before do
- group.add_developer(user)
- end
+ # a private project in an internal/public group is valid
+ context 'when project is private in an internal/public group' do
+ let(:group) { create(:group, :internal) }
+
+ context 'when user is not a member of the project' do
+ it 'removes project todos for the provided user' do
+ expect { subject }.to change { Todo.count }.from(6).to(3)
- it 'does not remove any todos' do
- expect { subject }.not_to change { Todo.count }
+ expect(user.todos).to match_array([todo_group_user])
+ expect(user2.todos).to match_array([todo_issue_c_user2, todo_group_user2])
end
end
- context 'when the user is guest of a parent group' do
- before do
- project.add_guest(user)
- end
+ context 'access permissions' do
+ # rubocop:disable RSpec/LeakyConstantDeclaration
+ PRIVATE_PROJECT_INTERNAL_GROUP_ACCESS_TABLE =
+ lambda do |_|
+ [
+ # :group_access, :project_access, :c_todos, :mr_todos, :method
+ [nil, :reporter, :keep, :keep, :does_not_remove_any_todos],
+ [nil, :guest, :delete, :delete, :removes_confidential_issues_and_merge_request_todos],
+ [:reporter, nil, :keep, :keep, :does_not_remove_any_todos],
+ [:guest, nil, :delete, :delete, :removes_confidential_issues_and_merge_request_todos],
+ [:guest, :reporter, :keep, :keep, :does_not_remove_any_todos],
+ [:guest, :guest, :delete, :delete, :removes_confidential_issues_and_merge_request_todos]
+ ]
+ end
+ # rubocop:enable RSpec/LeakyConstantDeclaration
- it 'removes only confidential issues todos' do
- expect { subject }.to change { Todo.count }.from(5).to(4)
- end
+ it_behaves_like 'using different access permissions', PRIVATE_PROJECT_INTERNAL_GROUP_ACCESS_TABLE
end
end
+ # an internal project in an internal/public group is valid
context 'when project is not private' do
- before do
- group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
- project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
- end
+ let(:group) { create(:group, :internal) }
+ let(:project) { create(:project, :internal, group: group) }
+ let(:issue) { create(:issue, project: project) }
+ let(:issue_c) { create(:issue, project: project, confidential: true) }
it 'enqueues the PrivateFeaturesWorker' do
expect(TodosDestroyer::PrivateFeaturesWorker)
@@ -84,50 +152,42 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
context 'confidential issues' do
context 'when a user is not an author of confidential issue' do
- it 'removes only confidential issues todos' do
- expect { subject }.to change { Todo.count }.from(5).to(4)
- end
+ it_behaves_like 'removes only confidential issues todos'
end
context 'when a user is an author of confidential issue' do
before do
- issue.update!(author: user)
+ issue_c.update!(author: user)
end
- it 'does not remove any todos' do
- expect { subject }.not_to change { Todo.count }
- end
+ it_behaves_like 'does not remove any todos'
end
context 'when a user is an assignee of confidential issue' do
before do
- issue.assignees << user
+ issue_c.assignees << user
end
- it 'does not remove any todos' do
- expect { subject }.not_to change { Todo.count }
- end
+ it_behaves_like 'does not remove any todos'
end
- context 'when a user is a project guest' do
- before do
- project.add_guest(user)
- end
-
- it 'removes only confidential issues todos' do
- expect { subject }.to change { Todo.count }.from(5).to(4)
- end
- end
-
- context 'when a user is a project guest but group developer' do
- before do
- project.add_guest(user)
- group.add_developer(user)
- end
-
- it 'does not remove any todos' do
- expect { subject }.not_to change { Todo.count }
- end
+ context 'access permissions' do
+ # rubocop:disable RSpec/LeakyConstantDeclaration
+ INTERNAL_PROJECT_INTERNAL_GROUP_ACCESS_TABLE =
+ lambda do |_|
+ [
+ # :group_access, :project_access, :c_todos, :mr_todos, :method
+ [nil, :reporter, :keep, :keep, :does_not_remove_any_todos],
+ [nil, :guest, :delete, :keep, :removes_only_confidential_issues_todos],
+ [:reporter, nil, :keep, :keep, :does_not_remove_any_todos],
+ [:guest, nil, :delete, :keep, :removes_only_confidential_issues_todos],
+ [:guest, :reporter, :keep, :keep, :does_not_remove_any_todos],
+ [:guest, :guest, :delete, :keep, :removes_only_confidential_issues_todos]
+ ]
+ end
+ # rubocop:enable RSpec/LeakyConstantDeclaration
+
+ it_behaves_like 'using different access permissions', INTERNAL_PROJECT_INTERNAL_GROUP_ACCESS_TABLE
end
end
@@ -138,42 +198,43 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
end
it 'removes only users issue todos' do
- expect { subject }.to change { Todo.count }.from(5).to(4)
+ expect { subject }.to change { Todo.count }.from(6).to(5)
end
end
end
end
end
- context 'when a user leaves a group' do
+ describe 'updating a Group' do
subject { described_class.new(user.id, group.id, 'Group').execute }
context 'when group is private' do
- it 'removes group and subproject todos for the user' do
- expect { subject }.to change { Todo.count }.from(5).to(2)
-
- expect(user.todos).to be_empty
- expect(user2.todos).to match_array([todo_issue_user2, todo_group_user2])
- end
-
- context 'when the user is member of the group' do
- before do
- group.add_developer(user)
- end
+ context 'when a user leaves a group' do
+ it 'removes group and subproject todos for the user' do
+ expect { subject }.to change { Todo.count }.from(6).to(2)
- it 'does not remove any todos' do
- expect { subject }.not_to change { Todo.count }
+ expect(user.todos).to be_empty
+ expect(user2.todos).to match_array([todo_issue_c_user2, todo_group_user2])
end
end
- context 'when the user is member of the group project but not the group' do
- before do
- project.add_developer(user)
- end
+ context 'access permissions' do
+ # rubocop:disable RSpec/LeakyConstantDeclaration
+ PRIVATE_GROUP_PRIVATE_PROJECT_ACCESS_TABLE =
+ lambda do |_|
+ [
+ # :group_access, :project_access, :c_todos, :mr_todos, :method
+ [nil, :reporter, :keep, :keep, :does_not_remove_any_todos],
+ [nil, :guest, :delete, :delete, :removes_confidential_issues_and_merge_request_todos],
+ [:reporter, nil, :keep, :keep, :does_not_remove_any_todos],
+ [:guest, nil, :delete, :delete, :removes_confidential_issues_and_merge_request_todos],
+ [:guest, :reporter, :keep, :keep, :does_not_remove_any_todos],
+ [:guest, :guest, :delete, :delete, :removes_confidential_issues_and_merge_request_todos]
+ ]
+ end
+ # rubocop:enable RSpec/LeakyConstantDeclaration
- it 'does not remove any todos' do
- expect { subject }.not_to change { Todo.count }
- end
+ it_behaves_like 'using different access permissions', PRIVATE_GROUP_PRIVATE_PROJECT_ACCESS_TABLE
end
context 'with nested groups' do
@@ -191,12 +252,12 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
context 'when the user is not a member of any groups/projects' do
it 'removes todos for the user including subprojects todos' do
- expect { subject }.to change { Todo.count }.from(11).to(4)
+ expect { subject }.to change { Todo.count }.from(12).to(4)
expect(user.todos).to be_empty
expect(user2.todos)
.to match_array(
- [todo_issue_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2]
+ [todo_issue_c_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2]
)
end
end
@@ -208,9 +269,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
parent_group.add_developer(user)
end
- it 'does not remove any todos' do
- expect { subject }.not_to change { Todo.count }
- end
+ it_behaves_like 'does not remove any todos'
end
context 'when the user is member of a subgroup' do
@@ -219,12 +278,12 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
end
it 'does not remove group and subproject todos' do
- expect { subject }.to change { Todo.count }.from(11).to(7)
+ expect { subject }.to change { Todo.count }.from(12).to(7)
expect(user.todos).to match_array([todo_group_user, todo_subgroup_user, todo_subproject_user])
expect(user2.todos)
.to match_array(
- [todo_issue_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2]
+ [todo_issue_c_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2]
)
end
end
@@ -235,12 +294,12 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
end
it 'does not remove subproject and group todos' do
- expect { subject }.to change { Todo.count }.from(11).to(7)
+ expect { subject }.to change { Todo.count }.from(12).to(7)
expect(user.todos).to match_array([todo_subgroup_user, todo_group_user, todo_subproject_user])
expect(user2.todos)
.to match_array(
- [todo_issue_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2]
+ [todo_issue_c_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2]
)
end
end
@@ -248,10 +307,10 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
end
context 'when group is not private' do
- before do
- group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
- project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
- end
+ let(:group) { create(:group, :internal) }
+ let(:project) { create(:project, :internal, group: group) }
+ let(:issue) { create(:issue, project: project) }
+ let(:issue_c) { create(:issue, project: project, confidential: true) }
it 'enqueues the PrivateFeaturesWorker' do
expect(TodosDestroyer::PrivateFeaturesWorker)
@@ -260,31 +319,24 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
subject
end
- context 'when user is not member' do
- it 'removes only confidential issues todos' do
- expect { subject }.to change { Todo.count }.from(5).to(4)
- end
- end
-
- context 'when user is a project guest' do
- before do
- project.add_guest(user)
- end
-
- it 'removes only confidential issues todos' do
- expect { subject }.to change { Todo.count }.from(5).to(4)
- end
- end
-
- context 'when user is a project guest & group developer' do
- before do
- project.add_guest(user)
- group.add_developer(user)
- end
+ context 'access permissions' do
+ # rubocop:disable RSpec/LeakyConstantDeclaration
+ INTERNAL_GROUP_INTERNAL_PROJECT_ACCESS_TABLE =
+ lambda do |_|
+ [
+ # :group_access, :project_access, :c_todos, :mr_todos, :method
+ [nil, nil, :delete, :keep, :removes_only_confidential_issues_todos],
+ [nil, :reporter, :keep, :keep, :does_not_remove_any_todos],
+ [nil, :guest, :delete, :keep, :removes_only_confidential_issues_todos],
+ [:reporter, nil, :keep, :keep, :does_not_remove_any_todos],
+ [:guest, nil, :delete, :keep, :removes_only_confidential_issues_todos],
+ [:guest, :reporter, :keep, :keep, :does_not_remove_any_todos],
+ [:guest, :guest, :delete, :keep, :removes_only_confidential_issues_todos]
+ ]
+ end
+ # rubocop:enable RSpec/LeakyConstantDeclaration
- it 'does not remove any todos' do
- expect { subject }.not_to change { Todo.count }
- end
+ it_behaves_like 'using different access permissions', INTERNAL_GROUP_INTERNAL_PROJECT_ACCESS_TABLE
end
end
end
diff --git a/spec/support/shared_examples/cached_response_shared_examples.rb b/spec/support/shared_examples/cached_response_shared_examples.rb
new file mode 100644
index 00000000000..34e5f741b4e
--- /dev/null
+++ b/spec/support/shared_examples/cached_response_shared_examples.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+#
+# Negates lib/gitlab/no_cache_headers.rb
+#
+
+RSpec.shared_examples 'cached response' do
+ it 'defines a cached header response' do
+ expect(response.headers["Cache-Control"]).not_to include("no-store", "no-cache")
+ expect(response.headers["Pragma"]).not_to eq("no-cache")
+ expect(response.headers["Expires"]).not_to eq("Fri, 01 Jan 1990 00:00:00 GMT")
+ end
+end
diff --git a/spec/validators/future_date_validator_spec.rb b/spec/validators/future_date_validator_spec.rb
new file mode 100644
index 00000000000..6814ba7c820
--- /dev/null
+++ b/spec/validators/future_date_validator_spec.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe FutureDateValidator do
+ subject do
+ Class.new do
+ include ActiveModel::Model
+ include ActiveModel::Validations
+ attr_accessor :expires_at
+ validates :expires_at, future_date: true
+ end.new
+ end
+
+ before do
+ subject.expires_at = date
+ end
+
+ context 'past date' do
+ let(:date) { Date.yesterday }
+
+ it { is_expected.not_to be_valid }
+ end
+
+ context 'current date' do
+ let(:date) { Date.today }
+
+ it { is_expected.to be_valid }
+ end
+
+ context 'future date' do
+ let(:date) { Date.tomorrow }
+
+ it { is_expected.to be_valid }
+ end
+end
diff --git a/spec/workers/remove_expired_members_worker_spec.rb b/spec/workers/remove_expired_members_worker_spec.rb
index cbdd5a68698..8a34b41834b 100644
--- a/spec/workers/remove_expired_members_worker_spec.rb
+++ b/spec/workers/remove_expired_members_worker_spec.rb
@@ -7,9 +7,13 @@ RSpec.describe RemoveExpiredMembersWorker do
describe '#perform' do
context 'project members' do
- let!(:expired_project_member) { create(:project_member, expires_at: 1.hour.ago, access_level: GroupMember::DEVELOPER) }
- let!(:project_member_expiring_in_future) { create(:project_member, expires_at: 10.days.from_now, access_level: GroupMember::DEVELOPER) }
- let!(:non_expiring_project_member) { create(:project_member, expires_at: nil, access_level: GroupMember::DEVELOPER) }
+ let_it_be(:expired_project_member) { create(:project_member, expires_at: 1.day.from_now, access_level: GroupMember::DEVELOPER) }
+ let_it_be(:project_member_expiring_in_future) { create(:project_member, expires_at: 10.days.from_now, access_level: GroupMember::DEVELOPER) }
+ let_it_be(:non_expiring_project_member) { create(:project_member, expires_at: nil, access_level: GroupMember::DEVELOPER) }
+
+ before do
+ travel_to(3.days.from_now)
+ end
it 'removes expired members' do
expect { worker.perform }.to change { Member.count }.by(-1)
@@ -28,9 +32,13 @@ RSpec.describe RemoveExpiredMembersWorker do
end
context 'group members' do
- let!(:expired_group_member) { create(:group_member, expires_at: 1.hour.ago, access_level: GroupMember::DEVELOPER) }
- let!(:group_member_expiring_in_future) { create(:group_member, expires_at: 10.days.from_now, access_level: GroupMember::DEVELOPER) }
- let!(:non_expiring_group_member) { create(:group_member, expires_at: nil, access_level: GroupMember::DEVELOPER) }
+ let_it_be(:expired_group_member) { create(:group_member, expires_at: 1.day.from_now, access_level: GroupMember::DEVELOPER) }
+ let_it_be(:group_member_expiring_in_future) { create(:group_member, expires_at: 10.days.from_now, access_level: GroupMember::DEVELOPER) }
+ let_it_be(:non_expiring_group_member) { create(:group_member, expires_at: nil, access_level: GroupMember::DEVELOPER) }
+
+ before do
+ travel_to(3.days.from_now)
+ end
it 'removes expired members' do
expect { worker.perform }.to change { Member.count }.by(-1)
@@ -49,7 +57,11 @@ RSpec.describe RemoveExpiredMembersWorker do
end
context 'when the last group owner expires' do
- let!(:expired_group_owner) { create(:group_member, expires_at: 1.hour.ago, access_level: GroupMember::OWNER) }
+ let_it_be(:expired_group_owner) { create(:group_member, expires_at: 1.day.from_now, access_level: GroupMember::OWNER) }
+
+ before do
+ travel_to(3.days.from_now)
+ end
it 'does not delete the owner' do
worker.perform
diff --git a/spec/workers/remove_unaccepted_member_invites_worker_spec.rb b/spec/workers/remove_unaccepted_member_invites_worker_spec.rb
new file mode 100644
index 00000000000..96d7cf535ed
--- /dev/null
+++ b/spec/workers/remove_unaccepted_member_invites_worker_spec.rb
@@ -0,0 +1,76 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe RemoveUnacceptedMemberInvitesWorker do
+ let(:worker) { described_class.new }
+
+ describe '#perform' do
+ context 'unaccepted members' do
+ before do
+ stub_const("#{described_class}::EXPIRATION_THRESHOLD", 1.day)
+ end
+
+ it 'removes unaccepted members', :aggregate_failures do
+ unaccepted_group_invitee = create(
+ :group_member, invite_token: 't0ken',
+ invite_email: 'group_invitee@example.com',
+ user: nil,
+ created_at: Time.current - 5.days)
+ unaccepted_project_invitee = create(
+ :project_member, invite_token: 't0ken',
+ invite_email: 'project_invitee@example.com',
+ user: nil,
+ created_at: Time.current - 5.days)
+
+ expect { worker.perform }.to change { Member.count }.by(-2)
+
+ expect(Member.where(id: unaccepted_project_invitee.id)).not_to exist
+ expect(Member.where(id: unaccepted_group_invitee.id)).not_to exist
+ end
+ end
+
+ context 'invited members still within expiration threshold' do
+ it 'leaves invited members', :aggregate_failures do
+ group_invitee = create(
+ :group_member, invite_token: 't0ken',
+ invite_email: 'group_invitee@example.com',
+ user: nil)
+ project_invitee = create(
+ :project_member, invite_token: 't0ken',
+ invite_email: 'project_invitee@example.com',
+ user: nil)
+
+ expect { worker.perform }.not_to change { Member.count }
+
+ expect(Member.where(id: group_invitee.id)).to exist
+ expect(Member.where(id: project_invitee.id)).to exist
+ end
+ end
+
+ context 'accepted members' do
+ before do
+ stub_const("#{described_class}::EXPIRATION_THRESHOLD", 1.day)
+ end
+
+ it 'leaves accepted members', :aggregate_failures do
+ user = create(:user)
+ accepted_group_invitee = create(
+ :group_member, invite_token: 't0ken',
+ invite_email: 'group_invitee@example.com',
+ user: user,
+ created_at: Time.current - 5.days)
+ accepted_project_invitee = create(
+ :project_member, invite_token: nil,
+ invite_email: 'project_invitee@example.com',
+ user: user,
+ created_at: Time.current - 5.days)
+
+ expect { worker.perform }.not_to change { Member.count }
+
+ expect(Member.where(id: accepted_group_invitee.id)).to exist
+ expect(Member.where(id: accepted_project_invitee.id)).to exist
+ end
+ end
+ end
+end
diff --git a/vendor/gitignore/C++.gitignore b/vendor/gitignore/C++.gitignore
index 259148fa18f..259148fa18f 100644..100755
--- a/vendor/gitignore/C++.gitignore
+++ b/vendor/gitignore/C++.gitignore
diff --git a/vendor/gitignore/Java.gitignore b/vendor/gitignore/Java.gitignore
index a1c2a238a96..a1c2a238a96 100644..100755
--- a/vendor/gitignore/Java.gitignore
+++ b/vendor/gitignore/Java.gitignore