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

459 commits

Author SHA1 Message Date
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
epriestley
4fcc634a99 Fix almost all remaining schemata issues
Summary:
Ref T1191. This fixes nearly every remaining blocker for utf8mb4 -- primarily, overlong keys.

Remaining issue is https://secure.phabricator.com/T1191#77467

Test Plan: I'll annotate inline.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley, hach-que

Maniphest Tasks: T6099, T6129, T6133, T6134, T6150, T6148, T6147, T6146, T6105, T1191

Differential Revision: https://secure.phabricator.com/D10601
2014-10-01 08:18:36 -07:00
epriestley
943c62d1e9 Add missing expected keys and uniqueness
Summary:
Ref T1191.

  - Adds definitions for missing keys and keys with wrong uniqueness. Generally, I defined these before fixing the key query to actually pull all keys and support uniqueness.
  - Moves "key uniqueness" to note severity; this is fixable (probably?) and there are no remaining issues.
  - Moves "Missing Key" to note severity; missing keys are fixable and all remaining missing keys are really missing (either missing edge keys, or missing PHID keys):

{F210089}

  - Moves "Surplus Key" to note seveirty; surplus keys are fixable all remaining surplus keys are really surplus (duplicate key in Harbormaster, key on unused column in Worker):

{F210090}

Test Plan:
  - Vetted missing/surplus/unique messages.
  - 146 issues remaining.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1191

Differential Revision: https://secure.phabricator.com/D10590
2014-10-01 07:53:50 -07:00
epriestley
1ead50c2cc Generate reasonable expected schemata for Chatlog, Conduit, Config, Countdown, Daemons
Summary: Ref T1191. Fills in some more of the databases. Nothing very notable here. I didn't encounter any issues or overlong keys.

Test Plan: Used web UI to click around and verify expected schemata match up against actual schemata well.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1191

Differential Revision: https://secure.phabricator.com/D10516
2014-09-18 11:15:29 -07:00
Joshua Spence
0151c38b10 Apply some autofix linter rules
Summary: Self-explanatory.

Test Plan: Eyeball it.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10454
2014-09-10 06:55:05 +10:00
Bob Trahan
b93bc7e479 phutil_utf8_shorten => PhutilUTF8StringTruncator
Summary: Ref T3307. Only one I thought was tricky was Excel; I went with bytes there like it was email.

Test Plan: played around on a few endpoints but mostly thought carefully

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T3307

Differential Revision: https://secure.phabricator.com/D10392
2014-08-29 15:15:13 -07:00
epriestley
d09d7ffe1f 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
2014-08-11 12:08:06 -07:00
Mukunda Modell
5f82705e12 First stab at a conduit method for creating projects.
Summary: This code is mostly lifted from the PhabricatorProjectCreateController.

Test Plan: currently untested

Reviewers: rush898, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, aklapper, Korvin

Maniphest Tasks: T5691

Differential Revision: https://secure.phabricator.com/D10036
2014-08-05 09:29:30 -07:00
Joshua Spence
1450c0e8d6 Omit unnecessary function call
Summary: As mentioned on rP8ce35e6b67e7e2a81b274bab7a6dd19dedb4df06, `setConcreteOnly(true)` can be omitted since (lacking magical powers) `loadObjects()` will always only instantiate concrete objects.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10050
2014-07-25 23:17:52 +10:00
Joshua Spence
8ce35e6b67 Fix an issue with ConduitQueryConduitAPIMethod
Fixes an issue with D9991. A user was hitting the following exception:

```
echo '{}' | arc --conduit-uri='http://phabricator.joshuaspence.com'
call-conduit conduit.query
Waiting for JSON parameters on stdin...
Exception
[HTTP/500] Internal Server Error
>>> UNRECOVERABLE FATAL ERROR <<<

Call to a member function getAPIMethodName() on a non-object

/usr/src/phabricator/src/applications/conduit/method/ConduitQueryConduitAPIMethod.php:34

┻━┻ ︵ ¯\_(ツ)_/¯ ︵ ┻━┻
(Run with --trace for a full exception trace.)
```

