Skip to content

Commit 489d2ea

Browse files
authored
Merge pull request #5 from nekudo/feature/hardening
Do not sent protected database fields to client
2 parents 216a711 + ba370e4 commit 489d2ea

File tree

4 files changed

+104
-40
lines changed

4 files changed

+104
-40
lines changed

src/ShinyDeploy/Action/WsDataAction/UpdateRepository.php

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,23 @@ public function __invoke(array $actionPayload) : bool
3939
return false;
4040
}
4141

42+
// get users encryption key:
43+
$auth = new Auth($this->config, $this->logger);
44+
$encryptionKey = $auth->getEncryptionKeyFromToken($this->token);
45+
if (empty($encryptionKey)) {
46+
$this->responder->setError('Could not get encryption key.');
47+
return false;
48+
}
49+
$repositories->setEnryptionKey($encryptionKey);
50+
4251
// check if url is okay:
4352
if (!isset($repositoryData['username'])) {
4453
$repositoryData['username'] = '';
4554
}
46-
if (!isset($repositoryData['password'])) {
47-
$repositoryData['password'] = '';
55+
if (empty($repositoryData['password'])) {
56+
$repositoryId = (int) $repositoryData['id'];
57+
$currentRepoData = $repositories->getRepositoryData($repositoryId);
58+
$repositoryData['password'] = $currentRepoData['password'] ?? '';
4859
}
4960
$repositoryData['url'] = preg_replace('/\.git$/s', '', $repositoryData['url']);
5061
$urlCheckResult = $this->checkUrl(
@@ -57,16 +68,7 @@ public function __invoke(array $actionPayload) : bool
5768
return false;
5869
}
5970

60-
// get users encryption key:
61-
$auth = new Auth($this->config, $this->logger);
62-
$encryptionKey = $auth->getEncryptionKeyFromToken($this->token);
63-
if (empty($encryptionKey)) {
64-
$this->responder->setError('Could not get encryption key.');
65-
return false;
66-
}
67-
6871
// update repository:
69-
$repositories->setEnryptionKey($encryptionKey);
7072
$addResult = $repositories->updateRepository($repositoryData);
7173
if ($addResult === false) {
7274
$this->responder->setError('Could not update repository.');

src/ShinyDeploy/Domain/Database/Repositories.php

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -155,21 +155,33 @@ public function updateRepository(array $repositoryData) : bool
155155
if ($repositoryData === false) {
156156
throw new RuntimeException('Data encryption failed.');
157157
}
158-
return $this->db->prepare(
159-
"UPDATE repositories
160-
SET `name` = %s,
161-
`type` = %s,
162-
`url` = %s,
163-
`username` = %s,
164-
`password` = %s
165-
WHERE id = %i",
166-
$repositoryData['name'],
167-
$repositoryData['type'],
168-
$repositoryData['url'],
169-
$repositoryData['username'],
170-
$repositoryData['password'],
171-
$repositoryData['id']
172-
)->execute();
158+
159+
$statement = "UPDATE repositories
160+
SET `name` = %s, `type` = %s, `url` = %s, `username` = %s, `password` = %s
161+
WHERE id = %i";
162+
if (empty($repositoryData['password'])) {
163+
$statement = str_replace(', `password` = %s', '', $statement);
164+
$result = $this->db->prepare(
165+
$statement,
166+
$repositoryData['name'],
167+
$repositoryData['type'],
168+
$repositoryData['url'],
169+
$repositoryData['username'],
170+
$repositoryData['id']
171+
)->execute();
172+
} else {
173+
$result = $this->db->prepare(
174+
$statement,
175+
$repositoryData['name'],
176+
$repositoryData['type'],
177+
$repositoryData['url'],
178+
$repositoryData['username'],
179+
$repositoryData['password'],
180+
$repositoryData['id']
181+
)->execute();
182+
}
183+
184+
return $result;
173185
}
174186

175187
/**

src/ShinyDeploy/Domain/Database/Servers.php

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -172,25 +172,43 @@ public function updateServer(array $serverData) : bool
172172
throw new RuntimeException('Data encryption failed.');
173173
}
174174

175-
return $this->db->prepare(
176-
"UPDATE servers
177-
SET `name` = %s,
175+
$statement = "UPDATE servers
176+
SET
177+
`name` = %s,
178178
`type` = %s,
179179
`hostname` = %s,
180180
`port` = %s,
181181
`username` = %s,
182182
`password` = %s,
183183
`root_path` = %s
184-
WHERE id = %i",
185-
$serverData['name'],
186-
$serverData['type'],
187-
$serverData['hostname'],
188-
$serverData['port'],
189-
$serverData['username'],
190-
$serverData['password'],
191-
$serverData['root_path'],
192-
$serverData['id']
193-
)->execute();
184+
WHERE id = %i";
185+
if (empty($serverData['password'])) {
186+
$statement = str_replace('`password` = %s,', '', $statement);
187+
$result = $this->db->prepare(
188+
$statement,
189+
$serverData['name'],
190+
$serverData['type'],
191+
$serverData['hostname'],
192+
$serverData['port'],
193+
$serverData['username'],
194+
$serverData['root_path'],
195+
$serverData['id']
196+
)->execute();
197+
} else {
198+
$result = $this->db->prepare(
199+
$statement,
200+
$serverData['name'],
201+
$serverData['type'],
202+
$serverData['hostname'],
203+
$serverData['port'],
204+
$serverData['username'],
205+
$serverData['password'],
206+
$serverData['root_path'],
207+
$serverData['id']
208+
)->execute();
209+
}
210+
211+
return $result;
194212
}
195213

196214
/**

src/ShinyDeploy/Responder/WsDataResponder.php

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,40 @@ public function getFrameData() : array
8484
if ($this->responseType === 'error') {
8585
$frameData['reason'] = $this->errorMsg;
8686
} else {
87-
$frameData['payload'] = $this->payload;
87+
$frameData['payload'] = $this->getSecurePayload();
8888
}
8989
return $frameData;
9090
}
91+
92+
/**
93+
* Returns payload with all protected/secure fields removed.
94+
*
95+
* @return array
96+
*/
97+
protected function getSecurePayload(): array
98+
{
99+
$protectedFields = ['password'];
100+
$securePayload = $this->payload;
101+
foreach ($protectedFields as $protectedField) {
102+
$this->unsetProtectedField($securePayload, $protectedField);
103+
}
104+
105+
return $securePayload;
106+
}
107+
108+
/**
109+
* Recursively removes given field/key from an array.
110+
*
111+
* @param array $payload
112+
* @param string $protectedField
113+
*/
114+
private function unsetProtectedField(array &$payload, string $protectedField): void
115+
{
116+
unset($payload[$protectedField]);
117+
foreach ($payload as &$value) {
118+
if (is_array($value)) {
119+
$this->unsetProtectedField($value, $protectedField);
120+
}
121+
}
122+
}
91123
}

0 commit comments

Comments
 (0)