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
Test Plan:
Created a listener that adds some patterns to $matches array, reloaded
Differential, some changesets were not shown as generated.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley, mareksapota
Differential Revision: 1200
Summary: There was some documentation for this but it was kind of buried in a
random, difficult-to-discover file. Separate it into its own file and link to it
from the previous location.
Test Plan: Regenerated documentation and read through it without catching
anything terrible.
Reviewers: btrahan
Reviewed By: btrahan
CC: zeeg, aran, btrahan
Maniphest Tasks: T643
Differential Revision: 1161
Summary:
- Update documentation for changes in D1148.
- Link to Remarkup documentation from Maniphest.
- Support "Note:" syntax in Phabricator (previously, it was only supported in
Diviner, but I've found it pretty good and useful).
Test Plan: Regenerated and perused documentation; made a "NOTE:".
Reviewers: btrahan, broofa, fugalh, jungejason, nh, aran
Reviewed By: btrahan
CC: aran, btrahan
Differential Revision: 1149
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
Summary:
This is an attempt to satisfy a lot of the one-off requests a little more
generally, by providing a relatively generic piece of event architecture.
Allow the registation of event listeners which can react to various application
events (currently, task editing).
I'll doc this a bit better but I wanted to see if anyone had massive objections
to doing this or the broad approach. The specific problem I want to address is
that one client wants to do a bunch of routing for tasks via email, so it's
either build a hook, or have them override most of ManiphestReplyHandler, or
something slightly more general like this.
Test Plan: Wrote a silly listener that adds "Quack!" to a task every time it is
edited and edited some tasks. I was justly rewarded.
Reviewers: nh, jungejason, tuomaspelkonen, aran
Reviewed By: aran
CC: aran, epriestley
Differential Revision: 881
Summary: Explains how to use the immutable history doctrine and mercurial.
Recommends "one idea is one commit".
Test Plan: Read documentation.
Reviewers: fratrik, Makinde, aran, jungejason, tuomaspelkonen, cpiro
Reviewed By: cpiro
CC: aran, cpiro, epriestley, ide
Differential Revision: 861
Summary: Feedback from @makinde. These are easy (and necessary) to configure so
we might as well give the user a heads up.
Test Plan: Regenerated the documentation and read "Configuration Guide".
Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran
Reviewed By: Makinde
CC: aran, Makinde
Differential Revision: 929
Summary:
This is pretty straightforward, except:
- We need to request read/write access to the address book to get the account
ID (which we MUST have) and real name, email and account name (which we'd like
to have). This is way more access than we should need, but there's apparently no
"get_loggedin_user_basic_information" type of call in the Google API suite (or,
at least, I couldn't find one).
- We can't get the profile picture or profile URI since there's no Plus API
access and Google users don't have meaningful public pages otherwise.
- Google doesn't save the fact that you've authorized the app, so every time
you want to login you need to reaffirm that you want to give us silly amounts of
access. Phabricator sessions are pretty long-duration though so this shouldn't
be a major issue.
Test Plan:
- Registered, logged out, and logged in with Google.
- Registered, logged out, and logged in with Facebook / Github to make sure I
didn't break anything.
- Linked / unlinked Google accounts.
Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran
Reviewed By: aran
CC: aran, epriestley, Makinde
Differential Revision: 916
Summary: If the link text is a URI, just treat it as a nameable (and possibly
relative) URI link. See tasks.
Test Plan: Copy/pasted the doc example into Phriction, links worked.
Reviewers: skrul, hunterbridges, jungejason, tuomaspelkonen, aran
Reviewed By: jungejason
CC: aran, jungejason
Differential Revision: 882
Summary: "set -e" causes the script to exit if any command returns nonzero.
Without it, we incorrectly discard the failure code. At Facebook everything runs
in "set -e" or some equivalent so I never picked this up in testing.
Test Plan:
Added the hook to my svn local, it blocked bad commits and allowed good ones.
>>> orbital:~/devtools/svnroot $ svn commit -m "quaa"
Sending test.php
Transmitting file data .svn: Commit failed (details follow):
svn: Commit blocked by pre-commit hook (exit code 1) with output:
LINT ERRORS
This changeset has lint errors. You must fix all lint errors before you can
commit.
You can add '@bypass-lint' to your commit message to disable lint checks for
this commit, or '@nolint' to the file with errors to disable lint for that
file.
>>> Lint for test.php:
Error (XHP1) PHP Syntax Error!
This file contains a syntax error: XHPAST Parse Error: syntax error,
unexpected '}' on line 1
>>> 1 <?php asdddddd;!}}
Reviewers: svemir, jungejason, tuomaspelkonen, aran
Reviewed By: svemir
CC: aran, svemir, epriestley
Differential Revision: 864
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
Summary:
- Fix a bug where 'caption' didn't do anything.
- Provide an abstract base implementation for extensions.
- Add some documentation.
- Expose aux fields via conduit.
Test Plan: Added some fields like "Dinosaur", "Kilograms" and "derp" on my local
install. Read documentation.
Reviewed By: jungejason
Reviewers: hunterbridges, jungejason, tuomaspelkonen, aran
CC: aran, philc, jungejason
Differential Revision: 785
Summary:
Provide a {T123} syntax which pulls in the entire name of an object, not just a
link to it. A major use for this is organizing projects using wiki pages. Since
handle links show object status now, this lets you organize stuff in an ad-hoc
way and get a reasonable overview of it. We can make handles richer in the
future, too.
The performance on this isn't perfect (it adds some potential single gets) but I
think it's okay for now and I don't want to make remarkup engine even more
complex until the preprocess/postprocess stuff has had a chance to settle and
I'm more confident it works.
In Differential and Maniphest we'll also incorrectly cache the object
state/name, but that'll fix itself once I move the cache code to use
preprocess/postprocess correctly.
Test Plan:
- See https://secure.phabricator.com/file/view/PHID-FILE-5f9ca32407bec20899b9/
for an example.
- Generated and looked over the documentation.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran, hunterbridges
CC: skrul, aran, jungejason, epriestley
Differential Revision: 784
Summary:
See D431, where I promised to document this like 2 months ago. Document that:
- Everything should be UTF-8.
- ASCII is recommended.
- How to identify and repair files which aren't valid UTF-8.
- What to do if you're using some other encoding.
Test Plan: Generated and read documentation.
Reviewed By: codeblock
Reviewers: edward, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, codeblock
Differential Revision: 776
Summary: This syntax is somewhat obscure and awkward, try to document it more completely in the location users are most likely to run into it for the first time.
Test Plan: Generated documentation: https://secure.phabricator.com/file/view/PHID-FILE-ba958fd365e5312d9077/
Reviewers: codeblock, egillth, jungejason, tuomaspelkonen, aran
CC:
Differential Revision: 775
query
Summary:
- Provide an example unit test, and document it.
- Document database isolation better.
- When we issue an unsimulated query to the isolated connection, throw a
helpful message.
- Pygments is complaining about my madeup "lang=demo", change it to
"lang=text".
Test Plan:
- Ran the unit test (sanity check).
- Ran all other unit tests (verify I didn't break isolation).
- Added a queryfx(..., 'SELECT 1') to a test and verified it throws.
- Read the documentation.
Reviewed By: edward
Reviewers: edward, jungejason, tuomaspelkonen, aran
CC: aran, edward
Differential Revision: 773
Summary: This needs work but is better than nothing.
Test Plan: Generated and read documentation, clicked links.
Reviewed By: edward
Reviewers: edward
CC: aran, edward
Differential Revision: 765
Summary:
See T344. Currently, there's a hard-coded 12MB filesize limit and some awkward
interactions with MySQL's max_allowed_packet. Make this system generally more
robust:
- Move the upload limit to configuration.
- Add setup steps which reconcile max_allowed_packet vs MySQL file storage
limits.
- Add a layer of indirection between uploading files and storage engines.
- Allow the definition of new storage engines.
- Define a local disk storage engine.
- Add a "storage engine selector" class which manages choosing which storage
engines to put files in.
- Document storage engines.
- Document file storage classes.
Test Plan:
Setup mode:
- Disabled MySQL storage engine, misconfigured it, configured it correctly.
- Disabled file storage engine, set it to something invalid, set it to
something valid.
- Verified max_allowed_packet is read correctly.
Application mode:
- Configured local file storage.
- Uploaded large and small files.
- Verified larger files were written to local storage.
- Verified smaller files were written to MySQL blob storage.
Documentation:
- Read documentation.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley, jungejason
Differential Revision: 695
Summary: See task and P91, this just adapts my Discussion post to be more
general. I'll follow up by linking to it from the Arcanist error message.
Test Plan: Read the document.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 701
Summary:
Single brackets are getting some troublesome false positives in Facebook's
install. Particularly, there's a weird convention at Facebook of tagging diffs
by putting stuff like "[perf]" or "[chat]" in the title, although this isn't
turned into structured data at any stage. When commits appear in Diffusion, we
currently link such ad-hoc tags to Phriction.
Wikipedia uses double-bracket sytnax, as do many other wikis, so this seems like
a reasonable burden to place on the lightweightness of the markup. The
alternative is selectively disabling Phriction markup in some interfaces, but
I'd rather allow integration in commit messages and just guard the syntax more
closely.
(I'm not providing any sort of migration plan since this landed less than a week
ago and I'm pretty confident no one has built a huge wiki yet, but I added a
CHANGELOG note.)
Test Plan: Edited a wiki document and added some links. Verified single brackets
were unlinked and double brackets were linked.
Reviewed By: jungejason
Reviewers: hsb, aran, jungejason, tuomaspelkonen
CC: aran, jungejason, epriestley
Differential Revision: 689
Summary:
A few minor things:
- Add Phriction to the homepage.
- Link to remarkup reference on the edit screen.
- Add a help tab with a super basic help document.
Test Plan: Cursory inspection.
Reviewed By: codeblock
Reviewers: hsb, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, epriestley, codeblock
Differential Revision: 655
Summary: Port slowvote. This has some style/layout roughness but gets us most of
the way there. I'll followup to fix some of the markup issues.
Test Plan: Created and voted in several different kinds of poll.
Reviewed By: codeblock
Reviewers: codeblock, tomo, jungejason, aran, tuomaspelkonen
Commenters: aran, jungejason
CC: aran, codeblock, jungejason, epriestley
Differential Revision: 613
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
Summary:
Phabricator generates a bunch of data that we don't need to keep around forever,
add a GC daemon to get rid of it with some basic configuration options.
This needs a couple more diffs to get some of the details but I think this is a
reasonable start.
I also fixed a couple of UI things related to this, e.g. the daemon logs page
going crazy when a daemon gets stuck in a loop and dumps tons of data to stdout.
Test Plan:
- Ran gc daemon in 'phd debug' mode and saw it delete stuff, then sleep once
it had cleaned everything up.
- Mucked around with TTLs and verified they work correctly.
- Viewed gc'd transcripts in the web interface and made sure they displayed
okay.
- Viewed daemon logs before/after garbage collection.
- Running some run-at / run-for tests now, I'll update if the daemon doesn't
shut off in ~10-15 minutes. :P
Reviewed By: tuomaspelkonen
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 583
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:
Make it slightly more clear how to go about this.
Test Plan:
Generated and reviewed the documentation.
Reviewed By: moskov
Reviewers: moskov, aran, jungejason, tuomaspelkonen
CC: aran, moskov
Differential Revision: 467
Summary:
Add a note on configuring Lamson to the configuring inbound email article.
Test Plan:
Generated and proofread the documentation.
Reviewed By: epriestley
Reviewers: epriestley
CC: aran, epriestley
Differential Revision: 442
Summary:
Point users at the install scripts. Fix some common points of confusion and
update some other documentation points.
The links point to a directory which ultimately serves these scripts out of the
git checkout on the machine:
http://phabricator.com/rsrc/install/install_rhel-derivs.shhttp://phabricator.com/rsrc/install/install_ubuntu.sh
Test Plan:
Read documentation.
Reviewed By: aran
Reviewers: codeblock, jungejason, tuomaspelkonen, aran
Commenters: jungejason
CC: aran, epriestley, jungejason
Differential Revision: 439
Summary:
You don't need these anymore after D444.
Test Plan:
Generated documentation.
Reviewed By: aran
Reviewers: toulouse, jungejason, tuomaspelkonen, aran
CC: aran
Differential Revision: 445
Summary:
Make it easier to find obvious problems in daemons by letting them run
undaemonized in the console without requiring the user to know the magical
incantations for loading libraries, etc.
Test Plan:
Ran "phd debug nice", simulated some failures (e.g., bringing down Phabricator,
daemon fatal) and got useful error messages.
Reviewed By: jungejason
Reviewers: toulouse, jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 448
Summary:
Add a section about image macros to the documentation for Remarkup
Test Plan:
Read the text
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 435
Summary:
This is not obvious and worth calling out explicitly.
Test Plan:
Generated and proofread the documentation.
Reviewed By: toulouse
Reviewers: toulouse
CC: aran, toulouse
Differential Revision: 437
Summary: Normally this gives you a prompt about taking down services, provide a noninteractive mode for scripting the upgrade process.
Also drop a generally bad/confusing/irrelevant piece of advice from the documentation and replace it with information about -f.
Test Plan: Ran with and without -f. Ran with -h.
Reviewers: moskov, tuomaspelkonen, jungejason, aran
CC:
Differential Revision: 387
Summary:
Allows you to link to comments with "D123#3" or "T123#3", then adds a pile of JS
to try to make it not terrible. :/
The thing I'm trying to avoid here is when someone says "look at this!
http://blog.com/#comment-239291" and you click and your browser jumps somewhere
random and you have no idea which comment they meant. Since I really hate this,
I've tried to avoid it by making sure the comment is always highlighted.
Test Plan:
Put T1#1 and D1#1 in remarkup and verified they linked properly.
Clicked anchors on individual comments.
Faked all comments hidden in Differential and verified they expanded on anchor
or anchor change.
Reviewed By: aran
Reviewers: aran, tomo, mroch, jungejason, tuomaspelkonen
CC: aran, epriestley
Differential Revision: 383
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
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
Summary:
GitHub needs an unguessably specific URI in its configuration for OAuth.
Test Plan:
Regenerated documentation and read it over. (These instructions are based on the
existing applications, which appear to function correctly.)
Reviewed By: j3kuntz
Reviewers: jungejason, aran, tuomaspelkonen, j3kuntz
CC: aran, epriestley, j3kuntz
Differential Revision: 307
Summary:
There was an old "create_user.php" script but it really was only useful for
creating agents. Provide a more user-friendly script for creating the first
account.
Depends on D278.
Test Plan:
Used 'accountadmin' to create and edit accounts. Read documentation.
Reviewed By: tuomaspelkonen
Reviewers: jungejason, tuomaspelkonen, aran
CC: ccheever, aran, tuomaspelkonen
Differential Revision: 279
Summary:
ccheever did an install and gave me some feedback about issues he hit. This
tries to:
- properly document how to configure outbound email;
- test outbound email configuration in the setup mode;
- provide basic daemon documentation;
- document that phabricator.base-uri is required for all installs.
Test Plan:
read documentation, jumped through all the setup branches to test configuration
error detection
Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran, rm
CC: ccheever, aran
Differential Revision: 276
Summary:
Alters the installation instructions to guide installers into a "setup" mode
which does config file sanity checking.
Test Plan:
Put myself in setup mode, simulated all the failures it detects, took myself out
of setup mode, Phabricator works OK.
Reviewed By: tuomaspelkonen
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 230
Summary:
After rP2a39fd09ebe7f4fc8cd2ab0b39bbb0e466f357c3, you need to run this command
after cloning Phabricator.
Git, world's hardest revision control system
Test Plan:
read text
Reviewed By: rm
Reviewers: tomo, tuomaspelkonen, jungejason, aran, rm
CC: aran, rm, epriestley
Differential Revision: 252
Summary:
Sendmail isn't actually OK with passing ENV stuff via 'aliases', accept it as an
argument instead.
Test Plan:
Sent real email to a real server, got differential updates!
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 233
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
Summary:
Orient potential contributors to stuff they should read first, the Facebook CLA,
how they can get started, and the general philosophy of the project.
Test Plan:
read the document
Reviewed By: aran
Reviewers: aran, jungejason, tuomaspelkonen, davidrecordon
Commenters: davidrecordon
CC: aran, epriestley, davidrecordon
Differential Revision: 208
Summary:
Some users have had problems with the database initialization process, simplify
it by creating a new "initialize.sql" dump at v34.
I also populated this dump with the right landing screen (so all the tools
actually have links) and a default avatar.
Test Plan:
Dropped all databases, initialized according to documentation, ended up in a
good state with sensible defaults.
Reviewed By: jungejason
Reviewers: aran, jungejason, tuomaspelkonen
CC: aran, jungejason
Differential Revision: 210
Summary:
In a basically reasonable configuration where you connect
with a non-privileged user from the web workflow, upgrade_schema.php
won't have enough privileges. Allow the user to override the normal
auth with -u and -p.
Test Plan:
Tried to do a schema upgrade with an underprivileged user,
got a useful error message instead of garbage.
Reviewed By: Girish
Reviewers: Girish, davidrecordon, jungejason, tuomaspelkonen, aran
CC: aran, epriestley, Girish
Differential Revision: 191
Summary:
Documentation describes how to use the script to upgrade schema.
Test Plan:
Generated the documentation and it looked good.
Reviewed By: epriestley
Reviewers: epriestley
CC: jungejason, epriestley
Differential Revision: 124
Summary:
add the conduit URI and the username together with the arc
certificate to the setting page.
Test Plan:
run arc diff to make sure it still works after copying the
generated test into the .arcrc file.
Reviewed By: epriestley
Reviewers: epriestley
CC: epriestley
Differential Revision: 73
Summary:
Update documentation to mention the need to apply sql patches.
Task ID: #
Test Plan:
No
Reviewed By: epriestley
Reviewers: epriestley
CC: epriestley
Revert Plan:
OK
- begin *PUBLIC* platform impact section -
Bugzilla: #
- end platform impact -
Differential Revision: 52
