New upstream version 13.2.10

CHANGELOG.md

@@ -2,6 +2,34 @@
 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)
+
+- Fix ActiveRecord::IrreversibleOrderError during restore from backup. !40789
+- Update the 2FA user update check to account for rounding errors. !41327
+
+
 ## 13.2.8 (2020-09-02)
 
 ### Security (1 change)

GITALY_SERVER_VERSION

@@ -1 +1 @@
-13.2.8
+13.2.10

VERSION

@@ -1 +1 @@
-13.2.8
+13.2.10

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
 

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

app/controllers/concerns/authenticates_with_two_factor.rb

@@ -125,6 +125,10 @@ module AuthenticatesWithTwoFactor
   def user_changed?(user)
     return false unless session[:user_updated_at]
 
-    user.updated_at != session[:user_updated_at]
+    # See: https://gitlab.com/gitlab-org/gitlab/-/issues/244638
+    # Rounding errors happen when the user is updated, as the Rails ActiveRecord
+    # object has higher precision than what is stored in the database, therefore
+    # using .to_i to force truncation to the timestamp
+    user.updated_at.to_i != session[:user_updated_at].to_i
   end
 end

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|

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

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

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

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

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

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

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,34 +83,6 @@ class ActiveSession
     end
   end
 
-  def self.destroy(user, session_id)
-    return unless session_id
-
-    Gitlab::Redis::SharedState.with do |redis|
-      destroy_sessions(redis, user, [session_id])
-    end
-  end
-
-  def self.destroy_with_public_id(user, public_id)
-    decrypted_id = decrypt_public_id(public_id)
-
-    return if decrypted_id.nil?
-
-    session_id = Rack::Session::SessionId.new(decrypted_id)
-    destroy(user, session_id)
-  end
-
-  def self.destroy_sessions(redis, user, session_ids)
-    key_names = session_ids.map { |session_id| key_name(user.id, session_id.public_id) }
-
-    redis.srem(lookup_key_name(user.id), session_ids.map(&:public_id))
-
-    Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do
-      redis.del(key_names)
-      redis.del(rack_session_keys(session_ids))
-    end
-  end
-
   def self.cleanup(user)
     Gitlab::Redis::SharedState.with do |redis|
       clean_up_old_sessions(redis, user)
@@ -105,12 +90,52 @@ class ActiveSession
     end
   end
 
-  def self.destroy_all_but_current(user, current_session)
-    session_ids = not_impersonated(user)
-    session_ids.reject! { |session| session.current?(current_session) } if current_session
+  # 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
 
     Gitlab::Redis::SharedState.with do |redis|
-      destroy_sessions(redis, user, session_ids.map(&:session_id)) if session_ids.any?
+      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) }
+
+    redis.srem(lookup_key_name(user.id), session_ids)
+
+    Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do
+      redis.del(key_names)
+      redis.del(rack_session_keys(session_ids))
+    end
+  end
+
+  # 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|
+      destroy_sessions(redis, user, [session_id, decrypted_session_id, rack_session_private_id].compact)
+    end
+  end
+
+  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|
+      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
-
-    # 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

app/models/application_setting.rb

@@ -426,6 +426,8 @@ class ApplicationSetting < ApplicationRecord
   end
 
   def self.create_from_defaults
+    check_schema!
+
     transaction(requires_new: true) do
       super
     end
@@ -434,6 +436,22 @@ class ApplicationSetting < ApplicationRecord
     current_without_cache
   end
 
+  # Due to the frequency with which settings are accessed, it is
+  # likely that during a backup restore a running GitLab process
+  # will insert a new `application_settings` row before the
+  # constraints have been added to the table. This would add an
+  # extra row with ID 1 and prevent the primary key constraint from
+  # being added, which made ActiveRecord throw a
+  # IrreversibleOrderError anytime the settings were accessed
+  # (https://gitlab.com/gitlab-org/gitlab/-/issues/36405).  To
+  # prevent this from happening, we do a sanity check that the
+  # primary key constraint is present before inserting a new entry.
+  def self.check_schema!
+    return if ActiveRecord::Base.connection.primary_key(self.table_name).present?
+
+    raise "The `#{self.table_name}` table is missing a primary key constraint in the database schema"
+  end
+
   # By default, the backend is Rails.cache, which uses
   # ActiveSupport::Cache::RedisStore. Since loading ApplicationSetting
   # can cause a significant amount of load on Redis, let's cache it in

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'
 

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],

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 }

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)

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

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)

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)

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

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')

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: 

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

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'

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!
 
     ##

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

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

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