Auditors: epriestley
2014-07-25 15:54:14 +10:00
epriestley
20589389de Fix some issues with new Conduit method implementations
Summary: Ref T5655. A few of these were missed.

Test Plan:
Checked all other methods like this:

```
    foreach ($method_map as $k => $v) {
      $v = preg_replace('/ConduitAPIMethod$/', '', $v);
      $k = str_replace('.', '', $k);
      $v = strtolower($v);
      if ($k != $v) {
        echo "{$k} x {$v}!\n";
      }
    }
    echo "OK\n";
```

Reviewers: hach-que, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D10049
2014-07-24 21:57:03 -07:00
Joshua Spence
bff217efd3 Don't log Conduit 404s as errors
Summary: Fixes T5695. A Conduit "method does not exist" exception is somewhat expected... there is no need to `phlog` the exception.

Test Plan: Called a non-existent Conduit method. Saw no exceptions in the error logs.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5695

Differential Revision: https://secure.phabricator.com/D10042
2014-07-25 11:24:31 +10:00
Joshua Spence
023dee0d3b Rename Conduit classes
Summary: Ref T5655. Rename Conduit classes and provide a `getAPIMethodName` method to declare the API method.

Test Plan:
```
> echo '{}' | arc --conduit-uri='http://phabricator.joshuaspence.com' call-conduit user.whoami
Waiting for JSON parameters on stdin...
{"error":null,"errorMessage":null,"response":{"phid":"PHID-USER-lioqffnwn6y475mu5ndb","userName":"josh","realName":"Joshua Spence","image":"http:\/\/phabricator.joshuaspence.com\/res\/1404425321T\/phabricator\/3eb28cd9\/rsrc\/image\/avatar.png","uri":"http:\/\/phabricator.joshuaspence.com\/p\/josh\/","roles":["admin","verified","approved","activated"]}}
```

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin, hach-que

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D9991
2014-07-25 10:54:15 +10:00
Joshua Spence
0c8f487b0f Implement the getName method in PhabricatorApplication subclasses
Summary: Provide an implementation for the `getName` method rather than automagically determining the application name.

Test Plan: Saw reasonable application names in the launcher.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10027
2014-07-23 23:52:50 +10:00
Joshua Spence
86c399b657 Rename PhabricatorApplication subclasses
Summary: Ref T5655. Some discussion in D9839. Generally speaking, `Phabricator{$name}Application` is clearer than `PhabricatorApplication{$name}`.

Test Plan:
# Pinned and uninstalled some applications.
# Applied patch and performed migrations.
# Verified that the pinned applications were still pinned and that the uninstalled applications were still uninstalled.
# Performed a sanity check on the database contents.

Reviewers: btrahan, epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: hach-que, epriestley, Korvin

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D9982
2014-07-23 10:03:09 +10:00
James Rhodes
9cb6b2cfcc Remove user-independent date and time functions from Phabricator
Summary: These have been moved into libphutil.

Test Plan: Browsed Phabricator, didn't see a crash.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9907
2014-07-13 12:03:17 +10:00
Bob Trahan
e281c5ee90 Security - disable conduit act as user by default
Summary: Introduce a new configuration setting that by default disables the conduit as as user method. Wordily explain that turning it on is not recommended. Fixes T3818.

