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

417 commits

Author SHA1 Message Date
epriestley
90078fe06e Clarify that 'order' is an optional parameter in Conduit API methods
Summary: Fixes T8603. For automatic 'order' parameters provided by infrastructure en route to T7715, clarify that they are optional (we will use the default builtin order for the underlying Query if an order is not provided).

Test Plan: Used web UI to see "optional" hint.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8603

Differential Revision: https://secure.phabricator.com/D13342
2015-06-18 13:05:44 -07:00
Joshua Spence
1239cfdeaf Add a bunch of tests for subclass implementations
Summary: Add a bunch of tests to ensure that subclasses behave.

Test Plan: `arc unit`

Reviewers: eadler, #blessed_reviewers, epriestley

Reviewed By: eadler, #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D13272
2015-06-15 18:13:27 +10:00
Joshua Spence
b6d745b666 Extend from Phobject
Summary: All classes should extend from some other class. See D13275 for some explanation.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D13283
2015-06-15 18:02:27 +10:00
epriestley
992c199577 Add "Mailing List" users
Summary:
Ref T8387. Adds new mailing list users.

This doesn't migrate anything yet. I also need to update the "Email Addresses" panel to let administrators change the list address.

Test Plan:
  - Created and edited a mailing list user.
  - Viewed profile.
  - Viewed People list.
  - Searched for lists / nonlists.
  - Grepped for all uses of `getIsDisabled()` / `getIsSystemAgent()` and added relevant corresponding behaviors.
  - Hit the web/api/ssh session blocks.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: eadler, tycho.tatitscheff, epriestley

Maniphest Tasks: T8387

Differential Revision: https://secure.phabricator.com/D13123
2015-06-03 18:42:33 -07:00
epriestley
8440b3efc0 Update account roles documentation and remove actAsUser
Summary:
Ref T8387. This describes changes I haven't made yet, but plan to make.

Also removes the long-deprecated actAsUser capability so I can remove the caveat about it from the documentation.

Test Plan: `grep`, reading

Reviewers: btrahan, eadler

Reviewed By: btrahan, eadler

Subscribers: eadler, epriestley

Maniphest Tasks: T8387

Differential Revision: https://secure.phabricator.com/D13120
2015-06-03 18:42:09 -07:00
Joshua Spence
bf81fda036 Linter fixes
Summary: Apply various minor linter fixes.

Test Plan: `arc lint`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D13109
2015-06-02 22:14:01 +10:00
epriestley
7d757483a0 Make everything 1000x or 1000000x slower
Summary:
In the great `pht()` conversion, some strings like "123,456" are now being printed as numbers with "%d". These come out as "123" instead of "123,456".

Use "%s" and "PhutilNumber" to present numbers with comma groupings.

Test Plan:
  - Viewed DarkConsole.
  - Viewed conduit logs.
  - Viewed daemon logs.
  - Grepped for `%d ms` and `%d us`.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12979
2015-05-23 05:36:02 -07:00
Joshua Spence
36e2d02d6e phtize all the things
Summary: `pht`ize a whole bunch of strings in rP.

Test Plan: Intense eyeballing.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: hach-que, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12797
2015-05-22 21:16:39 +10:00
Joshua Spence
acb45968d8 Use __CLASS__ instead of hard-coding class names
Summary: Use `__CLASS__` instead of hard-coding class names. Depends on D12605.

Test Plan: Eyeball it.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: hach-que, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12806
2015-05-14 07:21:13 +10:00
epriestley
3a34d948b9 Show how to call Conduit API methods from clients
Summary: Fixes T3628. Ref T5955.

Test Plan:
On the method page, you see a generic example:

{F396471}

After making a call, you see a specific example with your parameters:

{F396472}

{F396474}

{F396475}

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3628, T5955

Differential Revision: https://secure.phabricator.com/D12770
2015-05-08 12:19:52 -07:00
Joshua Spence
70c8649142 Use phutil_json_decode instead of json_decode
Summary: Generally, `phutil_json_decode` should be preferred over `json_decode`.

