Skip to content

Commit 1df7d40

Browse files
author
epriestley
committed
Store repository credentials with repositories
Summary: Move toward storing credentials in configuration so it's easier to get the daemons working. This should eventually solve all the key juggling junk you have to do right now. This only gets us part of the way to actually using these credentials in the daemons since I have to go swap everything for $repository->execBlah(). I tried to write a web "Test Connection" button but it was too much of a mess to get git to work since git doesn't give you access to its SSH command and SSH has a bunch of interactive prompts which you can't really do anything about without it or a bunch of ~/.ssh/config editing. This is what Git recommends: https://git.wiki.kernel.org/index.php/GitFaq#How_do_I_specify_what_ssh_key_git_should_use.3F ..but it's not a great match for this use case. Test Plan: - Only partial. - Ran "test_connection.php" on a Git repo with and without SSH, and with and without valid credentials. This part works properly. - Ran "test_connection.php" on a public SVN repo, but I don't have private or WEBDAV repos set up at the moment. - Mercurial doesn't work yet. - Daemons haven't been converted yet. Reviewers: jungejason, tuomaspelkonen, aran Reviewed By: jungejason CC: aran, abdul, nmalcolm, epriestley, jungejason Differential Revision: 888
1 parent e875c81 commit 1df7d40

File tree

7 files changed

+355
-22
lines changed

7 files changed

+355
-22
lines changed

‎scripts/repository/test_connection.php‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
#!/usr/bin/env php
2+
<?php
3+
4+
/*
5+
* Copyright 2011 Facebook, Inc.
6+
*
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
*/
19+
20+
$root = dirname(dirname(dirname(__FILE__)));
21+
require_once $root.'/scripts/__init_script__.php';
22+
require_once $root.'/scripts/__init_env__.php';
23+
24+
phutil_require_module('phutil', 'console');
25+
phutil_require_module('phutil', 'future/exec');
26+
27+
if (empty($argv[1])) {
28+
echo "usage: test_connection.php <repository_callsign>\n";
29+
exit(1);
30+
}
31+
32+
echo phutil_console_wrap(
33+
phutil_console_format(
34+
'This script will test that you have configured valid credentials for '.
35+
'access to a repository, so the Phabricator daemons can pull from it. '.
36+
'You should run this as the **same user you will run the daemons as**, '.
37+
'from the **same machine they will run from**. Doing this will help '.
38+
'detect various problems with your configuration, such as SSH issues.'));
39+
40+
list($whoami) = execx('whoami');
41+
$whoami = trim($whoami);
42+
43+
$ok = phutil_console_confirm("Do you want to continue as '{$whoami}'?");
44+
if (!$ok) {
45+
die(1);
46+
}
47+
48+
$callsign = $argv[1];
49+
echo "Loading '{$callsign}' repository...\n";
50+
$repository = id(new PhabricatorRepository())->loadOneWhere(
51+
'callsign = %s',
52+
$argv[1]);
53+
if (!$repository) {
54+
throw new Exception("No such repository exists!");
55+
}
56+
57+
$vcs = $repository->getVersionControlSystem();
58+
59+
PhutilServiceProfiler::installEchoListener();
60+
61+
echo "Trying to connect to the remote...\n";
62+
switch ($vcs) {
63+
case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
64+
$err = $repository->passthruRemoteCommand(
65+
'--limit 1 log %s',
66+
$repository->getRemoteURI());
67+
break;
68+
case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
69+
// Do an ls-remote on a nonexistent ref, which we expect to just return
70+
// nothing.
71+
$err = $repository->passthruRemoteCommand(
72+
'ls-remote %s %s',
73+
$repository->getRemoteURI(),
74+
'just-testing');
75+
break;
76+
default:
77+
throw new Exception("Unsupported repository type.");
78+
}
79+
80+
if ($err) {
81+
echo phutil_console_format(
82+
"<bg:red>** FAIL **</bg> Connection failed. The credentials for this ".
83+
"repository appear to be incorrectly configured.\n");
84+
exit(1);
85+
} else {
86+
echo phutil_console_format(
87+
"<bg:green>** OKAY **</bg> Connection successful. The credentials for ".
88+
"this repository appear to be correctly configured.\n");
89+
}
90+

