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

8724 commits

Author SHA1 Message Date
epriestley
803c50c1e7 Allow Harbormaster HTTP steps to pass credentials
Summary: Fixes T4590. Use the credentials custom field to allow Harbormaster HTTP requests to include usernames/passwords.

Test Plan: Ran a build plan with credentials, verified they were sent to the remote server.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4590

Differential Revision: https://secure.phabricator.com/D8786
2014-04-16 13:01:38 -07:00
epriestley
c402d7d307 Implement smart waits for rarely updated repositories
Summary:
Ref T4605. When figuring out how long to wait to update a repository, factor in when it was last pushed. For rarely updated repositories, wait longer between updates.

(A slightly funky thing about this is that empty repos update every 15 seconds, but that seems OK for the moment.)

Test Plan:
Ran `bin/phd debug pulllocal` and saw sensible calculations and output:

```
...
<VERB> PhabricatorRepositoryPullLocalDaemon Last commit to repository "rPOEMS" was 1,239,608 seconds ago; considering a wait of 6,198 seconds before update.
>>> [79] <query> SELECT * FROM `repository` r   ORDER BY r.id DESC
<<< [79] <query> 514 us
>>> [80] <query> SELECT * FROM `repository_statusmessage` WHERE statusType = 'needs-update'
<<< [80] <query> 406 us
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rINIH" is not due for an update for 8,754 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rDUCK" is not due for an update for 14 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rMTESTX" is not due for an update for 21,598 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rQWER" is not due for an update for 14 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rBT" is not due for an update for 13 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rSVNX" is not due for an update for 21,598 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rINIG" is not due for an update for 13 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rHGTEST" is not due for an update for 21,598 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rBTX" is not due for an update for 14 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rGX" is not due for an update for 13 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rMTX" is currently updating.
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rPOEMS" is not due for an update for 6,198 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rPHU" is currently updating.
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rSVN" is not due for an update for 21,598 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rPHY" is currently updating.
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rGTEST" is not due for an update for 21,598 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rINIS" is not due for an update for 6,894 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rARCLINT" is not due for an update for 21,599 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rLPHX" is not due for an update for 1,979 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rARC" is not due for an update for 1,824 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rINIHG" is not due for an update for 21,599 second(s).
...
```

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4605

Differential Revision: https://secure.phabricator.com/D8782
2014-04-16 13:01:04 -07:00
epriestley
5671c4b276 Make PullLocal daemon more flexible and transparent about scheduling
Summary:
Ref T4605. Fixes T3466. The major change here is that we now run up to four simultaneous updates. This should ease cases where, e.g., one very slow repository was blocking other repositories. It also tends to increase load; the next diff will introduce smart backoff for cold repositories to ease this.

The rest of this is just a ton of logging so I can IRC debug these things by having users run them in `phd debug pulllocal` mode.

For T3466:

  - You now have to hit four simultaneous hangs to completely block the update process.
  - Importing repository updates are killed after 4 hours.
  - Imported repository updates are killed after 15 minutes.

Test Plan:
  - Ran `phd debug pulllocal` and observed sensible logs and behavior.
  - Interrupted daemon from sleeps and processing with `diffusion.looksoon`.
  - Ran with various `--not`, `--no-discovery` flags.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3466, T4605

Differential Revision: https://secure.phabricator.com/D8785
2014-04-16 13:00:49 -07:00
epriestley
417056932e Make discovery slightly cheaper in the common case
Summary:
Ref T4605. Before discovering branches, try to prefill the cache in bulk. For repositories with large numbers of branches, this allows us to issue dramatically fewer queries.

(Before D8780, this cache was usually held across discovery events, so being able to fill it cheaply was not as relevant.)

Test Plan: Ran discovery on Git, Mercurial and SVN repositories. Observed fewer queries for Git/Mercurial.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4605

Differential Revision: https://secure.phabricator.com/D8781
2014-04-16 13:00:38 -07:00
epriestley
118c696f72 Separate repository updates from the pull daemon
Summary:
Ref T4605. Currently, the PullLocal daemon is responsible for two relatively distinct things:

  - scheduling repository updates; and
  - actually updating repositories.

Move the "actually updating" part into a new `bin/repository update` command, which basically runs the pull, discover, refs and mirror commands. This will let the parent process focus on scheduling in a more understandable way and update multiple repositories at once. It also makes it easier to debug and understand update behavior since the non-scheduling pipeline can be run separately.

