Project

General

Profile

Feature #30820 » 0001-Remove-setting-Blind-carbon-copy-recipients-bcc.patch

Marius BĂLTEANU, 2019-02-20 09:55

View differences:

app/models/mailer.rb
651 651
      redmine_headers 'Sender' => @author.login
652 652
    end
653 653

  
654
    # Blind carbon copy recipients
655
    if Setting.bcc_recipients?
656
      headers[:bcc] = [headers[:to], headers[:cc]].flatten.uniq.reject(&:blank?)
657
      headers[:to] = nil
658
      headers[:cc] = nil
659
    end
660

  
661 654
    if @message_id_object
662 655
      headers[:message_id] = "<#{self.class.message_id_for(@message_id_object)}>"
663 656
    end
app/views/settings/_notifications.html.erb
4 4
<div class="box tabular settings">
5 5
<p><%= setting_text_field :mail_from, :size => 60 %></p>
6 6

  
7
<p><%= setting_check_box :bcc_recipients %></p>
8

  
9 7
<p><%= setting_check_box :plain_text_mail %></p>
10 8
</div>
11 9

  
config/settings.yml
78 78
  default: 10
79 79
mail_from:
80 80
  default: redmine@example.net
81
bcc_recipients:
82
  default: 1
83 81
plain_text_mail:
84 82
  default: 0
85 83
text_formatting:
db/migrate/20190217164229_remove_bcc_recipients_setting.rb
1
class RemoveBccRecipientsSetting < ActiveRecord::Migration[5.2]
2
  def change
3
    Setting.where(:name => 'bcc_recipients').delete_all
4
  end
5
end
test/functional/account_controller_test.rb
310 310
              :firstname => 'John',
311 311
              :lastname => 'Doe',
312 312
              :mail => 'register@example.com'
313
              
313

  
314 314
            }
315 315
          }
316 316
        assert_redirected_to '/my/account'
......
324 324
      assert user.active?
325 325
    end
326 326
  end
327
  
327

  
328 328
  def test_post_register_with_registration_off_should_redirect
329 329
    with_settings :self_registration => '0' do
330 330
      assert_no_difference 'User.count' do
......
336 336
              :firstname => 'John',
337 337
              :lastname => 'Doe',
338 338
              :mail => 'register@example.com'
339
              
339

  
340 340
            }
341 341
          }
342 342
        assert_redirected_to '/'
......
355 355
              :firstname => 'John',
356 356
              :lastname => 'Doe',
357 357
              :mail => 'register@example.com'
358
              
359
            },  
358

  
359
            },
360 360
            :pref => {
361 361
              :hide_mail => '1'
362
              
362

  
363 363
            }
364 364
          }
365 365
      end
......
406 406
      end
407 407
    end
408 408
    mail = ActionMailer::Base.deliveries.last
409
    assert_equal ['jsmith@somenet.foo'], mail.bcc
409
    assert_equal ['jsmith@somenet.foo'], mail.to
410 410
  end
411 411

  
412 412
  def test_lost_password_using_additional_email_address_should_send_email_to_the_address
......
422 422
      end
423 423
    end
424 424
    mail = ActionMailer::Base.deliveries.last
425
    assert_equal ['anotherAddress@foo.bar'], mail.bcc
425
    assert_equal ['anotherAddress@foo.bar'], mail.to
426 426
  end
427 427

  
428 428
  def test_lost_password_for_unknown_user_should_fail
test/functional/admin_controller_test.rb
93 93
    mail = ActionMailer::Base.deliveries.last
94 94
    assert_not_nil mail
95 95
    user = User.find(1)
96
    assert_equal [user.mail], mail.bcc
96
    assert_equal [user.mail], mail.to
97 97
  end
98 98

  
99 99
  def test_test_email_failure_should_display_the_error
test/functional/email_addresses_controller_test.rb
133 133
      assert_select 'a[href^=?]', 'http://localhost:3000/my/account', :text => 'My account'
134 134
    end
135 135
    # The old email address should be notified about a new address for security purposes
136
    assert [mail.bcc, mail.cc].flatten.include?(User.find(2).mail)
137
    assert [mail.bcc, mail.cc].flatten.include?('something@example.fr')
136
    assert mail.to.include?(User.find(2).mail)
137
    assert mail.to.include?('something@example.fr')
138 138
  end
139 139

  
140 140
  def test_update
......
182 182
    assert_mail_body_match I18n.t(:mail_body_security_notification_notify_disabled, value: 'another@somenet.foo'), mail
