Protect file data with a one-time-token

Test Plan: currently untested work in progress

Reviewers: #blessed_reviewers, epriestley

Subscribers: rush898, aklapper, Korvin, epriestley

Projects: #wikimedia

Maniphest Tasks: T5685

Differential Revision: https://secure.phabricator.com/D10054
Add Phund app icon

Summary: Super important. This single icon unlocks untold riches and tens of corporate dollars.

Test Plan: Photoshop

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10219
Show file cacheability in Files application

Summary: Ref T5685. We've added a new `canCDN` flag to control whether or not files can be cached and delivered over a CDN. Show this flag in the UI.

Test Plan: Viewed several files, saw correct/expected UI values.

Reviewers: btrahan, 20after4

Reviewed By: 20after4

Subscribers: epriestley

Maniphest Tasks: T5685

Differential Revision: https://secure.phabricator.com/D10213
Generate a 403 page with a nice dialog when a file token is invalid

Summary:
Ref T5685. Currently we just 403 on an invalid token, but we can be a little more helpful.

The issues here are:

  - If we **do** redirect you on this page and something goes wrong, you might get stuck in a redirect loop.
  - If we **don't** redirect you, copy/pasting the link to someone (or reloading the page) gives them a pretty confusing result, since the link doesn't work any more. Prior to this diff, they get a 403.

To mitigate this, do a little better than a bare 403: give them a link to auth and generate a new URI for the file.

If this is still confusing, the next best thing I can come up with is something like this:

  - Put some modulous of the timestamp in the URI.
  - If the current time is within 2 seconds of the generation time, show this dialog.
  - Otherwise, redirect.

That seems like it would be okay, but I worry that "2" has to be small (so links you copy/paste -> chat -> click still work) and a small value means that a small amount of clock skew breaks things. We could use the database clock, but ehhh.

Other ideas:

  - Put a hash of the remote IP in the URI, redirect if it doesn't match. Fails for companies behind a NAT gateway but should work in a lot of other cases.
  - Just redirect always, there's no reason it should ever loop and browsers don't really do anything bad when there's a loop (they'll show an error after too many redirects).

I'm leaning toward letting this stabilize in the wild for a bit, then trying "always redirect".

Test Plan: {F188914}

Reviewers: btrahan, 20after4

Reviewed By: 20after4

Subscribers: epriestley

Maniphest Tasks: T5685

Differential Revision: https://secure.phabricator.com/D10215
Consolidate handling of special properties for newly uploaded files

Summary:
Fixes T5849. When a new file is created, we might have to actually write the data to a storage engine, or we might be able to just point at data which is already there.

Currently, these two paths handle `$params` with different code and mild behavioral differences. Instead, have them call the same code so they get the same behavior.

Test Plan:
  - Uploaded the same file multiple times to home page.
  - Uploaded the same file multiple times as profile picture.
  - Generated files via Diffusion.
  - All the files got the expected properties, whether they were reusing data or not.

Reviewers: btrahan, 20after4

Reviewed By: 20after4

Subscribers: epriestley

Maniphest Tasks: T5849

Differential Revision: https://secure.phabricator.com/D10216
Make payment method management somewhat more reasonable in Phortune

Summary: Ref T2787. Shows somewhat-useful information, allows payment methods to be disabled and renamed.

Test Plan: Created, renamed, disabled payment methods.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2787

Differential Revision: https://secure.phabricator.com/D10203
Fix string construction in Conduit exceptions

Summary:
Fixes T5838.

  - We currently try to use a `ConduitAPIMethod` object as a string.
  - We then pass that string to the parent's `__construct()` method as `$message`.

Test Plan: Uninstalled Maniphest, then tried to execute `maniphest.createtask`. Got a useful exception message instead of an error during message construction.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5838

Differential Revision: https://secure.phabricator.com/D10211
Fix several issues with application interactions while importing commits

