1. 29 Mar, 2020 1 commit
    • Daniel Stone's avatar
      HACK: Conduit: Accept OAuth2 Authorization header · 5e9cf61a
      Daniel Stone authored
      This is really lame. The Ruby OAuth2 client can only pass its token in
      the form data (which Phab is not prepared to accept), or as part of the
      Authorization header (which PHP strips out).
      Use a function only available in newer PHP to scrape the Authorization
      header from the raw stream.
      I have no idea what the correct fix is.
  2. 22 Nov, 2019 1 commit
  3. 24 Sep, 2019 1 commit
  4. 31 Jul, 2019 1 commit
    • epriestley's avatar
      Support "date" custom fields in "*.edit" endpoints · 8e263a2f
      epriestley authored
      Summary: Fixes T13355. This didn't appear to be a ton of extra work, we just didn't get it for free in the original implementation in D14635.
      Test Plan:
        - Saw "date" custom fields appear in Conduit API documentation for "maniphest.edit".
        - Set custom "date" field to null and non-null values via the API.
      Maniphest Tasks: T13355
      Differential Revision: https://secure.phabricator.com/D20690
  5. 24 Jul, 2019 1 commit
    • epriestley's avatar
      Modularize user activity log message types · 32dd13d4
      epriestley authored
      Depends on D20670. Ref T13343. The user activity message log types are currently hard-coded, so only upstream code can really use the log construct.
      Under the theory that we're going to keep this log around going forward (just focus it a little bit), modularize things so the log is extensible.
      Test Plan:
      Grepped for `UserLog::`, viewed activity logs in People and Settings.
      (If I missed something here -- say, misspelled a constant -- the effect should just be that older logs don't get a human-readable label, so stakes are very low.)
      Reviewers: amckinley
      Reviewed By: amckinley
      Maniphest Tasks: T13343
      Differential Revision: https://secure.phabricator.com/D20671
  6. 07 Mar, 2019 1 commit
  7. 25 Feb, 2019 1 commit
  8. 23 Jan, 2019 1 commit
  9. 19 Jan, 2019 1 commit
  10. 03 Jan, 2019 1 commit
    • Austin McKinley's avatar
      Raise warning when accidentally submitting Conduit parameters as a JSON-encoded body · 05a94741
      Austin McKinley authored
      Summary: See T12447 for discussion. It is reasonably intuitive to try and pass Conduit parameters via a JSON-encoded HTTP body, but if you do so, you'll get an unhelpful messsage about how method so-and-so does not accept a parameter named "your_entire_json_body". Instead, detect this mistake and advise developers to use form-encoded parameters.
      Test Plan:
      Got a better error when attempting to make Conduit calls from React code. Tested the following additional invocations of Conduit and got the expected results without an error:
      * From the Conduit UI
      * With cURL:
      ~ $ curl http://local.phacility.com:8080/api/conpherence.querythread \
      >     -d api.token=api-tvv2zb565zrtueab5ddprmpxvrwb \
      >     -d ids[0]=1
      * With `arc call-conduit`:
      ~ $ echo '{
      >   "ids": [
      >     1
      >   ]
      > }' | arc call-conduit --conduit-uri http://local.phacility.com:8080/ --conduit-token api-tvv2zb565zrtueab5ddprmpxvrwb conpherence.querythread
      Reviewers: epriestley
      Reviewed By: epriestley
      Subscribers: Korvin
      Differential Revision: https://secure.phabricator.com/D19944
  11. 09 Nov, 2018 1 commit
  12. 10 Sep, 2018 1 commit
    • epriestley's avatar
      Make DiffusionCommitSearch accept modern (string) constants · 8eb8e8e1
      epriestley authored
      Depends on D19650. Ref T13197. Allow `SearchCheckboxesField` to have a "deprecated" map of older aliases, then convert them to modern values.
      On the API method page, show all the values.
      This technically resolves the issue in PHI841, although I still plan to migrate behind this.
      Test Plan:
      - Queried audits, fiddled with `?status=1,audited`, etc.
      Reviewers: amckinley
      Reviewed By: amckinley
      Maniphest Tasks: T13197
      Differential Revision: https://secure.phabricator.com/D19651
  13. 06 Sep, 2018 1 commit
    • epriestley's avatar
      In Conduit, let checkbox constraints self-document · 5a38b75f
      epriestley authored
      Ref T13195. Ref PHI851. Currently, checkbox constraints don't tell you what values are supported on the API page.
      You can figure this out with "Inspect Element" or by viewing the source code, but just render a nice table instead.
      Test Plan: {F5862969}
      Reviewers: amckinley
      Reviewed By: amckinley
      Maniphest Tasks: T13195
      Differential Revision: https://secure.phabricator.com/D19641
  14. 08 Aug, 2018 1 commit
    • epriestley's avatar
      Add an "--as" flag to "bin/conduit call ..." to improve flexibility and ease of profiling · 3ca3f09a
      epriestley authored
      Ref T13164. In PHI801, an install reported a particular slow Conduit method call.
      Conduit calls aren't easily profilable with normal tools (for example, `arc call-conduit --xprofile ...` gives you a profile of the //client//). They can be profiled most easily with `bin/conduit call ... --xprofile`.
      However, `bin/conduit call` currently doesn't let you pick a user to execute the command on behalf of, so it's not terribly useful for profiling `*.edit`-style methods which do a write: these need a real acting user.
      Test Plan:
      Ran `bin/conduit call --method user.whoami --as epriestley ...` with valid, invalid, and no acting users.
      $ echo '{}' | ./bin/conduit call --method user.whoami --as epriestley --input -
      Reading input from stdin...
        "result": {
          "phid": "PHID-USER-icyixzkx3f4ttv67avbn",
          "userName": "epriestley",
          "realName": "Evan Priestley",
      Reviewers: amckinley
      Reviewed By: amckinley
      Maniphest Tasks: T13164
      Differential Revision: https://secure.phabricator.com/D19566
  15. 20 Jul, 2018 1 commit
    • epriestley's avatar
      Make the Conduit auth error for an unrecognized public key a little more useful · eb80a5ed
      epriestley authored
      Summary: Ref T13168. This is just a small quality-of-life fix: we can disclose which public key we're talking about because public keys are public.
      Test Plan:
        - Hit public key error (through my own bumbling / not reading or following instructions). Specifically, I haven't associated the key with a device in Almanac.
        - Before: vague error.
        - After: more specific error with enough key material that I could grep for it.
      Reviewers: amckinley
      Reviewed By: amckinley
      Subscribers: yelirekim
      Maniphest Tasks: T13168
      Differential Revision: https://secure.phabricator.com/D19516
  16. 05 Feb, 2018 1 commit
    • epriestley's avatar
      Add a `bin/conduit call` support binary · 956c4058
      epriestley authored
      Ref T13060. See PHI343. Triaging this bug required figuring out where in the pipeline UTF8 was being dropped, and bisecting the pipeline required making calls to Conduit.
      Currently, there's no easy way to debug/inspect arbitrary Conduit calls, especially when they are `diffusion.*` calls which route to a different host (even if you have a real session and use the web console for these, you just see an HTTP service call to the target host in DarkConsole).
      Add a `bin/conduit` utility to make this kind of debugging easier, with an eye toward the Phacility production cluster (or other similar clusters) specifically.
      Test Plan:
        - Ran `echo '{}' | bin/conduit call --method conduit.ping --input -` and similar.
        - Used a similar approach to successfully diagnose the UTF8 issue in T13060.
      Reviewers: amckinley
      Reviewed By: amckinley
      Maniphest Tasks: T13060
      Differential Revision: https://secure.phabricator.com/D18987
  17. 31 Jan, 2018 1 commit
  18. 23 Jan, 2018 1 commit
    • epriestley's avatar
      Fix failure to record `pullerPHID` in repository pull logs · 29146134
      epriestley authored
      See PHI305. Ref T13046.
      The SSH workflows currently extend `PhabricatorManagementWorkflow` to benefit from sharing all the standard argument parsing code. Sharing the parsing code is good, but it also means they inherit a `getViewer()` method which returns the ommnipotent viewer.
      This is appropriate for everything else which extends `ManagementWorkflow` (like `bin/storage`, `bin/auth`, etc.) but not appropriate for SSH workflows, which have a real user.
      This caused a bug with the pull logs where `pullerPHID` was not recorded properly. We used `$this->getViewer()->getPHID()` but the correct code was `$this->getUser()->getPHID()`.
      To harden this against future mistakes:
        - Don't extend `ManagementWorkflow`. Extend `PhutilArgumentWorkflow` instead. We **only** want the argument parsing code.
        - Rename `get/setUser()` to `get/setSSHUser()` to make them explicit.
      Then, fix the pull log bug by calling `getSSHUser()` instead of `getViewer()`.
      Test Plan:
        - Pulled and pushed to a repository over SSH.
        - Grepped all the SSH stuff for the altered symbols.
        -  Saw pulls record a valid `pullerPHID` in the pull log.
        - Used `echo {} | ssh ... conduit conduit.ping` to test conduit over SSH.
      Reviewers: amckinley
      Reviewed By: amckinley
      Maniphest Tasks: T13046
      Differential Revision: https://secure.phabricator.com/D18912
  19. 06 Sep, 2017 1 commit
  20. 04 Aug, 2017 1 commit
    • Chad Little's avatar
      Update Settings to use full side-navigation · 83f66ce5
      Chad Little authored
      Summary: Moves Settings to use a normal side navigation vs. a two column side navigation. It also updates Edit Engine to do the same, but I don't think there are other callsites. Added a consistent header for better clarification if you were editng your settings, global settings, or a bot's settings.
      Test Plan: Test each page on a personal account, create global settings, test each page there, create a bot account, and test each page on the bot account. Anything else?
      Reviewers: epriestley
      Reviewed By: epriestley
      Subscribers: Korvin
      Differential Revision: https://secure.phabricator.com/D18342
  21. 18 Jul, 2017 1 commit
  22. 05 Jun, 2017 1 commit
    • Chad Little's avatar
      Separate button CSS classes · d3c464a6
      Chad Little authored
      Summary: Try to dis-ambiguate various button types and colors. Moves `simple` to `phui-button-simple` and moves colors to `button-color`.
      Test Plan: Grep for buttons still inline, UIExamples, PHUIX, Herald, and Email Preferences.
      Reviewers: epriestley
      Reviewed By: epriestley
      Subscribers: Korvin
      Differential Revision: https://secure.phabricator.com/D18077
  23. 24 Mar, 2017 1 commit
  24. 22 Feb, 2017 1 commit
  25. 17 Feb, 2017 1 commit
  26. 13 Feb, 2017 1 commit
  27. 09 Jan, 2017 1 commit
    • epriestley's avatar
      Mark "v3" API methods as stable; mark obsoleted methods as "Frozen" · aa6e788f
      epriestley authored
      Ref T12074. The "v3" API methods (`*.search`, `*.edit`) are currently marked as "unstable", but they're pretty stable and essentially all new code should be using them.
      Although these methods are seeing some changes, almost all changes are additive (support for new constraints or attachemnts) and do not break backward compatibility. We have no major, compatibility-breaking changes planned.
      I don't want to mark the older methods "deprecated" yet since `arc` still uses a lot of them and there are some capabilities not yet available on the v3 methods, but introduce a new "frozen" status with pointers to the new methods.
      Overall, this should gently push users toward the newer methods.
      Test Plan: {F2325323}
      Reviewers: chad
      Reviewed By: chad
      Maniphest Tasks: T12074
      Differential Revision: https://secure.phabricator.com/D17158
  28. 08 Dec, 2016 1 commit
    • epriestley's avatar
      Use futures to improve clustered repository main page performance · 5f26dd9b
      epriestley authored
      Ref T11954. In cluster configurations, we get repository information by making HTTP calls over Conduit.
      These are slower than local calls, so clustering imposes a performance penalty. However, we can use futures and parallelize them so that clustering actually improves overall performance.
      When not running in clustered mode, this just makes us run stuff inline.
      Test Plan:
        - Browsed Git, Mercurial and Subversion repositories.
        - Locally, saw a 700ms wall time page drop to 200ms.
      Reviewers: chad
      Reviewed By: chad
      Maniphest Tasks: T11954
      Differential Revision: https://secure.phabricator.com/D17009
  29. 06 Dec, 2016 2 commits
  30. 09 Nov, 2016 1 commit
  31. 17 Oct, 2016 1 commit
  32. 14 Oct, 2016 1 commit
  33. 27 Sep, 2016 1 commit
    • Josh Cox's avatar
      Remove "Application" field from ConduitSearchEngine · 6649b0ce
      Josh Cox authored
      Summary: Fixes T9063. Removes the "Application" field from the search because it was largely redundant with the 'Name Contains' field.
      Test Plan: Went to `/conduit/query/modern/`, clicked on `Edit Query` and noted that there is no "Application" field anymore. The 'Name Contains' field still works however.
      Reviewers: epriestley, #blessed_reviewers
      Reviewed By: epriestley, #blessed_reviewers
      Subscribers: Korvin, epriestley, yelirekim
      Maniphest Tasks: T9063
      Differential Revision: https://secure.phabricator.com/D16602
  34. 07 Sep, 2016 1 commit
    • epriestley's avatar
      Throw when callers pass an invalid constraint to a "*.search" method · 38ae81fb
      epriestley authored
      Ref T11593. When you call a `*.search` method like `maniphest.search`, we don't currently validate that all the constraints you pass are recognized.
      I think there were two very weak arguments for not doing this:
        - It makes compatibility in `arc` across versions slightly easier: if we add a new constraint, we could add it to `arc` but also do client-side filtering for a while.
        - Conduit parameter types //could//, in theory, accept multiple inputs or optional/alias inputs.
      These reasons are pretty fluff and T11593 is a concrete issue caused by not validating. Just validate instead.
      Test Plan:
        - Made a `maniphest.search` call with a bogus constraint, got an explicit error about the bad constraint.
        - Made a `maniphest.search` call with a valid constraint (`"ids"`).
      Reviewers: chad
      Reviewed By: chad
      Maniphest Tasks: T11593
      Differential Revision: https://secure.phabricator.com/D16507
  35. 23 Aug, 2016 1 commit
  36. 14 Aug, 2016 1 commit
    • epriestley's avatar
      Don't allow empty list constraints in Conduit calls · 07082d28
      epriestley authored
      Ref T11473. If you write a method like `get_stuff(ids)` and then call it with an empty list of IDs, you can end up passing an empty constraint to Conduit.
      If you run a `*.search` method with such a constraint, like this one:
        "ids": []
      ...we have three possible beahviors:
        # Treat it like the user passed no constraint (basically, ignore the constraint).
        # Respect the constraint (return no results).
        # Error.
      Currently, we do (1). However, this is pretty confusing and I think clearly the worst option, since it means `get_stuff(array())` in client code will often tend to return a ton of results.
      We could do (2) instead, but this is also sort of confusing (it may not be obvious why nothing matched, even though it's an application bug) and I think most reasonable client code should be doing an `if ($ids)` test: this test makes clients a little more complicated, but they can save a network call, and I think they often need to do this test anyway (for example, to show the user a different message).
      This implements (3), and just considers these to be errors: this is the least tricky behavior, it's consistent with what we do in PHP, makes fairly good sense, and the only cost for this is that client code may need to be slightly more complex, but this slightly more complex code is usually better code.
      Test Plan: Ran Conduit `*.search` queries with `"ids":[]` and `"phids":[]`, got sensible errors instead of runaway result sets.
      Reviewers: chad
      Reviewed By: chad
      Maniphest Tasks: T11473
      Differential Revision: https://secure.phabricator.com/D16396
  37. 31 Jul, 2016 1 commit
    • epriestley's avatar
      Allow `*.search` Conduit API methods to have data bulk-loaded by extensions · 6e57582a
      epriestley authored
      Ref T11404. Currently, SearchEngineAttachments can bulk-load data but SearchEngineExtensions can not.
      This leads to poor performance of custom fields. See T11404 for discussion.
      This changes the API to support a bulk load + format pattern like the one Attachments use. The next change will use it to bulk-load custom field data.
      Test Plan:
        - Ran `differential.query`, `differential.revision.search` as a sanity check.
        - No behavioral changes are expected
        - See next revision.
      Reviewers: yelirekim, chad
      Reviewed By: chad
      Maniphest Tasks: T11404
      Differential Revision: https://secure.phabricator.com/D16350
  38. 01 Jul, 2016 1 commit
    • epriestley's avatar
      Convert all remaining old tabs to new PHUITabGroupViews · 65980ac6
      epriestley authored
      Summary: Ref T10628. This moves everything else over. I'll clean up the cruft in the next diff.
      Test Plan:
      - Viewed Conduit API page, toggled tabs.
      - Viewed Harbormaster build, toggled tabs.
      - Viewed a Drydock lease, swapped tabs.
      - Viewed a Drydock resource, swapped tabs.
      - Viewed mail, swapped tabs.
      - Grepped for `addPropertyList(...)`, looked for any remaining calls with a second argument.
      - Also checked rSAAS for any calls, but we don't have anything there that uses tabs.
      Reviewers: chad
      Reviewed By: chad
      Maniphest Tasks: T10628
      Differential Revision: https://secure.phabricator.com/D16207
  39. 28 Jun, 2016 1 commit
    • epriestley's avatar
      Clean up redirect URIs for "Temporary Tokens" and "API Tokens" settings panels · ec8581ab
      epriestley authored
      Summary: Fixes T11223. I missed a few of these; most of them kept working anyway because we have redirects in place, but make them a bit more modern/not-hard-coded.
      Test Plan:
        - Generated and revoked API tokens for myself.
        - Generated and revoked API tokens for bots.
        - Revoked temporary tokens for myself.
        - Clicked the link to the API tokens panel from the Conduit console.
        - Clicked all the cancel buttons in all the dialogs, too.
      In all cases, everything now points at the correct URIs. Previously, some things pointed at the wrong URIs (mostly dealing with stuff for bots).
      Reviewers: chad
      Reviewed By: chad
      Maniphest Tasks: T11223
      Differential Revision: https://secure.phabricator.com/D16185