183 183

  
184 184
    # The changed address should be notified for security purposes
185
    assert [mail.bcc, mail.cc].flatten.include?('another@somenet.foo')
185
    assert mail.to.include?('another@somenet.foo')
186 186
  end
187 187

  
188 188

  
......
241 241
    assert_mail_body_match I18n.t(:mail_body_security_notification_remove, field: I18n.t(:field_mail), value: 'another@somenet.foo'), mail
242 242

  
243 243
    # The removed address should be notified for security purposes
244
    assert [mail.bcc, mail.cc].flatten.include?('another@somenet.foo')
244
    assert mail.to.include?('another@somenet.foo')
245 245
  end
246 246
end
test/functional/issues_controller_test.rb
3419 3419
    # Watchers notified
3420 3420
    mail = ActionMailer::Base.deliveries.last
3421 3421
    assert_not_nil mail
3422
    assert [mail.bcc, mail.cc].flatten.include?(User.find(3).mail)
3422
    assert mail.to.include?(User.find(3).mail)
3423 3423
  end
3424 3424

  
3425 3425
  def test_post_create_subissue
test/functional/issues_custom_fields_visibility_test.rb
172 172
          :id => @issue.id,
173 173
          :issue => {
174 174
            :custom_field_values => {
175
            @field1.id.to_s => "User#{user.id}Value0",    
176
                    @field2.id.to_s => "User#{user.id}Value1",  
175
            @field1.id.to_s => "User#{user.id}Value0",
176
                    @field2.id.to_s => "User#{user.id}Value1",
177 177
                  @field3.id.to_s => "User#{user.id}Value2",
178 178
                }
179 179
        }
......
268 268

  
269 269
    ActionMailer::Base.deliveries.clear
270 270
    @request.session[:user_id] = 1
271
    with_settings :bcc_recipients => '1' do
272 271
      assert_difference 'Issue.count' do