Test Plan: Eyellballed.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12680
2015-05-05 20:48:55 +10:00
epriestley
ba6b1376f2 Provide an "api" context to Multimeter for API events
Summary: Ref T6930. This gives us a better context for API calls -- "api.call.name" instead of always "web.ConduitAPIController".

Test Plan: {F391658}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6930

Differential Revision: https://secure.phabricator.com/D12677
2015-05-04 10:06:57 -07:00
epriestley
f5580c7a08 Make buildWhereClause() a method of AphrontCursorPagedPolicyAwareQuery
Summary:
Ref T4100. Ref T5595.

To support a unified "Projects:" query across all applications, a future diff is going to add a set of "Edge Logic" capabilities to `PolicyAwareQuery` which write the required SELECT, JOIN, WHERE, HAVING and GROUP clauses for you.

With the addition of "Edge Logic", we'll have three systems which may need to build components of query claues: ordering/paging, customfields/applicationsearch, and edge logic.

For most clauses, queries don't currently call into the parent explicitly to get default components. I want to move more query construction logic up the class tree so it can be shared.

For most methods, this isn't a problem, but many subclasses define a `buildWhereClause()`. Make all such definitions protected and consistent.

This causes no behavioral changes.

Test Plan: Ran `arc unit --everything`, which does a pretty through job of verifying this statically.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: yelirekim, hach-que, epriestley

Maniphest Tasks: T4100, T5595

Differential Revision: https://secure.phabricator.com/D12453
2015-04-20 10:06:09 -07:00
epriestley
09ad69238e Drive conduit result ordering through Query order specifications
Summary:
Ref T7803. Ref T5873. Allows Query methods to expose orderings from the underlying Query class nearly-for-free.

Callers can specify a string to use a builtin ordering, or an array to use a low-level column ordering.

Test Plan: {F368236}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5873, T7803

Differential Revision: https://secure.phabricator.com/D12381
2015-04-13 11:58:37 -07:00
epriestley
156b156e77 Give Conduit params/return/errors protected visibility
Summary:
Ref T7803. Ref T5873. I want to drive Conduit through more shared infrastructure, but can't currently add parameters automatically.

Put a `getX()` around the `defineX()` methods so the parent can provide default behaviors.

Also like 60% of methods don't define any special error types; don't require them to implement this method. I want to move away from this in general.

Test Plan:
  - Ran `arc unit --everything`.
  - Called `conduit.query`.
  - Browsed Conduit UI.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: hach-que, epriestley

Maniphest Tasks: T5873, T7803

Differential Revision: https://secure.phabricator.com/D12380
2015-04-13 11:58:35 -07:00
Joshua Spence
ec12b710aa Re-enable the deprecated calls setup check
Summary: This reverts commit 55c00ebfa1. T7111, T7112 and T7113 have all been resolved now.

Test Plan: Maybe wait a few more weeks... I just wanted to ensure that this wasn't forgotten.

Reviewers: btrahan, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11910
2015-04-07 18:08:49 +10:00
Joshua Spence
ea376685ae Fix some odd looking arrays
Summary: These arrays looks a little odd, most likely due to the autofix applied by `ArcanistXHPASTLinter::LINT_ARRAY_SEPARATOR`. See D12296 in which I attempt to improve the autocorrection from this linter rule.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12281
2015-04-05 22:29:39 +10:00
epriestley
c169199e64 Allow applications to have multiple "help" menu items
Summary:
Ref T7199. Convert the single help menu item into a dropdown and allow applications to list multiple items there.

When an application has mail command objects, link them in the menu.

Test Plan:
{F355925}

{F355926}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12244
2015-04-01 11:51:48 -07:00
Chad Little
423180430e Modernize Conduit app a bit
Summary: Remove some AphrontPanels, add some phts, fix some table layouts.

Test Plan: Browse many Conduit pages, test a few calls.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7427

