Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions apps/files_sharing/lib/Controller/ShareController.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,14 @@
use OCP\ISession;
use OCP\IURLGenerator;
use OCP\IUserManager;
use OCP\Security\Events\GenerateSecurePasswordEvent;
use OCP\Security\ISecureRandom;
use OCP\Security\PasswordContext;
use OCP\Share;
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\IManager as ShareManager;
use OCP\Share\IPublicShareTemplateFactory;
use OCP\Share\IShare;
use OCP\Template;

/**
* @package OCA\Files_Sharing\Controllers
Expand Down Expand Up @@ -156,7 +157,7 @@ protected function validateIdentity(?string $identityToken = null): bool {
* Generates a password for the share, respecting any password policy defined
*/
protected function generatePassword(): void {
$event = new \OCP\Security\Events\GenerateSecurePasswordEvent();
$event = new GenerateSecurePasswordEvent(PasswordContext::SHARING);
$this->eventDispatcher->dispatchTyped($event);
$password = $event->getPassword() ?? $this->secureRandom->generate(20);

Expand Down
3 changes: 2 additions & 1 deletion apps/sharebymail/lib/ShareByMailProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
use OCP\Security\Events\GenerateSecurePasswordEvent;
use OCP\Security\IHasher;
use OCP\Security\ISecureRandom;
use OCP\Security\PasswordContext;
use OCP\Share\Exceptions\GenericShareException;
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\IAttributes;
Expand Down Expand Up @@ -131,7 +132,7 @@ protected function autoGeneratePassword(IShare $share): string {
);
}

$passwordEvent = new GenerateSecurePasswordEvent();
$passwordEvent = new GenerateSecurePasswordEvent(PasswordContext::SHARING);
$this->eventDispatcher->dispatchTyped($passwordEvent);

$password = $passwordEvent->getPassword();
Expand Down
90 changes: 36 additions & 54 deletions apps/sharebymail/tests/ShareByMailProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
use OC\Mail\Message;
use OCA\ShareByMail\Settings\SettingsManager;
use OCA\ShareByMail\ShareByMailProvider;
use OCP\Activity\IManager as IActivityManager;
use OCP\Defaults;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\File;
use OCP\Files\IRootFolder;
use OCP\Files\Node;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IL10N;
Expand All @@ -25,9 +27,11 @@
use OCP\Security\Events\GenerateSecurePasswordEvent;
use OCP\Security\IHasher;
use OCP\Security\ISecureRandom;
use OCP\Security\PasswordContext;
use OCP\Share\IAttributes;
use OCP\Share\IManager;
use OCP\Share\IShare;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
use Test\TestCase;

