1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-04 20:52:43 +01:00
Commit graph

77 commits

Author SHA1 Message Date
Bob Trahan
06384f5f87 fix mail handling error -- return empty array if we have no $addresses
Test Plan: pushing it live to test
2012-11-16 06:57:15 -08:00
Bob Trahan
bd1bc6d71a fix error where blank email addresses were sneaking in the stack
Summary: array_filter to the rescue

Test Plan: mostly lots of reasoning (as opposed to making a fake user with a blank email address to reproduce)

Reviewers: epriestley, chad

Reviewed By: chad

CC: aran, Korvin

Maniphest Tasks: T2052

Differential Revision: https://secure.phabricator.com/D3927
2012-11-08 12:21:50 -08:00
vrana
ef85f49adc Delete license headers from files
Summary:
This commit doesn't change license of any file. It just makes the license implicit (inherited from LICENSE file in the root directory).

We are removing the headers for these reasons:

- It wastes space in editors, less code is visible in editor upon opening a file.
- It brings noise to diff of the first change of any file every year.
- It confuses Git file copy detection when creating small files.
- We don't have an explicit license header in other files (JS, CSS, images, documentation).
- Using license header in every file is not obligatory: http://www.apache.org/dev/apply-license.html#new.

This change is approved by Alma Chao (Lead Open Source and IP Counsel at Facebook).

Test Plan: Verified that the license survived only in LICENSE file and that it didn't modify externals.

Reviewers: epriestley, davidrecordon

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2035

Differential Revision: https://secure.phabricator.com/D3886
2012-11-05 11:16:51 -08:00
Bob Trahan
afae26ad94 robustify Differential and Maniphest mailhandlers wrt attachments
Summary:
a few things

- make the parent mailhandler class not send "blank body" error if you have attachments
- make both differential and maniphest append a list of attachments to the body if any exist
- BONUS - made the cc stuff work in Maniphest

Test Plan: I haven't actually tested this yet. :(  i need to figure out how to send a mail with an attachment from the command-line and figured I'd serve this up first.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2012

Differential Revision: https://secure.phabricator.com/D3868
2012-11-01 15:18:06 -07:00
epriestley
5903ed650c Move completed tasks to an "archive" table and delete them in the GC
Summary:
Currently, when taskmasters complete a task it is immediately deleted. This prevents us from doing some general things, like:

  - Supporting the idea of permanent failure (e.g., after N failures just stop trying).
  - Showing the user how fast taskmasters are completing tasks.
  - Showing the user how long tasks took to complete.

Having better visibility into this is important to Drydock, which builds on the task system. Also, generally buff debug output for task execution.

Test Plan: Ran `bin/phd debug taskmaster`. Ran `bin/phd debug garbage`. Queued some tasks via various systems.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2015

Differential Revision: https://secure.phabricator.com/D3852
2012-10-31 15:22:16 -07:00
epriestley
1a8232f4c9 Fix an issue where excluded recipients are not respected
Summary: I broke this in D3778. We modify `$parameters` and then ignore it in favor of `$params` for the rest of the method. Unit tests work great since they're one level below this.

Test Plan: Verified "Send email about my own actions" behaved correctly.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3796
2012-10-23 10:48:03 -07:00
epriestley
90ccfe4da1 Clean up and expose filtered recipient lists for PhabricatorMetaMTAMail
Summary: Provide a public interface to get all the filtered recipients of an email. The intent is to pass this along to Notifications so it can mark notifications as read if the user is also receiving an email, possibly based on some preference (see T1403). This also simplifies the enormous sendNow() method a little bit.

Test Plan: Added unit tests, and sent a few mails that should cover most/all of these cases. They appeared to produce the correct recipients.

Reviewers: btrahan, vrana, nh

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1403

Differential Revision: https://secure.phabricator.com/D3778
2012-10-22 16:25:43 -07:00
Bob Trahan
d9c6e07f2c If users are on the email to Phabricator, do not send them the Phabricator reply.
Summary: When we receive an email, figure out if any of the other tos and ccs are users. If they are, pass their phids through the stach as "exclude phids" and exclude them from getting the email.

