Skip to content

Commit fd1b563

Browse files
PVince81CarlSchwan
PVince81
and
CarlSchwan
committed
Sharing from review
Signed-off-by: Vincent Petry <vincent@nextcloud.com> Co-authored-by: Carl Schwan <carl@carlschwan.eu>
1 parent 70e790c commit fd1b563

File tree

8 files changed

+77
-118
lines changed

8 files changed

+77
-118
lines changed

‎apps/files_sharing/lib/Controller/ShareAPIController.php‎

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1523,30 +1523,27 @@ private function parseDate(string $expireDate): \DateTime {
15231523

15241524
/**
15251525
* Set the share's password expiration time
1526-
*
1527-
* @param IShare $share
1528-
*
15291526
*/
1530-
private function setSharePasswordExpirationTime($share) {
1527+
private function setSharePasswordExpirationTime(IShare $share): void {
15311528
if ($this->config->getSystemValue('allow_mail_share_permanent_password')) {
15321529
// Sets password expiration date to NULL
15331530
$share->setPasswordExpirationTime();
1534-
} else {
1535-
// Sets password expiration date
1536-
$expirationTime = null;
1537-
try {
1538-
$now = new \DateTime();
1539-
$expirationInterval = $this->config->getSystemValue('share_temporary_password_expiration_interval');
1540-
if ($expirationInterval === '' || is_null($expirationInterval)) {
1541-
$expirationInterval = 'P0DT15M';
1542-
}
1543-
$expirationTime = $now->add(new \DateInterval($expirationInterval));
1544-
} catch (\Exception $e) {
1545-
// Catches invalid format for system value 'share_temporary_password_expiration_interval'
1546-
$expirationTime = $now->add(new \DateInterval('P0DT15M'));
1547-
} finally {
1548-
$share->setPasswordExpirationTime($expirationTime);
1531+
return;
1532+
}
1533+
// Sets password expiration date
1534+
$expirationTime = null;
1535+
try {
1536+
$now = new \DateTime();
1537+
$expirationInterval = $this->config->getSystemValue('share_temporary_password_expiration_interval');
1538+
if ($expirationInterval === '' || is_null($expirationInterval)) {
1539+
$expirationInterval = 'P0DT15M';
15491540
}
1541+
$expirationTime = $now->add(new \DateInterval($expirationInterval));
1542+
} catch (\Exception $e) {
1543+
// Catches invalid format for system value 'share_temporary_password_expiration_interval'
1544+
$expirationTime = $now->add(new \DateInterval('P0DT15M'));
1545+
} finally {
1546+
$share->setPasswordExpirationTime($expirationTime);
15501547
}
15511548
}
15521549

‎apps/files_sharing/lib/Controller/ShareController.php‎

Lines changed: 18 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -85,56 +85,21 @@
8585
* @package OCA\Files_Sharing\Controllers
8686
*/
8787
class ShareController extends AuthPublicShareController {
88+
protected IConfig $config;
89+
protected IUserManager $userManager;
90+
protected ILogger $logger;
91+
protected \OCP\Activity\IManager $activityManager;
92+
protected IPreview $previewManager;
93+
protected IRootFolder $rootFolder;
94+
protected FederatedShareProvider $federatedShareProvider;
95+
protected IAccountManager $accountManager;
96+
protected IEventDispatcher $eventDispatcher;
97+
protected IL10N $l10n;
98+
protected Defaults $defaults;
99+
protected ShareManager $shareManager;
100+
protected ISecureRandom $secureRandom;
101+
protected ?Share\IShare $share = null;
88102

89-
/** @var IConfig */
90-
protected $config;
91-
/** @var IUserManager */
92-
protected $userManager;
93-
/** @var ILogger */
94-
protected $logger;
95-
/** @var \OCP\Activity\IManager */
96-
protected $activityManager;
97-
/** @var IPreview */
98-
protected $previewManager;
99-
/** @var IRootFolder */
100-
protected $rootFolder;
101-
/** @var FederatedShareProvider */
102-
protected $federatedShareProvider;
103-
/** @var IAccountManager */
104-
protected $accountManager;
105-
/** @var IEventDispatcher */
106-
protected $eventDispatcher;
107-
/** @var IL10N */
108-
protected $l10n;
109-
/** @var Defaults */
110-
protected $defaults;
111-
/** @var ShareManager */
112-
protected $shareManager;
113-
/** @var ISecureRandom */
114-
protected $secureRandom;
115-
116-
/** @var Share\IShare */
117-
protected $share;
118-
119-
/**
120-
* @param string $appName
121-
* @param IRequest $request
122-
* @param IConfig $config
123-
* @param IURLGenerator $urlGenerator
124-
* @param IUserManager $userManager
125-
* @param ILogger $logger
126-
* @param \OCP\Activity\IManager $activityManager
127-
* @param \OCP\Share\IManager $shareManager
128-
* @param ISession $session
129-
* @param IPreview $previewManager
130-
* @param IRootFolder $rootFolder
131-
* @param FederatedShareProvider $federatedShareProvider
132-
* @param IAccountManager $accountManager
133-
* @param IEventDispatcher $eventDispatcher
134-
* @param IL10N $l10n
135-
* @param ISecureRandom $secureRandom
136-
* @param Defaults $defaults
137-
*/
138103
public function __construct(string $appName,
139104
IRequest $request,
140105
IConfig $config,
@@ -237,10 +202,10 @@ protected function showIdentificationResult(bool $success = false): TemplateResp
237202
/**
238203
* Validate the identity token of a public share
239204
*
240-
* @param string $identityToken
205+
* @param ?string $identityToken
241206
* @return bool
242207
*/
243-
protected function validateIdentity(string $identityToken = null): bool {
208+
protected function validateIdentity(?string $identityToken = null): bool {
244209

245210
if ($this->share->getShareType() !== IShare::TYPE_EMAIL) {
246211
return false;
@@ -250,25 +215,19 @@ protected function validateIdentity(string $identityToken = null): bool {
250215
return false;
251216
}
252217

253-
if ($identityToken !== $this->share->getSharedWith()) {
254-
return false;
255-
}
256-
257-
return true;
258-
218+
return $identityToken === $this->share->getSharedWith();
259219
}
260220

261221
/**
262222
* Generates a password for the share, respecting any password policy defined
263223
*/
264-
protected function generatePassword() {
224+
protected function generatePassword(): void {
265225
$event = new \OCP\Security\Events\GenerateSecurePasswordEvent();
266226
$this->eventDispatcher->dispatchTyped($event);
267227
$password = $event->getPassword() ?? $this->secureRandom->generate(20);
268228

269229
$this->share->setPassword($password);
270230
$this->shareManager->updateShare($this->share);
271-
return;
272231
}
273232

274233
protected function verifyPassword(string $password): bool {

‎apps/sharebymail/lib/ShareByMailProvider.php‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,7 @@
7676
*/
7777
class ShareByMailProvider implements IShareProvider {
7878

79-
/** @var IConfig */
80-
private $config;
79+
private IConfig $config;
8180

8281
/** @var IDBConnection */
8382
private $dbConnection;
@@ -684,23 +683,24 @@ public function getChildren(IShare $parent) {
684683
}
685684

686685
/**
687-
* add share to the database and return the ID
688-
*
689-
* @param int $itemSource
690-
* @param string $itemType
691-
* @param string $shareWith
692-
* @param string $sharedBy
693-
* @param string $uidOwner
694-
* @param int $permissions
695-
* @param string $token
696-
* @param string $password
697-
* @param bool $sendPasswordByTalk
698-
* @param bool $hideDownload
699-
* @param string $label
700-
* @param \DateTime|null $expirationTime
701-
* @return int
686+
* Add share to the database and return the ID
702687
*/
703-
protected function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $password, $passwordExpirationTime, $sendPasswordByTalk, $hideDownload, $label, $expirationTime, $note = ''): int {
688+
protected function addShareToDB(
689+
?int $itemSource,
690+
?string $itemType,
691+
?string $shareWith,
692+
?string $sharedBy,
693+
?string $uidOwner,
694+
?int $permissions,
695+
?string $token,
696+
?string $password,
697+
?\DateTimeInterface $passwordExpirationTime,
698+
?bool $sendPasswordByTalk,
699+
?bool $hideDownload,
700+
?string $label,
701+
?\DateTimeInterface $expirationTime,
702+
?string $note = ''
703+
): int {
704704
$qb = $this->dbConnection->getQueryBuilder();
705705
$qb->insert('share')
706706
->setValue('share_type', $qb->createNamedParameter(IShare::TYPE_EMAIL))
@@ -1026,7 +1026,8 @@ protected function createShareObject($data) {
10261026
$share->setShareTime($shareTime);
10271027
$share->setSharedWith($data['share_with']);
10281028
$share->setPassword($data['password']);
1029-
$share->setPasswordExpirationTime($data['password_expiration_time']);
1029+
$passwordExpirationTime = \DateTime::createFromFormat('Y-m-d H:i:s', $data['password_expiration_time']);
1030+
$share->setPasswordExpirationTime($passwordExpirationTime !== false? $passwordExpirationTime : null);
10301031
$share->setLabel($data['label']);
10311032
$share->setSendPasswordByTalk((bool)$data['password_by_talk']);
10321033
$share->setHideDownload((bool)$data['hide_download']);

‎apps/sharebymail/tests/ShareByMailProviderTest.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,7 @@ public function testAddShareToDB() {
476476
$hideDownload = true;
477477
$label = 'label';
478478
$expiration = new \DateTime();
479+
$passwordExpirationTime = new \DateTime();
479480

480481

481482
$instance = $this->getInstance();
@@ -491,7 +492,7 @@ public function testAddShareToDB() {
491492
$permissions,
492493
$token,
493494
$password,
494-
null,
495+
$passwordExpirationTime,
495496
$sendPasswordByTalk,
496497
$hideDownload,
497498
$label,
@@ -518,6 +519,7 @@ public function testAddShareToDB() {
518519
$this->assertSame($permissions, (int)$result[0]['permissions']);
519520
$this->assertSame($token, $result[0]['token']);
520521
$this->assertSame($password, $result[0]['password']);
522+
$this->assertSame($passwordExpirationTime->getTimestamp(), \DateTime::createFromFormat('Y-m-d H:i:s', $result[0]['password_expiration_time'])->getTimestamp());
521523
$this->assertSame($sendPasswordByTalk, (bool)$result[0]['password_by_talk']);
522524
$this->assertSame($hideDownload, (bool)$result[0]['hide_download']);
523525
$this->assertSame($label, $result[0]['label']);

‎lib/private/Share20/Manager.php‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1554,12 +1554,8 @@ public function checkPassword(IShare $share, $password) {
15541554

15551555
// Makes sure password hasn't expired
15561556
$expirationTime = $share->getPasswordExpirationTime();
1557-
if ($expirationTime !== null) {
1558-
$expirationDateTime = new \DateTime($expirationTime);
1559-
$now = new \DateTime();
1560-
if ($expirationDateTime < $now) {
1561-
return false;
1562-
}
1557+
if ($expirationTime !== null && $expirationTime < new \DateTime()) {
1558+
return false;
15631559
}
15641560

15651561
$newHash = '';

‎lib/private/Share20/Share.php‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,7 @@ class Share implements IShare {
7373
private $expireDate;
7474
/** @var string */
7575
private $password;
76-
/** @var string */
77-
private $passwordExpirationTime;
76+
private ?\DateTimeInterface $passwordExpirationTime = null;
7877
/** @var bool */
7978
private $sendPasswordByTalk = false;
8079
/** @var string */
@@ -466,15 +465,15 @@ public function getPassword() {
466465
/**
467466
* @inheritdoc
468467
*/
469-
public function setPasswordExpirationTime($passwordExpirationTime = null) {
468+
public function setPasswordExpirationTime(?\DateTimeInterface $passwordExpirationTime = null): IShare {
470469
$this->passwordExpirationTime = $passwordExpirationTime;
471470
return $this;
472471
}
473472

474473
/**
475474
* @inheritdoc
476475
*/
477-
public function getPasswordExpirationTime() {
476+
public function getPasswordExpirationTime(): ?\DateTimeInterface {
478477
return $this->passwordExpirationTime;
479478
}
480479

‎lib/public/AppFramework/AuthPublicShareController.php‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,21 +98,26 @@ protected function showIdentificationResult(bool $success): TemplateResponse {
9898
*
9999
* @since 24.0.0
100100
*/
101-
abstract protected function validateIdentity(string $identityToken): bool;
101+
protected function validateIdentity(?string $identityToken = null): bool {
102+
return false;
103+
};
102104

103105
/**
104106
* Generates a password
105107
*
106-
* @since 14.0.0
108+
* @since 24.0.0
107109
*/
108-
abstract protected function generatePassword();
110+
protected function generatePassword(): void {
111+
};
109112

110113
/**
111114
* Verify the password
112115
*
113-
* @since 14.0.0
116+
* @since 24.0.0
114117
*/
115-
abstract protected function verifyPassword(string $password): bool;
118+
protected function verifyPassword(string $password): bool {
119+
return false;
120+
};
116121

117122
/**
118123
* Function called after failed authentication

‎lib/public/Share/IShare.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -451,16 +451,16 @@ public function getPassword();
451451
/**
452452
* Set the password's expiration time of this share.
453453
*
454-
* @return \OCP\Share\IShare The modified object
454+
* @return self The modified object
455+
* @since 24.0.0
455456
*/
456-
public function setPasswordExpirationTime($passwordExpirationTime = null);
457+
public function setPasswordExpirationTime(?\DateTimeInterface $passwordExpirationTime = null): IShare;
457458

458459
/**
459460
* Get the password's expiration time of this share.
460-
*
461-
* @return string
461+
* @since 24.0.0
462462
*/
463-
public function getPasswordExpirationTime();
463+
public function getPasswordExpirationTime(): ?\DateTimeInterface;
464464

465465
/**
466466
* Set if the recipient can start a conversation with the owner to get the

0 commit comments

Comments
 (0)