Test Plan:
  - Ran `update --trace` on SVN, Mercurial and Git repos.
  - Ran PullLocal daemon for a while without issues.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4605

Differential Revision: https://secure.phabricator.com/D8780
2014-04-16 13:00:29 -07:00
Chad Little
2cf2117eee Remove extra workboard margin on mobile
Summary: We have too much space on workboards when displayed on mobile devices.

Test Plan: Shrink browser display, note that all workboards align to common gutters.

Reviewers: epriestley, btrahan

Reviewed By: btrahan

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8790
2014-04-16 11:09:53 -07:00
Chad Little
f5eb7b6404 Add ability to edit Projects on mobile
Summary: sets action list to crumbs

Test Plan: shrink browser, see mobile action list, click on it, edit

Reviewers: epriestley, btrahan

Reviewed By: btrahan

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8789
2014-04-16 11:06:04 -07:00
epriestley
041c3fd19d Let project prefilling accept PHIDs
Summary: I recently made this better about accepting project names, but we use it in some cases with PHIDs. Make that work properly again.

Test Plan: Clicked "New Task" from a project page.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8778
2014-04-15 11:17:37 -07:00
epriestley
20abf37b88 Fix a lookup issue in Owners
Summary:
Fixes T4477. Sort of winging this but it's probably the right fix?

One error in T4477.

One error via email:

```
[2014-04-15 17:44:34] ERROR 8: Undefined index: /some_index/ at [/phab_path/phabricator/src/applications/owners/storage/PhabricatorOwnersPackage.php:213]
  #0 PhabricatorOwnersPackage::findLongestPathsPerPackage(Array of size 3 starting with: { 0 => Array of size 3 starting with: { id => 5 } }, Array of size 8 starting with: { / => Array of size 2 starting with: { /some_index/some_file.py => true } }) called at [/phab_path/phabricator/src/applications/owners/storage/PhabricatorOwnersPackage.php:170]
  #1 PhabricatorOwnersPackage::loadPackagesForPaths(Object PhabricatorRepository, Array of size 2 starting with: { 0 => /some_index/some_file.py }) called at [/phab_path/phabricator/src/applications/owners/storage/PhabricatorOwnersPackage.php:119]
...
```

Test Plan: Will make @zeeg do it.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley, zeeg

Maniphest Tasks: T4477

Differential Revision: https://secure.phabricator.com/D8779
2014-04-15 11:12:42 -07:00
Bob Trahan
c845b757ee Maniphest - remove "attach file" action
Summary: Fixes T4655. Basically leaves the display code intact for legacy installs but removes the option from the UI and removes "create" code.

Test Plan:
tried to attach file and the action was not in the dropdown!
made a new task and it worked!
commented on an old task and it worked!

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4655

Differential Revision: https://secure.phabricator.com/D8777
2014-04-15 10:49:05 -07:00
epriestley
04c07a7a7b Remove the developer-specific CSRF help in phabricator_form()
Summary:
Fixes T4802. For context, see T1921.

Originally (in T1921), a developer ran into an issue where rendering `phabricator_form()` with an absolute URI confusingly dropped CSRF tokens, and it wasn't obvious why. This is a security measure, but at the time it wasn't very clear how all the pieces fit together. To make it more clear, we:

  # expanded the exception text in developer mode to include a description of this issue; and
  # added an exception in developer mode when rendering a form like this.

However, (2) causes some undesirable interactions for file downloads. In particular, if:

  - developer mode is on; and
  - there's no alternate file domain configured; and
  - you try to download a file...

...we produce CDN URIs that are fully-qualified, and you get the exception from (2) above.

This is kind of a mess, and producing fully-qualified CDN URIs in all cases is simple and clear and desirable. To resolve this, just revert (2). We still have the clarification from (1) above and this hasn't caused further issues, so I think that's sufficient. This is a rare issue anyway and not particularly serious or error prone (at worst, a bit confusing and annoying, but hopefully easy to understand and resolve after the changes in (1)).

Test Plan: With develper mode and no alternate file domain, downloaded files from Files.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4802

