Skip to content

Commit e0f3f4c

Browse files
simonLeary42simonLeary42
committed
simplify ssh key adding behavior
1 parent 8348ca7 commit e0f3f4c

File tree

2 files changed

+101
-76
lines changed

2 files changed

+101
-76
lines changed

‎test/functional/SSHKeyAddTest.php‎

Lines changed: 91 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,69 @@
77

88
class SSHKeyAddTest extends UnityWebPortalTestCase
99
{
10-
private function addSshKeysPaste(array $keys): void
10+
public static function keyProvider()
1111
{
12-
foreach ($keys as $key) {
12+
$validKey =
13+
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIB+XqO25MUB9x/pS04I3JQ7rMGboWyGXh0GUzkOrTi7a foobar";
14+
$invalidKey = "foobar";
15+
return [[false, $invalidKey], [true, $validKey]];
16+
}
17+
18+
public static function keysProvider()
19+
{
20+
$validKey =
21+
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIB+XqO25MUB9x/pS04I3JQ7rMGboWyGXh0GUzkOrTi7a foobar";
22+
$validKey2 =
23+
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIB+XqO25MUB9x/pS04I3JQ7rMGboWyGXh0GUzkOrTi7a foobar2";
24+
$invalidKey = "foobar";
25+
return [
26+
[0, []],
27+
[0, [$invalidKey]],
28+
[1, [$validKey]],
29+
[0, [$validKey, $invalidKey]],
30+
[1, [$validKey, $validKey]],
31+
[2, [$validKey, $validKey2]],
32+
];
33+
}
34+
35+
public function getKeyCount()
36+
{
37+
global $USER;
38+
return count($USER->getSSHKeys());
39+
}
40+
41+
#[DataProvider("keyProvider")]
42+
public function testAddSshKeyPaste(bool $expectedKeyAdded, string $key)
43+
{
44+
global $USER;
45+
switchUser(...getUserHasNoSshKeys());
46+
$numKeysBefore = $this->getKeyCount();
47+
$this->assertEquals(0, $numKeysBefore);
48+
try {
1349
http_post(__DIR__ . "/../../webroot/panel/account.php", [
1450
"form_type" => "addKey",
1551
"add_type" => "paste",
1652
"key" => $key,
1753
]);
54+
$numKeysAfter = $this->getKeyCount();
55+
if ($expectedKeyAdded) {
56+
$this->assertEquals(1, $numKeysAfter - $numKeysBefore);
57+
} else {
58+
$this->assertEquals(0, $numKeysAfter - $numKeysBefore);
59+
}
60+
} finally {
61+
$USER->setSSHKeys([]);
1862
}
1963
}
2064

21-
private function addSshKeysImport(array $keys): void
65+
#[DataProvider("keyProvider")]
66+
public function testAddSshKeyImport(bool $expectedKeyAdded, string $key)
2267
{
23-
foreach ($keys as $key) {
68+
global $USER;
69+
switchUser(...getUserHasNoSshKeys());
70+
$numKeysBefore = $this->getKeyCount();
71+
$this->assertEquals(0, $numKeysBefore);
72+
try {
2473
$tmp = tmpfile();
2574
$tmp_path = stream_get_meta_data($tmp)["uri"];
2675
fwrite($tmp, $key);
@@ -34,82 +83,67 @@ private function addSshKeysImport(array $keys): void
3483
} finally {
3584
unset($_FILES["keyfile"]);
3685
}
86+
$numKeysAfter = $this->getKeyCount();
87+
if ($expectedKeyAdded) {
88+
$this->assertEquals(1, $numKeysAfter - $numKeysBefore);
89+
} else {
90+
$this->assertEquals(0, $numKeysAfter - $numKeysBefore);
91+
}
92+
} finally {
93+
$USER->setSSHKeys([]);
3794
}
3895
}
3996

40-
private function addSshKeysGenerate(array $keys): void
97+
#[DataProvider("keyProvider")]
98+
public function testAddSshKeyGenerate(bool $expectedKeyAdded, string $key)
4199
{
42-
foreach ($keys as $key) {
100+
global $USER;
101+
switchUser(...getUserHasNoSshKeys());
102+
$numKeysBefore = $this->getKeyCount();
103+
$this->assertEquals(0, $numKeysBefore);
104+
try {
43105
http_post(__DIR__ . "/../../webroot/panel/account.php", [
44106
"form_type" => "addKey",
45107
"add_type" => "generate",
46108
"gen_key" => $key,
47109
]);
48-
}
49-
}
50-
51-
private function addSshKeysGithub(array $keys): void
52-
{
53-
global $GITHUB;
54-
$oldGithub = $GITHUB;
55-
$GITHUB = $this->createMock(UnityGithub::class);
56-
$GITHUB->method("getSshPublicKeys")->willReturn($keys);
57-
try {
58-
http_post(__DIR__ . "/../../webroot/panel/account.php", [
59-
"form_type" => "addKey",
60-
"add_type" => "github",
61-
"gh_user" => "foobar",
62-
]);
110+
$numKeysAfter = $this->getKeyCount();
111+
if ($expectedKeyAdded) {
112+
$this->assertEquals(1, $numKeysAfter - $numKeysBefore);
113+
} else {
114+
$this->assertEquals(0, $numKeysAfter - $numKeysBefore);
115+
}
63116
} finally {
64-
$GITHUB = $oldGithub;
65-
}
66-
}
67-
68-
public static function provider()
69-
{
70-
$validKey =
71-
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIB+XqO25MUB9x/pS04I3JQ7rMGboWyGXh0GUzkOrTi7a foobar";
72-
$invalidKey = "foobar";
73-
$methods = [
74-
"addSshKeysPaste",
75-
"addSshKeysImport",
76-
"addSshKeysGenerate",
77-
"addSshKeysGithub",
78-
];
79-
$output = [];
80-
foreach ($methods as $method) {
81-
$output = array_merge($output, [
82-
[$method, 0, []],
83-
[$method, 0, [$invalidKey]],
84-
[$method, 1, [$validKey]],
85-
[$method, 1, [$validKey, $invalidKey]],
86-
[$method, 1, [$validKey, $validKey]],
87-
]);
117+
$USER->setSSHKeys([]);
88118
}
89-
return $output;
90-
}
91-
92-
public function getKeyCount()
93-
{
94-
global $USER;
95-
return count($USER->getSSHKeys());
96119
}
97120

98121
#[AllowMockObjectsWithoutExpectations]
99-
#[DataProvider("provider")]
100-
public function testAddSshKeys(string $methodName, int $expectedKeysAdded, array $keys)
122+
#[DataProvider("keysProvider")]
123+
public function testAddSshKeysGithub(int $expectedKeysAdded, array $keys)
101124
{
102-
global $USER;
125+
global $USER, $GITHUB;
103126
switchUser(...getUserHasNoSshKeys());
104127
$numKeysBefore = $this->getKeyCount();
105128
$this->assertEquals(0, $numKeysBefore);
129+
$oldGithub = $GITHUB;
106130
try {
107-
call_user_func([SSHKeyAddTest::class, $methodName], $keys);
108-
// $method($keys);
131+
$GITHUB = $this->createMock(UnityGithub::class);
132+
$GITHUB->method("getSshPublicKeys")->willReturn($keys);
133+
try {
134+
http_post(__DIR__ . "/../../webroot/panel/account.php", [
135+
"form_type" => "addKey",
136+
"add_type" => "github",
137+
"gh_user" => "foobar",
138+
]);
139+
} finally {
140+
$GITHUB = $oldGithub;
141+
}
109142
$numKeysAfter = $this->getKeyCount();
110143
$this->assertEquals($expectedKeysAdded, $numKeysAfter - $numKeysBefore);
111144
} finally {
112145
$USER->setSSHKeys([]);
146+
$GITHUB = $oldGithub;
113147
}
114148
}
115149
}

‎webroot/panel/account.php‎

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,42 +13,33 @@
1313
UnityHTTPD::validatePostCSRFToken();
1414
switch (UnityHTTPD::getPostData("form_type")) {
1515
case "addKey":
16-
$keys = array();
1716
switch (UnityHTTPD::getPostData("add_type")) {
1817
case "paste":
19-
array_push($keys, UnityHTTPD::getPostData("key"));
18+
$keys = [UnityHTTPD::getPostData("key")];
2019
break;
2120
case "import":
2221
try {
23-
$key = UnityHTTPD::getUploadedFileContents("keyfile");
22+
$keys = [UnityHTTPD::getUploadedFileContents("keyfile")];
2423
} catch (EncodingUnknownException | EncodingConversionException $e) {
2524
UnityHTTPD::badRequest("uploaded key has bad encoding", error: $e);
2625
}
27-
array_push($keys, $key);
2826
break;
2927
case "generate":
30-
array_push($keys, UnityHTTPD::getPostData("gen_key"));
28+
$keys = [UnityHTTPD::getPostData("gen_key")];
3129
break;
3230
case "github":
3331
$githubUsername = UnityHTTPD::getPostData("gh_user");
34-
$githubKeys = $GITHUB->getSshPublicKeys($githubUsername);
35-
$keys = array_merge($keys, $githubKeys);
32+
$keys = $GITHUB->getSshPublicKeys($githubUsername);
3633
break;
3734
}
38-
if (!empty($keys)) {
39-
$keys = array_map("trim", $keys);
40-
$validKeys = array_filter($keys, "testValidSSHKey");
41-
$USER->setSSHKeys(array_merge($USER->getSSHKeys(), $validKeys));
42-
if (count($keys) != count($validKeys)) {
43-
UnityHTTPD::badRequest(
44-
"one more more invalid SSH keys were not added",
45-
data: [
46-
"keys_valid_added" => $validKeys,
47-
"keys_invalid_not_added" => array_diff($keys, $validKeys),
48-
],
49-
);
35+
$keys = array_map("trim", $keys);
36+
foreach ($keys as $key) {
37+
if (!testValidSSHKey($key)) {
38+
UnityHTTPD::messageError("Invalid SSH Key", $key);
39+
UnityHTTPD::redirect();
5040
}
5141
}
42+
$USER->setSSHKeys(array_merge($USER->getSSHKeys(), $keys));
5243
break;
5344
case "delKey":
5445
$keys = $USER->getSSHKeys();

0 commit comments

Comments
 (0)