Test Plan:
```
15:25:19 ~/Dropbox/code/phalanx/src/applications/conduit (T3818)
~>  echo '{}' | arc call-conduit --conduit-uri http://phalanx.dev/ user.whoami
Waiting for JSON parameters on stdin...
{"error":null,"errorMessage":null,"response":{"phid":"PHID-USER-tghb3b2gbdyezdcuw2or","userName":"btrahan","realName":"Bob Trahan","image":"http:\/\/phalanx.dev\/file\/data\/yncjbh7phk7ktrdhuorn\/PHID-FILE-qyf4ui3x2ll3e52hpg5e\/profile-profile-gravatar","uri":"http:\/\/phalanx.dev\/p\/btrahan\/","roles":["admin","verified","approved","activated"]}}
15:25:34 ~/Dropbox/code/phalanx/src/applications/conduit (T3818)

<go edit libconfig/conduitclient to spoof another user...>

~>  echo '{}' | arc call-conduit --conduit-uri http://phalanx.dev/ user.whoami
Waiting for JSON parameters on stdin...
{"error":"ERR-CONDUIT-CORE","errorMessage":"ERR-CONDUIT-CORE: security.allow-conduit-act-as-user is disabled","response":null}
15:26:40 ~/Dropbox/code/phalanx/src/applications/conduit (T3818)

<enable option via bin/config....>

~>  echo '{}' | arc call-conduit --conduit-uri http://phalanx.dev/ user.whoami
Waiting for JSON parameters on stdin...
{"error":null,"errorMessage":null,"response":{"phid":"PHID-USER-6lcglnzbkiamdofishgi","userName":"xerxes","realName":"Xerxes Trahan","image":"http:\/\/phalanx.dev\/file\/data\/n2kyeevowetcuynbcxrg\/PHID-FILE-voquikectzpde256zzvm\/profile-1275455993.jpg","uri":"http:\/\/phalanx.dev\/p\/xerxes\/","roles":["verified","approved","activated"]}}
```

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: jevripio, sowedance, epriestley, Korvin

Maniphest Tasks: T3818

Differential Revision: https://secure.phabricator.com/D9881
2014-07-10 15:43:53 -07:00
Joshua Spence
8756d82cf6 Remove @group annotations
Summary: I'm pretty sure that `@group` annotations are useless now... see D9855. Also fixed various other minor issues.

Test Plan: Eye-ball it.

Reviewers: #blessed_reviewers, epriestley, chad

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D9859
2014-07-10 08:12:48 +10:00
epriestley
85e9f8374a Don't validate Conduit hosts
Summary:
(See rPd1d3bf4e / rPf371c7b3.) Just get rid of this logic, I don't think there's any value to it.

IIRC, this was added a long time ago to deal with some issues that users had configuring things, but I think modern Phabricator covers all this stuff and I haven't seen any confusion from users for a year or more.

(Generally, I want to generally make Conduit easier to use, and this makes it more difficult.)

Test Plan: `grep`

Reviewers: joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D9703
2014-06-23 17:41:02 -07:00
epriestley
46d9bebc84 Remove all device = true from page construction
Summary: Fixes T5446. Depends on D9687.

Test Plan: Mostly regexp'd this. Lint doesn't complain.

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley, hach-que

Maniphest Tasks: T5446

Differential Revision: https://secure.phabricator.com/D9690
2014-06-23 15:18:14 -07:00
Joshua Spence
21a2597421 Complain if a Conduit Client doesn't send a host key.
Summary: There is a TODO here that is a few years old... the Conduit Protocol is now at version 7.

Test Plan: One less TODO in the codebase.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9694
2014-06-24 04:23:07 +10:00
epriestley
b8bc0aa2b0 Allow users to select QueryPanel search engines from a list
Summary: Ref T4986. Instead of requiring users to know the name of an application search engine class, let them select from a list.

Test Plan:
Created a new panel.

{F165468}

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T4986

Differential Revision: https://secure.phabricator.com/D9500
2014-06-12 13:22:20 -07:00
Joshua Spence
d0128afa29 Applied various linter fixes.
Summary: Applied some more linter fixes that I previously missed because my global `arc` install was out-of-date.

Test Plan: Will run `arc unit` on another host.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9443
2014-06-09 16:04:12 -07:00
Joshua Spence
0a62f13464 Change double quotes to single quotes.
Summary: Ran `arc lint --apply-patches --everything` over rP, mainly to change double quotes to single quotes where appropriate. These changes also validate that the `ArcanistXHPASTLinter::LINT_DOUBLE_QUOTE` rule is working as expected.