db/schema_migrations/20200916120837

@@ -0,0 +1 @@
+ff246eb2761c4504b67b7d7b197990a671626038e50f1b82d6b3e4739a1ec3d4

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
 \.
 

doc/.vale/gitlab/Acronyms.yml

@@ -61,6 +61,7 @@ exceptions:
   - RSA
   - RSS
   - SAML
+  - SCIM
   - SCP
   - SCSS
   - SHA

doc/administration/pages/index.md

@@ -409,7 +409,7 @@ pages:
 ### Using a custom Certificate Authority (CA)
 
 NOTE: **Note:**
-[Before 13.2](https://gitlab.com/gitlab-org/omnibus-gitlab/-/merge_requests/4289), when using Omnibus, a [workaround was required](https://docs.gitlab.com/13.1/ee/administration/pages/index.html#using-a-custom-certificate-authority-ca).
+[Before 13.3](https://gitlab.com/gitlab-org/omnibus-gitlab/-/merge_requests/4411), when using Omnibus, a [workaround was required](https://docs.gitlab.com/13.1/ee/administration/pages/index.html#using-a-custom-certificate-authority-ca).
 
 When using certificates issued by a custom CA, [Access Control](../../user/project/pages/pages_access_control.md#gitlab-pages-access-control) and
 the [online view of HTML job artifacts](../../ci/pipelines/job_artifacts.md#browsing-artifacts)

doc/api/protected_branches.md

@@ -248,7 +248,7 @@ Example response:
 ### Example with user / group level access **(STARTER)**
 
 Elements in the `allowed_to_push` / `allowed_to_merge` / `allowed_to_unprotect` array should take the
-form `{user_id: integer}`, `{group_id: integer}` or `{access_level: integer}`. Each user must have access to the project and each group must [have this project shared](../user/project/members/share_project_with_groups.md). These access levels allow [more granular control over protected branch access](../user/project/protected_branches.md#restricting-push-and-merge-access-to-certain-users-starter) and were [added to the API in](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/3516) GitLab 10.3 EE.
+form `{user_id: integer}`, `{group_id: integer}` or `{access_level: integer}`. Each user must have access to the project and each group must [have this project shared](../user/project/members/share_project_with_groups.md). These access levels allow [more granular control over protected branch access](../user/project/protected_branches.md#restricting-push-and-merge-access-to-certain-users-starter) and were [added to the API](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/3516) in GitLab 10.3 EE.
 
 ```shell
 curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/5/protected_branches?name=*-stable&allowed_to_push%5B%5D%5Buser_id%5D=1"

doc/api/scim.md

@@ -2,7 +2,7 @@
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/9388) in [GitLab Silver](https://about.gitlab.com/pricing/) 11.10.
 
-The SCIM API implements [the RFC7644 protocol](https://tools.ietf.org/html/rfc7644).
+The SCIM API implements the [RFC7644 protocol](https://tools.ietf.org/html/rfc7644).
 
 CAUTION: **Caution:**
 This API is for internal system use for connecting with a SCIM provider. While it can be used directly, it is subject to change without notice.

doc/development/changelog.md

@@ -41,8 +41,8 @@ the `author` field. GitLab team members **should not**.
   a changelog entry regardless of these guidelines if the contributor wants one.
   Example: "Fixed a typo on the search results page."
 - Any docs-only changes **should not** have a changelog entry.
-- Any change behind a feature flag **should not** have a changelog entry - unless
-  the feature flag has been defaulted to true.
+- Any change behind a disabled feature flag **should not** have a changelog entry.
+- Any change behind an enabled feature flag **should** have a changelog entry.
 - A change that [removes a feature flag](feature_flags/development.md) **should** have a changelog entry -
   only if the feature flag did not default to true already.
 - A fix for a regression introduced and then fixed in the same release (i.e.,

doc/development/testing_guide/frontend_testing.md

@@ -638,6 +638,38 @@ unit tests.
 Instead of `setImmediate`, use `jest.runAllTimers` or `jest.runOnlyPendingTimers` to run pending timers.
 The latter is useful when you have `setInterval` in the code. **Remember:** our Jest configuration uses fake timers.
 
+## Avoid non-deterministic specs
+
+Non-determinism is the breeding ground for flaky and brittle specs. Such specs end up breaking the CI pipeline, interrupting the work flow of other contributors.
+
+1. Make sure your test subject's collaborators (e.g., axios, apollo, lodash helpers) and test environment (e.g., Date) behave consistently across systems and over time.
+1. Make sure tests are focused and not doing "extra work" (e.g., needlessly creating the test subject more than once in an individual test)
+
+### Faking `Date` for determinism
+
+Consider using `useFakeDate` to ensure a consistent value is returned with every `new Date()` or `Date.now()`.
+
+```javascript
+import { useFakeDate } from 'helpers/fake_date';
+
+describe('cool/component', () => {
+  useFakeDate();
+
+  // ...
+});
+```
+
+### Faking `Math.random` for determinism
+
+Consider replacing `Math.random` with a fake when the test subject depends on it.
+
+```javascript
+beforeEach(() => {
+  // https://xkcd.com/221/
+  jest.spyOn(Math, 'random').mockReturnValue(0.4);
+});
+```
+
 ## Factories
 
 TBU

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

doc/user/clusters/management_project.md

@@ -73,10 +73,10 @@ configure cluster:
     name: production
 ```
 
-### Setting the environment scope **(PREMIUM)**
+### Setting the environment scope
 
 [Environment
-scopes](../project/clusters/index.md#setting-the-environment-scope-premium)
+scopes](../project/clusters/index.md#setting-the-environment-scope)
 are usable when associating multiple clusters to the same management
 project.
 

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.
 

doc/user/project/clusters/add_eks_clusters.md

@@ -135,7 +135,7 @@ To create and add a new Kubernetes cluster to your project, group, or instance:
 1. Click **Authenticate with AWS**.
 1. Choose your cluster's settings:
    - **Kubernetes cluster name** - The name you wish to give the cluster.
-   - **Environment scope** - The [associated environment](index.md#setting-the-environment-scope-premium) to this cluster.
+   - **Environment scope** - The [associated environment](index.md#setting-the-environment-scope) to this cluster.
    - **Kubernetes version** - The Kubernetes version to use. Currently the only version supported is 1.14.
    - **Role name** - Select the [IAM role](https://docs.aws.amazon.com/eks/latest/userguide/service_IAM_role.html)
      to allow Amazon EKS and the Kubernetes control plane to manage AWS resources on your behalf. This IAM role is separate

doc/user/project/clusters/add_gke_clusters.md

@@ -55,7 +55,7 @@ To create and add a new Kubernetes cluster to your project, group, or instance:
    **Sign in with Google** button.
 1. Choose your cluster's settings:
    - **Kubernetes cluster name** - The name you wish to give the cluster.
-   - **Environment scope** - The [associated environment](index.md#setting-the-environment-scope-premium) to this cluster.
+   - **Environment scope** - The [associated environment](index.md#setting-the-environment-scope) to this cluster.
    - **Google Cloud Platform project** - Choose the project you created in your GCP
      console that will host the Kubernetes cluster. Learn more about
      [Google Cloud Platform projects](https://cloud.google.com/resource-manager/docs/creating-managing-projects).

doc/user/project/clusters/add_remove_clusters.md

@@ -169,7 +169,7 @@ To add a Kubernetes cluster to your project, group, or instance:
 1. Click the **Add existing cluster** tab and fill in the details:
    1. **Kubernetes cluster name** (required) - The name you wish to give the cluster.
    1. **Environment scope** (required) - The
-      [associated environment](index.md#setting-the-environment-scope-premium) to this cluster.
+      [associated environment](index.md#setting-the-environment-scope) to this cluster.
    1. **API URL** (required) -
       It's the URL that GitLab uses to access the Kubernetes API. Kubernetes
       exposes several APIs, we want the "base" URL that is common to all of them.

doc/user/project/clusters/index.md

@@ -74,10 +74,10 @@ project. That way you can have different clusters for different environments,
 like dev, staging, production, and so on.
 
 Simply add another cluster, like you did the first time, and make sure to
-[set an environment scope](#setting-the-environment-scope-premium) that will
+[set an environment scope](#setting-the-environment-scope) that will
 differentiate the new cluster with the rest.
 
-#### Setting the environment scope **(PREMIUM)**
+#### Setting the environment scope
 
 When adding more than one Kubernetes cluster to your project, you need to differentiate
 them with an environment scope. The environment scope associates clusters with [environments](../../../ci/environments/index.md) similar to how the

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 :

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))

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
 

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 ""
 

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

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

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]
 
-      it 'sends an email confirmation' do
-        expect { post(:create, params: { email: email_params }) }.to change { ActionMailer::Base.deliveries.size }
+        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 '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
 
-    it 'unable to resend an email confirmation' do
-      expect { put(:resend_confirmation_instructions, params: { id: 1 }) }.not_to change { ActionMailer::Base.deliveries.size }
+    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_behaves_like 'respects the rate limit' do
+      let(:action) { :profile_resend_email_confirmation }
     end
   end
 end

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
+          }
 
-        expect(requester.reload.human_access).to eq(label)
+          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
+
+      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

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

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

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

spec/frontend/helpers/fake_date.js

@@ -0,0 +1,49 @@
+// Frida Kahlo's birthday (6 = July)
+export const DEFAULT_ARGS = [2020, 6, 6];
+
+const RealDate = Date;
+
+const isMocked = val => Boolean(val.mock);
+
+export const createFakeDateClass = ctorDefault => {
+  const FakeDate = new Proxy(RealDate, {
+    construct: (target, argArray) => {
+      const ctorArgs = argArray.length ? argArray : ctorDefault;
+
+      return new RealDate(...ctorArgs);
+    },
+    apply: (target, thisArg, argArray) => {
+      const ctorArgs = argArray.length ? argArray : ctorDefault;
+
+      return RealDate(...ctorArgs);
+    },
+    // We want to overwrite the default 'now', but only if it's not already mocked
+    get: (target, prop) => {
+      if (prop === 'now' && !isMocked(target[prop])) {
+        return () => new RealDate(...ctorDefault).getTime();
+      }
+
+      return target[prop];
+    },
+    getPrototypeOf: target => {
+      return target.prototype;
+    },
+    // We need to be able to set props so that `jest.spyOn` will work.
+    set: (target, prop, value) => {
+      // eslint-disable-next-line no-param-reassign
+      target[prop] = value;
+      return true;
+    },
+  });
+
+  return FakeDate;
+};
+
+export const useFakeDate = (...args) => {
+  const FakeDate = createFakeDateClass(args.length ? args : DEFAULT_ARGS);
+  global.Date = FakeDate;
+};
+
+export const useRealDate = () => {
+  global.Date = RealDate;
+};

spec/frontend/helpers/fake_date_spec.js

@@ -0,0 +1,33 @@
+import { createFakeDateClass, DEFAULT_ARGS, useRealDate } from './fake_date';
+
+describe('spec/helpers/fake_date', () => {
+  describe('createFakeDateClass', () => {
+    let FakeDate;
+
+    beforeAll(() => {
+      useRealDate();
+    });
+
+    beforeEach(() => {
+      FakeDate = createFakeDateClass(DEFAULT_ARGS);
+    });
+
+    it('should use default args', () => {
+      expect(new FakeDate()).toEqual(new Date(...DEFAULT_ARGS));
+      expect(FakeDate()).toEqual(Date(...DEFAULT_ARGS));
+    });
+
+    it('should have deterministic now()', () => {
+      expect(FakeDate.now()).not.toBe(Date.now());
+      expect(FakeDate.now()).toBe(new Date(...DEFAULT_ARGS).getTime());
+    });
+
+    it('should be instanceof Date', () => {
+      expect(new FakeDate()).toBeInstanceOf(Date);
+    });
+
+    it('should be instanceof self', () => {
+      expect(new FakeDate()).toBeInstanceOf(FakeDate);
+    });
+  });
+});

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

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

spec/lib/gitlab/current_settings_spec.rb

@@ -115,6 +115,16 @@ RSpec.describe Gitlab::CurrentSettings do
           expect(settings).to have_attributes(settings_from_defaults)
         end
 
+        context 'when ApplicationSettings does not have a primary key' do
+          before do
+            allow(ActiveRecord::Base.connection).to receive(:primary_key).with('application_settings').and_return(nil)
+          end
+
+          it 'raises an exception if ApplicationSettings does not have a primary key' do
+            expect { described_class.current_application_settings }.to raise_error(/table is missing a primary key constraint/)
+          end
+        end
+
         context 'with pending migrations' do
           let(:current_settings) { described_class.current_application_settings }
 

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

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}", '')
 
-      ActiveSession.destroy_with_public_id(user, session.public_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
 
-      total_sessions = ActiveSession.list(user).count
-      expect(total_sessions).to eq 0
+      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
+
+      it 'removes the lookup entry' do
+        subject
+
+        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) }
 
-      expect do
-        ActiveSession.destroy_with_public_id(user, "")
-      end.not_to raise_error
+      it 'calls .destroy_sessions' do
+        expect(ActiveSession).to(
+          receive(:destroy_sessions)
+            .with(anything, user, [rack_session.private_id]))
 
-      expect do
-        ActiveSession.destroy_with_public_id(user, "aaaaaaaa")
-      end.not_to raise_error
+        subject
+      end
+
+      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 }
+
+        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 }
 
-      ActiveSession.destroy_with_public_id(user, nil)
-      ActiveSession.destroy_with_public_id(user, "")
-      ActiveSession.destroy_with_public_id(user, "aaaaaaaa")
+      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
+
+      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

spec/models/application_setting_spec.rb

@@ -654,6 +654,16 @@ RSpec.describe ApplicationSetting do
     end
   end
 
+  context 'when ApplicationSettings does not have a primary key' do
+    before do
+      allow(ActiveRecord::Base.connection).to receive(:primary_key).with(described_class.table_name).and_return(nil)
+    end
+
+    it 'raises an exception' do
+      expect { described_class.create_from_defaults }.to raise_error(/table is missing a primary key constraint/)
+    end
+  end
+
   describe '#disabled_oauth_sign_in_sources=' do
     before do
       allow(Devise).to receive(:omniauth_providers).and_return([:github])

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]) }

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

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
 

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

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)
+      it_behaves_like 'uncached response' do
+        before do
+          url = route('.gitignore') + "/raw"
+          expect(Gitlab::Workhorse).to receive(:send_git_blob)
 
-        get api(url, current_user), params: params
-
-        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

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

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
 

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

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
 

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
 
-      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 a string' do
+        let(:params) { { access_level: Gitlab::Access::GUEST.to_s } }
 
-        updated_member = described_class.new(current_user, params).execute(member, permission: permission)
+        it_behaves_like 'schedules to delete confidential todos'
+      end
 
-        expect(updated_member).to be_valid
-        expect(updated_member.access_level).to eq(Gitlab::Access::GUEST)
+      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
+
+    context 'when access_level is invalid' do
+      let(:params) { { access_level: 'invalid' } }
+
+      it 'raises an error' do
+        expect { described_class.new(current_user, params) }.to raise_error(ArgumentError, 'invalid value for Integer(): "invalid"')
       end
     end
   end

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)

spec/services/todos/destroy/entity_leave_service_spec.rb

@@ -3,78 +3,146 @@
 require 'spec_helper'
 
 RSpec.describe Todos::Destroy::EntityLeaveService do
-  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_it_be(:user, reload: true)    { create(:user) }
+  let_it_be(:user2, reload: true)   { create(:user) }
 
-  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(:group)   { create(:group, :private) }
+  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)
+        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)
 
-          expect(user.todos).to match_array([todo_group_user])
-          expect(user2.todos).to match_array([todo_issue_user2, todo_group_user2])
-        end
-
-        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
-        end
-
-        context 'when the user is member of a parent group' do
-          before do
-            group.add_developer(user)
-          end
-
-          it 'does not remove any todos' do
-            expect { subject }.not_to change { Todo.count }
-          end
-        end
-
-        context 'when the user is guest of a parent group' 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
+          it_behaves_like 'using different access permissions', PRIVATE_PROJECT_PRIVATE_GROUP_ACCESS_TABLE
         end
       end
 
-      context 'when project is not private' do
-        before do
-          group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
-          project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
+      # 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)
+
+            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 '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_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
+        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)
             .to receive(:perform_async).with(project.id, user.id)
@@ -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
+          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 '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
+            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)
+        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)
 
-          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
-
-          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 '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
 
-        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
-
-          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

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

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

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

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