‎src/__celerity_resource_map__.php‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -652,6 +652,17 @@
652652
),
653653
'disk' => '/rsrc/js/application/maniphest/behavior-transaction-preview.js',
654654
),
655+
0 =>
656+
array(
657+
'uri' => '/res/1da00bfe/rsrc/js/javelin/lib/__tests__/URI.js',
658+
'type' => 'js',
659+
'requires' =>
660+
array(
661+
0 => 'javelin-uri',
662+
1 => 'javelin-php-serializer',
663+
),
664+
'disk' => '/rsrc/js/javelin/lib/__tests__/URI.js',
665+
),
655666
'javelin-behavior-owners-path-editor' =>
656667
array(
657668
'uri' => '/res/9cf78ffc/rsrc/js/application/owners/owners-path-editor.js',
@@ -1283,15 +1294,6 @@
12831294
),
12841295
'disk' => '/rsrc/js/application/core/Prefab.js',
12851296
),
1286-
0 =>
1287-
array(
1288-
'uri' => '/res/936e8e81/rsrc/js/javelin/docs/onload.js',
1289-
'type' => 'js',
1290-
'requires' =>
1291-
array(
1292-
),
1293-
'disk' => '/rsrc/js/javelin/docs/onload.js',
1294-
),
12951297
'phabricator-profile-css' =>
12961298
array(
12971299
'uri' => '/res/ebe1ac2f/rsrc/css/application/profile/profile-view.css',

‎src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php‎

Lines changed: 65 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,9 @@ private function processTrackingRequest() {
239239
'default-owners-path',
240240
'/'));
241241

242+
$repository->setDetail('ssh-login', $request->getStr('ssh-login'));
243+
$repository->setDetail('ssh-key', $request->getStr('ssh-key'));
244+
242245
$repository->setDetail(
243246
'herald-disabled',
244247
$request->getInt('herald-disabled', 0));
@@ -329,10 +332,14 @@ private function processTrackingRequest() {
329332
'Differential, Diffusion, Herald, and other services. To enable '.
330333
'tracking for a repository, configure it here and then start (or '.
331334
'restart) the daemons. More information is available in the '.
332-
'<strong>'.$user_guide_link.'</strong>.</p>')
335+
'<strong>'.$user_guide_link.'</strong>.</p>');
336+
337+
$form
338+
->appendChild(
339+
'<h1>Basics</h1><div class="aphront-form-inset">')
333340
->appendChild(
334341
id(new AphrontFormStaticControl())
335-
->setLabel('Repository')
342+
->setLabel('Repository Name')
336343
->setValue($repository->getName()))
337344
->appendChild(
338345
id(new AphrontFormSelectControl())
@@ -345,13 +352,19 @@ private function processTrackingRequest() {
345352
->setValue(
346353
$repository->getDetail('tracking-enabled')
347354
? 'enabled'
348-
: 'disabled'));
355+
: 'disabled'))
356+
->appendChild('</div>');
357+
358+
$form->appendChild(
359+
'<h1>Remote URI</h1>'.
360+
'<div class="aphront-form-inset">');
349361