Test Plan: Eyeballed it.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D9431
2014-06-09 11:36:50 -07:00
epriestley
6df1a02413 (Redesign) Clean up older "Tile" code
Summary:
This does some backend cleanup of the tile stuff, and some general cleanup of other application things:

  - Users who haven't customized preferences get a small, specific set of pinned applications: Differential, Maniphest, Diffusion, Audit, Phriction, Projects (and, for administrators, Auth, Config and People).
  - Old tile size methods are replaced with `isPinnnedByDefault()`.
  - Shortened some short descriptions.
  - `shouldAppearInLaunchView()` replaced by less ambiguous `isLaunchable()`.
  - Added a marker for third-party / extension applications.

Test Plan: Faked away my preferences and viewed the home page, saw a smaller set of default pins.

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D9358
2014-06-03 15:47:27 -07:00
epriestley
81d95cf682 Make default view of "Applications" app a full-page launcher
Summary:
This probably needs some tweaks, but the idea is to make it easier to browse and access applications without necessarily needing them to be on the homepage.

Open to feedback.

Test Plan:
(This screenshot merges "Organization", "Communication" and "Core" into a single "Core" group. We can't actually do this yet because it wrecks the homepage.)

{F160052}

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T5176

Differential Revision: https://secure.phabricator.com/D9297
2014-05-29 12:17:54 -07:00
epriestley
15561a27c3 When a conduit method requires a string constant, call it "string-const" not "enum"
Summary: Ref T5058. The use of "enum" is confusing; we mean "choose one of these specific string constants". Make this more clear.

Test Plan: Viewed each call from the web UI.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5058

Differential Revision: https://secure.phabricator.com/D9127
2014-05-14 21:59:03 -07:00
epriestley
9d1cfcd8ec Move Conduit rendering to SearchEngine
Summary: Ref T4986. Nothing special.

Test Plan: Looked at Conduit, made a panel.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4986

Differential Revision: https://secure.phabricator.com/D9013
2014-05-08 20:04:18 -07:00
epriestley
50376aad04 Require multiple auth factors to establish web sessions
Summary:
Ref T4398. This prompts users for multi-factor auth on login.

Roughly, this introduces the idea of "partial" sessions, which we haven't finished constructing yet. In practice, this means the session has made it through primary auth but not through multi-factor auth. Add a workflow for bringing a partial session up to a full one.

Test Plan:
  - Used Conduit.
  - Logged in as multi-factor user.
  - Logged in as no-factor user.
  - Tried to do non-login-things with a partial session.
  - Reviewed account activity logs.

{F149295}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4398

Differential Revision: https://secure.phabricator.com/D8922
2014-05-01 10:23:02 -07:00
epriestley
3fde020049 Make many actions require high security
Summary:
Ref T4398. Protects these actions behind a security barrier:

  - Link external account.
  - Retrieve Conduit token.
  - Reveal Passphrase credential.
  - Create user.
  - Admin/de-admin user.
  - Rename user.
  - Show conduit certificate.
  - Make primary email.
  - Change password.
  - Change VCS password.
  - Add SSH key.
  - Generate SSH key.

Test Plan: Tried to take each action and was prompted for two-factor.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4398

Differential Revision: https://secure.phabricator.com/D8921
2014-04-30 17:44:59 -07:00
Bob Trahan
08d9e5ec99 Use initializeNewLog rather than instantiate the UserLog
Summary:
Use initializeNewLog rather than instantiate the UserLog,
Closes T4912

Test Plan: Run install-certificate

Reviewers: #blessed_reviewers, btrahan

Reviewed By: #blessed_reviewers, btrahan

Subscribers: epriestley

Maniphest Tasks: T4912

Differential Revision: https://secure.phabricator.com/D8887
2014-04-28 15:44:52 -07:00
epriestley
27d426e3fe Allow Conduit console to be browsed by logged-out users
Summary:
Ref T4830. A few methods, like `conduit.ping`, are callable without authentication, so this even has some use cases. Also:

  - Make some Differential stuff a little more consistent.
  - Use slightly more modern rendering.
  - Deprecate the status-oriented `user` calls; these will be replaced by Calendar methods.

Test Plan: Browsed console as logged out / logged in users.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4830

Differential Revision: https://secure.phabricator.com/D8826
2014-04-21 15:32:48 -07:00
epriestley
3b0be0961c Add a rough harbormaster.querybuildables Conduit API method
Summary: Ref T4809. I need to sort out some of the "status" stuff we're doing before this is actually useful (there's no sensible "status" value to expose right now) but once that happens `arc` can query this to figure out whether it needs to warn the user about pending/failed builds.

