-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Require token for local editing #34559
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
nickvergessen
merged 5 commits into
master
from
bugfix/noid/require-token-for-local-editing
Oct 12, 2022
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
cfbbace
Require a token for "Edit locally"
nickvergessen c167fb5
Also remove token when we successfully validated
nickvergessen 9c5fef3
Add a background job to delete expired tokens
nickvergessen 9b20464
Fix psalm error on migration
nickvergessen edb64b1
Also throttle on expiration
nickvergessen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
61 changes: 61 additions & 0 deletions
61
apps/files/lib/BackgroundJob/DeleteExpiredOpenLocalEditor.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| /** | ||
| * @copyright Copyright (c) 2022 Joas Schilling <[email protected]> | ||
| * | ||
| * @author Joas Schilling <[email protected]> | ||
| * | ||
| * @license GNU AGPL version 3 or any later version | ||
| * | ||
| * This program is free software: you can redistribute it and/or modify | ||
| * it under the terms of the GNU Affero General Public License as | ||
| * published by the Free Software Foundation, either version 3 of the | ||
| * License, or (at your option) any later version. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| * GNU Affero General Public License for more details. | ||
| * | ||
| * You should have received a copy of the GNU Affero General Public License | ||
| * along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
| * | ||
| */ | ||
|
|
||
| namespace OCA\Files\BackgroundJob; | ||
|
|
||
| use OCA\Files\Controller\OpenLocalEditorController; | ||
| use OCA\Files\Db\OpenLocalEditorMapper; | ||
| use OCP\AppFramework\Utility\ITimeFactory; | ||
| use OCP\BackgroundJob\IJob; | ||
| use OCP\BackgroundJob\TimedJob; | ||
|
|
||
| /** | ||
| * Delete all expired "Open local editor" token | ||
| */ | ||
| class DeleteExpiredOpenLocalEditor extends TimedJob { | ||
| protected OpenLocalEditorMapper $mapper; | ||
|
|
||
| public function __construct( | ||
| ITimeFactory $time, | ||
| OpenLocalEditorMapper $mapper | ||
| ) { | ||
| parent::__construct($time); | ||
| $this->mapper = $mapper; | ||
|
|
||
| // Run every 12h | ||
| $this->interval = 12 * 3600; | ||
| $this->setTimeSensitivity(IJob::TIME_INSENSITIVE); | ||
| } | ||
|
|
||
| /** | ||
| * Makes the background job do its work | ||
| * | ||
| * @param array $argument unused argument | ||
| */ | ||
| public function run($argument): void { | ||
| $this->mapper->deleteExpiredTokens($this->time->getTime()); | ||
| } | ||
| } |
138 changes: 138 additions & 0 deletions
138
apps/files/lib/Controller/OpenLocalEditorController.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,138 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| /** | ||
| * @copyright Copyright (c) 2022 Joas Schilling <[email protected]> | ||
| * | ||
| * @author Joas Schilling <[email protected]> | ||
| * | ||
| * @license GNU AGPL version 3 or any later version | ||
| * | ||
| * This program is free software: you can redistribute it and/or modify | ||
| * it under the terms of the GNU Affero General Public License as | ||
| * published by the Free Software Foundation, either version 3 of the | ||
| * License, or (at your option) any later version. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| * GNU Affero General Public License for more details. | ||
| * | ||
| * You should have received a copy of the GNU Affero General Public License | ||
| * along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
| * | ||
| */ | ||
|
|
||
| namespace OCA\Files\Controller; | ||
|
|
||
| use OCA\Files\Db\OpenLocalEditor; | ||
| use OCA\Files\Db\OpenLocalEditorMapper; | ||
| use OCP\AppFramework\Db\DoesNotExistException; | ||
| use OCP\AppFramework\Http; | ||
| use OCP\AppFramework\Http\DataResponse; | ||
| use OCP\AppFramework\OCSController; | ||
| use OCP\AppFramework\Utility\ITimeFactory; | ||
| use OCP\DB\Exception; | ||
| use OCP\IRequest; | ||
| use OCP\Security\ISecureRandom; | ||
| use Psr\Log\LoggerInterface; | ||
|
|
||
| class OpenLocalEditorController extends OCSController { | ||
| public const TOKEN_LENGTH = 128; | ||
| public const TOKEN_DURATION = 600; // 10 Minutes | ||
| public const TOKEN_RETRIES = 50; | ||
|
|
||
| protected ITimeFactory $timeFactory; | ||
| protected OpenLocalEditorMapper $mapper; | ||
| protected ISecureRandom $secureRandom; | ||
| protected LoggerInterface $logger; | ||
| protected ?string $userId; | ||
|
|
||
| public function __construct( | ||
| string $appName, | ||
| IRequest $request, | ||
| ITimeFactory $timeFactory, | ||
| OpenLocalEditorMapper $mapper, | ||
| ISecureRandom $secureRandom, | ||
| LoggerInterface $logger, | ||
| ?string $userId | ||
| ) { | ||
| parent::__construct($appName, $request); | ||
|
|
||
| $this->timeFactory = $timeFactory; | ||
| $this->mapper = $mapper; | ||
| $this->secureRandom = $secureRandom; | ||
| $this->logger = $logger; | ||
| $this->userId = $userId; | ||
| } | ||
|
|
||
| /** | ||
| * @NoAdminRequired | ||
| * @UserRateThrottle(limit=10, period=120) | ||
| */ | ||
| public function create(string $path): DataResponse { | ||
| $pathHash = sha1($path); | ||
|
|
||
| $entity = new OpenLocalEditor(); | ||
| $entity->setUserId($this->userId); | ||
| $entity->setPathHash($pathHash); | ||
| $entity->setExpirationTime($this->timeFactory->getTime() + self::TOKEN_DURATION); // Expire in 10 minutes | ||
|
|
||
| for ($i = 1; $i <= self::TOKEN_RETRIES; $i++) { | ||
| $token = $this->secureRandom->generate(self::TOKEN_LENGTH, ISecureRandom::CHAR_ALPHANUMERIC); | ||
| $entity->setToken($token); | ||
|
|
||
| try { | ||
| $this->mapper->insert($entity); | ||
|
|
||
| return new DataResponse([ | ||
| 'userId' => $this->userId, | ||
| 'pathHash' => $pathHash, | ||
| 'expirationTime' => $entity->getExpirationTime(), | ||
| 'token' => $entity->getToken(), | ||
| ]); | ||
| } catch (Exception $e) { | ||
| if ($e->getCode() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { | ||
| // Only retry on unique constraint violation | ||
| throw $e; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| $this->logger->error('Giving up after ' . self::TOKEN_RETRIES . ' retries to generate a unique local editor token for path hash: ' . $pathHash); | ||
| return new DataResponse([], Http::STATUS_INTERNAL_SERVER_ERROR); | ||
| } | ||
|
|
||
| /** | ||
| * @NoAdminRequired | ||
| * @BruteForceProtection(action=openLocalEditor) | ||
| */ | ||
| public function validate(string $path, string $token): DataResponse { | ||
| $pathHash = sha1($path); | ||
|
|
||
| try { | ||
| $entity = $this->mapper->verifyToken($this->userId, $pathHash, $token); | ||
Check noticeCode scanning / Psalm PossiblyNullArgument
Argument 1 of OCA\Files\Db\OpenLocalEditorMapper::verifyToken cannot be null, possibly null value provided
|
||
| } catch (DoesNotExistException $e) { | ||
| $response = new DataResponse([], Http::STATUS_NOT_FOUND); | ||
| $response->throttle(['userId' => $this->userId, 'pathHash' => $pathHash]); | ||
| return $response; | ||
| } | ||
|
|
||
| $this->mapper->delete($entity); | ||
|
|
||
| if ($entity->getExpirationTime() <= $this->timeFactory->getTime()) { | ||
| $response = new DataResponse([], Http::STATUS_NOT_FOUND); | ||
| $response->throttle(['userId' => $this->userId, 'pathHash' => $pathHash]); | ||
| return $response; | ||
| } | ||
|
|
||
| return new DataResponse([ | ||
| 'userId' => $this->userId, | ||
| 'pathHash' => $pathHash, | ||
| 'expirationTime' => $entity->getExpirationTime(), | ||
| 'token' => $entity->getToken(), | ||
| ]); | ||
| } | ||
|
|
||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| /** | ||
| * @copyright Copyright (c) 2022 Joas Schilling <[email protected]> | ||
| * | ||
| * @author Joas Schilling <[email protected]> | ||
| * | ||
| * @license GNU AGPL version 3 or any later version | ||
| * | ||
| * This program is free software: you can redistribute it and/or modify | ||
| * it under the terms of the GNU Affero General Public License as | ||
| * published by the Free Software Foundation, either version 3 of the | ||
| * License, or (at your option) any later version. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| * GNU Affero General Public License for more details. | ||
| * | ||
| * You should have received a copy of the GNU Affero General Public License | ||
| * along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
| * | ||
| */ | ||
|
|
||
| namespace OCA\Files\Db; | ||
|
|
||
| use OCP\AppFramework\Db\Entity; | ||
|
|
||
| /** | ||
| * @method void setUserId(string $userId) | ||
| * @method string getUserId() | ||
| * @method void setPathHash(string $pathHash) | ||
| * @method string getPathHash() | ||
| * @method void setExpirationTime(int $expirationTime) | ||
| * @method int getExpirationTime() | ||
| * @method void setToken(string $token) | ||
| * @method string getToken() | ||
| */ | ||
| class OpenLocalEditor extends Entity { | ||
| /** @var string */ | ||
| protected $userId; | ||
Check noticeCode scanning / Psalm PropertyNotSetInConstructor
Property OCA\Files\Db\OpenLocalEditor::$userId is not defined in constructor of OCA\Files\Db\OpenLocalEditor or in any methods called in the constructor
|
||
|
|
||
| /** @var string */ | ||
| protected $pathHash; | ||
Check noticeCode scanning / Psalm PropertyNotSetInConstructor
Property OCA\Files\Db\OpenLocalEditor::$pathHash is not defined in constructor of OCA\Files\Db\OpenLocalEditor or in any methods called in the constructor
|
||
|
|
||
| /** @var int */ | ||
| protected $expirationTime; | ||
Check noticeCode scanning / Psalm PropertyNotSetInConstructor
Property OCA\Files\Db\OpenLocalEditor::$expirationTime is not defined in constructor of OCA\Files\Db\OpenLocalEditor or in any methods called in the constructor
|
||
|
|
||
| /** @var string */ | ||
| protected $token; | ||
Check noticeCode scanning / Psalm PropertyNotSetInConstructor
Property OCA\Files\Db\OpenLocalEditor::$token is not defined in constructor of OCA\Files\Db\OpenLocalEditor or in any methods called in the constructor
|
||
|
|
||
| public function __construct() { | ||
| $this->addType('userId', 'string'); | ||
| $this->addType('pathHash', 'string'); | ||
| $this->addType('expirationTime', 'integer'); | ||
| $this->addType('token', 'string'); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Check notice
Code scanning / Psalm
PossiblyNullArgument