1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-12 08:36:13 +01:00
Commit graph

436 commits

Author SHA1 Message Date
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
epriestley
b2890eeb0e Add "final" to all Phabricator "Controller" classes
Summary:
These are all unambiguously unextensible. Issues I hit:

  - Maniphest Change/Diff controllers, just consolidated them.
  - Some search controllers incorrectly extend from "Search" but should extend from "SearchBase". This has no runtime effects.
  - D1836 introduced a closure, which we don't handle correctly (somewhat on purpose; we target PHP 5.2). See T962.

Test Plan: Ran "testEverythingImplemented" unit test to identify classes extending from `final` classes. Resolved issues.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T795

Differential Revision: https://secure.phabricator.com/D1843
2012-03-09 15:46:25 -08:00
epriestley
b571f0b229 Forbid mailing list names contianing spaces or commas
Summary:
They end up in "CCs:" fields where they can't be parsed.

Not bothering to migrate since I think only Dropbox has hit this.

Also improved another error condition's handling.

Test Plan: Tried to save a mailing list with spaces and commas in the name.

Reviewers: btrahan, Makinde

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T947

Differential Revision: https://secure.phabricator.com/D1813
2012-03-07 13:18:00 -08:00
Nick Harper
6ddb98f804 Add retry count to meta mta view
Summary:
Show the retry count in the meta mta view (in addition to the list of
messages) - I find this info useful when I'm trying to debug what's going on
with mail failures.

Task ID: #

Blame Rev:

Test Plan:
loaded /mail/view/NNNNN/ and saw the retry count

Revert Plan:

Tags:

Reviewers: epriestley, btrahan, jungejason

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1782
2012-03-05 11:36:55 -08:00
Nick Harper
8f27f4272c Provide a way to view MetaMTA messages by status
Summary:
Added a query option of status for the MetaMTA list controller. There currently
isn't a ui for accessing this.

Task ID: #

Blame Rev:

Test Plan:
loaded /mail/, /mail/?status=queued, /mail?phid=PHID...&status=...
each request returned a sane list of data

Revert Plan:

Tags:

Reviewers: epriestley, btrahan, jungejason

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1784
2012-03-05 11:36:38 -08: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
blair
913c931cb0 [mailing list] add paging
Summary:
The mailing list page in MetaMTA only showed the first 100
sorted by ID, so it made it seem like lists were missing. Changed it to
do paging and short by name, so it has some user-understandable order.

Test Plan:
 - Go to /mail/lists/
 - Step through pager, confirm ordering.

Reviewers: nh, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1670
2012-02-22 12:47:13 -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
47631530a5 Remove admin requirement from MetaMTASendGridReceiveController
Summary:
This got caught in the crossfire when we admin-only'd the whole MetaMTA tool. It
should not be admin only.

(Generally, we should probably separate this out better at some point.)

Test Plan: Hit /mail/sendgrid/ as a logged-out, non-admin user (like SendGrid
does).

Reviewers: s, btrahan