Expand All @@ -38,65 +42,36 @@
* @group DB
*/
class ShareByMailProviderTest extends TestCase {
/** @var IConfig */
private $config;

/** @var IDBConnection */
private $connection;

/** @var IManager | \PHPUnit\Framework\MockObject\MockObject */
private $shareManager;

/** @var IL10N | \PHPUnit\Framework\MockObject\MockObject */
private $l;

/** @var LoggerInterface | \PHPUnit\Framework\MockObject\MockObject */
private $logger;

/** @var IRootFolder | \PHPUnit\Framework\MockObject\MockObject */
private $rootFolder;

/** @var IUserManager | \PHPUnit\Framework\MockObject\MockObject */
private $userManager;

/** @var ISecureRandom | \PHPUnit\Framework\MockObject\MockObject */
private $secureRandom;

/** @var IMailer | \PHPUnit\Framework\MockObject\MockObject */
private $mailer;

/** @var IURLGenerator | \PHPUnit\Framework\MockObject\MockObject */
private $urlGenerator;

/** @var IShare | \PHPUnit\Framework\MockObject\MockObject */
private $share;

/** @var \OCP\Activity\IManager | \PHPUnit\Framework\MockObject\MockObject */
private $activityManager;

/** @var SettingsManager | \PHPUnit\Framework\MockObject\MockObject */
private $settingsManager;

/** @var Defaults|\PHPUnit\Framework\MockObject\MockObject */
private $defaults;

/** @var IHasher | \PHPUnit\Framework\MockObject\MockObject */
private $hasher;

/** @var IEventDispatcher */
private $eventDispatcher;

private IDBConnection $connection;

private IL10N&MockObject $l;
private IShare&MockObject $share;
private IConfig&MockObject $config;
private IMailer&MockObject $mailer;
private IHasher&MockObject $hasher;
private Defaults&MockObject $defaults;
private IManager&MockObject $shareManager;
private LoggerInterface&MockObject $logger;
private IRootFolder&MockObject $rootFolder;
private IUserManager&MockObject $userManager;
private ISecureRandom&MockObject $secureRandom;
private IURLGenerator&MockObject $urlGenerator;
private SettingsManager&MockObject $settingsManager;
private IActivityManager&MockObject $activityManager;
private IEventDispatcher&MockObject $eventDispatcher;

protected function setUp(): void {
parent::setUp();

$this->config = $this->getMockBuilder(IConfig::class)->getMock();
$this->connection = \OC::$server->getDatabaseConnection();
$this->connection = \OCP\Server::get(IDBConnection::class);

$this->l = $this->getMockBuilder(IL10N::class)->getMock();
$this->l->method('t')
->willReturnCallback(function ($text, $parameters = []) {
return vsprintf($text, $parameters);
});
$this->config = $this->getMockBuilder(IConfig::class)->getMock();
$this->logger = $this->getMockBuilder(LoggerInterface::class)->getMock();
$this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock();
$this->userManager = $this->getMockBuilder(IUserManager::class)->getMock();
Expand Down Expand Up @@ -165,7 +140,10 @@ private function getInstance(array $mockedMethods = []) {
}

protected function tearDown(): void {
$this->connection->getQueryBuilder()->delete('share')->execute();
$this->connection
->getQueryBuilder()
->delete('share')
->executeStatement();

parent::tearDown();
}
Expand Down Expand Up @@ -305,7 +283,11 @@ public function testCreateSendPasswordByMailWithPasswordAndWithoutEnforcedPasswo
// Assume the mail address is valid.
$this->mailer->expects($this->any())->method('validateMailAddress')->willReturn(true);

$instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity', 'sendEmail', 'sendPassword', 'sendPasswordToOwner']);
$instance = $this->getInstance([
'getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject',
'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity',
'sendEmail', 'sendPassword', 'sendPasswordToOwner',
]);

$instance->expects($this->once())->method('getSharedWith')->willReturn([]);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42);
Expand Down Expand Up @@ -361,7 +343,7 @@ public function testCreateSendPasswordByMailWithEnforcedPasswordProtectionWithPe
->willReturn('autogeneratedPassword');
$this->eventDispatcher->expects($this->once())
->method('dispatchTyped')
->with(new GenerateSecurePasswordEvent());
->with(new GenerateSecurePasswordEvent(PasswordContext::SHARING));

// Assume the mail address is valid.
$this->mailer->expects($this->any())->method('validateMailAddress')->willReturn(true);
Expand Down Expand Up @@ -822,7 +804,7 @@ public function dataUpdateSendPassword() {
* @param bool sendMail
*/
public function testUpdateSendPassword($plainTextPassword, string $originalPassword, string $newPassword, $originalSendPasswordByTalk, $newSendPasswordByTalk, bool $sendMail) {
$node = $this->getMockBuilder(File::class)->getMock();
$node = $this->createMock(File::class);
$node->expects($this->any())->method('getName')->willReturn('filename');

$this->settingsManager->method('sendPasswordByMail')->willReturn(true);
Expand Down Expand Up @@ -927,7 +909,7 @@ public function testGetShareByPath() {
$permissions = 1;
$token = 'token';

$node = $this->getMockBuilder('OCP\Files\Node')->getMock();
$node = $this->createMock(Node::class);
$node->expects($this->once())->method('getId')->willReturn($itemSource);


Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,7 @@
'OCP\\Security\\Ip\\IFactory' => $baseDir . '/lib/public/Security/Ip/IFactory.php',
'OCP\\Security\\Ip\\IRange' => $baseDir . '/lib/public/Security/Ip/IRange.php',
'OCP\\Security\\Ip\\IRemoteAddress' => $baseDir . '/lib/public/Security/Ip/IRemoteAddress.php',
'OCP\\Security\\PasswordContext' => $baseDir . '/lib/public/Security/PasswordContext.php',
'OCP\\Security\\RateLimiting\\ILimiter' => $baseDir . '/lib/public/Security/RateLimiting/ILimiter.php',
'OCP\\Security\\RateLimiting\\IRateLimitExceededException' => $baseDir . '/lib/public/Security/RateLimiting/IRateLimitExceededException.php',
'OCP\\Security\\VerificationToken\\IVerificationToken' => $baseDir . '/lib/public/Security/VerificationToken/IVerificationToken.php',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OCP\\Security\\Ip\\IFactory' => __DIR__ . '/../../..' . '/lib/public/Security/Ip/IFactory.php',
'OCP\\Security\\Ip\\IRange' => __DIR__ . '/../../..' . '/lib/public/Security/Ip/IRange.php',
'OCP\\Security\\Ip\\IRemoteAddress' => __DIR__ . '/../../..' . '/lib/public/Security/Ip/IRemoteAddress.php',
'OCP\\Security\\PasswordContext' => __DIR__ . '/../../..' . '/lib/public/Security/PasswordContext.php',
'OCP\\Security\\RateLimiting\\ILimiter' => __DIR__ . '/../../..' . '/lib/public/Security/RateLimiting/ILimiter.php',
'OCP\\Security\\RateLimiting\\IRateLimitExceededException' => __DIR__ . '/../../..' . '/lib/public/Security/RateLimiting/IRateLimitExceededException.php',
'OCP\\Security\\VerificationToken\\IVerificationToken' => __DIR__ . '/../../..' . '/lib/public/Security/VerificationToken/IVerificationToken.php',
Expand Down
34 changes: 32 additions & 2 deletions lib/public/Security/Events/GenerateSecurePasswordEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,55 @@
namespace OCP\Security\Events;

use OCP\EventDispatcher\Event;
use OCP\Security\PasswordContext;

/**
* Event to request a secure password to be generated
* @since 18.0.0
*/
class GenerateSecurePasswordEvent extends Event {
/** @var null|string */
private $password;
private ?string $password;

/**
* Request a secure password to be generated.
*
* By default passwords are generated for the user account context,
* this can be adjusted by passing another `PasswordContext`.
* @since 31.0.0
*/
public function __construct(
private PasswordContext $context = PasswordContext::ACCOUNT,
) {
parent::__construct();
$this->password = null;
}

/**
* Get the generated password.
*
* If a password generator is registered and successfully generated a password
* that password can get read back. Otherwise `null` is returned.
* @since 18.0.0
*/
public function getPassword(): ?string {
return $this->password;
}

/**
* Set the generated password.
*
* This is used by password generators to set the generated password.
* @since 18.0.0
*/
public function setPassword(string $password): void {
$this->password = $password;
}

/**
* Get the context this password should generated for.
* @since 31.0.0
*/
public function getContext(): PasswordContext {
return $this->context;
}
}
23 changes: 19 additions & 4 deletions lib/public/Security/Events/ValidatePasswordPolicyEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,41 @@
namespace OCP\Security\Events;

use OCP\EventDispatcher\Event;
use OCP\Security\PasswordContext;

/**
* This event can be emitted to request a validation of a password.
*
* If a password policy app is installed and the password
* is invalid, an `\OCP\HintException` will be thrown.
* @since 18.0.0
*/
class ValidatePasswordPolicyEvent extends Event {
/** @var string */
private $password;

/**
* @since 18.0.0
* @since 31.0.0 - $context parameter added
*/
public function __construct(string $password) {
public function __construct(
private string $password,
private PasswordContext $context = PasswordContext::ACCOUNT,
) {
parent::__construct();
$this->password = $password;
}

/**
* Get the password that should be validated.
* @since 18.0.0
*/
public function getPassword(): string {
return $this->password;
}

/**
* Get the context this password should validated for.
* @since 31.0.0
*/
public function getContext(): PasswordContext {
return $this->context;
}
}
29 changes: 29 additions & 0 deletions lib/public/Security/PasswordContext.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCP\Security;

/**
* Define the context in which a password is used.
* This allows setting a context for password validation and password generation.
*
* @package OCP\Security
* @since 31.0.0
*/
enum PasswordContext {
/**
* Password used for an user account
* @since 31.0.0
*/
case ACCOUNT;

/**
* Password used for (public) shares
* @since 31.0.0
*/
case SHARING;
}
33 changes: 33 additions & 0 deletions tests/lib/Security/Events/GenerateSecurePasswordEventTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace Test\Security\Events;

use OCP\Security\Events\GenerateSecurePasswordEvent;
use OCP\Security\PasswordContext;

class GenerateSecurePasswordEventTest extends \Test\TestCase {

public function testDefaultProperties() {
$event = new GenerateSecurePasswordEvent();
$this->assertNull($event->getPassword());
$this->assertEquals(PasswordContext::ACCOUNT, $event->getContext());
}

public function testSettingPassword() {
$event = new GenerateSecurePasswordEvent();
$event->setPassword('example');
$this->assertEquals('example', $event->getPassword());
}

public function testSettingContext() {
$event = new GenerateSecurePasswordEvent(PasswordContext::SHARING);
$this->assertEquals(PasswordContext::SHARING, $event->getContext());
}
}
Loading