diff options
44 files changed, 568 insertions, 164 deletions
diff --git a/app/assets/stylesheets/pages/profile.scss b/app/assets/stylesheets/pages/profile.scss index 67abe6e88ed..6c521bb06ee 100644 --- a/app/assets/stylesheets/pages/profile.scss +++ b/app/assets/stylesheets/pages/profile.scss @@ -392,11 +392,11 @@ table.u2f-registrations { } } -.gpg-email-badge { +.email-badge { display: inline; margin-right: $gl-padding / 2; - .gpg-email-badge-email { + .email-badge-email { display: inline; margin-right: $gl-padding / 4; } diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 676a7203c7d..156a8e2c515 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -155,7 +155,7 @@ class Admin::UsersController < Admin::ApplicationController def remove_email email = user.emails.find(params[:email_id]) - success = Emails::DestroyService.new(current_user, user: user, email: email.email).execute + success = Emails::DestroyService.new(current_user, user: user).execute(email) respond_to do |format| if success diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb index 0c2646d7bf0..80ab681ed87 100644 --- a/app/controllers/confirmations_controller.rb +++ b/app/controllers/confirmations_controller.rb @@ -10,13 +10,14 @@ class ConfirmationsController < Devise::ConfirmationsController users_almost_there_path end - def after_confirmation_path_for(resource_name, resource) - if signed_in?(resource_name) + def after_confirmation_path_for(_resource_name, resource) + # incoming resource can either be a :user or an :email + if signed_in?(:user) after_sign_in(resource) else Gitlab::AppLogger.info("Email Confirmed: username=#{resource.username} email=#{resource.email} ip=#{request.remote_ip}") flash[:notice] += " Please sign in." - new_session_path(resource_name) + new_session_path(:user) end end diff --git a/app/controllers/profiles/emails_controller.rb b/app/controllers/profiles/emails_controller.rb index 97db84b92d4..bbd7ba49d77 100644 --- a/app/controllers/profiles/emails_controller.rb +++ b/app/controllers/profiles/emails_controller.rb @@ -1,15 +1,14 @@ class Profiles::EmailsController < Profiles::ApplicationController + before_action :find_email, only: [:destroy, :resend_confirmation_instructions] + def index - @primary = current_user.email + @primary_email = current_user.email @emails = current_user.emails.order_id_desc end def create @email = Emails::CreateService.new(current_user, email_params.merge(user: current_user)).execute - - if @email.errors.blank? - NotificationService.new.new_email(@email) - else + unless @email.errors.blank? flash[:alert] = @email.errors.full_messages.first end @@ -17,9 +16,7 @@ class Profiles::EmailsController < Profiles::ApplicationController end def destroy - @email = current_user.emails.find(params[:id]) - - Emails::DestroyService.new(current_user, user: current_user, email: @email.email).execute + Emails::DestroyService.new(current_user, user: current_user).execute(@email) respond_to do |format| format.html { redirect_to profile_emails_url, status: 302 } @@ -27,9 +24,23 @@ class Profiles::EmailsController < Profiles::ApplicationController end end + def resend_confirmation_instructions + if Emails::ConfirmService.new(current_user, user: current_user).execute(@email) + flash[:notice] = "Confirmation email sent to #{@email.email}" + else + flash[:alert] = "There was a problem sending the confirmation email" + end + + redirect_to profile_emails_url + end + private def email_params params.require(:email).permit(:email) end + + def find_email + @email = current_user.emails.find(params[:id]) + end end diff --git a/app/mailers/emails/profile.rb b/app/mailers/emails/profile.rb index c401030e34a..4f5edeb9bda 100644 --- a/app/mailers/emails/profile.rb +++ b/app/mailers/emails/profile.rb @@ -7,12 +7,6 @@ module Emails mail(to: @user.notification_email, subject: subject("Account was created for you")) end - def new_email_email(email_id) - @email = Email.find(email_id) - @current_user = @user = @email.user - mail(to: @user.notification_email, subject: subject("Email was added to your account")) - end - def new_ssh_key_email(key_id) @key = Key.find_by(id: key_id) 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 diff --git a/app/services/emails/base_service.rb b/app/services/emails/base_service.rb index 7f591c89411..5bbceeb3b3f 100644 --- a/app/services/emails/base_service.rb +++ b/app/services/emails/base_service.rb @@ -1,9 +1,8 @@ module Emails class BaseService - def initialize(current_user, opts) - @current_user = current_user - @user = opts.delete(:user) - @email = opts[:email] + def initialize(current_user, params = {}) + @current_user, @params = current_user, params.dup + @user = params.delete(:user) end end end diff --git a/app/services/emails/confirm_service.rb b/app/services/emails/confirm_service.rb new file mode 100644 index 00000000000..b5301bf2b82 --- /dev/null +++ b/app/services/emails/confirm_service.rb @@ -0,0 +1,7 @@ +module Emails + class ConfirmService < ::Emails::BaseService + def execute(email) + email.resend_confirmation_instructions + end + end +end diff --git a/app/services/emails/create_service.rb b/app/services/emails/create_service.rb index b6491ee9804..94a841af7c3 100644 --- a/app/services/emails/create_service.rb +++ b/app/services/emails/create_service.rb @@ -1,7 +1,7 @@ module Emails class CreateService < ::Emails::BaseService - def execute - @user.emails.create(email: @email) + def execute(extra_params = {}) + @user.emails.create(@params.merge(extra_params)) end end end diff --git a/app/services/emails/destroy_service.rb b/app/services/emails/destroy_service.rb index 44011cc36c8..1ed131fe326 100644 --- a/app/services/emails/destroy_service.rb +++ b/app/services/emails/destroy_service.rb @@ -1,7 +1,7 @@ module Emails class DestroyService < ::Emails::BaseService - def execute - update_secondary_emails! if Email.find_by_email!(@email).destroy + def execute(email) + email.destroy && update_secondary_emails! end private diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index e2a80db06a6..8d5da459882 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -31,13 +31,6 @@ class NotificationService end end - # Always notify user about email added to profile - def new_email(email) - if email.user&.can?(:receive_notifications) - mailer.new_email_email(email.id).deliver_later - end - end - # When create an issue we should send an email to: # # * issue assignee if their notification level is not Disabled diff --git a/app/views/devise/mailer/_confirmation_instructions_account.html.haml b/app/views/devise/mailer/_confirmation_instructions_account.html.haml new file mode 100644 index 00000000000..65565b7b8a8 --- /dev/null +++ b/app/views/devise/mailer/_confirmation_instructions_account.html.haml @@ -0,0 +1,16 @@ +- confirmation_link = confirmation_url(@resource, confirmation_token: @token) +- if @resource.unconfirmed_email.present? + #content + = email_default_heading(@resource.unconfirmed_email) + %p Click the link below to confirm your email address. + #cta + = link_to 'Confirm your email address', confirmation_link +- else + #content + - if Gitlab.com? + = email_default_heading('Thanks for signing up to GitLab!') + - else + = email_default_heading("Welcome, #{@resource.name}!") + %p To get started, click the link below to confirm your account. + #cta + = link_to 'Confirm your account', confirmation_link diff --git a/app/views/devise/mailer/_confirmation_instructions_account.text.erb b/app/views/devise/mailer/_confirmation_instructions_account.text.erb new file mode 100644 index 00000000000..01f09aa763d --- /dev/null +++ b/app/views/devise/mailer/_confirmation_instructions_account.text.erb @@ -0,0 +1,14 @@ +<% if @resource.unconfirmed_email.present? %> +<%= @resource.unconfirmed_email %>, + +Use the link below to confirm your email address. +<% else %> + <% if Gitlab.com? %> +Thanks for signing up to GitLab! + <% else %> +Welcome, <%= @resource.name %>! + <% end %> +To get started, use the link below to confirm your account. +<% end %> + +<%= confirmation_url(@resource, confirmation_token: @token) %> diff --git a/app/views/devise/mailer/_confirmation_instructions_secondary.html.haml b/app/views/devise/mailer/_confirmation_instructions_secondary.html.haml new file mode 100644 index 00000000000..3d0a1f622a5 --- /dev/null +++ b/app/views/devise/mailer/_confirmation_instructions_secondary.html.haml @@ -0,0 +1,8 @@ +#content + = email_default_heading("#{@resource.user.name}, you've added an additional email!") + %p Click the link below to confirm your email address (#{@resource.email}) + #cta + = link_to 'Confirm your email address', confirmation_url(@resource, confirmation_token: @token) + %p + If this email was added in error, you can remove it here: + = link_to "Emails", profile_emails_url diff --git a/app/views/devise/mailer/_confirmation_instructions_secondary.text.erb b/app/views/devise/mailer/_confirmation_instructions_secondary.text.erb new file mode 100644 index 00000000000..a3b28cb0b84 --- /dev/null +++ b/app/views/devise/mailer/_confirmation_instructions_secondary.text.erb @@ -0,0 +1,7 @@ +<%= @resource.user.name %>, you've added an additional email! + +Use the link below to confirm your email address (<%= @resource.email %>) + +<%= confirmation_url(@resource, confirmation_token: @token) %> + +If this email was added in error, you can remove it here: <%= profile_emails_url %> diff --git a/app/views/devise/mailer/confirmation_instructions.html.haml b/app/views/devise/mailer/confirmation_instructions.html.haml index 4d1037807be..50ee7b53d8f 100644 --- a/app/views/devise/mailer/confirmation_instructions.html.haml +++ b/app/views/devise/mailer/confirmation_instructions.html.haml @@ -1,16 +1 @@ -- confirmation_link = confirmation_url(@resource, confirmation_token: @token) -- if @resource.unconfirmed_email.present? - #content - = email_default_heading(@resource.unconfirmed_email) - %p Click the link below to confirm your email address. - #cta - = link_to confirmation_link, confirmation_link -- else - #content - - if Gitlab.com? - = email_default_heading('Thanks for signing up to GitLab!') - - else - = email_default_heading("Welcome, #{@resource.name}!") - %p To get started, click the link below to confirm your account. - #cta - = link_to confirmation_link, confirmation_link += render partial: "confirmation_instructions_#{@resource.is_a?(User) ? 'account' : 'secondary'}" diff --git a/app/views/devise/mailer/confirmation_instructions.text.erb b/app/views/devise/mailer/confirmation_instructions.text.erb index 9f76edb76a4..05fddddf415 100644 --- a/app/views/devise/mailer/confirmation_instructions.text.erb +++ b/app/views/devise/mailer/confirmation_instructions.text.erb @@ -1,9 +1 @@ -Welcome, <%= @resource.name %>! - -<% if @resource.unconfirmed_email.present? %> -You can confirm your email (<%= @resource.unconfirmed_email %>) through the link below: -<% else %> -You can confirm your account through the link below: -<% end %> - -<%= confirmation_url(@resource, confirmation_token: @token) %> +<%= render partial: "confirmation_instructions_#{@resource.is_a?(User) ? 'account' : 'secondary'}" %>
\ No newline at end of file diff --git a/app/views/notify/new_email_email.html.haml b/app/views/notify/new_email_email.html.haml deleted file mode 100644 index 4a0448a573c..00000000000 --- a/app/views/notify/new_email_email.html.haml +++ /dev/null @@ -1,10 +0,0 @@ -%p - Hi #{@user.name}! -%p - A new email was added to your account: -%p - email: - %code= @email.email -%p - If this email was added in error, you can remove it here: - = link_to "Emails", profile_emails_url diff --git a/app/views/notify/new_email_email.text.erb b/app/views/notify/new_email_email.text.erb deleted file mode 100644 index 51cba99ad0d..00000000000 --- a/app/views/notify/new_email_email.text.erb +++ /dev/null @@ -1,7 +0,0 @@ -Hi <%= @user.name %>! - -A new email was added to your account: - -email.................. <%= @email.email %> - -If this email was added in error, you can remove it here: <%= profile_emails_url %> diff --git a/app/views/profiles/emails/index.html.haml b/app/views/profiles/emails/index.html.haml index 612ecbbb96a..df1df4f5d72 100644 --- a/app/views/profiles/emails/index.html.haml +++ b/app/views/profiles/emails/index.html.haml @@ -32,19 +32,25 @@ All email addresses will be used to identify your commits. %ul.well-list %li - = @primary + = render partial: 'shared/email_with_badge', locals: { email: @primary_email, verified: current_user.confirmed? } %span.pull-right %span.label.label-success Primary email - - if @primary === current_user.public_email + - if @primary_email === current_user.public_email %span.label.label-info Public email - - if @primary === current_user.notification_email + - if @primary_email === current_user.notification_email %span.label.label-info Notification email - @emails.each do |email| %li - = email.email + = render partial: 'shared/email_with_badge', locals: { email: email.email, verified: email.confirmed? } %span.pull-right - if email.email === current_user.public_email %span.label.label-info Public email - if email.email === current_user.notification_email %span.label.label-info Notification email - = link_to 'Remove', profile_email_path(email), data: { confirm: 'Are you sure?'}, method: :delete, class: 'btn btn-sm btn-warning prepend-left-10' + - unless email.confirmed? + - confirm_title = "#{email.confirmation_sent_at ? 'Resend' : 'Send'} confirmation email" + = link_to confirm_title, resend_confirmation_instructions_profile_email_path(email), method: :put, class: 'btn btn-sm btn-warning prepend-left-10' + + = link_to profile_email_path(email), data: { confirm: 'Are you sure?'}, method: :delete, class: 'btn btn-sm btn-danger prepend-left-10' do + %span.sr-only Remove + = icon('trash') diff --git a/app/views/profiles/gpg_keys/_key.html.haml b/app/views/profiles/gpg_keys/_key.html.haml index b04981f90e3..970e92aadaa 100644 --- a/app/views/profiles/gpg_keys/_key.html.haml +++ b/app/views/profiles/gpg_keys/_key.html.haml @@ -3,7 +3,7 @@ = icon 'key', class: "settings-list-icon hidden-xs" .key-list-item-info - key.emails_with_verified_status.map do |email, verified| - = render partial: 'email_with_badge', locals: { email: email, verified: verified } + = render partial: 'shared/email_with_badge', locals: { email: email, verified: verified } .description %code= key.fingerprint diff --git a/app/views/profiles/gpg_keys/_email_with_badge.html.haml b/app/views/shared/_email_with_badge.html.haml index 5f7844584e1..b7bbc109238 100644 --- a/app/views/profiles/gpg_keys/_email_with_badge.html.haml +++ b/app/views/shared/_email_with_badge.html.haml @@ -2,7 +2,7 @@ - css_classes << (verified ? 'verified': 'unverified') - text = verified ? 'Verified' : 'Unverified' -.gpg-email-badge - .gpg-email-badge-email= email +.email-badge + .email-badge-email= email %div{ class: css_classes } = text diff --git a/changelogs/unreleased/feature-verify_secondary_emails.yml b/changelogs/unreleased/feature-verify_secondary_emails.yml new file mode 100644 index 00000000000..e1ecc527f85 --- /dev/null +++ b/changelogs/unreleased/feature-verify_secondary_emails.yml @@ -0,0 +1,5 @@ +--- +title: A confirmation email is now sent when adding a secondary email address +merge_request: +author: digitalmoksha +type: added diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 0ba0d791054..c6ec0aeda7b 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -175,7 +175,7 @@ Devise.setup do |config| # Configure the default scope given to Warden. By default it's the first # devise role declared in your routes (usually :user). - # config.default_scope = :user + config.default_scope = :user # now have an :email scope as well, so set the default # Configure sign_out behavior. # Sign_out action can be scoped (i.e. /users/sign_out affects only :user scope). diff --git a/config/routes/profile.rb b/config/routes/profile.rb index 3e4e6111ab8..ddc852f0132 100644 --- a/config/routes/profile.rb +++ b/config/routes/profile.rb @@ -1,3 +1,6 @@ +# for secondary email confirmations - uses the same confirmation controller as :users +devise_for :emails, path: 'profile/emails', controllers: { confirmations: :confirmations } + resource :profile, only: [:show, :update] do member do get :audit_log @@ -28,7 +31,11 @@ resource :profile, only: [:show, :update] do put :revoke end end - resources :emails, only: [:index, :create, :destroy] + resources :emails, only: [:index, :create, :destroy] do + member do + put :resend_confirmation_instructions + end + end resources :chat_names, only: [:index, :new, :create, :destroy] do collection do delete :deny diff --git a/db/migrate/20170904092148_add_email_confirmation.rb b/db/migrate/20170904092148_add_email_confirmation.rb new file mode 100644 index 00000000000..17ff424b319 --- /dev/null +++ b/db/migrate/20170904092148_add_email_confirmation.rb @@ -0,0 +1,33 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddEmailConfirmation < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index", "remove_concurrent_index" or + # "add_column_with_default" you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + add_column :emails, :confirmation_token, :string + add_column :emails, :confirmed_at, :datetime_with_timezone + add_column :emails, :confirmation_sent_at, :datetime_with_timezone + end +end diff --git a/db/migrate/20170909090114_add_email_confirmation_index.rb b/db/migrate/20170909090114_add_email_confirmation_index.rb new file mode 100644 index 00000000000..a8c1023c482 --- /dev/null +++ b/db/migrate/20170909090114_add_email_confirmation_index.rb @@ -0,0 +1,36 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddEmailConfirmationIndex < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index", "remove_concurrent_index" or + # "add_column_with_default" you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + disable_ddl_transaction! + + # Not necessary to remove duplicates, as :confirmation_token is a new column + def up + add_concurrent_index :emails, :confirmation_token, unique: true + end + + def down + remove_concurrent_index :emails, :confirmation_token if index_exists?(:emails, :confirmation_token) + end +end diff --git a/db/schema.rb b/db/schema.rb index fa1aad257db..17be774e9de 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -514,8 +514,12 @@ ActiveRecord::Schema.define(version: 20171004121444) do t.string "email", null: false t.datetime "created_at" t.datetime "updated_at" + t.string "confirmation_token" + t.datetime "confirmed_at" + t.datetime "confirmation_sent_at" end + add_index "emails", ["confirmation_token"], name: "index_emails_on_confirmation_token", unique: true, using: :btree add_index "emails", ["email"], name: "index_emails_on_email", unique: true, using: :btree add_index "emails", ["user_id"], name: "index_emails_on_user_id", using: :btree diff --git a/doc/user/project/repository/gpg_signed_commits/index.md b/doc/user/project/repository/gpg_signed_commits/index.md index 29e04a0ccf0..6b9976d133c 100644 --- a/doc/user/project/repository/gpg_signed_commits/index.md +++ b/doc/user/project/repository/gpg_signed_commits/index.md @@ -26,7 +26,7 @@ to be uploaded to GitLab. For a signature to be verified three conditions need to be met: 1. The public key needs to be added your GitLab account -1. One of the emails in the GPG key matches your **primary** email +1. One of the emails in the GPG key matches a **verified** email address you use in GitLab 1. The committer's email matches the verified email from the gpg key ## Generating a GPG key @@ -94,7 +94,7 @@ started: ``` 1. Enter you real name, the email address to be associated with this key (should - match the primary email address you use in GitLab) and an optional comment + match a verified email address you use in GitLab) and an optional comment (press <kbd>Enter</kbd> to skip): ``` diff --git a/lib/api/users.rb b/lib/api/users.rb index d07dc302717..b6f97a1eac2 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -331,7 +331,6 @@ module API email = Emails::CreateService.new(current_user, declared_params(include_missing: false).merge(user: user)).execute if email.errors.blank? - NotificationService.new.new_email(email) present email, with: Entities::Email else render_validation_error!(email) @@ -369,10 +368,8 @@ module API not_found!('Email') unless email destroy_conditionally!(email) do |email| - Emails::DestroyService.new(current_user, user: user, email: email.email).execute + Emails::DestroyService.new(current_user, user: user).execute(email) end - - user.update_secondary_emails! end desc 'Delete a user. Available only for admins.' do @@ -677,7 +674,6 @@ module API email = Emails::CreateService.new(current_user, declared_params.merge(user: current_user)).execute if email.errors.blank? - NotificationService.new.new_email(email) present email, with: Entities::Email else render_validation_error!(email) @@ -693,10 +689,8 @@ module API not_found!('Email') unless email destroy_conditionally!(email) do |email| - Emails::DestroyService.new(current_user, user: current_user, email: email.email).execute + Emails::DestroyService.new(current_user, user: current_user).execute(email) end - - current_user.update_secondary_emails! end desc 'Get a list of user activities' diff --git a/spec/controllers/profiles/emails_controller_spec.rb b/spec/controllers/profiles/emails_controller_spec.rb new file mode 100644 index 00000000000..ecf14aad54f --- /dev/null +++ b/spec/controllers/profiles/emails_controller_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' + +describe Profiles::EmailsController do + let(:user) { create(:user) } + + before do + sign_in(user) + end + + describe '#create' do + let(:email_params) { { email: "add_email@example.com" } } + + it 'sends an email confirmation' do + expect { post(:create, { email: email_params }) }.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" + end + end + + describe '#resend_confirmation_instructions' do + let(:email_params) { { email: "add_email@example.com" } } + + it 'resends an email confirmation' do + email = user.emails.create(email: 'add_email@example.com') + + expect { put(:resend_confirmation_instructions, { 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" + end + + it 'unable to resend an email confirmation' do + expect { put(:resend_confirmation_instructions, { id: 1 }) }.not_to change { ActionMailer::Base.deliveries.size } + end + end +end diff --git a/spec/controllers/profiles_controller_spec.rb b/spec/controllers/profiles_controller_spec.rb index b52b63e05a4..ce5040ff02b 100644 --- a/spec/controllers/profiles_controller_spec.rb +++ b/spec/controllers/profiles_controller_spec.rb @@ -15,6 +15,20 @@ describe ProfilesController do expect(user.unconfirmed_email).to eq('john@gmail.com') end + it "allows an email update without confirmation if existing verified email" do + user = create(:user) + create(:email, :confirmed, user: user, email: 'john@gmail.com') + sign_in(user) + + put :update, + user: { email: "john@gmail.com", name: "John" } + + user.reload + + expect(response.status).to eq(302) + expect(user.unconfirmed_email).to eq nil + end + it "ignores an email update from a user with an external email address" do stub_omniauth_setting(sync_profile_from_provider: ['ldap']) stub_omniauth_setting(sync_profile_attributes: true) diff --git a/spec/factories/emails.rb b/spec/factories/emails.rb index 8303861bcfe..c9ab87a15aa 100644 --- a/spec/factories/emails.rb +++ b/spec/factories/emails.rb @@ -2,5 +2,7 @@ FactoryGirl.define do factory :email do user email { generate(:email_alias) } + + trait(:confirmed) { confirmed_at Time.now } end end diff --git a/spec/features/profiles/emails_spec.rb b/spec/features/profiles/emails_spec.rb new file mode 100644 index 00000000000..11cc8aae6f3 --- /dev/null +++ b/spec/features/profiles/emails_spec.rb @@ -0,0 +1,71 @@ +require 'rails_helper' + +feature 'Profile > Emails' do + let(:user) { create(:user) } + + before do + sign_in(user) + end + + describe 'User adds an email' do + before do + visit profile_emails_path + end + + scenario 'saves the new email' do + fill_in('Email', with: 'my@email.com') + click_button('Add email address') + + expect(page).to have_content('my@email.com Unverified') + expect(page).to have_content("#{user.email} Verified") + expect(page).to have_content('Resend confirmation email') + end + + scenario 'does not add a duplicate email' do + fill_in('Email', with: user.email) + click_button('Add email address') + + email = user.emails.find_by(email: user.email) + expect(email).to be_nil + expect(page).to have_content('Email has already been taken') + end + end + + scenario 'User removes email' do + user.emails.create(email: 'my@email.com') + visit profile_emails_path + expect(page).to have_content("my@email.com") + + click_link('Remove') + expect(page).not_to have_content("my@email.com") + end + + scenario 'User confirms email' do + email = user.emails.create(email: 'my@email.com') + visit profile_emails_path + expect(page).to have_content("#{email.email} Unverified") + + email.confirm + expect(email.confirmed?).to be_truthy + + visit profile_emails_path + expect(page).to have_content("#{email.email} Verified") + end + + scenario 'User re-sends confirmation email' do + email = user.emails.create(email: 'my@email.com') + visit profile_emails_path + + expect { click_link("Resend confirmation email") }.to change { ActionMailer::Base.deliveries.size } + expect(page).to have_content("Confirmation email sent to #{email.email}") + end + + scenario 'old unconfirmed emails show Send Confirmation button' do + email = user.emails.create(email: 'my@email.com') + email.update_attribute(:confirmation_sent_at, nil) + visit profile_emails_path + + expect(page).not_to have_content('Resend confirmation email') + expect(page).to have_content('Send confirmation email') + end +end diff --git a/spec/features/profiles/gpg_keys_spec.rb b/spec/features/profiles/gpg_keys_spec.rb index 623e4f341c5..b0f6848bc4b 100644 --- a/spec/features/profiles/gpg_keys_spec.rb +++ b/spec/features/profiles/gpg_keys_spec.rb @@ -4,7 +4,7 @@ feature 'Profile > GPG Keys' do let(:user) { create(:user, email: GpgHelpers::User2.emails.first) } before do - login_as(user) + sign_in(user) end describe 'User adds a key' do diff --git a/spec/mailers/emails/profile_spec.rb b/spec/mailers/emails/profile_spec.rb index 09e5094cf84..1f7be415e35 100644 --- a/spec/mailers/emails/profile_spec.rb +++ b/spec/mailers/emails/profile_spec.rb @@ -120,29 +120,4 @@ describe Emails::Profile do it { expect { Notify.new_gpg_key_email('foo') }.not_to raise_error } end end - - describe 'user added email' do - let(:email) { create(:email) } - - subject { Notify.new_email_email(email.id) } - - it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like 'a user cannot unsubscribe through footer link' - - it 'is sent to the new user' do - is_expected.to deliver_to email.user.email - end - - it 'has the correct subject' do - is_expected.to have_subject /^Email was added to your account$/i - end - - it 'contains the new email address' do - is_expected.to have_body_text /#{email.email}/ - end - - it 'includes a link to emails page' do - is_expected.to have_body_text /#{profile_emails_path}/ - end - end end diff --git a/spec/models/email_spec.rb b/spec/models/email_spec.rb index 1d6fabe48b1..b32dd31ae6d 100644 --- a/spec/models/email_spec.rb +++ b/spec/models/email_spec.rb @@ -11,4 +11,33 @@ describe Email do expect(described_class.new(email: ' inFO@exAMPLe.com ').email) .to eq 'info@example.com' end + + describe '#update_invalid_gpg_signatures' do + let(:user) do + create(:user, email: 'tula.torphy@abshire.ca').tap do |user| + user.skip_reconfirmation! + end + end + let(:user) { create(:user) } + + it 'synchronizes the gpg keys when the email is updated' do + email = user.emails.create(email: 'new@email.com') + + expect(user).to receive(:update_invalid_gpg_signatures) + + email.confirm + end + end + + describe 'scopes' do + let(:user) { create(:user) } + + it 'scopes confirmed emails' do + create(:email, :confirmed, user: user) + create(:email, user: user) + + expect(user.emails.count).to eq 2 + expect(user.emails.confirmed.count).to eq 1 + end + end end diff --git a/spec/models/gpg_key_spec.rb b/spec/models/gpg_key_spec.rb index 4a4d079b721..743f2cfcab5 100644 --- a/spec/models/gpg_key_spec.rb +++ b/spec/models/gpg_key_spec.rb @@ -90,11 +90,20 @@ describe GpgKey do it 'email is verified if the user has the matching email' do user = create :user, email: 'bette.cartwright@example.com' gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user + create :email, user: user + user.reload expect(gpg_key.emails_with_verified_status).to eq( 'bette.cartwright@example.com' => true, 'bette.cartwright@example.net' => false ) + + create :email, :confirmed, user: user, email: 'bette.cartwright@example.net' + user.reload + expect(gpg_key.emails_with_verified_status).to eq( + 'bette.cartwright@example.com' => true, + 'bette.cartwright@example.net' => true + ) end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 62890dd5002..9f517e4af72 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -360,9 +360,22 @@ describe User do expect(external_user.projects_limit).to be 0 end end + + describe '#check_for_verified_email' do + let(:user) { create(:user) } + let(:secondary) { create(:email, :confirmed, email: 'secondary@example.com', user: user) } + + it 'allows a verfied secondary email to be used as the primary without needing reconfirmation' do + user.update_attributes!(email: secondary.email) + user.reload + expect(user.email).to eq secondary.email + expect(user.unconfirmed_email).to eq nil + expect(user.confirmed?).to be_truthy + end + end end - describe 'after update hook' do + describe 'after commit hook' do describe '.update_invalid_gpg_signatures' do let(:user) do create(:user, email: 'tula.torphy@abshire.ca').tap do |user| @@ -376,10 +389,50 @@ describe User do end it 'synchronizes the gpg keys when the email is updated' do - expect(user).to receive(:update_invalid_gpg_signatures) + expect(user).to receive(:update_invalid_gpg_signatures).at_most(:twice) user.update_attributes!(email: 'shawnee.ritchie@denesik.com') end end + + describe '#update_emails_with_primary_email' do + before do + @user = create(:user, email: 'primary@example.com').tap do |user| + user.skip_reconfirmation! + end + @secondary = create :email, email: 'secondary@example.com', user: @user + @user.reload + end + + it 'gets called when email updated' do + expect(@user).to receive(:update_emails_with_primary_email) + + @user.update_attributes!(email: 'new_primary@example.com') + end + + it 'adds old primary to secondary emails when secondary is a new email ' do + @user.update_attributes!(email: 'new_primary@example.com') + @user.reload + + expect(@user.emails.count).to eq 2 + expect(@user.emails.pluck(:email)).to match_array([@secondary.email, 'primary@example.com']) + end + + it 'adds old primary to secondary emails if secondary is becoming a primary' do + @user.update_attributes!(email: @secondary.email) + @user.reload + + expect(@user.emails.count).to eq 1 + expect(@user.emails.first.email).to eq 'primary@example.com' + end + + it 'transfers old confirmation values into new secondary' do + @user.update_attributes!(email: @secondary.email) + @user.reload + + expect(@user.emails.count).to eq 1 + expect(@user.emails.first.confirmed_at).not_to eq nil + end + end end describe '#update_tracked_fields!', :clean_gitlab_redis_shared_state do @@ -467,6 +520,7 @@ describe User do describe '#generate_password' do it "does not generate password by default" do user = create(:user, password: 'abcdefghe') + expect(user.password).to eq('abcdefghe') end end @@ -474,6 +528,7 @@ describe User do describe 'authentication token' do it "has authentication token" do user = create(:user) + expect(user.authentication_token).not_to be_blank end end @@ -481,6 +536,7 @@ describe User do describe 'ensure incoming email token' do it 'has incoming email token' do user = create(:user) + expect(user.incoming_email_token).not_to be_blank end end @@ -523,6 +579,7 @@ describe User do it 'ensures an rss token on read' do user = create(:user, rss_token: nil) rss_token = user.rss_token + expect(rss_token).not_to be_blank expect(user.reload.rss_token).to eq rss_token end @@ -633,6 +690,7 @@ describe User do it "blocks user" do user.block + expect(user.blocked?).to be_truthy end end @@ -966,6 +1024,7 @@ describe User do it 'is case-insensitive' do user = create(:user, username: 'JohnDoe') + expect(described_class.find_by_username('JOHNDOE')).to eq user end end @@ -978,6 +1037,7 @@ describe User do it 'is case-insensitive' do user = create(:user, username: 'JohnDoe') + expect(described_class.find_by_username!('JOHNDOE')).to eq user end end @@ -1067,11 +1127,13 @@ describe User do it 'is true if avatar is image' do user.update_attribute(:avatar, 'uploads/avatar.png') + expect(user.avatar_type).to be_truthy end it 'is false if avatar is html page' do user.update_attribute(:avatar, 'uploads/avatar.html') + expect(user.avatar_type).to eq(['only images allowed']) end end @@ -1094,6 +1156,50 @@ describe User do end end + describe '#all_emails' do + let(:user) { create(:user) } + + it 'returns all emails' do + email_confirmed = create :email, user: user, confirmed_at: Time.now + email_unconfirmed = create :email, user: user + user.reload + + expect(user.all_emails).to match_array([user.email, email_unconfirmed.email, email_confirmed.email]) + end + end + + describe '#verified_emails' do + let(:user) { create(:user) } + + it 'returns only confirmed emails' do + email_confirmed = create :email, user: user, confirmed_at: Time.now + create :email, user: user + user.reload + + expect(user.verified_emails).to match_array([user.email, email_confirmed.email]) + end + end + + describe '#verified_email?' do + let(:user) { create(:user) } + + it 'returns true when the email is verified/confirmed' do + email_confirmed = create :email, user: user, confirmed_at: Time.now + create :email, user: user + user.reload + + expect(user.verified_email?(user.email)).to be_truthy + expect(user.verified_email?(email_confirmed.email.titlecase)).to be_truthy + end + + it 'returns false when the email is not verified/confirmed' do + email_unconfirmed = create :email, user: user + user.reload + + expect(user.verified_email?(email_unconfirmed.email)).to be_falsy + end + end + describe '#requires_ldap_check?' do let(:user) { described_class.new } @@ -1101,6 +1207,7 @@ describe User do # Create a condition which would otherwise cause 'true' to be returned allow(user).to receive(:ldap_user?).and_return(true) user.last_credential_check_at = nil + expect(user.requires_ldap_check?).to be_falsey end @@ -1111,6 +1218,7 @@ describe User do it 'is false for non-LDAP users' do allow(user).to receive(:ldap_user?).and_return(false) + expect(user.requires_ldap_check?).to be_falsey end @@ -1121,11 +1229,13 @@ describe User do it 'is true when the user has never had an LDAP check before' do user.last_credential_check_at = nil + expect(user.requires_ldap_check?).to be_truthy end it 'is true when the last LDAP check happened over 1 hour ago' do user.last_credential_check_at = 2.hours.ago + expect(user.requires_ldap_check?).to be_truthy end end @@ -1136,16 +1246,19 @@ describe User do describe '#ldap_user?' do it 'is true if provider name starts with ldap' do user = create(:omniauth_user, provider: 'ldapmain') + expect(user.ldap_user?).to be_truthy end it 'is false for other providers' do user = create(:omniauth_user, provider: 'other-provider') + expect(user.ldap_user?).to be_falsey end it 'is false if no extern_uid is provided' do user = create(:omniauth_user, extern_uid: nil) + expect(user.ldap_user?).to be_falsey end end @@ -1153,6 +1266,7 @@ describe User do describe '#ldap_identity' do it 'returns ldap identity' do user = create :omniauth_user + expect(user.ldap_identity.provider).not_to be_empty end end @@ -1162,6 +1276,7 @@ describe User do it 'blocks user flaging the action caming from ldap' do user.ldap_block + expect(user.blocked?).to be_truthy expect(user.ldap_blocked?).to be_truthy end @@ -1234,18 +1349,22 @@ describe User do expect(user.starred?(project2)).to be_falsey star1 = UsersStarProject.create!(project: project1, user: user) + expect(user.starred?(project1)).to be_truthy expect(user.starred?(project2)).to be_falsey star2 = UsersStarProject.create!(project: project2, user: user) + expect(user.starred?(project1)).to be_truthy expect(user.starred?(project2)).to be_truthy star1.destroy + expect(user.starred?(project1)).to be_falsey expect(user.starred?(project2)).to be_truthy star2.destroy + expect(user.starred?(project1)).to be_falsey expect(user.starred?(project2)).to be_falsey end @@ -1257,9 +1376,13 @@ describe User do project = create(:project, :public) expect(user.starred?(project)).to be_falsey + user.toggle_star(project) + expect(user.starred?(project)).to be_truthy + user.toggle_star(project) + expect(user.starred?(project)).to be_falsey end end @@ -1438,9 +1561,11 @@ describe User do user = create(:user) member = group.add_developer(user) + expect(user.authorized_projects).to include(project) member.destroy + expect(user.authorized_projects).not_to include(project) end @@ -1461,9 +1586,11 @@ describe User do project = create(:project, :private, namespace: user1.namespace) project.team << [user2, Gitlab::Access::DEVELOPER] + expect(user2.authorized_projects).to include(project) project.destroy + expect(user2.authorized_projects).not_to include(project) end @@ -1473,9 +1600,11 @@ describe User do user = create(:user) group.add_developer(user) + expect(user.authorized_projects).to include(project) group.destroy + expect(user.authorized_projects).not_to include(project) end end @@ -2019,7 +2148,9 @@ describe User do it 'creates the namespace' do expect(user.namespace).to be_nil + user.save! + expect(user.namespace).not_to be_nil end end @@ -2040,11 +2171,13 @@ describe User do it 'updates the namespace name' do user.update_attributes!(username: new_username) + expect(user.namespace.name).to eq(new_username) end it 'updates the namespace path' do user.update_attributes!(username: new_username) + expect(user.namespace.path).to eq(new_username) end @@ -2058,6 +2191,7 @@ describe User do it 'adds the namespace errors to the user' do user.update_attributes(username: new_username) + expect(user.errors.full_messages.first).to eq('Namespace name has already been taken') end end @@ -2074,56 +2208,49 @@ describe User do end end - describe '#verified_email?' do - it 'returns true when the email is the primary email' do - user = build :user, email: 'email@example.com' - - expect(user.verified_email?('email@example.com')).to be true - end - - it 'returns false when the email is not the primary email' do - user = build :user, email: 'email@example.com' - - expect(user.verified_email?('other_email@example.com')).to be false - end - end - describe '#sync_attribute?' do let(:user) { described_class.new } context 'oauth user' do it 'returns true if name can be synced' do stub_omniauth_setting(sync_profile_attributes: %w(name location)) + expect(user.sync_attribute?(:name)).to be_truthy end it 'returns true if email can be synced' do stub_omniauth_setting(sync_profile_attributes: %w(name email)) + expect(user.sync_attribute?(:email)).to be_truthy end it 'returns true if location can be synced' do stub_omniauth_setting(sync_profile_attributes: %w(location email)) + expect(user.sync_attribute?(:email)).to be_truthy end it 'returns false if name can not be synced' do stub_omniauth_setting(sync_profile_attributes: %w(location email)) + expect(user.sync_attribute?(:name)).to be_falsey end it 'returns false if email can not be synced' do stub_omniauth_setting(sync_profile_attributes: %w(location email)) + expect(user.sync_attribute?(:name)).to be_falsey end it 'returns false if location can not be synced' do stub_omniauth_setting(sync_profile_attributes: %w(location email)) + expect(user.sync_attribute?(:name)).to be_falsey end it 'returns true for all syncable attributes if all syncable attributes can be synced' do stub_omniauth_setting(sync_profile_attributes: true) + expect(user.sync_attribute?(:name)).to be_truthy expect(user.sync_attribute?(:email)).to be_truthy expect(user.sync_attribute?(:location)).to be_truthy @@ -2139,6 +2266,7 @@ describe User do context 'ldap user' do it 'returns true for email if ldap user' do allow(user).to receive(:ldap_user?).and_return(true) + expect(user.sync_attribute?(:name)).to be_falsey expect(user.sync_attribute?(:email)).to be_truthy expect(user.sync_attribute?(:location)).to be_falsey @@ -2147,6 +2275,7 @@ describe User do it 'returns true for email and location if ldap user and location declared as syncable' do allow(user).to receive(:ldap_user?).and_return(true) stub_omniauth_setting(sync_profile_attributes: %w(location)) + expect(user.sync_attribute?(:name)).to be_falsey expect(user.sync_attribute?(:email)).to be_truthy expect(user.sync_attribute?(:location)).to be_truthy diff --git a/spec/services/emails/confirm_service_spec.rb b/spec/services/emails/confirm_service_spec.rb new file mode 100644 index 00000000000..2b2c31e2521 --- /dev/null +++ b/spec/services/emails/confirm_service_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe Emails::ConfirmService do + let(:user) { create(:user) } + + subject(:service) { described_class.new(user) } + + describe '#execute' do + it 'sends a confirmation email again' do + email = user.emails.create(email: 'new@email.com') + mail = service.execute(email) + expect(mail.subject).to eq('Confirmation instructions') + end + end +end diff --git a/spec/services/emails/create_service_spec.rb b/spec/services/emails/create_service_spec.rb index 75812c17309..54692c88623 100644 --- a/spec/services/emails/create_service_spec.rb +++ b/spec/services/emails/create_service_spec.rb @@ -12,6 +12,11 @@ describe Emails::CreateService do expect(Email.where(opts)).not_to be_empty end + it 'creates an email with additional attributes' do + expect { service.execute(confirmation_token: 'abc') }.to change { Email.count }.by(1) + expect(Email.where(opts).first.confirmation_token).to eq 'abc' + end + it 'has the right user association' do service.execute diff --git a/spec/services/emails/destroy_service_spec.rb b/spec/services/emails/destroy_service_spec.rb index 7726fc0ef81..c3204fac3df 100644 --- a/spec/services/emails/destroy_service_spec.rb +++ b/spec/services/emails/destroy_service_spec.rb @@ -4,11 +4,11 @@ describe Emails::DestroyService do let!(:user) { create(:user) } let!(:email) { create(:email, user: user) } - subject(:service) { described_class.new(user, user: user, email: email.email) } + subject(:service) { described_class.new(user, user: user) } describe '#execute' do it 'removes an email' do - expect { service.execute }.to change { user.emails.count }.by(-1) + expect { service.execute(email) }.to change { user.emails.count }.by(-1) end end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index f4b36eb7eeb..b64ca5be8fc 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -105,18 +105,6 @@ describe NotificationService, :mailer do end end - describe 'Email' do - describe '#new_email' do - let!(:email) { create(:email) } - - it { expect(notification.new_email(email)).to be_truthy } - - it 'sends email to email owner' do - expect { notification.new_email(email) }.to change { ActionMailer::Base.deliveries.size }.by(1) - end - end - end - describe 'Notes' do context 'issue note' do let(:project) { create(:project, :private) } |