Test Plan: used the various applications (audit, differential, maniphest) and noted emails were sent as expected.

Reviewers: epriestley, vrana

Reviewed By: vrana

CC: aran, Korvin, vrana

Maniphest Tasks: T1676

Differential Revision: https://secure.phabricator.com/D3645
2012-10-10 10:18:23 -07:00
Bob Trahan
a754fdfca3 Create metamta.user-address-format configuration option
Summary: this lets users specify what "name" to use in email addresses

Test Plan: changed the conf setting in my local instance and used phabricator. observed name format changes

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1862

Differential Revision: https://secure.phabricator.com/D3639
2012-10-05 11:27:58 -07:00
Bob Trahan
cdfc71ced5 Only send the "this is blank silly" error message email if the email is sent *just* to Phabricator.
Summary: said differently, if the user included another to address or one or more cc's, don't send the error message email.

Test Plan: played around in the metamta test console and verified that blank replies generated the error handler.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1643

Differential Revision: https://secure.phabricator.com/D3345
2012-08-28 14:09:37 -07:00
epriestley
21ebd1a609 Fix an issue with To/CC placeholders
Summary: Currently, if no placeholder is configured we always move "Cc" up to "To", even if we have a valid "To". Instead, move "Cc" to "To" only if there's no "To" and no placeholder.

Test Plan: Sent email with "to" and "cc", email with "cc" only with a placeholder, and email with "cc" only without a placeholder. Verified recipients ended up in the right location in all cases.

Reviewers: nh, btrahan

Reviewed By: btrahan

CC: klimek, aran

Differential Revision: https://secure.phabricator.com/D3342
2012-08-20 14:08:45 -07:00
epriestley
74b438db13 Move "Mailing Lists" to a separate application
Summary:
There's currently no way to get here from the UI since nav tabs don't exist anymore. It's also always been hard to find this feature even when we had the tabs, since it's surprising that it's inside "MetaMTA".

  - Move mailing lists to a separate application.
  - Add `buildApplicationPage()`, since we don't really need `buildStandardPageResponse()` any more -- we can infer all the information from `PhabricatorApplication`. This will let us get rid of a lot of the `PhabricatorXXXController` classes which just define application information.
  - Add `getApplicationURI()` to reduce code duplication, and in case we want to let you move applications around some day.

Test Plan: Looked/edited/saved mailing lists.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T631

Differential Revision: https://secure.phabricator.com/D3248
2012-08-12 19:19:46 -07:00
epriestley
59db282624 Fix an issue with email "Re:" prefix
Summary: See detailed discussion in T1543.

Test Plan:
  - Enabled multiplexing.
  - Set user A to "enable Re".
  - Created a task owned by user B with user A cc'd.
  - Verified A got no "Re:" before this patch.
  - Applied patch.
  - Verified A got "Re:" after this patch.

Reviewers: nh, btrahan, vrana

Reviewed By: nh

CC: aran

Maniphest Tasks: T1543

Differential Revision: https://secure.phabricator.com/D3031
2012-07-23 09:23:54 -07:00
Nick Harper
67c302ae4f Send messages with only a CC
Summary: This keeps people in the correct To or CC field on multiplexed messages.

Test Plan:
with multiplexing on, checked that I received an email with me in the CC
field instead of the To field for a diff I'm CC'd on.

Reviewers: epriestley, jungejason, vrana

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2999
2012-07-18 11:45:46 -07:00
epriestley
2b690372de Fix several issues with Differential exception email
Summary:
  - Assigning $cc_phids clobbers the correct value assigned on line 80.
  - Remove stack trace noise, the trace is always meaningless and well-known.
  - Actually show the original body.

