Skip to content

Commit a329466

Browse files
theCalcaholictheCalcaholic
authored
Merge pull request #79 from theCalcaholic/fix/password-protected-share
Fix password protected shares on NC 32+
2 parents 8ce1253 + 6bf12c3 commit a329466

27 files changed

+718
-527
lines changed

‎composer.json‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,11 @@
88
"name": "Tobias Knöppler"
99
}
1010
],
11-
"repositories": [
12-
{
13-
"type": "vcs",
14-
"url": "https://github.com/nextcloud/openapi-extractor"
15-
}
16-
],
1711
"require-dev": {
1812
"bamarni/composer-bin-plugin": "^1.8",
13+
"nextcloud/coding-standard": "^v1.2.1",
1914
"nextcloud/ocp": "dev-master",
20-
"nextcloud/openapi-extractor": "dev-main",
21-
"nextcloud/coding-standard": "^v1.2.1"
15+
"nextcloud/openapi-extractor": "^1.4"
2216
},
2317
"scripts": {
2418
"bin": "echo 'bin not installed'",

‎composer.lock‎

Lines changed: 140 additions & 87 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/Controller/PageController.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public function __construct(IRequest $request, IManager $notificationManager, IU
2424
parent::__construct(Application::APP_ID, $request);
2525
$this->notificationManager = $notificationManager;
2626
$this->userId = $userSession->getUser()?->getUID();
27-
$this->debug = $config->getSystemValueBool("debug");
27+
$this->debug = $config->getSystemValueBool('debug');
2828
}
2929

3030
/**
@@ -33,12 +33,12 @@ public function __construct(IRequest $request, IManager $notificationManager, IU
3333
* @NoCSRFRequired
3434
*
3535
* @return TemplateResponse<Http::STATUS_OK, string>
36-
* 200: Return secrets page
36+
* 200: Return secrets page
3737
*/
3838
public function index(): TemplateResponse {
3939
Util::addScript(Application::APP_ID, 'secrets-main');
4040

41-
return new TemplateResponse(Application::APP_ID, 'main', ["debug" => $this->debug]);
41+
return new TemplateResponse(Application::APP_ID, 'main', ['debug' => $this->debug]);
4242
}
4343

4444
/**
@@ -48,7 +48,7 @@ public function index(): TemplateResponse {
4848
*
4949
* @param string $uuid The uuid of the secret
5050
* @return TemplateResponse<Http::STATUS_OK, string>
51-
* 200: Return secrets page and show specific secret
51+
* 200: Return secrets page and show specific secret
5252
*/
5353
public function show(string $uuid): TemplateResponse {
5454
$notification = $this->notificationManager->createNotification();

‎lib/Controller/RedirectController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class RedirectController extends Controller {
2020
* @NoCSRFRequired
2121
*
2222
* @return TemplateResponse<Http::STATUS_OK, string>
23-
* 200: Show secret share page
23+
* 200: Show secret share page
2424
*/
2525
public function share(): TemplateResponse {
2626
Util::addScript(Application::APP_ID, 'redirect');

‎lib/Controller/SecretApiController.php‎

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,15 @@ class SecretApiController extends OCSController {
3737

3838
use Errors;
3939

40-
public function __construct(IRequest $request,
41-
SecretService $service,
40+
public function __construct(IRequest $request,
41+
SecretService $service,
4242
ISession $session,
43-
NotificationService $notificationService,
43+
NotificationService $notificationService,
4444
INotificationManager $notificationManager,
45-
IURLGenerator $urlGenerator,
46-
IAppManager $appManager,
47-
LoggerInterface $logger,
48-
?string $userId) {
45+
IURLGenerator $urlGenerator,
46+
IAppManager $appManager,
47+
LoggerInterface $logger,
48+
?string $userId) {
4949
parent::__construct(Application::APP_ID, $request);
5050
$this->service = $service;
5151
$this->notificationService = $notificationService;
@@ -54,7 +54,7 @@ public function __construct(IRequest $request,
5454
$this->urlGenerator = $urlGenerator;
5555
$this->logger = $logger;
5656
$this->session = $session;
57-
$this->appVersion = $appManager->getAppInfo(Application::APP_ID)["version"];
57+
$this->appVersion = $appManager->getAppInfo(Application::APP_ID)['version'];
5858
}
5959

6060
/**
@@ -82,8 +82,8 @@ public function getAll(): DataResponse {
8282
* @param string $uuid The uuid of the secret
8383
*
8484
* @return DataResponse<Http::STATUS_OK, SecretsData, array{}>|DataResponse<Http::STATUS_NOT_FOUND, array{message: string}, array{}>
85-
* 200: Return secret with given uuid
86-
* 404: Secret not found
85+
* 200: Return secret with given uuid
86+
* 404: Secret not found
8787
*/
8888
public function get(string $uuid): DataResponse {
8989
return $this->handleNotFound(function () use ($uuid) {
@@ -98,7 +98,7 @@ public function get(string $uuid): DataResponse {
9898
* @NoCSRFRequired
9999
*
100100
* @return DataResponse<Http::STATUS_OK, array{version: string}, array{}>
101-
* 200: Return application/api version
101+
* 200: Return application/api version
102102
*
103103
*/
104104
#[AnonRateLimit(limit: 120, period: 60)]
@@ -116,9 +116,9 @@ public function getVersion(): DataResponse {
116116
* @param string|null $password The password for the secret share
117117
*
118118
* @return DataResponse<Http::STATUS_NOT_FOUND, array{message: string}, array{}>|DataResponse<Http::STATUS_UNAUTHORIZED, array{message: string}, array{}>|DataResponse<Http::STATUS_OK, array{iv: string, encrypted: string}, array{}>
119-
* 200: Return requested secret
120-
* 404: Secret not found for uuid
121-
* 401: Unauthorized
119+
* 200: Return requested secret
120+
* 404: Secret not found for uuid
121+
* 401: Unauthorized
122122
*
123123
*/
124124
#[UserRateLimit(limit: 500, period: 60)]
@@ -128,22 +128,25 @@ public function getVersion(): DataResponse {
128128
public function retrieveSharedSecret(string $uuid, ?string $password): DataResponse {
129129

130130
$pwHash = null;
131-
$pwHashLegacy = null;
132131
if ($password) {
133-
$pwHashLegacy = hash("sha256", $password . $uuid);
134132
$pwHash = $this->service->verifyPassword($uuid, $password);
133+
if ($pwHash === null) {
134+
$pwHash = hash('sha256', $password . $uuid);
135+
}
135136
} elseif ($this->session->get('public_link_authenticated_token') === $uuid) {
136137
$pwHash = $this->session->get('public_link_authenticated_password_hash');
137-
$pwHashLegacy = $this->session->get('public_link_authenticated_password_hash_legacy');
138+
} elseif ($this->session->get('public_link_authenticated_frontend')) {
139+
$authPayload = json_decode($this->session->get('public_link_authenticated_frontend'));
140+
$pwHash = $authPayload->$uuid;
138141
}
139142
try {
140-
$secret = $this->service->retrieveAndInvalidateSecret($uuid, $pwHash, $pwHashLegacy);
143+
$secret = $this->service->retrieveAndInvalidateSecret($uuid, $pwHash);
141144
} catch (SecretNotFound $e) {
142-
$resp = new DataResponse(["message" => "No secret with the given uuid was found"], Http::STATUS_NOT_FOUND);
145+
$resp = new DataResponse(['message' => 'No secret with the given uuid was found'], Http::STATUS_NOT_FOUND);
143146
$resp->throttle(['action' => 'retrieval']);
144147
return $resp;
145148
} catch (UnauthorizedException $e) {
146-
$resp = new DataResponse(["message" => "Forbidden"], Http::STATUS_UNAUTHORIZED);
149+
$resp = new DataResponse(['message' => 'Forbidden'], Http::STATUS_UNAUTHORIZED);
147150
$resp->throttle(['action' => 'password']);
148151
return $resp;
149152
}
@@ -169,8 +172,8 @@ public function retrieveSharedSecret(string $uuid, ?string $password): DataRespo
169172
* @param ?string $password (Optional) password to protect the secret share
170173
*
171174
* @return DataResponse<Http::STATUS_CREATED, SecretsData, array{}>|DataResponse<Http::STATUS_UNAUTHORIZED, array{message: string}, array{}>
172-
* 201: Secret created
173-
* 401: Unauthorized
175+
* 201: Secret created
176+
* 401: Unauthorized
174177
*/
175178
public function createSecret(string $title, string $encrypted, string $iv, ?string $expires, ?string $password) {
176179
if (!$this->userId) {
@@ -187,8 +190,8 @@ public function createSecret(string $title, string $encrypted, string $iv, ?stri
187190
* @param string $title The new title of the secret
188191
*
189192
* @return DataResponse<Http::STATUS_OK, SecretsData, array{}>|DataResponse<Http::STATUS_NOT_FOUND, array{message: string}, array{}>
190-
* 200: Return updated secret
191-
* 404: Secret not found
193+
* 200: Return updated secret
194+
* 404: Secret not found
192195
*/
193196
public function updateTitle(string $uuid, string $title): DataResponse {
194197
return $this->handleNotFound(function () use ($uuid, $title) {
@@ -203,8 +206,8 @@ public function updateTitle(string $uuid, string $title): DataResponse {
203206
* @param string $uuid The uuid of the secret
204207
*
205208
* @return DataResponse<Http::STATUS_OK, array{message: string}, array{}>|DataResponse<Http::STATUS_NOT_FOUND, array{message: string}, array{}>
206-
* 200: Secret deleted
207-
* 404: Secret not found
209+
* 200: Secret deleted
210+
* 404: Secret not found
208211
*/
209212
public function delete(string $uuid): DataResponse {
210213
try {

‎lib/Controller/SecretShareController.php‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,15 @@ class SecretShareController extends AuthPublicShareController {
2727
private Secret $secret;
2828
private bool $debug;
2929

30-
public function __construct(IRequest $request,
31-
ISession $session,
32-
SecretService $service,
33-
IURLGenerator $urlGenerator,
34-
IConfig $config,
30+
public function __construct(IRequest $request,
31+
ISession $session,
32+
SecretService $service,
33+
IURLGenerator $urlGenerator,
34+
IConfig $config,
3535
LoggerInterface $logger) {
3636
parent::__construct(Application::APP_ID, $request, $session, $urlGenerator);
3737
$this->service = $service;
38-
$this->debug = $config->getSystemValueBool("debug");
38+
$this->debug = $config->getSystemValueBool('debug');
3939
}
4040

4141
/**
@@ -53,7 +53,7 @@ protected function getPasswordHash(): string {
5353
private function getSecret(): ?Secret {
5454
if (!isset($this->secret)) {
5555
if (!$this->getToken()) {
56-
throw new InvalidArgumentException("secret uuid is not defined");
56+
throw new InvalidArgumentException('secret uuid is not defined');
5757
}
5858
$this->secret = $this->service->findPublic($this->getToken());
5959
}
@@ -81,9 +81,9 @@ protected function isPasswordProtected(): bool {
8181
protected function verifyPassword(string $password): bool {
8282
try {
8383
$pwHash = $this->service->verifyPassword($this->secret->getUuid(), $password);
84-
return $pwHash !== null ||
84+
return $pwHash !== null
8585
# for backwards compatibility. TODO: remove after some time
86-
hash("sha256", $password . $this->getSecret()->getUuid()) === $this->getPasswordHash();
86+
|| hash('sha256', $password . $this->getSecret()->getUuid()) === $this->getPasswordHash();
8787
} catch (SecretNotFound $e) {
8888
return false;
8989
}
@@ -97,20 +97,20 @@ protected function verifyPassword(string $password): bool {
9797
* @NoCSRFRequired
9898
*
9999
* @return TemplateResponse<Http::STATUS_OK, string>
100-
* 200: Show authentication page
100+
* 200: Show authentication page
101101
*
102102
* @since 14.0.0
103103
*/
104104
public function showAuthenticate(): TemplateResponse {
105105
return new TemplateResponse('secrets', 'publicshareauth',
106-
["debug" => $this->debug], 'guest');
106+
['debug' => $this->debug], 'guest');
107107
}
108108

109109
/**
110110
* The template to show when authentication failed
111111
*
112112
* @return TemplateResponse<Http::STATUS_OK, string>
113-
* 200: Show authentication failure page
113+
* 200: Show authentication failure page
114114
*
115115
* @since 14.0.0
116116
*
@@ -123,7 +123,7 @@ protected function showAuthFailed(): TemplateResponse {
123123
* The template to show after user identification
124124
*
125125
* @return TemplateResponse<Http::STATUS_OK, string>
126-
* 200: Show user identification success page
126+
* 200: Show user identification success page
127127
*
128128
* @since 24.0.0
129129
*/
@@ -138,7 +138,7 @@ protected function showIdentificationResult(bool $success): TemplateResponse {
138138
* @NoCSRFRequired
139139
*
140140
* @return TemplateResponse<Http::STATUS_OK, string>
141-
* 200: Show secret share page
141+
* 200: Show secret share page
142142
*/
143143
public function showShare(): TemplateResponse {
144144
Util::addScript(Application::APP_ID, 'secrets-public');

‎lib/Cron/SecretCleanup.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public function __construct(ITimeFactory $time, SecretService $service, LoggerIn
2929
* @throws Exception
3030
*/
3131
protected function run($argument) {
32-
$this->logger->info("CRON: Cleaning expired secrets...");
32+
$this->logger->info('CRON: Cleaning expired secrets...');
3333
$dt = new DateTime('now');
3434
$dt = $dt->sub(new DateInterval('P7D'));
3535
$this->service->deleteExpiredAfter($dt->format('Y-m-d'));

‎lib/Db/SecretMapper.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public function findPublic(string $uuid): Secret {
6363
->from('secrets')
6464
->where($qb->expr()->eq('uuid', $qb->createNamedParameter($uuid)))
6565
->andWhere($qb->expr()->isNotNull('encrypted'))
66-
->andWhere($qb->expr()->gt('expires', $qb->createNamedParameter(date("Y-m-d"))));
66+
->andWhere($qb->expr()->gt('expires', $qb->createNamedParameter(date('Y-m-d'))));
6767
return $this->findEntity($qb);
6868
}
6969

‎lib/Migration/Version1001Date20230122142756.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,11 @@ public function __construct(IDBConnection $connection) {
5757
*/
5858
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
5959
$schema = $schemaClosure();
60-
$table = $schema->getTable("secrets");
61-
if ($table->hasColumn("iv_str")) {
60+
$table = $schema->getTable('secrets');
61+
if ($table->hasColumn('iv_str')) {
6262
return null;
6363
}
64-
$table->addColumn("iv_str", Types::TEXT, ['notnull' => false, 'length' => null, 'default' => '']);
64+
$table->addColumn('iv_str', Types::TEXT, ['notnull' => false, 'length' => null, 'default' => '']);
6565
return $schema;
6666
}
6767

@@ -78,7 +78,7 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array
7878
->executeQuery();
7979
while ($secret = $results->fetch()) {
8080
$qb = $this->connection->getQueryBuilder();
81-
$qb->update("secrets")
81+
$qb->update('secrets')
8282
->where($qb->expr()->eq('id', $qb->createNamedParameter($secret['id'])))
8383
->set('iv_str', $qb->createNamedParameter(self::fixSerialization($secret['iv'])));
8484
$qb->executeStatement();

‎lib/Migration/Version1001Date20230123002443.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,12 @@ public function __construct(IDBConnection $connection) {
5757
*/
5858
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
5959
$schema = $schemaClosure();
60-
$table = $schema->getTable("secrets");
61-
if (!$table->hasColumn("iv_str")) {
60+
$table = $schema->getTable('secrets');
61+
if (!$table->hasColumn('iv_str')) {
6262
return null;
6363
}
64-
$table->dropColumn("iv");
65-
$table->addColumn("iv", Types::TEXT, ['notnull' => false, 'length' => null, 'default' => '']);
64+
$table->dropColumn('iv');
65+
$table->addColumn('iv', Types::TEXT, ['notnull' => false, 'length' => null, 'default' => '']);
6666
return $schema;
6767
}
6868

0 commit comments

Comments
 (0)