Differential Revision: https://secure.phabricator.com/D11957
2015-03-03 11:09:59 -08:00
Joshua Spence
aaf8d73ec7 Fix pht method calls
Summary: Ref T7046. This is mainly a proof-of-concept for D11661.

Test Plan: `arc lint`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7046

Differential Revision: https://secure.phabricator.com/D11680
2015-02-10 18:57:45 +11:00
Bob Trahan
5a9df1a225 Policy - filter app engines where the user can't see the application from panel editing
Summary: Fixes T7118. This does the basic "filter the list" thing, though it ends up being a little manual since I guess this hasn't come up before? There is also potential weird behavior if the user was using an app and lost access to it - they will have nothing selected on edit - but I think this is actually correct behavior in this circumstance.

Test Plan:
used a user who couldn't get access to the "quick create" apps and noted that the dropdown list on dashboard panel create was missing the expected engines

ran `arc unit --everything` to verify abstract method implemented everywhere

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7118

Differential Revision: https://secure.phabricator.com/D11687
2015-02-04 15:47:48 -08:00
epriestley
55c00ebfa1 Disable the deprecated calls setup check until we remedy upstream calls 2015-02-02 14:27:02 -08:00
epriestley
bd905352f5 Fix a bad setup check call 2015-02-02 14:22:09 -08:00
epriestley
9af376a743 Add a setup warning for calls to deprecated Conduit methods
Summary: We probably can't land this yet, since `arc tasks` still uses `maniphest.find` and `arc close` still uses `differential.getrevision`. We should clean those up and wait at least 30 days before committing this (maybe).

Test Plan: Saw setup issues for `maniphest.find` and `differential.getrevision` calls.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley, joshuaspence, FacebookPOC, aran

Differential Revision: https://secure.phabricator.com/D6333
2015-02-02 14:19:10 -08:00
Chad Little
8b06804394 Remove getIconName from all applications
Summary: Not used anymore

Test Plan: grep for 'getIconName'

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: hach-que, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11582
2015-01-30 12:11:21 -08:00
Chad Little
5d8bb61dde Add FontIcon bridge to AppIcons
Summary: Select a similar or better FontAwesome icon to represent each application

Test Plan: Visual inspection

Reviewers: epriestley, btrahan

Subscribers: hach-que, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11489
2015-01-24 23:43:01 -08:00
epriestley
d94d1da610 Proxy Diffusion Conduit API calls
Summary:
Fixes T7020. When an external user makes a Conduit request to Diffusion but the repository isn't hosted locally, we need to proxy it.

This also adds a guard layer to prevent requests from getting infinitely proxied inside the cluster.

In "trivial" configurations (where the repository is a service repository, but the service is on the local device) I'm making us always proxy anyway. This basically makes it reasonable to test this stuff (otherwise you'd have to set up two different installs) and this configuration doesn't make much sense in real life (if you're using multiple machines, making one a dedicating daemons+repo box is almost certainly the most reasonable configuration, even for a cluster size of 2).

Test Plan:
  - With a service-hosted repository, made Diffusion conduit calls and browsed the UI. Verified requests got proxied once, then resovled.
  - With a non-service repository, made Diffusion conduit calls and browsed UI. Verified requests were handled in-process immediately.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7020

Differential Revision: https://secure.phabricator.com/D11475
2015-01-23 13:30:52 -08:00
Joshua Spence
fff0481184 Add method description to conduit.query
Summary: As suggested in T6950, add the method description to the response from `conduit.query`.

Test Plan: Called `echo '{}' | arc call-conduit conduit.query` and verified that the response contained the method description.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11467
2015-01-23 07:16:26 +11:00
Joshua Spence
1258ed348b Add return type to conduit.query
Summary: Fixes T6950. Adds the return type of Conduit API methods to the `conduit.query` call.

Test Plan: Called `echo '{}' | arc call-conduit conduit.query` and verified that the return types were present in the response.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T6950