Differential Revision: https://secure.phabricator.com/D8776
2014-04-15 10:18:41 -07:00
epriestley
cb545856a9 Make task queue more robust against long-running tasks
Summary:
See discussion in D8773. Three small adjustments which should help prevent this kind of issue:

  - When queueing followup tasks, hold them on the worker until we finish the task, then queue them only if the work was successful.
  - Increase the default lease time from 60 seconds to 2 hours. Although most tasks finish in far fewer than 60 seconds, the daemons are generally stable nowadays and these short leases don't serve much of a purpose. I think they also date from an era where lease expiry and failure were less clearly distinguished.
  - Increase the default wait-after-failure from 60 seconds to 5 minutes. This largely dates from the MetaMTA era, where Facebook ran services with high failure rates and it was appropriate to repeatedly hammer them until things went through. In modern infrastructure, such failures are rare.

Test Plan:
  - Verified that tasks queued properly after the main task was updated.
  - Verified that leases default to 7200 seconds.
  - Intentionally failed a task and verified default 300 second wait before retry.
  - Removed all default leases shorter than 7200 seconds (there was only one).
  - Checked all the wait before retry implementations for anything much shorter than 5 minutes (they all seem reasonable).

Reviewers: btrahan, sowedance

Reviewed By: sowedance

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8774
2014-04-15 08:42:02 -07:00
Peng Li
6a4f126000 Give the commitownersparser a little more time
Summary:
Recently we see issues with huge commits (branch cuts for www) where people received hundreds of emails for the same commit. By checking all the active and archived tasks related to such commits, I saw the following pattern:
 - The commit itself is marked as importStatus = 15 which means all the processing was actually done;
 - In archived tasks, I see one PhabricatorRepositorySvnCommitMessageParserWorker, one PhabricatorRepositorySvnCommitChangeParserWorker, followed by many PhabricatorRepositoryCommitHeraldWorker, which means that the PhabricatorRepositoryCommitOwnersWorker (who schedule those herald tasks) was never done;
 - PhabricatorRepositoryCommitOwnersWorker is always active (for days) with failureCount = 0;
 - In daemon log I see a lot of lease expire exception for PhabricatorRepositoryCommitOwnersWorker.
So to me it looks like the following happened:
 - Everything is fine until we schedule the PhabricatorRepositoryCommitOwnersWorker
 - PhabricatorRepositoryCommitOwnersWorker actually successfully finished but its running time exceed 60s. Before it finishes, it scheduled the PhabricatorRepositoryCommitHeraldWorker task
 - When we try to archive it, the lease expiration exception happened. As a result, it stayed active and will be picked up immediately since it is in the head of the queue
 - The two steps above repeat forever until we kill it
I am not sure why we want to check lease expiration when we are archiving the task. For now I am giving the worker a little more time since parsing half million affected path needs some time..

Test Plan: Patched in our production and it worked.

Reviewers: lifeihuang, JoelB, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8773
2014-04-14 15:52:02 -07:00
Felipe Bessa Coelho
f27f7dce52 Install PHP mbstring extension on RHEL & friends
Summary:
The mbstring extension for PHP is not a dependency to any of the already
listed packages on RHEL-like systems, and is needed by Phabricator
(showing a "install mbstring" message as the first thing if it is not
installed)

RHEL seems to have some extra steps to allow php-mbstring to be installed, though:

http://snippets.roozbehk.com/post/35750940300/php-mbstring-missing-on-red-hat-enterprise-linux-6

PS: disabled lint for this change because of the already >80 chars long "yum install" string

Test Plan:
* Created a new container with docker using both centos:6.4 and fedora:20 images
* Ran install script
* Started httpd and mysqld services
* Browsed to server's address
* Got error message
* Installed php-mbstring & restarted httpd
* Works

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8772
2014-04-14 14:59:27 -07:00
epriestley
47245b1b59 Remove some ad-hoc loading of repositories from Releeph
Summary: Ref T3551. Since we now require repositories in order to perform policy checks, things that did loads properly don't need to load this data explicitly.

Test Plan: Edited a product, cut a new branch.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3551

Differential Revision: https://secure.phabricator.com/D8769
2014-04-14 12:08:04 -07:00
epriestley
f4c8a34abe Remove several "loadArcanistProject()" methods
Summary:
Ref T3551. Releeph has old-style `loadX()` methods; get rid of one of them.

Differential has a couple of copies of this too, clean them up.