Summary:
  - Fixes T5851. Currently, if a commit has `Fixes T123`, we generate an email with just that before generating the commit email. Don't send/publish transactions about a commit before it imports (this is a tiny bit hacky, but well-contained and I don't think it causes any problems).
  - Fixes T4864. Currently, we try to parse Differential information even if Differential is not installed. Instead, do this only if Differential is installed.
  - Fixes T5771. Currently, if we can't figure out who the committer/author of a commit is, we don't publish a `Fixes T123` transaction. Instead, fall back to acting as "Diffusion" if we can't find a better actor. Most of this diff expands the role of application actors. The existing application actors (Herald and Harbormaster) seem to be working well.

Test Plan:
  - Pushed a commit with `Fixes T123` and verified it did not generate email directly. (The task half of the transaction still does, correctly.)
  - Uninstalled Differential and pushed a commit, got a clean import instead of an exception.
  - Commented out author/committer PHIDs and pushed stuff, saw a "Diffusion" actor.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5771, T4864, T5851

Differential Revision: https://secure.phabricator.com/D10221
Show a very basic purchase history in Phortune

Summary: Ref T2787. This is very basic and just helps me know that the data is inserting correctly.

Test Plan: {F187765}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2787

Differential Revision: https://secure.phabricator.com/D10205
Don't send reset links to unverified addresses on accounts with verified addresses

Summary:
Via HackerOne. If a user adds an email address and typos it, entering `alinculne@gmailo.com`, and it happens to be a valid address which an evil user controls, the evil user can request a password reset and compromise the account.

This strains the imagination, but we can implement a better behavior cheaply.

  - If an account has any verified addresses, only send to verified addresses.
  - If an account has no verified addresses (e.g., is a new account), send to any address.

We've also received several reports about reset links not being destroyed as aggressively as researchers expect. While there's no specific scenario where this does any harm, revoke all outstanding reset tokens when a reset link is used to improve the signal/noise ratio of the reporting channel.

Test Plan:
  - Tried to send a reset link to an unverified address on an account with a verified address (got new error).
  - Tried to send a reset link to a verified adddress on an account with a verified address (got email).
  - Tried to send a reset link to an invalid address (got old error).
  - Tried to send a reset link to an unverified address on an account with only unverified addresses -- a new user (got email).
  - Requested several reset links, used one, verified all the others were revoked.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10206
Minor, fix one more Actor -> Acting As PHID

Summary: See D10221, I missed one of these callsites.

Auditors: btrahan
Rename Lisk internal getProperties() methods to avoid collisions

Summary:
See some discussion here:

https://github.com/phacility/phabricator/commit/24a6eeb8d8a52b592e456d110934670d4ace9b31#commitcomment-7334892

The `protected $properties;` storage parameter added to `ProjectColumn` is shadowed by `getProperties()` in the base class.

Although this works correctly for me, it's ambiguous and worth fixing. Make the base class methods explicit.

Test Plan: Used `grep` to find callers for both methods and renamed them.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10210
Move object monogram rules later in the parse order

Summary:
Fixes T5837. The problem is that the hash is being recognized as a commit hash. We currently fire the object monogram rules fairly early, but there's no real reason to do this. Move them after all of the hyperlink rules:

    0 PhutilRemarkupEscapeRemarkupRule
    100 PhutilRemarkupMonospaceRule
    150 PhutilRemarkupDocumentLinkRule
    175 PhrictionRemarkupRule

  <<< OLD OBJECT RULE POSITION

    200 PhabricatorIconRemarkupRule
    200 PhabricatorMemeRemarkupRule
    200 DivinerSymbolRemarkupRule
    350 DoorkeeperRemarkupRuleJIRA
    350 PhabricatorYoutubeRemarkupRule
    350 DoorkeeperRemarkupRuleAsana
    400 PhutilRemarkupHyperlinkRule

  >>> NEW OBJECT RULE POSITION

    500 PhabricatorImageMacroRemarkupRule
    500 CustomInlineJIRA5Rule
    500 PhabricatorMentionRemarkupRule
    500 CustomInlineCodeRule
    1000 PhutilRemarkupDelRule
    1000 PhutilRemarkupBoldRule
    1000 PhutilRemarkupItalicRule
    1000 PhutilRemarkupUnderlineRule

  - The disadvantage of this approach is that `{F123, alt=go look at http://lol.com/ omg}` will parse the URL first, and then fail to resolve the object embed. This seems very rare / unusual.
  - The advantage is that all URLs which happen to have monograms in them work.