Test Plan: Ran query with various different parameters.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4809

Differential Revision: https://secure.phabricator.com/D8794
2014-04-17 16:00:25 -07:00
epriestley
38cc38eaf6 Modernize documentation links
Summary:
  - Point them at the new Diviner.
  - Make them a little less cumbersome to write.

Test Plan: Found almost all of these links in the UI and clicked them.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8553
2014-03-17 15:01:31 -07:00
Joshua Spence
e11adc4ad7 Added some additional assertion methods.
Summary:
There are quite a few tests in Arcanist, libphutil and Phabricator that do something similar to `$this->assertEqual(false, ...)` or `$this->assertEqual(true, ...)`.

This is unnecessarily verbose and it would be cleaner if we had `assertFalse` and `assertTrue` methods.

Test Plan: I contemplated adding a unit test for the `getCallerInfo` method but wasn't sure if it was required / where it should live.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D8460
2014-03-08 19:16:21 -08:00
epriestley
72a062d802 Truncate Conduit 'clientDescription' so we don't overflow the column
Summary: We store the `arc` commandline in this 255-character column, but it can be more than 255-characters long. If it's huge, truncate it.

Test Plan:
Executed:

  arc list --conduit-uri=http://local.aphront.com:8080/ --conduit-version 6.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

Works fine after this patch.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D8327
2014-02-25 12:35:03 -08:00
epriestley
d112b6c15d Add diffusion.querycommits and deprecate diffusion.getcommits
Summary:
Fixes T4344. `diffusion.getcommits` is nasty old bad news. Implement a modern query method.

This method provides limit/paging in a somewhat abstract way so it's sort of ultramodern, but I didn't want the default behavior to return a million rows. I'll probably move more stuff toward this over time, now that cursor paging is pervasive. Here, we needed extra metadata (the identifier map) anyway.

Test Plan: Used console to execute command.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4344

Differential Revision: https://secure.phabricator.com/D8077
2014-01-27 17:14:21 -08:00
epriestley
9f35c7cc26 Complete modularization of the GC daemon
Summary: This modularizes the rest of the GC submethods. Turned out there was nothing tricky.

Test Plan: Ran `bin/phd debug garbage` and got reasonable looking behavior and output.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7971
2014-01-15 10:02:31 -08:00
epriestley
d392a8f157 Replace "web" and "conduit" magic session strings with constants
Summary: Ref T4310. Ref T3720. We use bare strings to refer to session types in several places right now; use constants instead.

Test Plan: grep; logged out; logged in; ran Conduit commands.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4310, T3720

Differential Revision: https://secure.phabricator.com/D7963
2014-01-14 13:22:34 -08:00
epriestley
eef314b701 Separate session management from PhabricatorUser
Summary: Ref T4310. Ref T3720. Session operations are currently part of PhabricatorUser. This is more tightly coupled than needbe, and makes it difficult to establish login sessions for non-users. Move all the session management code to a `SessionEngine`.

Test Plan:
  - Viewed sessions.
  - Regenerated Conduit certificate.
  - Verified Conduit sessions were destroyed.
  - Logged out.
  - Logged in.
  - Ran conduit commands.
  - Viewed sessions again.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4310, T3720

