Merge remote-tracking branch 'origin/master' into feature/sm/35954-create-kubernetes-cluster-on-gke-from-k8s-service

2 files changed, 48 insertions, 11 deletions

diff --git a/app/models/email.rb b/app/models/email.rb
index 826d4f16edb..384f38f2db7 100644
--- a/app/models/email.rb
+++ b/app/models/email.rb
@@ -7,6 +7,13 @@ class Email < ActiveRecord::Base
   validates :email, presence: true, uniqueness: true, email: true
   validate :unique_email, if: ->(email) { email.email_changed? }
 
+  scope :confirmed, -> { where.not(confirmed_at: nil) }
+
+  after_commit :update_invalid_gpg_signatures, if: -> { previous_changes.key?('confirmed_at') }
+
+  devise :confirmable
+  self.reconfirmable = false  # currently email can't be changed, no need to reconfirm
+
   def email=(value)
     write_attribute(:email, value.downcase.strip)
   end
@@ -14,4 +21,9 @@ class Email < ActiveRecord::Base
   def unique_email
     self.errors.add(:email, 'has already been taken') if User.exists?(email: self.email)
   end
+
+  # once email is confirmed, update the gpg signatures
+  def update_invalid_gpg_signatures
+    user.update_invalid_gpg_signatures if confirmed?
+  end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 4e71a3e11c2..4ba9130a75a 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -163,15 +163,16 @@ class User < ActiveRecord::Base
   before_validation :sanitize_attrs
   before_validation :set_notification_email, if: :email_changed?
   before_validation :set_public_email, if: :public_email_changed?
-
-  after_update :update_emails_with_primary_email, if: :email_changed?
   before_save :ensure_authentication_token, :ensure_incoming_email_token
   before_save :ensure_user_rights_and_limits, if: :external_changed?
   before_save :skip_reconfirmation!, if: ->(user) { user.email_changed? && user.read_only_attribute?(:email) }
+  before_save :check_for_verified_email, if: ->(user) { user.email_changed? && !user.new_record? }
   after_save :ensure_namespace_correct
+  after_destroy :post_destroy_hook
+  after_commit :update_emails_with_primary_email, on: :update, if: -> { previous_changes.key?('email') }
   after_commit :update_invalid_gpg_signatures, on: :update, if: -> { previous_changes.key?('email') }
+
   after_initialize :set_projects_limit
-  after_destroy :post_destroy_hook
 
   # User's Layout preference
   enum layout: [:fixed, :fluid]
@@ -525,12 +526,24 @@ class User < ActiveRecord::Base
     errors.add(:public_email, "is not an email you own") unless all_emails.include?(public_email)
   end
 
+  # see if the new email is already a verified secondary email
+  def check_for_verified_email
+    skip_reconfirmation! if emails.confirmed.where(email: self.email).any?
+  end
+
+  # Note: the use of the Emails services will cause `saves` on the user object, running
+  # through the callbacks again and can have side effects, such as the `previous_changes`
+  # hash and `_was` variables getting munged.
+  # By using an `after_commit` instead of `after_update`, we avoid the recursive callback
+  # scenario, though it then requires us to use the `previous_changes` hash
   def update_emails_with_primary_email
+    previous_email = previous_changes[:email][0]  # grab this before the DestroyService is called
     primary_email_record = emails.find_by(email: email)
-    if primary_email_record
-      Emails::DestroyService.new(self, user: self, email: email).execute
-      Emails::CreateService.new(self, user: self, email: email_was).execute
-    end
+    Emails::DestroyService.new(self, user: self).execute(primary_email_record) if primary_email_record
+
+    # the original primary email was confirmed, and we want that to carry over.  We don't
+    # have access to the original confirmation values at this point, so just set confirmed_at
+    Emails::CreateService.new(self, user: self, email: previous_email).execute(confirmed_at: confirmed_at)
   end
 
   def update_invalid_gpg_signatures
@@ -816,6 +829,10 @@ class User < ActiveRecord::Base
     avatar_path(args) || GravatarService.new.execute(email, size, scale, username: username)
   end
 
+  def primary_email_verified?
+    confirmed? && !temp_oauth_email?
+  end
+
   def all_emails
     all_emails = []
     all_emails << email unless temp_oauth_email?
@@ -823,6 +840,18 @@ class User < ActiveRecord::Base
     all_emails
   end
 
+  def verified_emails
+    verified_emails = []
+    verified_emails << email if primary_email_verified?
+    verified_emails.concat(emails.confirmed.pluck(:email))
+    verified_emails
+  end
+
+  def verified_email?(check_email)
+    downcased = check_email.downcase
+    email == downcased ? primary_email_verified? : emails.confirmed.where(email: downcased).exists?
+  end
+
   def hook_attrs
     {
       name: name,
@@ -1047,10 +1076,6 @@ class User < ActiveRecord::Base
     ensure_rss_token!
   end
 
-  def verified_email?(email)
-    self.email == email
-  end
-
   def sync_attribute?(attribute)
     return true if ldap_user? && attribute == :email