Test Plan: Piped mail to the mail receiver and verified the errors didn't CC revision CCs, no longer had traces, and included the original raw text body.

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2969
2012-07-12 13:33:26 -07:00
epriestley
16c20c92fe Minor, fix an issue where we may fail to select a translation for mail. 2012-07-02 07:17:56 -07:00
vrana
c5c0324e1b Send several X-Differential-CC headers
Summary: People want to create filters checking if they are in CC which is almost impossible with multiplexing in Outlook.

Test Plan: Sent e-mail with multiple CCs, verified headers.

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2829
2012-06-22 11:10:23 -07:00
vrana
e84f9f9ec9 Send Differential e-mails in user's language
Summary:
Works this way:

- Select users' language with multiplexing.
- Select default language otherwise (it can be different from current user's language).
- Build body and subject for each user individually.
- Set the original language after sending the mails.

Test Plan:
- Comment on a diff of user with custom translation.
- Set default to a custom translation. Comment on a diff of user with default translation.
- Set default to a default translation. Comment on a diff of user with default translation.

Repeat with/without multiplexing.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1139

Differential Revision: https://secure.phabricator.com/D2774
2012-06-18 12:41:09 -07:00
vrana
2793828795 Refactor setting e-mail subjects
Summary:
It seems that Outlook and Mail.app mostly ignores the threading headers and thread primarily by subject.
They are also very picky about the Re: part in the header.
I guess that's because users of these clients often hit Reply when they want to create a new message to the sender of an e-mail.

We need both of these applications to work with the same setting because we don't use multiplexing to prevent sending multiple e-mails to people in lists.
I also believe that the default behavior should just work in most setups.

I've tried several different combinations of putting "Re:" and none of them seems to always work in both clients.

This diff at least adds more abstraction to the code which should prevent copy/paste errors (two fixed by this diff!).

Test Plan: Sent several e-mails with varying subject, verified that they look as before in Outlook and Mail.app.

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2709
2012-06-11 19:07:21 -07:00
vrana
6cc196a2e5 Move files in Phabricator one level up
Summary:
- `kill_init.php` said "Moving 1000 files" - I hope that this is not some limit in `FileFinder`.
- [src/infrastructure/celerity] `git mv utils.php map.php; git mv api/utils.php api.php`
- Comment `phutil_libraries` in `.arcconfig` and run `arc liberate`.

NOTE: `arc diff` timed out so I'm pushing it without review.

Test Plan:
/D1234
Browsed around, especially in `applications/repository/worker/commitchangeparser` and `applications/` in general.

Auditors: epriestley

Maniphest Tasks: T1103
2012-06-01 12:32:44 -07:00
vrana
1ebf9186b4 Depend on class autoloading
Test Plan:
Run setup.
/differential/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2612
2012-05-30 16:57:21 -07:00
epriestley
09c8af4de0 Upgrade phabricator to libphutil v2
Summary: Mechanical changes from D2588. No "Class.php" moves yet.

Test Plan: See D2588.

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2591
2012-05-30 14:26:29 -07:00
epriestley
0461cd6e4f Prevent loops in received mail
Summary:
It's currently possible to configure Phabricator to send mail to some address it recognizes as relating to an object.

When we receive mail from Phabricator, drop it unconditionally.

Test Plan: Wrote two emails, one with the header and one without. Piped them to `mail_handler.php`, one was dropped immediately.

Reviewers: btrahan, nh, mikaaay, jungejason

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2529
2012-05-22 06:02:05 -07:00
Nick Harper
c2a9a8079f Remove email from handles
Summary:
Since user emails aren't in the user table, we had to do extra data fetching
for handles, and the emails are only used in MetaMTA, so we move the email
code into MetaMTA and remove it from handles.

Test Plan: send test emails

Reviewers: jungejason, vrana, epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2494
2012-05-21 17:37:26 -07:00
epriestley
87207b2f4e Allow users to have multiple email addresses, and verify emails
Summary:
  - Move email to a separate table.
  - Migrate existing email to new storage.
  - Allow users to add and remove email addresses.
  - Allow users to verify email addresses.
  - Allow users to change their primary email address.
  - Convert all the registration/reset/login code to understand these changes.
  - There are a few security considerations here but I think I've addressed them. Principally, it is important to never let a user acquire a verified email address they don't actually own. We ensure this by tightening the scoping of token generation rules to be (user, email) specific.
  - This should have essentially zero impact on Facebook, but may require some minor changes in the registration code -- I don't exactly remember how it is set up.

Not included here (next steps):

  - Allow configuration to restrict email to certain domains.
  - Allow configuration to require validated email.

Test Plan:
This is a fairly extensive, difficult-to-test change.

  - From "Email Addresses" interface:
    - Added new email (verified email verifications sent).
    - Changed primary email (verified old/new notificactions sent).
    - Resent verification emails (verified they sent).
    - Removed email.
    - Tried to add already-owned email.
  - Created new users with "accountadmin". Edited existing users with "accountadmin".
  - Created new users with "add_user.php".
  - Created new users with web interface.
  - Clicked welcome email link, verified it verified email.
  - Reset password.
  - Linked/unlinked oauth accounts.
  - Logged in with oauth account.
  - Logged in with email.
  - Registered with Oauth account.
  - Tried to register with OAuth account with duplicate email.
  - Verified errors for email verification with bad tokens, etc.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1184

Differential Revision: https://secure.phabricator.com/D2393
2012-05-07 10:29:33 -07:00
epriestley
9531496d66 Add "X-Auto-Response-Suppress" header to all outgoing mail
Summary: This appears to sometimes be effective (for MS clients), and we've seen it in the wild on inbound mail.

Test Plan: Sent myself some mail, verified it had the right header.

Reviewers: btrahan, vrana, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T571

Differential Revision: https://secure.phabricator.com/D2241
2012-04-17 06:35:28 -07:00
epriestley
c458768415 Fix various threading issues, particularly in Gmail
Summary:
  - Add an explicit multiplexing option, and enable it by default. This is necessary for Mail.app to coexist with other clients ("Re:" breaks outlook at the very least, and generally sucks in the common case), and allows users with flexible clients to enable subject variance.
  - Add an option for subject line variance. Default to not varying the subject, so mail no longer says [Committed], [Closed], etc. This is so the defaults thread correctly in Gmail (not entirely sure this actually works).
  - Add a preference to enable subject line variance.
  - Unless all mail is multiplexed, don't enable or respect the "Re" or "vary subject" preferences. These are currently shown and respected in non-multiplex cases, which creates inconsistent results.

NOTE: @jungejason @nh @vrana This changes the default behavior (from non-multiplexing to multiplexing), and might break Facebook's integration. You should be able to keep the same behavior by setting the options appropriately, although if you can get the new defaults working they're probably better.

Test Plan:
Send mail from Maniphest, Differential and Audit. Updated preferences. Enabled/disabled multiplexing. Things seem OK?

NOTE: I haven't actually been able to repro the Gmail threading issue so I'm not totally sure what's going on there, maybe it started respecting "Re:" (or always has), but @cpiro and @20after4 both reported it independently. This fixes a bunch of bugs in any case and gives us more conservative set of defaults.

I'll see if I can buff out the Gmail story a bit but every client is basically a giant black box of mystery. :/

Reviewers: btrahan, vrana, jungejason, nh

Reviewed By: btrahan

CC: cpiro, 20after4, aran

Maniphest Tasks: T1097, T847

Differential Revision: https://secure.phabricator.com/D2206
2012-04-12 09:31:03 -07:00
epriestley
23fd936b47 Add some basic signature stripping
Summary: See discussion in T789. Covered the obvious cases, at least. We can refine this as we get a larger sample size.

Test Plan: Unit test coverage.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T789

Differential Revision: https://secure.phabricator.com/D2154
2012-04-08 15:04:12 -07:00
vrana
8813c7be0e Use assert_instances_of() everywhere but Differential and Diffusion
Summary: NOTE: This is not produced by a script so there might be errors. Please review carefully.

Test Plan: Browse around.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2091
2012-04-03 14:53:20 -07:00
vrana
4fba549a99 Use PhabricatorEnv::newObjectFromConfig() wherever possible
Test Plan:
/mail/send/
scripts/aphront/aphrontpath.php /

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1983
2012-03-21 14:57:52 -07:00
epriestley
d0af617818 Add "final" to (almost) everything else
Summary: Last of the big final patches. Left a few debatable classes (12 out of about 400) that I'll deal with individually eventually.

Test Plan: Ran testEverythingImplemented.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T795

Differential Revision: https://secure.phabricator.com/D1881
2012-03-13 16:21:04 -07:00
Nick Harper
d8c601f21b Move functionality of PhabricatorMetaMTADaemon to a worker task
Summary:
This will allow sending mail to be done by task workers. See T750.

Task ID: #

Blame Rev:

Test Plan:
- started taskmaster daemon in test env
- used "send new test message" feature in MetMTA (with send now unchecked)
- confirmed receipt of 1 email
- repeated 2 & 3 with send now checked

Revert Plan:

Tags:

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T388, T750

Differential Revision: https://secure.phabricator.com/D1723
2012-03-01 22:01:55 -08:00
epriestley
94daf63ca5 Add an explicit "this mail came from Phabricator" header
Summary:
See T926. If you want to write a mail rule that, e.g., captures Differential
mail but ignores people replying to it, it's kind of tricky right now. You can
use the 'X-Mail-Transport-Agent' header but that's not obvious and it's not
necessarily stable.

Add a nice, obvious "X-Phabricator-Sent-This-Message" header.

Test Plan: Sent myself some mail, verified the header appeared.

Reviewers: vrana, btrahan, fugalh, jungejason

Reviewed By: jungejason

CC: aran, epriestley

Maniphest Tasks: T926

Differential Revision: https://secure.phabricator.com/D1732
2012-02-29 15:30:56 -08:00
epriestley
d7a7bca85c Enable email for audits
Summary:
When users submit an audit, send email to relevant parties informing them.

Allow email to be replied to. Just basic support so far; no "!raise" stuff and
no threading with the Herald commit notification.

Test Plan: Made comments, got email. Replied to email, got comments.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1698
2012-02-27 12:57:57 -08:00
epriestley
bfea830d09 Add email preferences to receive fewer less-important notifications
Summary:
A few similar requests have come in across several tools and use cases that I
think this does a reasonable job of resolving.

We currently send one email for each update an object receives, but these aren't
always appreciated:

  - Asana does post-commit review via Differential, so the "committed" mails are
useless.
  - Quora wants to make project category edits to bugs without spamming people
attached to them.
  - Some users in general are very sensitive to email volumes, and this gives us
a good way to reduce the volumes without incurring the complexity of
delayed-send-batching.

The technical mechanism is basically:

  - Mail may optionally have "mail tags", which indicate content in the mail
(e.g., "maniphest-priority, maniphest-cc, maniphest-comment" for a mail which
contains a priority change, a CC change, and a comment).
  - If a mail has tags, remove any recipients who have opted out of all the
tags.
  - Some tags can't be opted out of via the UI, so this ensures that important
email is still delivered (e.g., cc + assign + comment is always delivered
because you can't opt out of "assign" or "comment").

Test Plan:
  - Disabled all mail tags in the web UI.
  - Used test console to send myself mail with an opt-outable tag, it was
immediately dropped.
  - Used test console to send myself mail with an opt-outable tag and a custom
tag, it was delivered.
  - Made Differential updates affecting CCs with and without comments, got
appropriate delivery.
  - Made Maniphest updates affecting project, priority and CCs with and without
comments, got appropriate delivery.
  - Verified mail headers in all cases.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley, moskov

Maniphest Tasks: T616, T855

Differential Revision: https://secure.phabricator.com/D1635
2012-02-17 22:57:07 -08:00
epriestley
ad36865e50 Add optional "Re:" prefix to all threaded mail and allow disabling mail about
your own actions

Summary:
  - Mail.app on Lion has cumbersome threading rules, see T782. Add an option to
stick "Re: " in front of all threaded mail so it behaves. This is horrible, but
apparently the least-horrible option.
  - While I was in there, I added an option for T228.

Test Plan:
  - Sent a bunch of threaded and unthreaded mail with varous "Re:" settings,
seemed to get "Re:" in the right places.
  - Disabled email about my stuff, created a task with just me, got voided mail,
added a CC, got mail to just the CC.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, mkjones

Maniphest Tasks: T228, T782

Differential Revision: https://secure.phabricator.com/D1448
2012-01-18 15:20:50 -08:00
vrana
6472dbe168 Change fileName to filename
Summary: There are lots of callsites to $changeset->getFilename() so it seemed
easier to rename getFileName() to getFilename() even if it includes database
change. Plus I think that getFilename() is better.

Test Plan:
Alter database.
Open revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1437
2012-01-17 10:50:14 -08:00
epriestley
79205481e6 Minor, fix \n in MetaMTA headers. 2012-01-08 10:26:16 -08:00
epriestley
e45ffda55a Move most remaining sha1() calls to HMAC
Summary:
  - For context, see T547. This is the last (maybe?) in a series of diffs that
moves us off raw sha1() calls in order to make it easier to audit the codebase
for correct use of hash functions.
  - This breaks CSRF tokens. Any open forms will generate an error when
submitted, so maybe upgrade off-peak.
  - We now generate HMAC mail keys but accept MAC or HMAC. In a few months, we
can remove the MAC version.
  - The only remaining callsite is Conduit. We can't use HMAC since Arcanist
would need to know the key. {T550} provides a better solution to this, anyway.

Test Plan:
  - Verified CSRF tokens generate properly.
  - Manually changed CSRF to an incorrect value and got an error.
  - Verified mail generates with a new mail hash.
  - Verified Phabricator accepts both old and new mail hashes.
  - Verified Phabricator rejects bad mail hashes.
  - Checked user log, things look OK.

Reviewers: btrahan, jungejason, benmathews

Reviewed By: btrahan

CC: aran, epriestley, btrahan

Maniphest Tasks: T547

Differential Revision: 1237
2011-12-19 08:56:53 -08:00
epriestley
98c8e150b0 Prevent delivery of email to disabled objects
Summary: See T625. Facebook's REST-based MTA layer had a check for this so I
overlooked it in porting it out. We should not attempt to deliver email to
disabled users.

Test Plan:
Used MetaMTA console to send email to:

  - No users: received "no To" exception.
  - A disabled user: received "all To disabled" exception.
  - A valid user: received email.
  - A valid user and a disabled user: received email to valid user only.

(Note that you can't easily send to disabled users directly since they don't
appear in the typeahead, but you can prefill it and then disable the user by
hitting "Send".)

Reviewers: btrahan, jungejason, nh, tuomaspelkonen, aran

Reviewed By: aran

CC: skrul, aran, epriestley

Differential Revision: 1120
2011-11-16 11:07:50 -08:00
Marek Sapota
b71a55900a Allow tweaking of Differential mail by using events
Summary: Allow tweaking Differential mail before sending.

Test Plan:
Wrote a listener renaming Differential attachments and it worked without
problems.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, mareksapota, davidreuss

Differential Revision: 1091
2011-11-09 10:13:53 -08:00
David Reuss
b81231b3dc Corrected manipest reference in exception thrown
Summary: This should hopefully kill off the last of these :P

Test Plan: Should be self explanatory

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1043
2011-10-28 08:05:49 -07:00
David Reuss
c20608f066 use correct key looking for a public author of a task
Test Plan:
Used the scripts/mail_handler.php with and without patch and saw
the maniphest task being created with patch applied.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, davidreuss

Differential Revision: 1041
2011-10-28 08:04:25 -07:00
epriestley
4156cf6bd9 Add an optional configuration option to set 'Precedence: bulk' headers on
transactional mail

Summary: See T571. SES refuses to deliver mail with this header and there are
various reports of other issues on the internet so I'm defaulting it to off.

Test Plan: Set config to true, tried to send mail, SES rejected it because of
"Precedence: bulk" header.

Reviewers: bmaurer, ola, jungejason, nh, aran

Reviewed By: aran

CC: aran, epriestley, bmaurer

Differential Revision: 1032
2011-10-23 14:25:13 -07:00
epriestley
9a4bb3901e Allow bugs@ addresses to blanket-accept tasks
Summary: Allow configuration of a default author for bugs@ emails which don't
correspond to a known system user.

Test Plan: Configured a default author, sent some mails from nonsense addresses,
tasks were created.

Reviewers: davidreuss, jungejason, nh, tuomaspelkonen, aran

Reviewed By: aran

CC: aran, epriestley, ide

Differential Revision: 1013
2011-10-20 14:26:19 -07:00
Marek Sapota
5148741ab2 Prevent duplicated emails with send-immedialtely = true and MTA daemon running
Test Plan:
Set 'metamta.send-immediately' to true.  Start up several MTA daemons, without
the patch you'll probably get multiple emails, with the patch you should get
only one.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, mareksapota, epriestley

Differential Revision: 1021
2011-10-19 14:51:28 -07:00
Marek Sapota
fee7184350 Phabricator mail
Test Plan: EMPTY

Reviewers: aran, epriestley

Reviewed By: epriestley

CC: aran, mareksapota, epriestley, jungejason, nh, drnikki

Differential Revision: 1002
2011-10-14 12:12:41 -07:00
epriestley
d1134810d6 Allow insecure mail auth with "Reply-To" header
Summary: Quora wants to handle some moderation tasks with Phabricator, but want
to lower the barrier to entry for the install and let moderators adopt it
gradually. One request is to allow auth rules to be relaxed so we can auth based
on Reply-To to make things easier. This is insecure if configured but not really
a big deal and the patch isn't big or complicated.

Test Plan: Sent a test email with bogus "From" but valid "Reply-To". It was
rejected with this setting off, and allowed with this setting on.

Reviewers: jungejason, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 842
2011-08-23 14:13:04 -07:00
David Reuss
c236e4ad72 Enable support for a single reply-handler for outbound emails
Summary:
This allows you to configure a single mailbox for all mail sent by phabricator,
so you
can keep a mailaddress like bugs@example.com and don't need a catchall on your
domain/subdomain.

Test Plan:
Enabled and disabled suffix. Saw mails generated have to correct prefix. Also
piped raw mails
into the scripts/mail/mail_handler.php and ensured comments went into
phabricator for both maniphest
and differential.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 815
2011-08-22 10:20:49 +02:00
epriestley
6deddbfc70 Better enforce uniqueness for email delivery
Summary:
@skrul reports receiving multiple copies of notification emails since
@hunterbridges configured some bizarre dystopian email replication factory on
their outbound route. Two fixes:

  - Ensure "To" and "Cc" are unique. Email shouldn't be replicated for "To:
x@y.com, x@y.com" but it's silly that we do this.
  - Remove "To" addresses from "Cc". Email shouldn't be replicated here either,
but we don't really lose anything by accommodating this.

Test Plan:
Sent a mail to the same to/cc, verified I was to'd only and not cc'd when the
mail was delivered.

@hunterbridges, can you apply this patch locally and verify it fixes the issue?
You can test by going to MetaMTA -> Send New Message and sending a message to
yourself as both To and CC.

Reviewed By: skrul
Reviewers: skrul, hunterbridges, jungejason, tuomaspelkonen, aran
CC: aran, skrul, epriestley
Differential Revision: 751
2011-08-01 15:52:51 -07:00