Differential Revision: https://secure.phabricator.com/D7962
2014-01-14 13:22:27 -08:00
epriestley
3fc807bed1 Fix exception for new error stuff in Conduit
Summary: This might be `null`.

Test Plan: Loaded, got exception, applied patch, no exception. Viewed a method with an actual message too, that also worked.

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D7955
2014-01-13 15:32:37 -08:00
Chad Little
b74c7a3d37 Simplify PHUIObjectBoxViews handling of Save and Error states
Summary: This removes the bulk of the "Form Errors" text, some variations likely exists. These are a bit redundant and space consuming. I'd also like to back ErrorView more into PHUIObjectBox.

Test Plan: Test out the forms, see errors without the text.

Reviewers: epriestley, btrahan

CC: Korvin, epriestley, aran, hach-que

Differential Revision: https://secure.phabricator.com/D7924
2014-01-10 09:17:37 -08:00
epriestley
44c9a94abe Make conduit certificate readonly and select-on-click
Summary: See comments in <https://secure.phabricator.com/D6331#comment-3> -- make the Conduit Token and Conduit Certificate interfaces readonly and select-on-click.

Test Plan:
  - Viewed `/conduit/token/`, verified it was readonly and selected on click.
  - Viewed `/settings/panel/conduit/`, likewise.

Reviewers: Avish, btrahan, wotte

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7819
2013-12-23 10:43:53 -08:00
epriestley
a5dc9067af Provide convenience method addTextCrumb() to PhabricatorCrumbsView
Summary: We currently have a lot of calls to `addCrumb(id(new PhabricatorCrumbView())->...)` which can be expressed much more simply with a convenience method. Nearly all crumbs are only textual.

Test Plan:
  - This was mostly automated, then I cleaned up a few unusual sites manually.
  - Bunch of grep / randomly clicking around.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: hach-que, aran

Differential Revision: https://secure.phabricator.com/D7787
2013-12-18 17:47:34 -08:00
epriestley
7f11e8d740 Improve handling of email verification and "activated" accounts
Summary:
Small step forward which improves existing stuff or lays groudwork for future stuff:

  - Currently, to check for email verification, we have to single-query the email address on every page. Instead, denoramlize it into the user object.
    - Migrate all the existing users.
    - When the user verifies an email, mark them as `isEmailVerified` if the email is their primary email.
    - Just make the checks look at the `isEmailVerified` field.
  - Add a new check, `isUserActivated()`, to cover email-verified plus disabled. Currently, a non-verified-but-not-disabled user could theoretically use Conduit over SSH, if anyone deployed it. Tighten that up.
  - Add an `isApproved` flag, which is always true for now. In a future diff, I want to add a default-on admin approval queue for new accounts, to prevent configuration mistakes. The way it will work is:
    - When the queue is enabled, registering users are created with `isApproved = false`.
    - Admins are sent an email, "[Phabricator] New User Approval (alincoln)", telling them that a new user is waiting for approval.
    - They go to the web UI and approve the user.
    - Manually-created accounts are auto-approved.
    - The email will have instructions for disabling the queue.

I think this queue will be helpful for new installs and give them peace of mind, and when you go to disable it we have a better opportunity to warn you about exactly what that means.

Generally, I want to improve the default safety of registration, since if you just blindly coast through the path of least resistance right now your install ends up pretty open, and realistically few installs are on VPNs.

Test Plan:
  - Ran migration, verified `isEmailVerified` populated correctly.
  - Created a new user, checked DB for verified (not verified).
  - Verified, checked DB (now verified).
  - Used Conduit, People, Diffusion.

Reviewers: btrahan

Reviewed By: btrahan

CC: chad, aran