In the future, we could refine this by separating the rules, so the embed (`{...}`) versions fired at priority 200, while the normal versions fired at priority 450. We can wait for use cases, though. This is a little messy because the same code implements both rules.

Test Plan:
  - Verified example in T5837.
  - Marked up object rules like `F123` (works), `[[ asdf | F123 ]]` (works), `{F123, alt=http://example.com}` (does not work).

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5837

Differential Revision: https://secure.phabricator.com/D10212
Convert `pushlog` and `refcursor` to BLOB storage

Summary:
Fixes T5840. Some time ago I incorrectly believed that `latin1_bin` collation was synonymous with "binary". It is not, and does not permit UTF8 characters outside of BMP, among other sequences.

These two tables currently have `LONGTEXT` columns which should be `LONGBLOB`. The table design is explicilty intended to accommodate invalid/unreasonably long ref names, but the collation prevents this from working properly.

After T1191, we'll have a general system for resolving this, but a user hit an issue yesterday (T5840) with a brnach name containing Chinese characters.

Test Plan:
  - Tried emoji inserts into both tables, was rebuffed.
  - Ran migration.
  - Performed emoji inserts into both tables.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5840

Differential Revision: https://secure.phabricator.com/D10217
Improve usability of Phrequent "Stop Time" dialog

Summary:
Fixes T5848.

  - Disallow tracking negative time.
  - Preserve note if there's an error with the time selection.
  - Show start time and duration.
  - Slightly better error messages.

Test Plan: Started and stopped time. Tried to select future/negative ranges.

Reviewers: hach-que, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5848

Differential Revision: https://secure.phabricator.com/D10218
Account for preempting events on the Phrequent list view

Summary: Fixes T5850. Also fixes some logic where the wrong preempting events could be attached during a bulk query.

Test Plan: Phrequent list now shows preemption-aware times.

Reviewers: hach-que, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5850

Differential Revision: https://secure.phabricator.com/D10223
Updated PhabricatorManiphestTaskTestDataGenerator to assign projects to the generated tasks.

Summary: To assist with {T5245}, I have added projects back into the lipsum maniphest generator with the edge infrastructure.

Test Plan: Run the lipsum script for PhabricatorManiphestTaskTestDataGenerator and make sure it generates project data.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10202
Measure how long build targets take in Harbormaster

Summary:
Ref T1049.  This keeps track of how long a build target takes to execute in Harbormaster and displays it in the build view page.  I'm not sure whether "Started" is really that useful once the target has completed?

Also, I change the name of the time taken depending on whether or not the target has completed; if it's still in progress it's called "Elapsed" and if it's completed then it's "Duration".  The primary reason for this is that "Duration" sounds like post tense, whereas "Elapsed" is current tense.  I'm not sure whether this is okay or not?

Test Plan: Ran a Sleep build step and saw the target dates / times appear correctly.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: talshiri, epriestley, Korvin

Maniphest Tasks: T5824, T1049

Differential Revision: https://secure.phabricator.com/D10174
Allow Drydock blueprints to define and use custom fields

Summary: This allows Drydock blueprints to define custom fields for blueprint settings.

Test Plan: Pulled out of EC2 allocator diff.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10224
Allow timeouts to be specified on Drydock SSH connections

Summary: This allows timeouts to be specified on SSH connections that Drydock makes.  Used in the EC2 allocator to poll for the SSH server starting.

Test Plan: Used in EC2 allocator diff.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10225