Test Plan:
  - Viewed various differential revisions (with and without projects).
  - Viewed and edited Releeph products.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3551

Differential Revision: https://secure.phabricator.com/D8768
2014-04-14 12:07:32 -07:00
epriestley
05e30a024d Unban releeph product name "branch"
Summary:
Fixes T3657. We no longer construct ambiguous URIs, so product names are no longer restricted.

Also fix some minor URI construction stuff.

Test Plan: Created a product called "branch".

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3657

Differential Revision: https://secure.phabricator.com/D8767
2014-04-14 12:07:14 -07:00
epriestley
6deab24475 Remove ReleephProjectController
Summary:
Ref T3657. General changes here:

  - Removes `ReleephProjectController`, which is the source of T3657.
  - Mostly moves requests from "RQ" as a monogram to "Y" (looks like a merge, mnemonic for "yank"?, we don't have too many characters left). This should be essentially only cosmetic. This reduces ambiguity with "rQ" and "R123", which are current and future repository monograms. This will continue in the next few diffs.
  - Makes requests implement policies correctly.

Test Plan: Created, edited, browsed requests.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3657

Differential Revision: https://secure.phabricator.com/D8766
2014-04-14 12:06:56 -07:00
epriestley
6899fbcf29 Add DifferentialHunkQuery to start hiding hunk storage details
Summary:
Ref T4045. We have a lot of direct queries against the hunk table right now. These are messy, not really policy-aware, and limit our options on T4045.

This query is unusual (it requires changesets, and does not accept IDs). This keeps us from having to load changeset -> diff -> revision in order to do policy checks. We could also fix this with smarter policy checks and caching, but I'd rather not open that can of worms for now. This object is very low level and relatively unusual, and this small deviation from convention seems like the cleanest cut to make to keep this from snowballing.

Test Plan: Used Herald dry runs to verify that the affected rules still output the same data.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4045

Differential Revision: https://secure.phabricator.com/D8765
2014-04-14 12:06:26 -07:00
epriestley
aaf1320b02 Simplify Herald logic for loading Differential changes
Summary: Ref T4045. These three methods are fairly copy-pastey. Provide a more formal DifferentialHunk API for querying various types of line ranges.

Test Plan: Used test console to verify that "added content", "removed content", and "changed content" rules still produce the same data.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4045

Differential Revision: https://secure.phabricator.com/D8764
2014-04-14 12:06:20 -07:00
lkassianik
2509c6b0f5 Fixing z-index on calendar date badges.
Summary: Fixes T4787, decreased z-index on calendar badges so that they don't sit on top of  notification dropdowns when dropdowns are expanded. Not sure why the badges had z-index 10, but please let me know if there was a more substantial reason for this.

Test Plan: If neither notification dropdowns have content, create enough messages to populate at least 5 rows, open calendar, expand messages dropdown, verify that underlying calendar date badges do not appear over the dropdown.

Reviewers: chad, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4787

Differential Revision: https://secure.phabricator.com/D8770
2014-04-13 08:18:16 -07:00
epriestley
2712091a71 Move Releeph branch controllers toward a modern/stable state
Summary:
Ref T3644. Ref T3657. Ref T3549. Basically:

  - Move these controllers to modern query/policy infrastructure.
  - Move them to consistent, ID-based URIs.
  - Rename "Project" to "Product"; "Pick Request" to "Pull Request".
  - Clean up a few UI things here and there.

Test Plan:
  - Created and edited branches.
  - Opened and closed branches.
  - Viewed branch history.
  - Searched within a branch.
  - Browsed to branches from products.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3644, T3549, T3657

Differential Revision: https://secure.phabricator.com/D8646
2014-04-12 16:53:51 -07:00
epriestley
cbfa99174e Fixing tooltips not appearing in fullscreen editor
Summary: Ref T4714, fixing tooltips not appearing in editor when in fullscreen mode

Test Plan: Create paste, verify tooltips appear in comment text-editing bar, make comment box fullscreen, verify tooltips still appear.

Reviewers: chad, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4714

Differential Revision: https://secure.phabricator.com/D8763
2014-04-12 05:59:36 -07:00
lkassianik
8af84a9b7d Adding getBoundingClientRect math to getPos in Vector.js
Summary: Ref T4714, tooltips in fullscreen mode need special math, due to fixed position throwing off position of tooltips.

Test Plan: Create work board, create several tasks, create several columns, drag tasks among columns and within columns. Create a paste, check tooltips in comment box show and are positioned correctly. Fullscreen comment box. Verify tooltips still show and position correctly.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T4714

Differential Revision: https://secure.phabricator.com/D8762
2014-04-11 16:11:35 -07:00
Bob Trahan
cb675a46a2 Differential - fix bug writing affected paths
Summary: Fixes T4774. With the new code and configuration instructions downplaying the role of arcanist project we weren't writing affected paths at all! I had this issue on my installation - no affected paths were written. We seem to always have the repository now though if we can see it, so not too bad of a fix.

Test Plan: updated a diff and was able to browse in diffusion.

Reviewers: epriestley, bitglue

Reviewed By: epriestley, bitglue

Subscribers: bitglue, epriestley, Korvin

Maniphest Tasks: T4774

Differential Revision: https://secure.phabricator.com/D8757
2014-04-11 12:54:21 -07:00
Bob Trahan
f86ab666f6 Differential - make diffs you authored + are reviewer for show up in appropos boxes
Summary: Fixes T2328. Note the audit part is fixed now.

Test Plan: Tried to reproduce the audit issue by raising my own commit as a problem; it showed up before code changes! Made a diff with my self as author and reviewer; it showed up as expected

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T2328

Differential Revision: https://secure.phabricator.com/D8755
2014-04-11 10:31:07 -07:00
epriestley
499f66963d Minor, fix the scoping of a static variable
Auditors: chad, btrahan
2014-04-11 04:24:58 -07:00
Ben Alpert
ac0ad2e016 Fix typo in variable name
Summary: Follow-up to D8758.

Test Plan: Crossed fingers.

Reviewers: #blessed_reviewers, chad, epriestley

Reviewed By: #blessed_reviewers, chad, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8759
2014-04-11 04:18:24 -07:00
Bob Trahan
524e8a64f4 Countdown - use better date control
Summary: Fixes T3576

Test Plan: made a countdown and it looked right on view. edited it and it had the right values pre and post edit.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T3576

Differential Revision: https://secure.phabricator.com/D8754
2014-04-10 18:14:47 -07:00
epriestley
650bc97ed7 Don't load every commit if there are no local hashes
Summary: We make a silly query for every commit if you copy/paste a diff.

Test Plan: Copy/pasted diffs now render in fewer than 30 seconds.

Reviewers: btrahan, spicyj

Reviewed By: btrahan, spicyj

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8758
2014-04-10 18:09:54 -07:00
Joshua Spence
e7419d85f7 Set celerity.minify true in production environments.
Summary: I haven't been able to understand why this isn't set by default in production environments (since it is recommended to do so anyway).

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8743
2014-04-10 14:39:05 -07:00
Bob Trahan
3e0b3a1db5 Feed - fix some whacky "text mode" rendering code
Summary: ...add a "renderingTarget" to FeedStory and use it as appropos. Overall, not a ton of changes was necessary to make this work. I think this could be made to be even cleaner by going through each and every feed story and re-implementing as necessary with the full toolset available. But this is good enough for now I think, and just something to keep cleaning up when we're in here. Fixes T4630.

Test Plan: made a task. gave it a token. viewed my feed - saw stories. used conduit.feed.query with mode == 'text' and got good readable results.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: spicyj, epriestley, Korvin

Maniphest Tasks: T4630

Differential Revision: https://secure.phabricator.com/D8750
2014-04-10 13:46:02 -07:00
Bob Trahan
4b56dbed3a Herald - make tokenizers have the purdy icons
Summary: ...use the prefab stuff as it does fancier things than we were doing. Only trick then really is to pass username and the map of handle phids => icons to the client so prefab can work nicely. Fixes T4775.

Test Plan: made a herald rule with projects and users. Saw nice icons. Reloaded page and still saw nice icons.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4775

Differential Revision: https://secure.phabricator.com/D8749
2014-04-10 12:38:15 -07:00
epriestley
facd3ea8ef Modernize chatlog a bit
Summary:
Ref T4786. This doesn't fully fix the issue since there's no way to make channels public yet, but gets some of the infrastructure more up to date.

  - Allow public access to the list and log controllers.
  - Implement proper policy checks in the Events (this has no practical impact on the only controller that loads this stuff, it's just for general/future purposes).
  - Remove a old-style unused method for building page frames.

Test Plan: Viewed log list and log details as logged-in and logged out users.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4786

Differential Revision: https://secure.phabricator.com/D8746
2014-04-10 11:45:21 -07:00
epriestley
ab7d89edc8 Use better secrets in generating account tokens
Summary:
When we generate account tokens for CSRF keys and email verification, one of the inputs we use is the user's password hash. Users won't always have a password hash, so this is a weak input to key generation. This also couples CSRF weirdly with auth concerns.

Instead, give users a dedicated secret for use in token generation which is used only for this purpose.

Test Plan:
  - Ran upgrade scripts.
  - Verified all users got new secrets.
  - Created a new user.
  - Verified they got a secret.
  - Submitted CSRF'd forms, they worked.
  - Adjusted the CSRF token and submitted CSRF'd forms, verified they don't work.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8748
2014-04-10 11:45:10 -07:00
Chad Little
3018a5eb09 Better mobile display of ObjectItemView
Summary: Makes the default 3 rows on mobile devices, with larger fonts. Differential/Audit is much better, Maniphest is maybe a sidegrade depending on setup.

Test Plan: test maniphest, audit, differential list items.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8741
2014-04-10 11:23:38 -07:00
epriestley
0fd12f9cad Remove nonfunctional/obsolete 'reconcile.php'
Summary: Ref T4780. This no longer works and barely ever worked. T4237 is now the relevant task.

Test Plan: Grepped for `reconcile.php`

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4780

Differential Revision: https://secure.phabricator.com/D8739
2014-04-09 16:12:29 -07:00
epriestley
c887871d30 Disable rate limiting by default in general
Summary: This is still too rough to enable by default. We can turn it on for `secure.phabricator.com` and tweak it a bit first, and then release it in general with higher defaults and more sensible behavior in edge cases.

Test Plan: Loaded some pages.

Reviewers: btrahan, spicyj

Reviewed By: spicyj

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8736
2014-04-09 11:52:34 -07:00
Brayden Winterton
b660960771 Add a priorityColor property to the maniphest conduit endpoint
Summary:
I added a getTaskPriorityColor function to the ManiphestTaskPriority class which returns the color set in the maniphest config for the given priority.

This is in preparation for a change to arcanist which will allow it to display the priority color (if it is a supported color) upon running `arc tasks`.

Fixed some linting issues

Test Plan:
Invoke the maniphest.info method from conduit and ensure that:
 * The priorityColor property is given in the json
 * the priorityColor property is set correctly

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8734
2014-04-09 11:32:08 -07:00
Bob Trahan
2d43cf1296 OAuth - add a little notes section for admins to remember details about external accounts
Summary: Fixes T4755. This also includes putting in a note that Google might ToS you to use the Google+ API. Lots of code here as there was some repeated stuff between OAuth1 and OAuth2 so I made a base OAuth with less-base OAuth1 and OAuth2 inheriting from it. The JIRA provider remains an independent mess and didn't get the notes field thing.

Test Plan: looked at providers and read pretty instructions.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4755

Differential Revision: https://secure.phabricator.com/D8726
2014-04-09 11:09:50 -07:00
epriestley
8b3eced0c7 Make Maniphest project prefill more modern and standard
Summary: Fixes T4777. We technically support `?projects=...` already, but parse it in an unusual way and apply old, awkward, excessively strict lookups to it.

Test Plan: Used reasonable, standard, human-readable strings to prefill `?projects=` and got the results I expected.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T4777

Differential Revision: https://secure.phabricator.com/D8733
2014-04-09 08:37:23 -07:00
Brayden Winterton
dffbbaf0a6 Added isClosed property to maniphest conduit endpoint in order to fix an issue with arcanist when displaying tasks
Summary:
Arcanist is currently displaying all tasks as closed when invoking `arc tasks`.
This is because arcanist is setting the display to closed if there is anything in the `status` property. Adding an isClosed property will allow arcanist to properly display open/closed status on tasks by checking against the isClosed property. The isClosed property will be set according to the closed property that is set on each status in maniphest.

Test Plan:
Invoke the conduit maniphest.info method on any task and insure that:
 # The isClosed property is included in the properties
 # that it is set properly according to the statuses set for maniphest.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: chad, epriestley, Korvin

Maniphest Tasks: T4744

Differential Revision: https://secure.phabricator.com/D8731
2014-04-09 07:47:31 -07:00
lkassianik
01552d85de Show Projects bucket unconditionally in repository summary screen
Summary: fixes T4753

Test Plan: looked at repository with projects, looked at repository with no projects

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: chad, epriestley, Korvin

Maniphest Tasks: T4753

Differential Revision: https://secure.phabricator.com/D8730
2014-04-08 19:48:31 -07:00
epriestley
4d0935ba5e Rate limit requests by IP
Summary:
Fixes T3923. On `secure.phabricator.com`, we occasionally get slowed to a crawl when someone runs a security scanner against us, or 5 search bots decide to simultaneously index every line of every file in Diffusion.

Every time a user makes a request, give their IP address some points. If they get too many points in 5 minutes, start blocking their requests automatically for a while.

We give fewer points for logged in requests. We could futher refine this (more points for a 404, more points for a really slow page, etc.) but let's start simply.

Also, provide a mechanism for configuring this, and configuring the LB environment stuff at the same time (this comes up rarely, but we don't have a good answer right now).

Test Plan: Used `ab` and reloading over and over again to hit rate limits. Read documentation.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: chad, epriestley

Maniphest Tasks: T3923

Differential Revision: https://secure.phabricator.com/D8713
2014-04-08 18:36:21 -07:00
Ben Alpert
597c6c07f7 Switch back to janky array copying
Summary: Bad news @cpojer @tomo. IE8 doesn't like you.

Test Plan: Load a diff in IE8; see changes and don't get JS errors.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, tomo, Korvin, cpojer

Differential Revision: https://secure.phabricator.com/D8728
2014-04-08 18:07:58 -07:00
Ben Alpert
a7272dfb03 Switch back to zwsp for oncopy line marker
Summary:
Fixes T4759.

Turns out Chrome on windows doesn't really like the word joiner character. We'll switch back to zwsp but make it `position: absolute;` so it doesn't turn into a line break.

Test Plan: Looked at diffs in IE9 and Chrome Windows. Made sure copying still works as expected.

Reviewers: chad, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4759

Differential Revision: https://secure.phabricator.com/D8727
2014-04-08 17:55:48 -07:00
epriestley
563a1ac5bf Fix parsing and storage of generated SSH keys
Summary: Fixes T4772. We weren't parsing generated public keys properly, and were storing them in the wrong format.

Test Plan:
  - Updated a private key.
  - Generated a public key.
  - Saved the public key.
  - Used a generated private key to authenticate.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4772

Differential Revision: https://secure.phabricator.com/D8721
2014-04-08 14:52:37 -07:00
epriestley
f79320e64e Render default/current settings in the same format they'll be edited
Summary:
Fixes T4773. For config settings of type `list<string>`, `set`, or `list<regex>`, the "defaults" table and "examples" aren't always in the same format you should actually use when changing the setting.

This is pretty confusing. Instead, always show the settings in the desired format. For example, if the user should enter a newline-separated list, show them a newline separated list.

Test Plan:
  - Grepped for `list<string>`, `list<regex>`, and `'set'`; verified all the config had the right example format (most already did).
  - Viewed config settings of various kinds, including custom settings.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4773

Differential Revision: https://secure.phabricator.com/D8725
2014-04-08 12:09:43 -07:00
Bob Trahan
d5ded805b2 Herald - fix change type bug
Summary: wasn't working due to some type issues. Fixes T4756. I also made it display nicer while I was debugging this.

Test Plan: created a herald rule to block changes that added refs. git tag -a "test" -m "test test"; git push origin test got me blocked!

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4756

Differential Revision: https://secure.phabricator.com/D8724
2014-04-08 11:58:28 -07:00
epriestley
582ec54465 Add a checkbox to the LDAP auth configuration UI to "Always Search"
Summary: Fixes T3208. This forces us to bind+search even if there are no anonymous credentials.

Test Plan: Checked the box, saved the form. Unchecked the box, saved the form. LDAP??

Reviewers: Firehed

Reviewed By: Firehed

Subscribers: epriestley

Maniphest Tasks: T3208

Differential Revision: https://secure.phabricator.com/D8723
2014-04-08 11:36:23 -07:00