Summary: Facebook extends this, just unfinal it for now and we can figure out the right long-term fix at our leisure.
Test Plan: Inspection.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1917
Summary: No big surprises here, delted the unused "DarkConsole" class.
Test Plan: Ran 'testEverythingImplemented' to verify I wasn't finalizing anything we extend.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T795
Differential Revision: https://secure.phabricator.com/D1876
Summary:
Commenting on a diff causes adding the writer to the CCs. It doesn't make much
sense if the writer is author or reviewer who get all the copies anyway.
I've also moved the decision to DifferentialCommentEditor.
Test Plan:
Comment on a diff where I am author
Comment on a diff where I am reviewer
Comment on a diff where I am neither
Explicitely Add CCs where I am author
Reviewers: epriestley
Reviewed By: epriestley
CC: jungejason, aran, epriestley
Differential Revision: https://secure.phabricator.com/D1397
Summary: For reasons explained in the config I've omitted this from the default
action set, but it's trivial to support it. See D916.
Test Plan: Commented on a revision, was informed I could "!accept" in the email.
Used "!accept" to accept the revision.
Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran
Reviewed By: Makinde
CC: aran, Makinde
Differential Revision: 928
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
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
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
Summary:
See task. Allows users to unsubscribe via email.
Test Plan:
Used mail receiver to unsubscribe from a revision. Tested subscribe/unsubscribe
buttons. Verified "!unsubscribe" appears as an avilable action in email.
Reviewed By: ola
Reviewers: ola, mroch, jungejason, tuomaspelkonen, aran
CC: aran, epriestley, ola
Differential Revision: 385
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
Summary:
D254 removed DifferentialReplyHandler::getRevision(), but
is still using it in two places. Correct them.
Test Plan:
send email email handler and verified it works.
Reviewed By: epriestley
Reviewers: tuomaspelkonen, epriestley
CC: aran, epriestley
Blame Revision:
D254
Differential Revision: 277
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
Summary:
add email reply handler so that the user can reply to a
differential email to act on the revision. It generates the reply-to
email address, creates email body text with supported commands list, and
handle the action request on the differential revision.
Right now the reply-to handing is disabled in the config file. But a
site using Phabricator can enable it and implement a class
inheriting from DifferentialReplyHandler to enable customized email
handing.
Later we will need to add code to DifferentialMail.php to support
sending separate email to each email recipient to achieve better
security (see D226). The reply-to will be something like
D<revision_id>+<user_id>+<hash>@domain.com. We will create separate task
for it.
Test Plan:
tried comment on a revision from web UI and the email was
sent out as before without any change. When a subclass of
DifferentialReplyHandler is implemented and enabled, email's reply-to is
set and email text is added. Reply to the email with valid command did
create action to the revision.
Reviewed By: epriestley
Reviewers: tuomaspelkonen, epriestley, slawekbiel, dpepper
CC: aran, epriestley, jungejason
Differential Revision: 224