273
        post :create, :params => {
274
            :project_id => 1,
275
            :issue => {
276
              :tracker_id => 1,
277
              :status_id => 1,
278
              :subject => 'New issue',
279
              :priority_id => 5,
280
              :custom_field_values => {
281
                @field1.id.to_s => 'Value0', @field2.id.to_s => 'Value1', @field3.id.to_s => 'Value2'
282
              },    
283
              :watcher_user_ids => users_to_test.keys.map(&:id)
284
              
285
            }
272
      post :create, :params => {
273
          :project_id => 1,
274
          :issue => {
275
            :tracker_id => 1,
276
            :status_id => 1,
277
            :subject => 'New issue',
278
            :priority_id => 5,
279
            :custom_field_values => {
280
              @field1.id.to_s => 'Value0', @field2.id.to_s => 'Value1', @field3.id.to_s => 'Value2'
281
            },
282
            :watcher_user_ids => users_to_test.keys.map(&:id)
283

  
286 284
          }
287
        assert_response 302
288
      end
285
        }
286

  
287
      assert_response 302
289 288
    end
290 289

  
291 290
    assert_equal users_to_test.keys.size, ActionMailer::Base.deliveries.size
292 291
    # tests that each user receives 1 email with the custom fields he is allowed to see only
293 292
    users_to_test.each do |user, fields|
294
      mails = ActionMailer::Base.deliveries.select {|m| m.bcc.include? user.mail}
293
      mails = ActionMailer::Base.deliveries.select {|m| m.to.include? user.mail}
295 294
      assert_equal 1, mails.size
296 295
      mail = mails.first
297 296
      @fields.each_with_index do |field, i|
......
313 312
    end
314 313
    ActionMailer::Base.deliveries.clear
315 314
    @request.session[:user_id] = 1
316
    with_settings :bcc_recipients => '1' do
317
      put :update, :params => {
318
          :id => @issue.id,
319
          :issue => {
320
            :custom_field_values => {
321
              @field1.id.to_s => 'NewValue0', @field2.id.to_s => 'NewValue1', @field3.id.to_s => 'NewValue2'
322
            }    
323
            
315
    put :update, :params => {
316
        :id => @issue.id,
317
        :issue => {
318
          :custom_field_values => {
319
            @field1.id.to_s => 'NewValue0', @field2.id.to_s => 'NewValue1', @field3.id.to_s => 'NewValue2'
324 320
          }
321

  
325 322
        }
326
      assert_response 302
327
    end
323
      }
324

  
325
    assert_response 302
328 326
    assert_equal users_to_test.keys.size, ActionMailer::Base.deliveries.size
329 327
    # tests that each user receives 1 email with the custom fields he is allowed to see only
330 328
    users_to_test.each do |user, fields|
331
      mails = ActionMailer::Base.deliveries.select {|m| m.bcc.include? user.mail}
329
      mails = ActionMailer::Base.deliveries.select {|m| m.to.include? user.mail}
332 330
      assert_equal 1, mails.size
333 331
      mail = mails.first
334 332
      @fields.each_with_index do |field, i|
......
350 348
    end
351 349
    ActionMailer::Base.deliveries.clear
352 350
    @request.session[:user_id] = 1
353
    with_settings :bcc_recipients => '1' do
354
      put :update, :params => {
355
          :id => @issue.id,
356
          :issue => {
357
            :custom_field_values => {
358
              @field2.id.to_s => 'NewValue1', @field3.id.to_s => 'NewValue2'
359
            }    
360
            
351
    put :update, :params => {
352
        :id => @issue.id,
353
        :issue => {
354
          :custom_field_values => {
355
            @field2.id.to_s => 'NewValue1', @field3.id.to_s => 'NewValue2'
361 356
          }
357

  
362 358
        }
363
      assert_response 302
364
    end
359
      }
360
    assert_response 302
361

  
365 362
    users_to_test.each do |user, fields|
366
      mails = ActionMailer::Base.deliveries.select {|m| m.bcc.include? user.mail}
363
      mails = ActionMailer::Base.deliveries.select {|m| m.to.include? user.mail}
367 364
      if (fields & [@field2, @field3]).any?
368 365
        assert_equal 1, mails.size, "User #{user.id} was not notified"
369 366
      else
test/functional/messages_controller_test.rb
145 145
    end
146 146

  
147 147
    # author
148
    assert_equal ['jsmith@somenet.foo'], mails[0].bcc
148
    assert_equal ['jsmith@somenet.foo'], mails[0].to
149 149
    # project member
150
    assert_equal ['dlopper@somenet.foo'], mails[1].bcc
150
    assert_equal ['dlopper@somenet.foo'], mails[1].to
151 151
  end
152 152

  
153 153
  def test_get_edit
test/functional/my_controller_test.rb
391 391
      assert_select 'a[href^=?]', 'http://localhost:3000/my/account', :text => 'My account'
392 392
    end
393 393
    # The old email address should be notified about the change for security purposes
394
    assert [mail.bcc, mail.cc].flatten.include?(User.find(2).mail)
395
    assert [mail.bcc, mail.cc].flatten.include?('foobar@example.com')
394
    assert mail.to.include?(User.find(2).mail)
395
    assert mail.to.include?('foobar@example.com')
396 396
  end
397 397

  
398 398
  def test_my_account_should_show_destroy_link
test/functional/settings_controller_test.rb
76 76
    post :edit, :params => {
77 77
      :settings => {
78 78
        :mail_from => 'functional@test.foo',
79
        :bcc_recipients  => '0',
80 79
        :notified_events => %w(issue_added issue_updated news_added),
81 80
        :emails_footer => 'Test footer'
82 81
      }
83 82
    }
84 83
    assert_redirected_to '/settings'
85 84
    assert_equal 'functional@test.foo', Setting.mail_from
86
    assert !Setting.bcc_recipients?
87 85
    assert_equal %w(issue_added issue_updated news_added), Setting.notified_events
88 86
    assert_equal 'Test footer', Setting.emails_footer
89 87
  end
......
168 166
      assert_select 'a[href^=?]', 'http://localhost:3000/settings'
169 167
    end
170 168
    # All admins should receive this
171
    recipients = [mail.bcc, mail.cc].flatten
172 169
    User.active.where(admin: true).each do |admin|
173
      assert_include admin.mail, recipients
170
      assert_include admin.mail, mail.to
174 171
    end
175 172
  end
176 173

  
test/functional/users_controller_test.rb
211 211
  end
212 212

  
213 213
  def test_create
214
    Setting.bcc_recipients = '1'
215

  
216 214
    assert_difference 'User.count' do
217 215
      assert_difference 'ActionMailer::Base.deliveries.size' do
218 216
        post :create, :params => {
......
242 240

  
243 241
    mail = ActionMailer::Base.deliveries.last
244 242
    assert_not_nil mail
245
    assert_equal [user.mail], mail.bcc
243
    assert_equal [user.mail], mail.to
246 244
    assert_mail_body_match 'secret', mail
247 245
  end
248 246

  
......
369 367

  
370 368
    # All admins should receive this
371 369
    User.where(admin: true, status: Principal::STATUS_ACTIVE).each do |admin|
372
      assert_not_nil ActionMailer::Base.deliveries.detect{|mail| [mail.bcc, mail.cc].flatten.include?(admin.mail) }
370
      assert_not_nil ActionMailer::Base.deliveries.detect{|mail| [mail.to].flatten.include?(admin.mail) }
373 371
    end
374 372
  end
375 373

  
......
451 449
    u.status = User::STATUS_REGISTERED
452 450
    u.save!
453 451
    ActionMailer::Base.deliveries.clear
454
    Setting.bcc_recipients = '1'
455 452

  
456 453
    put :update, :params => {
457 454
      :id => u.id,
......
460 457
    assert u.reload.active?
461 458
    mail = ActionMailer::Base.deliveries.last
462 459
    assert_not_nil mail
463
    assert_equal ['foo.bar@somenet.foo'], mail.bcc
460
    assert_equal ['foo.bar@somenet.foo'], mail.to
464 461
    assert_mail_body_match ll('fr', :notice_account_activated), mail
465 462
  end
466 463

  
467 464
  def test_update_with_password_change_should_send_a_notification
468 465
    ActionMailer::Base.deliveries.clear
469
    Setting.bcc_recipients = '1'
470 466

  
471 467
    put :update, :params => {
472 468
      :id => 2,
......
478 474

  
479 475
    mail = ActionMailer::Base.deliveries.last
480 476
    assert_not_nil mail
481
    assert_equal [u.mail], mail.bcc
477
    assert_equal [u.mail], mail.to
482 478
    assert_mail_body_match 'newpass123', mail
483 479
  end
484 480

  
485 481
  def test_update_with_generate_password_should_email_the_password
486 482
    ActionMailer::Base.deliveries.clear
487
    Setting.bcc_recipients = '1'
488 483

  
489 484
    put :update, :params => {
490 485
      :id => 2,
......
582 577

  
583 578
    # All admins should receive this
584 579
    User.where(admin: true, status: Principal::STATUS_ACTIVE).each do |admin|
585
      assert_not_nil ActionMailer::Base.deliveries.detect{|mail| [mail.bcc, mail.cc].flatten.include?(admin.mail) }
580
      assert_not_nil ActionMailer::Base.deliveries.detect{|mail| mail.to == [admin.mail] }
586 581
    end
587 582
  end
588 583

  
......
602 597

  
603 598
    # All admins should receive this
604 599
    User.where(admin: true, status: Principal::STATUS_ACTIVE).each do |admin|
605
      assert_not_nil ActionMailer::Base.deliveries.detect{|mail| [mail.bcc, mail.cc].flatten.include?(admin.mail) }
600
      assert_not_nil ActionMailer::Base.deliveries.detect{|mail| mail.to == [admin.mail] }
606 601
    end
607 602
  end
608 603

  
......
622 617

  
623 618
    # All admins should receive this
624 619
    User.where(admin: true, status: Principal::STATUS_ACTIVE).each do |admin|
625
      assert_not_nil ActionMailer::Base.deliveries.detect{|mail| [mail.bcc, mail.cc].flatten.include?(admin.mail) }
620
      assert_not_nil ActionMailer::Base.deliveries.detect{|mail| mail.to == [admin.mail] }
626 621
    end
627 622

  
628 623
    # if user is already locked, destroying should not send a second mail
......
648 643

  
649 644
    # All admins should receive this
650 645
    User.where(admin: true, status: Principal::STATUS_ACTIVE).each do |admin|
651
      assert_not_nil ActionMailer::Base.deliveries.detect{|mail| [mail.bcc, mail.cc].flatten.include?(admin.mail) }
646
      assert_not_nil ActionMailer::Base.deliveries.detect{|mail| mail.to == [admin.mail] }
652 647
    end
653 648
  end
654 649

  
......
711 706

  
712 707
    # All admins should receive this
713 708
    User.where(admin: true, status: Principal::STATUS_ACTIVE).each do |admin|
714
      assert_not_nil ActionMailer::Base.deliveries.detect{|mail| [mail.bcc, mail.cc].flatten.include?(admin.mail) }
709
      assert_not_nil ActionMailer::Base.deliveries.detect{|mail| mail.to == [admin.mail] }
715 710
    end
716 711
  end
717 712
end
test/unit/issue_test.rb
2570 2570
      issue.assigned_to = nil
2571 2571
      issue.save!
2572 2572

  
2573
      assert_include [user.mail], ActionMailer::Base.deliveries.map(&:bcc)
2573
      assert_include [user.mail], ActionMailer::Base.deliveries.map(&:to)
2574 2574
    end
2575 2575
  end
2576 2576

  
test/unit/mailer_test.rb
281 281
    user.pref.save
282 282
    User.current = user
283 283
    Mailer.deliver_news_added(news.reload)
284
    assert_equal 1, last_email.bcc.size
284
    assert_equal 1, last_email.to.size
285 285

  
286 286
    # nobody to notify
287 287
    user.pref.no_self_notified = true
......
365 365
    issue = Issue.find(1)
366 366
    assert Mailer.deliver_issue_add(issue)
367 367

  
368
    assert mail = ActionMailer::Base.deliveries.find {|m| m.bcc.include?('dlopper@somenet.foo')}
369
    assert mail.bcc.include?('otheremail@somenet.foo')
368
    assert mail = ActionMailer::Base.deliveries.find {|m| m.to.include?('dlopper@somenet.foo')}
369
    assert mail.to.include?('otheremail@somenet.foo')
370 370
  end
371 371

  
372 372
  test "#issue_add should not notify project members that are not allow to view the issue" do
......
488 488
  def test_version_file_added
489 489
    attachements = [ Attachment.find_by_container_type('Version') ]
490 490
    assert Mailer.deliver_attachments_added(attachements)
491
    assert_not_nil last_email.bcc
492
    assert last_email.bcc.any?
491
    assert_not_nil last_email.to
492
    assert last_email.to.any?
493 493
    assert_select_email do
494 494
      assert_select "a[href=?]", "http://localhost:3000/projects/ecookbook/files"
495 495
    end
......
498 498
  def test_project_file_added
499 499
    attachements = [ Attachment.find_by_container_type('Project') ]
500 500
    assert Mailer.deliver_attachments_added(attachements)
501
    assert_not_nil last_email.bcc
502
    assert last_email.bcc.any?
501
    assert_not_nil last_email.to
502
    assert last_email.to.any?
503 503
    assert_select_email do
504 504
      assert_select "a[href=?]", "http://localhost:3000/projects/ecookbook/files"
505 505
    end
......
558 558
    Mailer.reminders(:days => 42)
559 559
    assert_equal 1, ActionMailer::Base.deliveries.size
560 560
    mail = last_email
561
    assert mail.bcc.include?('dlopper@somenet.foo')
561
    assert mail.to.include?('dlopper@somenet.foo')
562 562
    assert_mail_body_match 'Bug #3: Error 281 when updating a recipe', mail
563 563
    assert_equal '1 issue(s) due in the next 42 days', mail.subject
564 564
  end
......
570 570
      Mailer.reminders(:days => 42)
571 571
      assert_equal 1, ActionMailer::Base.deliveries.size
572 572
      mail = last_email
573
      assert mail.bcc.include?('dlopper@somenet.foo')
573
      assert mail.to.include?('dlopper@somenet.foo')
574 574
      assert_mail_body_match 'Bug #3: Error 281 when updating a recipe', mail
575 575
      assert_equal "1 demande(s) arrivent à échéance (42)", mail.subject
576 576
    end
......
587 587
      Mailer.reminders(:days => 42)
588 588
      assert_equal 1, ActionMailer::Base.deliveries.size
589 589
      mail = last_email
590
      assert mail.bcc.include?('dlopper@somenet.foo')
590
      assert mail.to.include?('dlopper@somenet.foo')
591 591
      assert_mail_body_no_match 'Closed issue', mail
592 592
    end
593 593
  end
......
598 598
    Mailer.reminders(:days => 42, :users => ['3'])
599 599
    assert_equal 1, ActionMailer::Base.deliveries.size # No mail for dlopper
600 600
    mail = last_email
601
    assert mail.bcc.include?('dlopper@somenet.foo')
601
    assert mail.to.include?('dlopper@somenet.foo')
602 602
    assert_mail_body_match 'Bug #3: Error 281 when updating a recipe', mail
603 603
  end
604 604

  
......
895 895

  
896 896
  # Returns an array of email addresses to which emails were sent
897 897
  def recipients
898
    ActionMailer::Base.deliveries.map(&:bcc).flatten.sort
898
    ActionMailer::Base.deliveries.map(&:to).flatten.sort
899 899
  end
900 900

  
901 901
  def last_email
(1-1/2)