Differential Revision: https://secure.phabricator.com/D7572
2013-11-12 14:37:04 -08:00
Jakub Vrana
a29b5b070f Replace some hsprintf() by phutil_tag()
Test Plan: Looked at a diff with inline comment.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7549
2013-11-11 09:23:23 -08:00
Anirudh Sanjeev
1bcf478c69 Fix weird issue where INF is not respected in PHP 5.5
Summary:
After upgrading to PHP 5.5, the conduit list was not fully visible because
INF was being treated as "0" for some reason. Fixed by making it a PHP_MAX_INT

Test Plan: Checked on PHP 5.5

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7530
2013-11-08 07:07:10 -08:00
epriestley
90a9e90675 Record Conduit calls in the service profiler
Summary: This puts Conduit calls into the "Services" tab. They aren't always real service calls, but I think they're big enough to belong there and be useful.

Test Plan: Viewed "Services" tab, saw conduit calls.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7482
2013-11-04 12:15:48 -08:00
epriestley
888b3839e7 Prepare to route VCS connections through SSH
Summary:
Fixes T2229. This sets the stage for a patch similar to D7417, but for SSH. In particular, SSH 6.2 introduced an `AuthorizedKeysCommand` directive, which lets us do this in a mostly-reasonable way without needing users to patch sshd (if they have a recent enough version, at least).

The way the `AuthorizedKeysCommand` works is that it gets run and produces an `authorized_keys`-style file fragment. This isn't ideal, because we have to dump every key into the result, but should be fine for most installs. The earlier patch against `sshd` passes the public key itself, which allows the script to just look up the key. We might use this eventually, since it can scale much better, so I haven't removed it.

Generally, auth is split into two scripts now which mostly do the same thing:

  - `ssh-auth` is the AuthorizedKeysCommand auth, which takes nothing and dumps the whole keyfile.
  - `ssh-auth-key` is the slightly cleaner and more scalable (but patch-dependent) version, which takes the public key and dumps only matching options.

I also reworked the argument parsing to be a bit more sane.

Test Plan:
This is somewhat-intentionally a bit obtuse since I don't really want anyone using it yet, but basically:

  - Copy `phabricator-ssh-hook.sh` to somewhere like `/usr/libexec/openssh/`, chown it `root` and chmod it `500`.
    - This script should probably also do a username check in the future.
  - Create a copy of `sshd_config` and fix the paths/etc. Point the KeyScript at your copy of the hook.
  - Start a copy of sshd (6.2 or newer) with `-f <your config file>` and maybe `-d -d -d` to foreground and debug.
  - Run `ssh -p 2222 localhost` or similar.

Specifically, I did this setup and then ran a bunch of commands like:

  - `ssh host` (denied, no command)
  - `ssh host ls` (denied, not supported)
  - `echo '{}' | ssh host conduit conduit.ping` (works)

Reviewers: btrahan

Reviewed By: btrahan

CC: hach-que, aran

Maniphest Tasks: T2229, T2230

Differential Revision: https://secure.phabricator.com/D7419
2013-10-29 15:32:40 -07:00
Chad Little
6bdedfe679 Add ObjectBox to Condiut Cert Page
Summary: adds a box, Fixes T4033

Test Plan: look at box

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T4033

Differential Revision: https://secure.phabricator.com/D7437
2013-10-29 14:54:10 -07:00
epriestley
90b83d7a92 Fix logged-out Diffusion calls to Conduit
Summary:
Conduit doesn't currently have an analog to "shouldAllowPublic", so the recent policy checks added here caught legitimate Conduit calls when viewing Diffusion as a logged-out user.

Add `shouldAllowPublic()` and set it for all the Diffusion queries.

(More calls probably need this, but we can add it when we hit them.)

Test Plan: Looked at Diffusion as a logged-out user with public access enabled.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7380
2013-10-22 13:47:52 -07:00
epriestley
2a5c987c71 Lock policy queries to their applications
Summary:
While we mostly have reasonable effective object accessibility when you lock a user out of an application, it's primarily enforced at the controller level. Users can still, e.g., load the handles of objects they can't actually see. Instead, lock the queries to the applications so that you can, e.g., never load a revision if you don't have access to Differential.