Differential Revision: https://secure.phabricator.com/D11466
2015-01-23 07:16:15 +11:00
Joshua Spence
daadf95537 Fix visibility of PhutilArgumentWorkflow::didConstruct methods
Summary: Ref T6822.

Test Plan: `grep`. This method is only called from within `PhutilArgumentWorkflow::__construct`.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T6822

Differential Revision: https://secure.phabricator.com/D11415
2015-01-16 07:42:07 +11:00
Joshua Spence
c2ac63e9ad Increase visibility of PhabricatorController::buildApplicationMenu methods
Summary: Ref T6822. This method needs to be `public` because it is called from `PhabricatorApplicationSearchController::buildApplicationMenu()`.

Test Plan: I wouldn't expect //increasing// method visibility to break anything.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6822

Differential Revision: https://secure.phabricator.com/D11416
2015-01-16 07:41:26 +11:00
Joshua Spence
463d094f96 Fix method visibility for PhabricatorPolicyAwareQuery subclasses
Summary: Ref T6822.

Test Plan:
`grep` for the following:

  - `->willFilterPage(`
  - `->loadPage(`
  - `->didFilterPage(`
  - `->getReversePaging(`
  - `->didFilterPage(`
  - `->willExecute(`
  - `->nextPage(`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: hach-que, Korvin, epriestley

Maniphest Tasks: T6822

Differential Revision: https://secure.phabricator.com/D11367
2015-01-14 07:01:16 +11:00
Joshua Spence
d6b882a804 Fix visiblity of LiskDAO::getConfiguration()
Summary: Ref T6822.

Test Plan: `grep`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: hach-que, Korvin, epriestley

Maniphest Tasks: T6822

Differential Revision: https://secure.phabricator.com/D11370
2015-01-14 06:54:13 +11:00
Bob Trahan
6de8fe418c Conduit - document that ERR-CONDUIT-CORE is coming atchya
Summary: Fixes T6863. Seems like this belongs there?

Test Plan: loaded up an API in conduit console and saw the new error text

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6863

Differential Revision: https://secure.phabricator.com/D11261
2015-01-06 14:41:28 -08:00
Joshua Spence
e448386d39 Fix method visibility for PhabricatorApplicationSearchEngine methods
Summary: Ref T6822.

Test Plan: Visual inspection. These methods are only called from within the `PhabricatorApplicationSearchEngine` class.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6822

Differential Revision: https://secure.phabricator.com/D11242
2015-01-07 07:34:52 +11:00
Bob Trahan
384b670709 Fix string truncation calls all over the codebase.
Summary: Fixes T6608, though I'll also clean up the comment for PhutilStringTruncator in another diff. If I understand correctly, before T1191, MySQL column length was by character count and post T1191 its by byte count. Ergo, most of these changes are going from codepoint -> bytes. See test plan for complete list of what was and was not done.

Test Plan:
Thought very carefully about each callsite and made changes as appropos. "Display" means the string is clearly used for display-only purposes and correctly uses "glyph" already.

grep -rn PhutilUTF8StringTruncator *