350362
$uri_label = 'Repository URI';
351363
if ($is_git) {
352364
$instructions =
353-
'NOTE: The user the tracking daemon runs as must have permission to '.
354-
'<tt>git clone</tt> from this URI.';
365+
'Enter the URI to clone this repository from. It should look like '.
366+
'<tt>git@github.com:example/example.git</tt> or '.
367+
'<tt>ssh://user@host.com/git/example.git</tt>';
355368
$form->appendChild(
356369
'<p class="aphront-form-instructions">'.$instructions.'</p>');
357370
} else if ($is_svn) {
@@ -360,10 +373,7 @@ private function processTrackingRequest() {
360373
'You can figure this out by running <tt>svn info</tt> and looking at '.
361374
'the value in the <tt>Repository Root</tt> field. It should be a URI '.
362375
'and look like <tt>http://svn.example.org/svn/</tt> or '.
363-
'<tt>svn+ssh://svn.example.com/svnroot/</tt>.'.
364-
'<br /><br />'.
365-
'NOTE: The user the daemons run as must be able to execute '.
366-
'<tt>svn log</tt> against this URI.';
376+
'<tt>svn+ssh://svn.example.com/svnroot/</tt>';
367377
$form->appendChild(
368378
'<p class="aphront-form-instructions">'.$instructions.'</p>');
369379
$uri_label = 'Repository Root';
@@ -374,9 +384,44 @@ private function processTrackingRequest() {
374384
id(new AphrontFormTextControl())
375385
->setName('uri')
376386
->setLabel($uri_label)
387+
->setID('remote-uri')
377388
->setValue($repository->getDetail('remote-uri'))
378389
->setError($e_uri));
379390

391+
$form->appendChild(
392+
'<div class="aphront-form-instructions">'.
393+
'If you want to connect to this repository over SSH, enter the '.
394+
'username and private key to use. You can leave these fields blank if '.
395+
'the repository does not use SSH. <strong>NOTE: This feature is not '.
396+
'yet fully supported.</strong>'.
397+
'</div>');
398+
399+
$form
400+
->appendChild(
401+
id(new AphrontFormTextControl())
402+
->setName('ssh-login')
403+
->setLabel('SSH User')
404+
->setValue($repository->getDetail('ssh-login')))
405+
->appendChild(
406+
id(new AphrontFormTextAreaControl())
407+
->setName('ssh-key')
408+
->setLabel('SSH Private Key')
409+
->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_SHORT)
410+
->setValue($repository->getDetail('ssh-key')))
411+
->appendChild(
412+
id(new AphrontFormMarkupControl())
413+
->setLabel('Test Connection')
414+
->setValue(
415+
'To test these credentials, <strong>save this form</strong> and '.
416+
'then run: <code>phabricator/scripts/repository/test_connection'.
417+
'.php '.phutil_escape_html($repository->getCallsign()).'</code>'));
418+
419+
$form->appendChild('</div>');
420+
421+
$form->appendChild(
422+
'<h1>Importing Repository Information</h1>'.
423+
'<div class="aphront-form-inset">');
424+
380425
if ($is_git) {
381426
$form->appendChild(
382427
'<p class="aphront-form-instructions">Select a path on local disk '.
@@ -415,6 +460,12 @@ private function processTrackingRequest() {
415460
'Number of seconds daemon should sleep between requests. Larger '.
416461
'numbers reduce load but also decrease responsiveness.'));
417462

463+
$form->appendChild('</div>');
464+
465+
$form->appendChild(
466+
'<h1>Application Configuration</h1>'.
467+
'<div class="aphront-form-inset">');
468+
418469
if ($is_git) {
419470
$form
420471
->appendChild(
@@ -452,8 +503,8 @@ private function processTrackingRequest() {
452503
1 => 'Disabled - Do Not Send Email',
453504
))
454505
->setCaption(
455-
'You can temporarily disable Herald notifications when reparsing '.
456-
'a repository or importing a new repository.'));
506+
'You can temporarily disable Herald commit notifications when '.
507+
'reparsing a repository or importing a new repository.'));
457508

458509
$parsers = id(new PhutilSymbolLoader())
459510
->setAncestorClass('PhabricatorRepositoryCommitMessageDetailParser')
@@ -487,10 +538,12 @@ private function processTrackingRequest() {
487538
->setCaption('Repository UUID from <tt>svn info</tt>.'));
488539
}
489540

541+
$form->appendChild('</div>');
542+
490543
$form
491544
->appendChild(
492545
id(new AphrontFormSubmitControl())
493-
->setValue('Save'));
546+
->setValue('Save Configuration'));
494547

495548
$panel = new AphrontPanelView();
496549
$panel->setHeader('Repository Tracking');

‎src/applications/repository/controller/edit/__init__.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
phutil_require_module('phabricator', 'infrastructure/env');
1818
phutil_require_module('phabricator', 'view/control/table');
1919
phutil_require_module('phabricator', 'view/form/base');
20+
phutil_require_module('phabricator', 'view/form/control/markup');
2021
phutil_require_module('phabricator', 'view/form/control/select');
2122
phutil_require_module('phabricator', 'view/form/control/static');
2223
phutil_require_module('phabricator', 'view/form/control/submit');

0 commit comments

Comments
 (0)