This has several parts:

  - For PolicyAware queries, provide an application class name method.
  - If the query specifies a class name and the user doesn't have permission to use it, fail the entire query unconditionally.
  - For handles, simplify query construction and count all the PHIDs as "restricted" so we get a UI full of "restricted" instead of "unknown" handles.

Test Plan:
  - Added a unit test to verify I got all the class names right.
  - Browsed around, logged in/out as a normal user with public policies on and off.
  - Browsed around, logged in/out as a restricted user with public policies on and off. With restrictions, saw all traces of restricted apps removed or restricted.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7367
2013-10-21 17:20:27 -07:00
epriestley
7180199d13 Fix daemon auth issue
Summary: I touched this code recently but it needs an unusual special case because we call through with the "omnipotent user" from the daemons. As per the TODO below, this will all get cleaned up at some point.

Test Plan: Will make @poop verify.

Reviewers: btrahan, poop

Reviewed By: poop

CC: poop, aran

Differential Revision: https://secure.phabricator.com/D7356
2013-10-18 18:29:36 -07:00
epriestley
5171e3684c Require application "Can Use" capability to call Conduit methods
Summary: Ref T603. If you don't have access to an application, prevent execution of its (authenticated) methods.

Test Plan: Restricted Tokens to only admins, then tried to view/call Token methods as a non-admin.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7342
2013-10-17 12:51:19 -07:00
epriestley
073cb0e78c Make PhabricatorPolicyInterface require a getPHID() method
Summary:
Ref T603. This cleans up an existing callsite in the policy filter, and opens up some stuff in the future.

Some policy objects don't have real PHIDs:

  PhabricatorTokenGiven
  PhabricatorSavedQuery
  PhabricatorNamedQuery
  PhrequentUserTime
  PhabricatorFlag
  PhabricatorDaemonLog
  PhabricatorConduitMethodCallLog
  ConduitAPIMethod
  PhabricatorChatLogEvent
  PhabricatorChatLogChannel

Although it would be reasonable to add real PHIDs to some of these (like `ChatLogChannel`), it probably doesn't make much sense for others (`DaemonLog`, `MethodCallLog`). Just let them return `null`.

Also remove some duplicate `$id` and `$phid` properties. These are declared on `PhabricatorLiskDAO` and do not need to be redeclared.

Test Plan: Ran the `testEverythingImplemented` unit test, which verifies that all classes conform to the interface.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7306
2013-10-14 14:35:47 -07:00
epriestley
2e5ac128b3 Explain policy exception rules to users
Summary:
Ref T603. Adds clarifying text which expands on policies and explains exceptions and rules. The goal is to provide an easy way for users to learn about special policy rules, like "task owners can always see a task".

This presentation might be a little aggressive. That's probably OK as we introduce policies, but something a little more tempered might be better down the road.

Test Plan: See screenshot.

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7150
2013-09-27 08:43:41 -07:00
Chad Little
9be7a948f9 Move PHUIFormBoxView to PHUIObjectBoxView
Summary: I'd like to reuse this for other content areas, renaming for now. This might be weird to keep setForm, but I can fix that later if we need.

Test Plan: reload a few forms in maniphest, projects, differential

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D7120
2013-09-25 11:23:29 -07:00
Chad Little
e8bb24fd60 Policy, Status in PHUIHeaderView
Summary: The adds the ability to set 'properties' such as state, privacy, due date to the header of objects.

Test Plan: Implemented in Paste, Pholio. Tested various states.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D7016
2013-09-17 09:12:37 -07:00
Chad Little
5ba20b8924 Move PhabricatorObjectItem to PHUIObjectItem, add 'plain' setting for lists.
Summary: Adds plain support for object lists that just look like lists

Test Plan: review UIexamples and a number of other applications

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6922
2013-09-09 14:14:34 -07:00