applications/calendar/query/PhabricatorCalendarEventSearchEngine.php:217:        ->addAttribute(id(new PhutilUTF8StringTruncator())  -- display
applications/chatlog/controller/PhabricatorChatLogChannelLogController.php:111:      $author = id(new PhutilUTF8StringTruncator())  -- display
applications/conduit/method/ConduitConnectConduitAPIMethod.php:62:    $client_description = id(new PhutilUTF8StringTruncator()) -- was codepoint, changed to bytes
applications/conpherence/view/ConpherenceFileWidgetView.php:22:        ->setFileName(id(new PhutilUTF8StringTruncator()) -- display
applications/differential/controller/DifferentialDiffViewController.php:65:            id(new PhutilUTF8StringTruncator()) -- display
applications/differential/event/DifferentialHovercardEventListener.php:69:        id(new PhutilUTF8StringTruncator()) -- display
applications/differential/parser/DifferentialCommitMessageParser.php:144:      $short = id(new PhutilUTF8StringTruncator()) -- was glyphs, made to bytes
applications/differential/view/DifferentialLocalCommitsView.php:80:      $summary = id(new PhutilUTF8StringTruncator()) -- display
applications/diffusion/controller/DiffusionBrowseFileController.php:686:            id(new PhutilUTF8StringTruncator()) -- display
applications/feed/story/PhabricatorFeedStory.php:392:      $text = id(new PhutilUTF8StringTruncator()) -- display, unless people are saving the results of renderSummary() somewhere...
applications/harbormaster/storage/build/HarbormasterBuild.php:216:    $log_source = id(new PhutilUTF8StringTruncator()) -- was codepoints now bytes
applications/herald/storage/transcript/HeraldObjectTranscript.php:55:        // NOTE: PhutilUTF8StringTruncator has huge runtime for giant strings. -- not applicable
applications/maniphest/export/ManiphestExcelDefaultFormat.php:107:        id(new PhutilUTF8StringTruncator()) -- bytes
applications/metamta/storage/PhabricatorMetaMTAMail.php:587:        $body = id(new PhutilUTF8StringTruncator()) -- bytes
applications/people/event/PhabricatorPeopleHovercardEventListener.php:62:        id(new PhutilUTF8StringTruncator()) -- display
applications/phame/conduit/PhameCreatePostConduitAPIMethod.php:93:      id(new PhutilUTF8StringTruncator()) -- was codepoints, now bytes
applications/pholio/storage/PholioTransaction.php:300:        id(new PhutilUTF8StringTruncator()) -- display
applications/phortune/provider/PhortuneBalancedPaymentProvider.php:147:    $charge_as = id(new PhutilUTF8StringTruncator()) -- bytes
applications/ponder/storage/PonderAnswerTransaction.php:86:          id(new PhutilUTF8StringTruncator()) -- display
applications/ponder/storage/PonderQuestionTransaction.php:267:            id(new PhutilUTF8StringTruncator()) -- display
applications/ponder/storage/PonderQuestionTransaction.php:276:            id(new PhutilUTF8StringTruncator()) -- display
applications/repository/storage/PhabricatorRepositoryCommitData.php:43:    $summary = id(new PhutilUTF8StringTruncator()) -- was codepoints, now bytes
applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php:20:    $data->setAuthorName(id(new PhutilUTF8StringTruncator()) -- was codepoints, now bytes
applications/slowvote/query/PhabricatorSlowvoteSearchEngine.php:158:        $item->addAttribute(id(new PhutilUTF8StringTruncator()) -- display
infrastructure/daemon/workers/query/PhabricatorWorkerLeaseQuery.php:317:    $host = id(new PhutilUTF8StringTruncator()) -- bytes
view/form/control/AphrontFormPolicyControl.php:61:      $policy_short_name = id(new PhutilUTF8StringTruncator()) -- glyphs, probably display only

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6608

Differential Revision: https://secure.phabricator.com/D11219
2015-01-05 11:14:54 -08:00
epriestley
fa7bb8ff7a Add cluster.addresses and require membership before accepting cluster authentication tokens
Summary:
Ref T2783. Ref T6706.

  - Add `cluster.addresses`. This is a whitelist of CIDR blocks which define cluster hosts.
  - When we recieve a request that has a cluster-based authentication token, require the cluster to be configured and require the remote address to be a cluster member before we accept it.
    - This provides a general layer of security for these mechanisms.
    - In particular, it means they do not work by default on unconfigured hosts.
  - When cluster addresses are configured, and we receive a request //to// an address not on the list, reject it.
    - This provides a general layer of security for getting the Ops side of cluster configuration correct.
    - If cluster nodes have public IPs and are listening on them, we'll reject requests.
    - Basically, this means that any requests which bypass the LB get rejected.

Test Plan:
  - With addresses not configured, tried to make requests; rejected for using a cluster auth mechanism.
  - With addresses configred wrong, tried to make requests; rejected for sending from (or to) an address outside of the cluster.
  - With addresses configured correctly, made valid requests.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6706, T2783

Differential Revision: https://secure.phabricator.com/D11159
2015-01-02 15:13:41 -08:00
Joshua Spence
4e28de07fe Rename PhabricatorSettingsPanel subclasses for consistency
Summary: Ref T5655.

Test Plan: `arc lint` and `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D11136
2015-01-02 15:20:08 +11:00
epriestley
f18ee5c237 Generate and use "cluster" Conduit API tokens
Summary:
Ref T5955. Ref T2783.

  - Removes the "temporary" type. I was going to use this for T3628 but it started taking more time than I wanted to spend on it.
  - Add a "cluster" type, which is an internal-only token type used within a cluster. This token value is never shown to the user.
  - Automatically generate, use, and cycle cluster tokens.

Test Plan:
  - Diffusion (mostly) works with a repository configured to use a remote service.
  - Saw cluster tokens generate; terminated a cluster token and saw it regenerate.
  - Viewed cluster token in settings panel and saw nice explanatory text instead, as expected (we might just hide these eventually).

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2783, T5955

Differential Revision: https://secure.phabricator.com/D10990
2014-12-15 11:15:14 -08:00
epriestley
288498f8d0 Add conduit.getcapabilities and a modern CLI handshake workflow
Summary:
Ref T5955.

  - Add `conduit.getcapabilities` to help arc (and other clients) determine formats, protocols, etc., the server supports.
  - Fixes T3117. Add a more modern version of the handshake workflow that allows all generated tokens to remain valid for an hour.
  - Generally, add a CLI token type. This token type expires after an hour when generated, then becomes permanent if used.

Test Plan:
  - See D10988.
  - Ran `conduit.getcapabilities` and inspected output.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3117, T5955

Differential Revision: https://secure.phabricator.com/D10989
2014-12-15 11:14:53 -08:00
epriestley
0507626f01 Accept Conduit tokens as an authentication mechanism
Summary:
  - Ref T5955. Accept the tokens introduced in D10985 as an authentication token.
  - Ref T3628. Permit simple `curl`-compatible decoding of parameters.

Test Plan:
  - Ran some sensible `curl` API commands:

```
epriestley@orbital ~/dev/phabricator $ curl -g "http://local.phacility.com/api/user.whoami?api.token=api-f7dfpoyelk4mmz6vxcueb6hcbtbk" ; echo
{"result":{"phid":"PHID-USER-cvfydnwadpdj7vdon36z","userName":"admin","realName":"asdf","image":"http:\/\/local.phacility.com\/res\/1410737307T\/phabricator\/3eb28cd9\/rsrc\/image\/avatar.png","uri":"http:\/\/local.phacility.com\/p\/admin\/","roles":["admin","verified","approved","activated"]},"error_code":null,"error_info":null}
```

```
epriestley@orbital ~/dev/phabricator $ curl -g "http://local.phacility.com/api/differential.query?api.token=api-f7dfpoyelk4mmz6vxcueb6hcbtbk&ids[]=1" ; echo
{"result":[{"id":"1","phid":"PHID-DREV-v3a67ixww3ccg5lqbxee","title":"zxcb","uri":"http:\/\/local.phacility.com\/D1","dateCreated":"1418405590","dateModified":"1418405590","authorPHID":"PHID-USER-cvfydnwadpdj7vdon36z","status":"0","statusName":"Needs Review","branch":null,"summary":"","testPlan":"zxcb","lineCount":"6","activeDiffPHID":"PHID-DIFF-pzbtc5rw6pe5j2kxtlr2","diffs":["1"],"commits":[],"reviewers":[],"ccs":[],"hashes":[],"auxiliary":{"phabricator:projects":[],"phabricator:depends-on":[],"organization.sqlmigration":null},"arcanistProjectPHID":null,"repositoryPHID":null,"sourcePath":null}],"error_code":null,"error_info":null}
```

  - Ran older-style commands like `arc list` against the local install.
  - Ran commands via web console.
  - Added and ran a unit test to make sure nothing is using forbidden parameter names.
  - Terminated a token and verified it no longer works.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3628, T5955

Differential Revision: https://secure.phabricator.com/D10986
2014-12-15 11:14:41 -08:00
epriestley
39f2bbaeea Add Conduit Tokens to make authentication in Conduit somewhat more sane
Summary:
Ref T5955. Summary of intended changes:

**Improve Granularity of Authorization**: Currently, users have one Conduit Certificate. This isn't very flexible, and means that you can't ever generate an API token with limited permissions or IP block controls (see T6706). This moves toward a world where you can generate multiple tokens, revoke them individually, and assign disparate privileges to them.

**Standardize Token Management**: This moves Conduit to work the same way that sessions, OAuth authorizations, and temporary tokens already work, instead of being this crazy bizarre mess.

**Make Authentication Faster**: Authentication currently requires a handshake (conduit.connect) to establish a session, like the web UI. This is unnecessary from a security point of view and puts an extra round trip in front of all Conduit activity. Essentially no other API anywhere works like this.

**Make Authentication Simpler**: The handshake is complex, and involves deriving hashes. The session is also complex, and creates issues like T4377. Handshake and session management require different inputs.

**Make Token Management Simpler**: The certificate is this huge long thing right now, which is not necessary from a security perspective. There are separate Arcanist handshake tokens, but they have a different set of issues. We can move forward to a token management world where neither of these problems exist.

**Lower Protocol Barrier**: The simplest possible API client is very complex right now. It should be `curl`. Simplifying authentication is a necessary step toward this.

**Unblock T2783**: T2783 is blocked on nodes in the cluster making authenticated API calls to other nodes. This provides a simpler way forward than the handshake mess (or enormous-hack-mess) which would currently be required.

Test Plan:
  - Generated tokens.
  - Generated tokens for a bot account.
  - Terminated tokens (and for a bot account).
  - Terminated all tokens (and for a bot account).
  - Ran GC and saw it reap all the expired tokens.

NOTE: These tokens can not actually be used to authenticate yet!

{F249658}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5955

Differential Revision: https://secure.phabricator.com/D10985
2014-12-15 11:14:23 -08:00
epriestley
db51d7d92a Make ConduitCall always local/in-process
Summary:
Ref T2783. ConduitCall currently has logic to pick a random remote server, but this is ultimately not appropriate: we always want to send requests to a specific server. For example, we want to send repository requests to a server which has that repository locally. The repository tier is not homogenous, so we can't do this below the call level.

Make ConduitCall always-local; logic above it will select ConduitCall for an in-process request or do service selection for an off-host request via ConduitClient.

Test Plan:
  - Browsed some pages using ConduitCall, everything worked.
  - Grepped for removed stuff.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2783

Differential Revision: https://secure.phabricator.com/D10959
2014-12-10 15:27:07 -08:00
epriestley
5e0f218fe4 Allow device SSH keys to be trusted
Summary:
Ref T6240. Some discussion in that task. In instance/cluster environments, daemons need to make Conduit calls that bypass policy checks.

We can't just let anyone add SSH keys with this capability to the web directly, because then an adminstrator could just add a key they own and start signing requests with it, bypassing policy checks.

Add a `bin/almanac trust-key --id <x>` workflow for trusting keys. Only trusted keys can sign requests.

Test Plan:
  - Generated a user key.
  - Generated a device key.
  - Trusted a device key.
  - Untrusted a device key.
  - Hit the various errors on trust/untrust.
  - Tried to edit a trusted key.

{F236010}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6240

Differential Revision: https://secure.phabricator.com/D10878
2014-11-20 17:33:30 -08:00
epriestley
ffc8a7edc7 Minor, correct spelling of PKCS8 key format
Summary: I faked this out locally because of the OSX stuff and goofed the
key format spelling.

Auditors: btrahan
2014-11-17 19:54:17 -08:00
epriestley
657b36dd06 Allow Phabricator to accept Conduit requests signed with an SSH key
Summary:
Ref T4209.  Depends on D10402.

This updates Conduit to support authenticating calls from other servers by signing the request parameters with the sending server's private key and verifying it with the public key stored in the database.

Test Plan:
  - Made like 500 bad calls using the stuff in D10402.
  - Made a few valid calls using the stuff in D10402.

Reviewers: hach-que, btrahan, #blessed_reviewers

Reviewed By: btrahan, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T6240, T4209

Differential Revision: https://secure.phabricator.com/D10401
2014-11-17 13:11:52 -08:00
epriestley
9352c76e81 Decouple some aspects of request routing and construction
Summary:
Ref T5702. This is a forward-looking change which provides some very broad API improvements but does not implement them. In particular:

  - Controllers no longer require `$request` to construct. This is mostly for T5702, directly, but simplifies things in general. Instead, we call `setRequest()` before using a controller. Only a small number of sites activate controllers, so this is less code overall, and more consistent with most constructors not having any parameters or effects.
  - `$request` now offers `getURIData($key, ...)`. This is an alternate way of accessing `$data` which is currently only available on `willProcessRequest(array $data)`. Almost all controllers which implement this method do so in order to read one or two things out of the URI data. Instead, let them just read this data directly when processing the request.
  - Introduce `handleRequest(AphrontRequest $request)` and deprecate (very softly) `processRequest()`. The majority of `processRequest()` calls begin `$request = $this->getRequest()`, which is avoided with the more practical signature.
  - Provide `getViewer()` on `$request`, and a convenience `getViewer()` on `$controller`. This fixes `$viewer = $request->getUser();` into `$viewer = $request->getViewer();`, and converts the `$request + $viewer` two-liner into a single `$this->getViewer()`.

Test Plan:
  - Browsed around in general.
  - Hit special controllers (redirect, 404).
  - Hit AuditList controller (uses new style).

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5702

Differential Revision: https://secure.phabricator.com/D10698
2014-10-17 05:01:40 -07:00
Joshua Spence
3cf9a5820f Minor formatting changes
Summary: Apply some autofix linter rules.

Test Plan: `arc lint` and `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D10585
2014-10-08 08:39:49 +11:00
epriestley
8fa8415c07 Automatically build all Lisk schemata
Summary:
Ref T1191. Now that the whole database is covered, we don't need to do as much work to build expected schemata. Doing them database-by-database was helpful in converting, but is just reudndant work now.

Instead of requiring every application to build its Lisk objects, just build all Lisk objects.

I removed `harbormaster.lisk_counter` because it is unused.

It would be nice to autogenerate edge schemata, too, but that's a little trickier.

Test Plan: Database setup issues are all green.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley, hach-que

Maniphest Tasks: T1191

Differential Revision: https://secure.phabricator.com/D10620
2014-10-02 09:51:20 -07:00
epriestley
300172e799 Support AUTO_INCREMENT in bin/storage adjust
Summary:
Ref T1191. When changing the column type of an AUTO_INCREMENT column, we currently may lose the autoincrement attribute.

Instead, support it. This is a bit messy because AUTO_INCREMENT columns interact with PRIMARY KEY columns (tables may only have one AUTO_INCREMENT column, and it must be a primary key). We need to migrate in more phases to avoid this issue.

Introduce new `auto` and `auto64` types to represent autoincrement IDs.

Test Plan:
  - Saw autoincrement show up correctly in web UI.
  - Fixed an autoincrement issue on the XHProf storage table with `bin/storage adjust` safely.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1191

Differential Revision: https://secure.phabricator.com/D10607
2014-10-01 08:24:51 -07:00