Reviewed By: s

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1588
2012-02-06 19:31:20 -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
cedb0c045a Lock down accepted next URI values for redirect after login
Summary:
I locked this down a little bit recently, but make
double-extra-super-sure that we aren't sending the user anywhere suspicious or
open-redirecty. This also locks down protocol-relative URIs (//evil.com/path)
although I don't think any browsers do bad stuff with them in this context, and
header injection URIs (although I don't think any of the modern PHP runtimes are
vulnerable).

Test Plan:
  - Ran tests.
  - Hit redirect page with valid and invalid next URIs; was punted to / for
invalid ones and to the right place for valid ones.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: arice, aran, epriestley, btrahan

Differential Revision: https://secure.phabricator.com/D1369
2012-01-13 11:58:45 -08:00
epriestley
65a56c6ce0 Improve mailing list edit form
Summary:
  - Add some captions to make it more clear what these fields mean.
  - Require "name", since tokenizers use it exclusively.
  - Limit URI to allowed protocols, since admins can currently XSS users by
entering a "javascript:" URI and then tricking the user into clicking the
mailing list name. This exploit is dumb, but technically privilege escallation.

Test Plan:
  - Created a new mailing list.
  - Edited a mailing list.
  - Tested URI: valid, invalid, omitted.
  - Tested name: valid, omitted.

Reviewers: btrahan, jungejason, davidreuss

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: https://secure.phabricator.com/D1365
2012-01-11 15:48:21 -08:00
epriestley
79205481e6 Minor, fix \n in MetaMTA headers. 2012-01-08 10:26:16 -08:00
Evan Priestley
71e1911dfc Lock down MetaMTA functionality to administrators
Summary:
We have a debug interface for sending various sorts of email, but normal users
don't really need to use it. In particular, they can:

  - Send arbitrary email to other users;
  - Discover other users' email addresses fairly easily (CC everyone);
  - Send arbitrary email to arbitrary addresses in conjunction with "Mailing
Lists"

In fact, normal users don't need to get to the MetaMTA web interface at all and
it has some somewhat-sensitive things beacuse it has a lot of detailed
information about mail. For instance, users can look at mail records to discover
things like password reset links and per-user object email addresses.

We should smooth out the UI here but I think I can do something about T21 fairly
soon and cover it then.

Test Plan:
Went to /mail/ with a non-admin, got 404'd. Went to /mail/ with an
admin, everything works, got a red admin header.

Reviewers: jungejason, btrahan

Reviewed By: btrahan

CC: aran, btrahan, jungejason

Maniphest Tasks: T718

Differential Revision: https://secure.phabricator.com/D1292
2011-12-30 14:37:13 -08:00
epriestley
5065db5a2a Reduce spew of some daemons
Summary:
It used to be more useful for daemons to spew random debugging information, but
features like "phd debug" and some fixes to error reporting like D1101 provide
better ways to debug, test, develop and diagnose daemons.

  - Stop writing "." every time MetaMTA sends a message.
  - Stop spewing the entire IRC protocol from the IRC bot unless in debug mode.
  - Stop writing GC daemon log entries about collecting daemon logs (DURRR)
unless in debug mode.

Test Plan: Ran daemons in debug and non-debug modes, got expected level of
noisiness.

Reviewers: jungejason, nh, btrahan

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: https://secure.phabricator.com/D1268
2011-12-22 17:49:33 -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
David Reuss
5f1b3937e5 Added Outlook boundaries for email parser
Summary:
Outlook wraps a message in 5 dashes on each side when doing replies.
This strips english and danish versions.

Test Plan:
Tried parsing emails with different messages and saw the
expected behaviour with patch applied. Ran arc unit, and saw test
passed.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1239
2011-12-19 08:40:18 -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
epriestley
802dcd4cfb Add attchment support to SendGridAdapter
Summary:
  - Add attachment support for SendGrid.
  - Add attachment support to the MetaMTA test console.

Test Plan:
  - Sent myself a file with Amazon SES via test console.
  - Sent myself a file with SendGrid via test console.

Reviewers: mareksapota, jungejason, nh, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 1089
2011-11-08 14:28:10 -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
Marek Sapota
0bf2753b88 PhabricatorMailImplementationPHPMailerLiteAdapter ignores parameter in setIsHTML
function.

Summary: Fix PhabricatorMailImplementationPHPMailerLiteAdapter to actually use
given parameter.

Test Plan: Use setIsHTML with false as parameter, sent mail should be in plain
text.

Reviewers: jungejason

Reviewed By: jungejason

CC: aran, jungejason, epriestley

Differential Revision: 1001
2011-10-14 12:12:41 -07:00
epriestley
f4c8525a9a Add "Fax" content source
Summary: Can't believe I missed this.

Test Plan: !!!

Reviewers: isaac, ola, g, jungejason

Reviewed By: ola

CC: aran, ola

Differential Revision: 886
2011-09-01 12:27:45 -07:00
epriestley
83f1140785 Use text, not icons, to indicate content sources
Summary:
oh god everyone hates this

revert revert

https://www.facebook.com/photo.php?fbid=787360256660&set=p.787360256660&type=1&theater

(I left the icons themselves since I have some plans to do other things with
them.)

Test Plan: I am not good at designer

Reviewers: ola, elynde, bh, ashwin, jungejason, kdelong, zrait, tomo, aran

Reviewed By: aran

CC: aran, epriestley, tomo

Differential Revision: 885
2011-09-01 10:07:16 -07:00
epriestley
69445222f7 Track content sources (email, web, conduit, mobile) for replies
Summary:
When an object is updated, record the content source for the update. This mostly
isn't terribly useful but one concrete thing I want to do with it is let admins
audit via-email replies more easily since there are a bunch of options which let
you do hyjinx if you intentionally configure them insecurely. I think having a
little more auditability around this feature is generally good. At some point
I'm going to turn this into a link admins can click to see details.

It also allows us to see how frequently different mechanisms are used, and lets
you see if someone is at their desk or on a mobile or whatever, at least
indirectly.

The "tablet" and "mobile" sources are currently unused but I figured I'd throw
them in anyway. SMS support should definitely happen at some point.

Not 100% sure about the design for this, I might change it to plain text at some
point.

Test Plan: Updated objects and saw update sources rendered.

Reviewers: jungejason, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, epriestley, jungejason

Differential Revision: 844
2011-08-30 11:08:27 -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
2bb4cf149e Fix SendGrid adapter for HTTPSFuture changes. 2011-08-18 09:55:35 -07:00
epriestley
6dc193d3d9 Fully update library map. 2011-08-18 09:52:36 -07:00
epriestley
a67ce60d67 Unguard two external POST endpoints from CSRF. 2011-08-18 07:25:45 -07:00
epriestley
d064c6efa8 Use exceptions for S3 error messages
Summary:
Right now, the "SimpleEmailService" class uses trigger_error() to communicate
error messages. This means they get lost in the error logs and aren't visible in
the MetaMTA interface.

Provide a flag to strengthen them into exceptions, instead.

(I've attempted to emulate the prevailing style so I can offer this upstream.)

Test Plan: Faked an error condition and got a detailed stack trace in MetaMTA
instead of an empty "Message" field.
Reviewed By: jungejason
Reviewers: hunterbridges, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 783
2011-08-04 14:17:13 -07: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
epriestley
7005cac32a Explicitly test for reply-to support before using the public reply-to address
Summary: We need to perform an explicit test for public reply support.
Previously, the existence of a valid result here was a sufficient implicit test
for public reply support, but it no longer is.
Test Plan: With an unmodified configuration, sent email. It generated with the
correct reply-to (me). Restored my original configuration and sent an email, it
generated with the correct (routed) reply-to.
Reviewed By: codeblock
Reviewers: codeblock
CC: aran, codeblock
Differential Revision: 626
2011-07-08 23:27:49 -07:00
Ricky Elrod
420235f9c4 Drag-drop file upload.
Summary:
- have files be uploaded by drag+drop instead of browse.
- Files are named by their uploaded filename, the user isn't given a chance to enter a file name. Is this bad?
- Store author PHID now with files
- Allow an ?author=<username> to limit the /files/ list by author.
- If one file is uploaded, the user is taken to its info page.
- If several are uploaded, they are taken to a list of their files.

Test Plan:
- Quickly tested everything and it still worked, I'd recommend some people try this out before it gets committed though. It's a rather huge revision.

Reviewers:
epriestley, Ttech

CC:

Differential Revision: 612
2011-07-08 15:20:57 -04:00
epriestley
f9599f4499 Allow configuration of a task-creation email address
Summary: This lets you configure an email address which will create tasks when
emails are sent to it. It's pretty basic but should get us most of the way
there.
Test Plan: Configured an address and created a task via email. Replied to a task
via email to check that I didn't break that.
Reviewed By: tuomaspelkonen
Reviewers: davidreuss, jungejason, tuomaspelkonen, aran
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 590
2011-07-05 17:17:27 -07:00
epriestley
a15f07cc33 Allow Phabricator to be configured to use a public Reply-To address
Summary:
We already support this (and Facebook uses it) but it is difficult to configure
and you have to write a bunch of code. Instead, provide a simple flag.

See the documentation changes for details, but when this flag is enabled we send
one email with a reply-to like "D2+public+23hf91fh19fh@phabricator.example.com".
Anyone can reply to this, and we figure out who they are based on their "From"
address instead of a unique hash. This is less secure, but a reasonable tradeoff
in many cases.

This also has the advantage over a naive implementation of at least doing object
hash validation.

@jungejason: I don't think this affects Facebook's implementation but this is an
area where we've had problems in the past, so watch out for it when you deploy.
Also note that you must set "metamta.public-replies" to true since Maniphest now
looks for that key specifically before going into public reply mode; it no
longer just tests for a public reply address being generateable (since it can
always generate one now).

Test Plan:
Swapped my local install in and out of public reply mode and commented on
objects. Got expected email behavior. Replied to public and private email
addresses.

Attacked public addresses by using them when the install was configured to
disallow them and by altering the hash and the from address. All this stuff was
rejected.

Reviewed By: jungejason
Reviewers: moskov, jungejason, tuomaspelkonen, aran
CC: aran, epriestley, moskov, jungejason
Differential Revision: 563
2011-07-03 12:31:00 -07:00
epriestley
74d57b0a42 Use phabricator_ time functions in more places
Summary:
Replace some more date() calls with locale-aware calls.

Also, at least on my system, the DateTimeZone / DateTime stuff didn't actually
work and always rendered in UTC. Fixed that.

Test Plan:
Viewed daemon console, differential revisions, files, and maniphest timestamps
in multiple timezones.

Reviewed By: toulouse
Reviewers: toulouse, fratrik, jungejason, aran, tuomaspelkonen
CC: aran, toulouse
Differential Revision: 530
2011-06-26 10:38:25 -07:00
epriestley
c915a064a9 Include both the Gmail and "natural" Message-IDs in the "References" header
Summary:
See T251, where gregprice correctly argues that we need both:

  None of the other people on the thread will have seen that message, so it
seems
  like a lot of clients would put the server's message in a new thread. In
  general, I think you want the References: header to mention every ancestor
  message in the thread that you know about, because that's how MUAs keep a
thread
  together in the face of missing some of its messages.

Test Plan:
Sent a reply email locally, got a response with both Message-IDs in
"references".

Reviewed By: rm
Reviewers: gregprice, rm
Commenters: gregprice
CC: aran, gregprice, epriestley, rm
Differential Revision: 499
2011-06-23 10:58:52 -07:00
epriestley
d6bfdf6ce7 Carry "Message-ID" across email replies to prevent Gmail conversation splitting
Summary:
See T251. In Gmail, conversations split if you reply to them and the next email
does not "In-Reply-To" your message ID. When an action is triggered by an email,
carry its Message-ID through the stack and use it for "In-Reply-To" and
"References" on the subsequent message.

Test Plan:
Live-patched phabricator.com and replied to a Maniphest thread in Gmail without
disrupting the thread. Locally replied to Maniphest and Differential threads and
verified Message-ID was carried across the reply boundary.

Reviewed By: rm
Reviewers: tcook, jungejason, aran, tuomaspelkonen, rm
CC: aran, epriestley, rm
Differential Revision: 498
2011-06-22 14:59:40 -07:00
Andrew Toulouse
9b522982fa Add timezone support
Summary:
Allows user-configurable timezones. Adds a preference panel, and migrates to the
new date rendering in easily-modified areas of the code. ***In progress***.

Test Plan:
Check database to make sure the field is being changed when the settings are
changed; check affected views to see how they render times.

Reviewed By: epriestley
Reviewers: epriestley
CC: aran, epriestley, toulouse
Differential Revision: 475
2011-06-18 13:07:43 -07:00
epriestley
d710fc097f Improve messaging and setup process for configuring outbound email.
Summary:
- Make the instructional text generally more useful.
  - Show the current configured adapter.
  - When the configuration prevents outbound email from being delivered, show a
warning.
  - Detect 'curl' extension during setup since it's more-or-less required
  - Add curl extension to the install scripts

codeblock: can you verify the rhel-derivs changes are correct?

Test Plan:
Set adapter to test, verified warning; entered setup mode and verified curl. Ran
apt-get on an ubuntu box. Ran yum on an amazon linux box.

Reviewed By: toulouse
Reviewers: toulouse, codeblock
Commenters: codeblock
CC: aran, jungejason, tuomaspelkonen, codeblock, epriestley, toulouse
Differential Revision: 438
2011-06-12 22:40:18 -07:00
epriestley
acdb215310 Fix UTF-8 in email
Summary:
Email was not being sent with the right headers/encoding for UTF-8.

Test Plan:
Sent UTF-8 mail using SES, default and SendGrid adapters. SendGrid already
worked; SES and default share the same code so this fixes both.

Reviewed By: slawekbiel
Reviewers: slawekbiel, aran, jungejason, tuomaspelkonen
CC: aran, slawekbiel
Differential Revision: 401
2011-06-06 17:53:20 -07:00
epriestley
f505584809 Fix reply email parsing for linebreaks in "On <date>, <user> wrote:" quote
identifier

Summary:
Move the parser to a separate class so it can be easily unit tested, add some
tests. Properly parse emails with linebreaks in the quote line.

Test Plan:
Ran unit tests, used mail receiver to reply to an object.

Reviewed By: cadamo
Reviewers: aran, jungejason, tuomaspelkonen, cadamo
CC: aran, cadamo, epriestley
Differential Revision: 392
2011-06-02 16:59:17 -07:00
epriestley
8ae765f6d7 Enable SendGrid Parse API as an inbound email handler
Summary:
Sendmail is seriously difficult to configure; SendGrid is extremely easy. It's
also pretty expensive ($80/mo) but there are a bunch of startups that already
have plans so it's effectively free for them.

Test Plan:
Configured SendGrid and sent reply email through it.

Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, epriestley
Differential Revision: 376
2011-05-30 12:28:33 -07:00
epriestley
0cdf4c2518 Properly parse email replies with varied 'To' address formats
Summary:
While my client and some others send email replies with an address like
##T1+x+y@example.com##, some other clients have sent either
##<T1+x+y@example.com>## or ##"T1+x+y@example.com" <T1+x+y@example.com>##.
Properly parse all the formats we've seen in the wild.

Test Plan:
Ran the regexp against all the formats observed in the wild (see
https://secure.phabricator.com/mail/received/) and verified it parses them
correctly.

Reviewed By: jungejason
Reviewers: tuomaspelkonen, jungejason, aran
CC: anjali, aran, jungejason
Differential Revision: 370
2011-05-30 00:38:01 -07:00
epriestley
e88d187362 Resolve some complete nonfunctionality in SendGrid adapter. 2011-05-27 16:22:05 -07:00
epriestley
dbedb012eb Add support for SendGrid as an outbound mail adapter
Summary: SendGrid is a popular mail delivery platform, similar to Amazon SES. Provide support for delivering email via their REST API.

Test Plan: Created a SendGrid account, configured my local install to use it, sent some mail, received mail.

Reviewers: tuomaspelkonen, jungejason, aran

CC: ccheever

Differential Revision: 347
2011-05-27 09:27:54 -07:00
epriestley
8fc79035b6 Provide an "algorithm" for stripping quoted text from email replies
Summary:
There's an undoubtedly-far-more-refined version of this in xmail if someone
wants to crib it for me. Otherwise we can anneal this as counterexamples arise.

This seems to be what mail.app and gmail do.

Test Plan:
Used mail receiver console to "send" some mail and verified it was correctly
truncated.

Reviewed By: jungejason
Reviewers: aran, tuomaspelkonen, jungejason
CC: aran, jungejason
Differential Revision: 290
2011-05-17 13:47:57 -07:00
epriestley
a69f217f98 Make reply-to fully work in Maniphest and Differential for open source
Phabricator

Summary:
Hook up the last pieces. This shouldn't impact the Facebook install, EXCEPT that
I removed "!accept" and added "!rethink" (plan changes). If you want to continue
supporting !accept, you should override the method in your subclass if you don't
already.

Test Plan:
Used the Mail Receiver test console to send mail to tasks and revisions.

Reviewed By: aran
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran
Differential Revision: 289
2011-05-16 15:34:11 -07:00
epriestley
f9f8ef0e6e Admin and disabled flags for users
Summary:
Provide an "isAdmin" flag for users, to designate administrative users.

Restore the account editing interface and allow it to set role flags and reset
passwords.

Provide an "isDisabled" flag for users and shut down all system access for them.

Test Plan:
Created "admin" and "disabled" users. Did administrative things with the admin
user. Tried to do stuff with the disabled user and was rebuffed. Tried to access
administrative interfaces with a normal non-admin user and was denied.

Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran
CC: ccheever, aran
Differential Revision: 278
2011-05-12 11:17:50 -07:00
epriestley
03b56c1035 Explicitly list To and Cc in multiplexed email
Summary:
When we multiplex email, add information to the body with an explicit list of
recipients. Also add some headers if people want to write mail rules.

Test Plan:
Commented on a task and a revision, got reasonable looking emails about them.

Reviewed By: jungejason
Reviewers: aran, jungejason, tuomaspelkonen
CC: aran, jungejason, epriestley
Differential Revision: 272
2011-05-12 11:14:28 -07:00
epriestley
71efb46ba7 Support email multiplexing for private Reply-To addresses
Summary:
Provide a base PhabricatorMailReplyHandler class which handles the plumbing for
multiplexing email if necessary and supporting public and private reply handler
addressses. DifferentialReplyHandler now extends it, and a new
ManiphestReplyHandler also does.

The general approach here is that we have three supported cases:

  - no reply handler, default config, same as what we're doing now
  - public reply handler, requires overriding classes but just sets "reply-to"
to some address the install generates and still sends only one email
  - private reply handler, provides a default generation mechanism or you can
override it and splits mail apart so we send one to each recipient

Test Plan:
Sent email from Maniphest and Differential with and without
reply-handler-domains set.

Reviewed By: aran
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley
Differential Revision: 254
2011-05-11 20:21:57 -07:00
epriestley
a64d5177a7 Spoof usernames with Amazon SES
Summary:
When I tested this earlier I was incorrectly interpreting PHPMailer errors as
SES errors. This works fine as long as you get around the peculiarities of
PHPMailer.

Test Plan:
Sent email to myself, received email from a human-readable address in my mail
client.

Reviewed By: rm
Reviewers: rm, tuomaspelkonen, jungejason, aran
CC: aran, rm, epriestley
Differential Revision: 246
2011-05-10 16:12:28 -07:00
jungejason
4a76174118 Adding paging to transcript pages
Summary:
enable paging by adding a AphrontPagerView to the two
transcript page.

Test Plan:
test the paging on both pages. Also test it with a certain
phid is given.

Reviewed By: epriestley
Reviewers: epriestley, tuomaspelkonen
CC: epriestley, jungejason
Differential Revision: 116
2011-05-08 15:59:39 -07:00
epriestley
25dee6ecd2 Support email replies in Phabricator
Summary:
Provides support for per-user x per-object unique reply-to email addresses, plus
SMTP integration.

This does not actually make Phabricator use these in outbound email.

Test Plan:
Used test console to validate in-Phabricator routing and handling.

Piped emails into the "mail_handler.php" script to validate mail parsing.

Configured sendmail and sent mail to Phabricator.

Technically I haven't conducted all parts of this test on the same machine since
I lost the will to configure more SMTP servers after configuring phabricator.com

Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley, jungejason
Differential Revision: 226
2011-05-05 14:58:57 -07:00
epriestley
6bec3d2e4f Simplify and demuddle MetaMTA send pathways
Summary:
I pretty shortsightedly made sending a side effect of save() in the case that a
server is configured for immediate sending. Move this out, make it explicit, and
get rid of all the tangles surrounding it.

The web tool now ignores the server setting and only repsects the checkbox,
which makes far more sense.

Test Plan:
Sent mails from Maniphest, Differential, and the web console. Also ran all the
unit tests. Verified headers from Maniphest.

Reviewed By: rm
Reviewers: aran, rm
CC: tuomaspelkonen, rm, jungejason, aran
Differential Revision: 200
2011-05-02 03:07:30 -07:00
epriestley
72e33c9e5a Fix a threading issue with Amazon SES
Summary:
Amazon SES does not allow us to set a Message-ID header, which means
that threads are incorrect in Mail.app (and presumably other applications
which respect In-Reply-To and References) because the initial email does not
have anything which attaches it to the rest of the thread. To fix this, never
rely on Message-ID if the mailer doesn't support Message-ID.

(In the Amazon SES case, Amazon generates its own Message-ID which we can't
know ahead of time).

I additionally used all the Lisk isolation from the other tests to make this
testable and wrote tests for it.

I also moved the idea of a thread ID lower in the stack and out of
DifferentialMail, which should not be responsible for implementation details.

NOTE: If you push this, it will cause a one-time break of threading for
everyone using Outlook since I've changed the seed for generating Thread-Index.
I feel like this is okay to avoid introducing more complexity here.

Test Plan:
Created and then updated a revision, messages delivered over Amazon
SES threaded correctly in Mail.app. Verified headers. Unit tests.

Reviewed By: rm
Reviewers: aran, tuomaspelkonen, jungejason, rm
Commenters: aran
CC: aran, rm, epriestley
Differential Revision: 195
2011-04-30 22:26:07 -07:00
jungejason
cca849c762 Fix 'Transcript' links in Differential
Summary:
add filtering for MetaMTA transcripts, add Herald
transcripts, also fixed PhabricatorObjectHandleData to support commits.
Note that paging in the transcripts pages will be in a different diff.

Test Plan:
test the transcripts for both MetaMTA and Herald.

Reviewed By: epriestley
Reviewers: epriestley, tuomaspelkonen
CC: jungejason, epriestley
Differential Revision: 114
2011-04-08 01:10:05 -07:00
epriestley
361ec78b03 Add missing includes from XHPAST parse bug. 2011-04-06 23:14:58 -07:00
epriestley
cc66c4890d Commit + Herald integration. 2011-04-04 14:13:14 -07:00
jungejason
e4fa1523bd Use PhabricatorDaemon for sending emails.
Summary:
Make PhabricatorMetaMTADaemon extend PhabricatorDaemon.

Test Plan:
send mail with the new daemon.

Reviewed By: epriestley
Reviewers: epriestley
CC: epriestley
Differential Revision: 74
2011-03-17 18:40:17 -07:00
jungejason
9bc04fe03d Change hard-coded PHID types to constants.
Summary:
add a constants module
src/applications/phid/constants/PhabricatorPHIDConstants.

Test Plan:
Execute applications which were using the hard-coded string.

Differential Revision: 44
Reviewed By: epriestley
Reviewers: epriestley
CC: epriestley
2011-03-03 12:00:53 -08:00
epriestley
33a9019a89 Improve availability of MetaMTA debugging info.
Summary:

Test Plan:

Reviewers:

CC:
2011-02-10 16:19:17 -08:00
epriestley
193dbf16b4 Very basic daemon infrastructure, plus MetaMTA daemon.
Summary: Amazon SES seems to be working well, except that it takes more than a
second to send mail in-process. Kick it out of process. (Between this and the
ImplementationAdapter layer, MetaMTA almost makes sense. :/)

Test Plan: Ran the daemon and got a flood of unsent test email.

Reviewers:

CC:
2011-02-09 17:39:55 -08:00
epriestley
b7801c44fe Don't double-generate message IDs.
Summary:

Test Plan:

Reviewers:

CC:
2011-02-09 16:46:33 -08:00
epriestley
8347729fc0 When sending with Amazon SES, use the AWS sendRawEmail API instead of sendEmail,
so we can send custom headers.

Summary:

Test Plan:

Reviewers:

CC:
2011-02-09 16:05:06 -08:00
epriestley
616c22eae2 I think this improves email a good deal, although it's hard to really test it
since Exchange has been down for like 30 minutes now. Push & Pray!

Summary:

Test Plan:

Reviewers:

CC:
2011-02-09 11:11:24 -08:00
epriestley
8098954776 Add basic Amazon SES support.
Summary:

Test Plan:

Reviewers:

CC:
2011-02-07 21:08:32 -08:00
epriestley
9dac0ed9f1 Bring in JX.Workflow and the inline commenting behavior, plus sync Javelin. 2011-02-01 15:52:04 -08:00
epriestley
4faad5b3f1 Add a default-send behavior to MetaMTA. 2011-01-31 21:13:38 -08:00
epriestley
29f7219a49 CSRF / Logout 2011-01-30 18:52:29 -08:00
epriestley
d37692ac06 Set 'From' correctly in MetaMTA. 2011-01-26 16:02:12 -08:00
epriestley
ccf7df6093 Authentication 2011-01-26 15:34:20 -08:00
epriestley
336a8b3212 Mailing lists 2011-01-26 11:30:23 -08:00
epriestley
2112673f06 PHPMailerLite 2011-01-26 09:57:33 -08:00
epriestley
a6274d9270 MetaMetaView 2011-01-26 09:20:09 -08:00
epriestley
bc57b12ef0 MetaMTA 2011-01-26 08:00:52 -08:00