diff --git a/apps/admin_audit/lib/Operation.php b/apps/admin_audit/lib/Operation.php index c88bffdff2178..7da7e737a50b2 100644 --- a/apps/admin_audit/lib/Operation.php +++ b/apps/admin_audit/lib/Operation.php @@ -36,6 +36,22 @@ enum Operation: string { case ShareExpirationRemoved = 'sharing.share.expiration_removed'; case ShareLinkAccessed = 'sharing.share.link_accessed'; + // Unified sharing API + // TODO: Merge with the legacy sharing.share.* operations above once the legacy sharing API is removed + case UnifiedShareCreated = 'unified_sharing.share.created'; + case UnifiedShareDeleted = 'unified_sharing.share.deleted'; + case UnifiedShareStateUpdated = 'unified_sharing.share.state_updated'; + case UnifiedShareUserStatusUpdated = 'unified_sharing.share.user_status_updated'; + case UnifiedShareSourceAdded = 'unified_sharing.source.added'; + case UnifiedShareSourceRemoved = 'unified_sharing.source.removed'; + case UnifiedShareRecipientAdded = 'unified_sharing.recipient.added'; + case UnifiedShareRecipientRemoved = 'unified_sharing.recipient.removed'; + case UnifiedShareRecipientSecretUpdated = 'unified_sharing.recipient_secret.updated'; + case UnifiedSharePropertyUpdated = 'unified_sharing.property.updated'; + case UnifiedSharePermissionUpdated = 'unified_sharing.permission.updated'; + case UnifiedShareRecipientPermissionUpdated = 'unified_sharing.recipient_permission.updated'; + case UnifiedSharePermissionPresetSelected = 'unified_sharing.permission_preset.selected'; + // Trash bin and versions case TrashbinFileDeleted = 'trashbin.file.deleted'; case TrashbinFileRestored = 'trashbin.file.restored'; @@ -49,6 +65,8 @@ enum Operation: string { case LogoutPerformed = 'auth.logout.performed'; case TwoFactorPassed = 'auth.twofactor.passed'; case TwoFactorFailed = 'auth.twofactor.failed'; + case PasswordResetRequested = 'auth.password_reset.requested'; + case RemoteWipeFinished = 'auth.remote_wipe.finished'; // Users case UserCreated = 'users.user.created'; diff --git a/core/Controller/AppPasswordController.php b/core/Controller/AppPasswordController.php index faa26a31913f7..f2835b1cbe0eb 100644 --- a/core/Controller/AppPasswordController.php +++ b/core/Controller/AppPasswordController.php @@ -33,6 +33,7 @@ use OCP\IRequest; use OCP\ISession; use OCP\IUserManager; +use OCP\Log\Audit\CriticalActionPerformedEvent; use OCP\Security\Bruteforce\IThrottler; use OCP\Security\ISecureRandom; @@ -104,6 +105,12 @@ public function getAppPassword(): DataResponse { $this->eventDispatcher->dispatchTyped( new AppPasswordCreatedEvent($generatedToken) ); + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent( + 'App password with ID "%s" and name "%s" was created', + ['tokenId' => $generatedToken->getId(), 'tokenName' => $generatedToken->getName()], + false, + 'auth.app_password.created', + )); return new DataResponse([ 'apppassword' => $token @@ -134,6 +141,12 @@ public function deleteAppPassword(): DataResponse { } $this->tokenProvider->invalidateTokenById($token->getUID(), $token->getId()); + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent( + 'App password with ID "%s" was deleted', + ['tokenId' => $token->getId()], + false, + 'auth.app_password.deleted', + )); return new DataResponse(); } @@ -242,6 +255,12 @@ public function getAppPasswordWithOneTimePassword(): DataResponse { $this->eventDispatcher->dispatchTyped( new AppPasswordCreatedEvent($generatedToken) ); + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent( + 'App password with ID "%s" and name "%s" was created', + ['tokenId' => $generatedToken->getId(), 'tokenName' => $generatedToken->getName()], + false, + 'auth.app_password.created', + )); return new DataResponse([ 'apppassword' => $token diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php index dbf1570e476eb..06a6db515a123 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -39,6 +39,7 @@ use OCP\ISession; use OCP\IURLGenerator; use OCP\IUserSession; +use OCP\Log\Audit\CriticalActionPerformedEvent; use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; use OCP\Session\Exceptions\SessionNotAvailableException; @@ -337,6 +338,12 @@ public function generateAppPassword( $this->eventDispatcher->dispatchTyped( new AppPasswordCreatedEvent($generatedToken) ); + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent( + 'App password with ID "%s" and name "%s" was created', + ['tokenId' => $generatedToken->getId(), 'tokenName' => $generatedToken->getName()], + false, + 'auth.app_password.created', + )); return new RedirectResponse($redirectUri); } diff --git a/core/Controller/LostController.php b/core/Controller/LostController.php index 184af1dee9126..b25052306ce77 100644 --- a/core/Controller/LostController.php +++ b/core/Controller/LostController.php @@ -37,6 +37,7 @@ use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; +use OCP\Log\Audit\CriticalActionPerformedEvent; use OCP\Mail\IMailer; use OCP\PreConditionNotMetException; use OCP\Security\VerificationToken\InvalidTokenException; @@ -283,6 +284,13 @@ protected function sendEmail(string $input): void { // Log the exception and continue $this->logger->error($e->getMessage(), ['app' => 'core', 'exception' => $e]); } + + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent( + 'Password reset was requested for user "%s"', + ['userId' => $user->getUID()], + false, + 'auth.password_reset.requested', + )); } /** diff --git a/core/Service/LoginFlowV2Service.php b/core/Service/LoginFlowV2Service.php index 5d6048230c638..47707b909ecf6 100644 --- a/core/Service/LoginFlowV2Service.php +++ b/core/Service/LoginFlowV2Service.php @@ -21,7 +21,9 @@ use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Authentication\Exceptions\InvalidTokenException; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; +use OCP\Log\Audit\CriticalActionPerformedEvent; use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; use Psr\Log\LoggerInterface; @@ -35,6 +37,7 @@ public function __construct( private ICrypto $crypto, private LoggerInterface $logger, private IProvider $tokenProvider, + private IEventDispatcher $eventDispatcher, ) { } @@ -153,7 +156,7 @@ public function flowDone(string $loginToken, string $sessionId, string $server, } $appPassword = $this->random->generate(72, ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_DIGITS); - $this->tokenProvider->generateToken( + $generatedToken = $this->tokenProvider->generateToken( $appPassword, $userId, $loginName, @@ -162,6 +165,12 @@ public function flowDone(string $loginToken, string $sessionId, string $server, IToken::PERMANENT_TOKEN, IToken::DO_NOT_REMEMBER ); + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent( + 'App password with ID "%s" and name "%s" was created', + ['tokenId' => $generatedToken->getId(), 'tokenName' => $generatedToken->getName()], + false, + 'auth.app_password.created', + )); $data->setLoginName($loginName); $data->setServer($server); diff --git a/lib/private/Authentication/Token/RemoteWipe.php b/lib/private/Authentication/Token/RemoteWipe.php index 08dce6fa5a1fb..48ec8a7639715 100644 --- a/lib/private/Authentication/Token/RemoteWipe.php +++ b/lib/private/Authentication/Token/RemoteWipe.php @@ -15,6 +15,7 @@ use OCP\Authentication\Exceptions\WipeTokenException; use OCP\EventDispatcher\IEventDispatcher; use OCP\IUser; +use OCP\Log\Audit\CriticalActionPerformedEvent; use Psr\Log\LoggerInterface; use function array_filter; @@ -119,6 +120,12 @@ public function finish(string $token): bool { $this->logger->info('user ' . $dbToken->getUID() . ' finished a remote wipe'); $this->eventDispatcher->dispatch(RemoteWipeFinished::class, new RemoteWipeFinished($dbToken)); + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent( + 'Remote wipe of app password with ID "%s" of user "%s" was finished', + ['tokenId' => $dbToken->getId(), 'userId' => $dbToken->getUID()], + false, + 'auth.remote_wipe.finished', + )); return true; } diff --git a/lib/private/Security/Bruteforce/Throttler.php b/lib/private/Security/Bruteforce/Throttler.php index e975399845aa4..4eb9373232200 100644 --- a/lib/private/Security/Bruteforce/Throttler.php +++ b/lib/private/Security/Bruteforce/Throttler.php @@ -65,6 +65,8 @@ public function registerAttempt(string $action, new CriticalActionPerformedEvent( 'Bruteforce attempt from "{ip}" detected for action "{action}" not throttled due to allow-list.', ['ip' => $ip, 'action' => $action], + false, + 'auth.bruteforce.bypassed', ) ); return; diff --git a/lib/private/Sharing/SharingManager.php b/lib/private/Sharing/SharingManager.php index 0190d1408864a..a6d36ca28e782 100644 --- a/lib/private/Sharing/SharingManager.php +++ b/lib/private/Sharing/SharingManager.php @@ -42,6 +42,7 @@ use OCP\IUser; use OCP\IUserManager; use OCP\L10N\IFactory; +use OCP\Log\Audit\CriticalActionPerformedEvent; use OCP\Security\ISecureRandom; use OCP\Share\IManager; use OCP\Snowflake\ISnowflakeGenerator; @@ -64,7 +65,7 @@ private IL10N $l10n; public function __construct( - IEventDispatcher $eventDispatcher, + private IEventDispatcher $eventDispatcher, private IUserManager $userManager, private IFactory $l10nFactory, private ISnowflakeGenerator $snowflakeGenerator, @@ -78,7 +79,7 @@ public function __construct( $this->randomizer = new Randomizer(); $this->l10n = $l10nFactory->get('sharing'); - $eventDispatcher->addServiceListener(BeforeUserDeletedEvent::class, self::class); + $this->eventDispatcher->addServiceListener(BeforeUserDeletedEvent::class, self::class); } #[\Override] @@ -175,6 +176,11 @@ public function createShare(ShareAccessContext $accessContext): Share { $share = $this->backend->getShare($accessContext, $id); [$share] = $this->processShareUpdates([$share]); + $this->audit('Share "%s" was created by "%s"', [ + 'shareId' => $share->id, + 'owner' => $share->owner->userId, + ], 'unified_sharing.share.created'); + return $share; } @@ -215,6 +221,12 @@ public function updateShareState(ShareAccessContext $accessContext, Share $share ); [$share] = $this->processShareUpdates([$share]); + + $this->audit('State of share "%s" was set to "%s"', [ + 'shareId' => $share->id, + 'state' => $state->value, + ], 'unified_sharing.share.state_updated'); + return $share; } @@ -245,6 +257,13 @@ public function updateShareUserStatus(ShareAccessContext $accessContext, Share $ ); [$share] = $this->processShareUpdates([$share]); + + $this->audit('Share "%s" was set to "%s" by recipient "%s"', [ + 'shareId' => $share->id, + 'userStatus' => $userStatus->value, + 'user' => $currentUser->getUID(), + ], 'unified_sharing.share.user_status_updated'); + return $share; } @@ -297,6 +316,13 @@ public function addShareSource(ShareAccessContext $accessContext, Share $share, [$share] = $this->backend->ensureDefaults([$share]); [$share] = $this->processShareUpdates([$share]); + + $this->audit('Share "%s": source %s "%s" was added', [ + 'shareId' => $share->id, + 'sourceClass' => $source->class, + 'sourceValue' => $source->value, + ], 'unified_sharing.source.added'); + return $share; } @@ -333,6 +359,13 @@ public function removeShareSource(ShareAccessContext $accessContext, Share $shar ); [$share] = $this->processShareUpdates([$share]); + + $this->audit('Share "%s": source %s "%s" was removed', [ + 'shareId' => $share->id, + 'sourceClass' => $source->class, + 'sourceValue' => $source->value, + ], 'unified_sharing.source.removed'); + return $share; } @@ -431,6 +464,14 @@ public function addShareRecipient(ShareAccessContext $accessContext, Share $shar [$share] = $this->backend->ensureDefaults([$share]); [$share] = $this->processShareUpdates([$share]); + + $this->audit('Share "%s": recipient %s "%s" on "%s" was added', [ + 'shareId' => $share->id, + 'recipientClass' => $recipient->class, + 'recipientValue' => $recipient->value, + 'instance' => $recipient->instance ?? 'local', + ], 'unified_sharing.recipient.added'); + return $share; } @@ -469,6 +510,14 @@ public function removeShareRecipient(ShareAccessContext $accessContext, Share $s ); [$share] = $this->processShareUpdates([$share]); + + $this->audit('Share "%s": recipient %s "%s" on "%s" was removed', [ + 'shareId' => $share->id, + 'recipientClass' => $recipient->class, + 'recipientValue' => $recipient->value, + 'instance' => $recipient->instance ?? 'local', + ], 'unified_sharing.recipient.removed'); + return $share; } @@ -568,6 +617,13 @@ public function updateShareRecipientSecret(ShareAccessContext $accessContext, Sh ); [$share] = $this->processShareUpdates([$share]); + + $this->audit('Share "%s": the secret of recipient %s "%s" was changed', [ + 'shareId' => $share->id, + 'recipientClass' => $recipient->class, + 'recipientValue' => $recipient->value, + ], 'unified_sharing.recipient_secret.updated'); + return $share; } @@ -612,6 +668,12 @@ public function updateShareProperty(ShareAccessContext $accessContext, Share $sh ); [$share] = $this->processShareUpdates([$share]); + + $this->audit('Share "%s": property %s was changed', [ + 'shareId' => $share->id, + 'propertyClass' => $property->class, + ], 'unified_sharing.property.updated'); + return $share; } @@ -648,6 +710,13 @@ public function updateSharePermission(ShareAccessContext $accessContext, Share $ $this->backend->updateSharePermission($share->id, $permission); [$share] = $this->processShareUpdates([$share]); + + $this->audit('Share "%s": permission %s was %s', [ + 'shareId' => $share->id, + 'permissionClass' => $permission->class, + 'enabled' => $permission->enabled ? 'enabled' : 'disabled', + ], 'unified_sharing.permission.updated'); + return $share; } @@ -703,6 +772,15 @@ public function updateShareRecipientPermission(ShareAccessContext $accessContext ); [$share] = $this->processShareUpdates([$share]); + + $this->audit('Share "%s": permission %s of recipient %s "%s" was %s', [ + 'shareId' => $share->id, + 'permissionClass' => $permission->class, + 'recipientClass' => $recipient->class, + 'recipientValue' => $recipient->value, + 'enabled' => $permission->enabled ? 'enabled' : 'disabled', + ], 'unified_sharing.recipient_permission.updated'); + return $share; } @@ -740,6 +818,12 @@ public function selectSharePermissionPreset(ShareAccessContext $accessContext, S ); [$share] = $this->processShareUpdates([$share]); + + $this->audit('Share "%s": permission preset %s was selected', [ + 'shareId' => $share->id, + 'presetClass' => $permissionPresetClass, + ], 'unified_sharing.permission_preset.selected'); + return $share; } @@ -750,6 +834,18 @@ public function deleteShare(ShareAccessContext $accessContext, Share $share): vo $this->validateShareEditPermissions($accessContext, $share); $this->backend->deleteShare($share->id); + + $this->audit('Share "%s" of "%s" was deleted', [ + 'shareId' => $share->id, + 'owner' => $share->owner->userId, + ], 'unified_sharing.share.deleted'); + } + + /** + * @param array $params + */ + private function audit(string $message, array $params, string $operation): void { + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent($message, $params, false, $operation)); } #[\Override] diff --git a/lib/public/Interaction/RestrictInteractionEvent.php b/lib/public/Interaction/RestrictInteractionEvent.php index 3fe4f0ab8bfee..db97d33167cb9 100644 --- a/lib/public/Interaction/RestrictInteractionEvent.php +++ b/lib/public/Interaction/RestrictInteractionEvent.php @@ -69,10 +69,10 @@ public function isInteractionRestricted(): false|string { $eventDispatcher = Server::get(IEventDispatcher::class); $params = [ - $this->action instanceof InteractionAction ? $this->action::class : '?', - $this->userId, - $this->resources === [] ? '?' : implode(', ', array_map(static fn (InteractionResource $resource): string => $resource->getID(), $this->resources)), - $this->receivers === [] ? '?' : implode(', ', array_map(static fn (InteractionReceiver $receiver): string => $receiver->getID() ?? $receiver::class, $this->receivers)), + 'action' => $this->action instanceof InteractionAction ? $this->action::class : '?', + 'userId' => $this->userId, + 'resources' => $this->resources === [] ? '?' : implode(', ', array_map(static fn (InteractionResource $resource): string => $resource->getID(), $this->resources)), + 'receivers' => $this->receivers === [] ? '?' : implode(', ', array_map(static fn (InteractionReceiver $receiver): string => $receiver->getID() ?? $receiver::class, $this->receivers)), ]; try { @@ -81,6 +81,8 @@ public function isInteractionRestricted(): false|string { $eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent( 'Interaction %s from user %s on %s to %s is allowed.', $params, + false, + 'users.interaction.allowed', )); return false; @@ -88,6 +90,8 @@ public function isInteractionRestricted(): false|string { $eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent( 'Interaction %s from user %s on %s to %s is restricted: ' . $interactionRestrictedException->getMessage(), $params, + false, + 'users.interaction.restricted', )); return $interactionRestrictedException->getHint(); diff --git a/tests/Core/Controller/AppPasswordControllerTest.php b/tests/Core/Controller/AppPasswordControllerTest.php index 123a61efa119b..c4e05ea63f9f4 100644 --- a/tests/Core/Controller/AppPasswordControllerTest.php +++ b/tests/Core/Controller/AppPasswordControllerTest.php @@ -8,6 +8,7 @@ namespace Tests\Core\Controller; +use OC\Authentication\Events\AppPasswordCreatedEvent; use OC\Authentication\Exceptions\InvalidTokenException; use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; @@ -19,11 +20,13 @@ use OCP\Authentication\Exceptions\PasswordUnavailableException; use OCP\Authentication\LoginCredentials\ICredentials; use OCP\Authentication\LoginCredentials\IStore; +use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; use OCP\IRequest; use OCP\ISession; use OCP\IUserManager; +use OCP\Log\Audit\CriticalActionPerformedEvent; use OCP\Security\Bruteforce\IThrottler; use OCP\Security\ISecureRandom; use PHPUnit\Framework\MockObject\MockObject; @@ -170,8 +173,7 @@ public function testGetAppPassword(): void { IToken::DO_NOT_REMEMBER ); - $this->eventDispatcher->expects($this->once()) - ->method('dispatchTyped'); + $this->expectAppPasswordCreatedEvents(); $this->controller->getAppPassword(); } @@ -215,8 +217,7 @@ public function testGetAppPasswordNoPassword(): void { IToken::DO_NOT_REMEMBER ); - $this->eventDispatcher->expects($this->once()) - ->method('dispatchTyped'); + $this->expectAppPasswordCreatedEvents(); $this->controller->getAppPassword(); } @@ -289,8 +290,62 @@ public function testDeleteAppPasswordSuccess(): void { 42 ); + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(fn (CriticalActionPerformedEvent $event): bool => $event->getOperation() === 'auth.app_password.deleted' + && $event->getParameters() === ['tokenId' => 42] + && $event->getLogMessage() === 'App password with ID "%s" was deleted')); + $result = $this->controller->deleteAppPassword(); $this->assertEquals(new DataResponse(), $result); } + + public function testGetAppPasswordWithOneTimePassword(): void { + $credentials = $this->createMock(ICredentials::class); + + $this->session->method('exists') + ->with('one_time_token') + ->willReturn(true); + $this->credentialStore->method('getLoginCredentials') + ->willReturn($credentials); + $credentials->method('getUid') + ->willReturn('myUID'); + $credentials->method('getPassword') + ->willReturn('myPassword'); + $credentials->method('getLoginName') + ->willReturn('myLoginName'); + $this->request->method('getHeader') + ->with('user-agent') + ->willReturn('myUA'); + $this->random->method('generate') + ->willReturn('myToken'); + + $this->tokenProvider->expects($this->once()) + ->method('generateToken') + ->with('myToken', 'myUID', 'myLoginName', 'myPassword', 'myUA', IToken::PERMANENT_TOKEN); + + $this->expectAppPasswordCreatedEvents(); + + $this->assertEquals(new DataResponse(['apppassword' => 'myToken']), $this->controller->getAppPasswordWithOneTimePassword()); + } + + /** + * Expects the AppPasswordCreatedEvent followed by its audit log entry. + */ + private function expectAppPasswordCreatedEvents(): void { + $calls = 0; + $this->eventDispatcher->expects($this->exactly(2)) + ->method('dispatchTyped') + ->willReturnCallback(function (Event $event) use (&$calls): void { + if (++$calls === 1) { + $this->assertInstanceOf(AppPasswordCreatedEvent::class, $event); + return; + } + $this->assertInstanceOf(CriticalActionPerformedEvent::class, $event); + $this->assertSame('auth.app_password.created', $event->getOperation()); + $this->assertSame('App password with ID "%s" and name "%s" was created', $event->getLogMessage()); + $this->assertSame(['tokenId', 'tokenName'], array_keys($event->getParameters())); + }); + } } diff --git a/tests/Core/Controller/ClientFlowLoginControllerTest.php b/tests/Core/Controller/ClientFlowLoginControllerTest.php index 772562507429c..bcd34c6c462fa 100644 --- a/tests/Core/Controller/ClientFlowLoginControllerTest.php +++ b/tests/Core/Controller/ClientFlowLoginControllerTest.php @@ -9,6 +9,7 @@ namespace Tests\Core\Controller; +use OC\Authentication\Events\AppPasswordCreatedEvent; use OC\Authentication\Exceptions\InvalidTokenException; use OC\Authentication\Exceptions\PasswordlessTokenException; use OC\Authentication\Token\IProvider; @@ -25,6 +26,7 @@ use OCP\AppFramework\Services\IInitialState; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Defaults; +use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; use OCP\IL10N; @@ -33,6 +35,7 @@ use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserSession; +use OCP\Log\Audit\CriticalActionPerformedEvent; use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; use OCP\Session\Exceptions\SessionNotAvailableException; @@ -409,8 +412,7 @@ public function testGeneratePasswordWithPassword(): void { ->method('getHeader') ->willReturn(''); - $this->eventDispatcher->expects($this->once()) - ->method('dispatchTyped'); + $this->expectAppPasswordCreatedEvents(); $expected = new RedirectResponse('nc://login/server:http://example.com&user:MyLoginName&password:MyGeneratedToken'); $this->assertEquals($expected, $this->clientFlowLoginController->generateAppPassword('MyStateToken')); @@ -500,8 +502,7 @@ public function testGeneratePasswordWithPasswordForOauthClient($redirectUri, $re ->with('MyClientIdentifier') ->willReturn($client); - $this->eventDispatcher->expects($this->once()) - ->method('dispatchTyped'); + $this->expectAppPasswordCreatedEvents(); $expected = new RedirectResponse($redirectUrl); $this->assertEquals($expected, $this->clientFlowLoginController->generateAppPassword('MyStateToken', 'MyClientIdentifier')); @@ -575,8 +576,7 @@ public function testGeneratePasswordWithoutPassword(): void { ->method('getHeader') ->willReturn(''); - $this->eventDispatcher->expects($this->once()) - ->method('dispatchTyped'); + $this->expectAppPasswordCreatedEvents(); $expected = new RedirectResponse('nc://login/server:http://example.com&user:MyLoginName&password:MyGeneratedToken'); $this->assertEquals($expected, $this->clientFlowLoginController->generateAppPassword('MyStateToken')); @@ -706,10 +706,28 @@ public function testGeneratePasswordWithHttpsProxy(array $headers, $protocol, $e ->method('getHeader') ->willReturnMap($headers); - $this->eventDispatcher->expects($this->once()) - ->method('dispatchTyped'); + $this->expectAppPasswordCreatedEvents(); $expected = new RedirectResponse('nc://login/server:' . $expected . '://example.com&user:MyLoginName&password:MyGeneratedToken'); $this->assertEquals($expected, $this->clientFlowLoginController->generateAppPassword('MyStateToken')); } + + /** + * Expects the AppPasswordCreatedEvent followed by its audit log entry. + */ + private function expectAppPasswordCreatedEvents(): void { + $calls = 0; + $this->eventDispatcher->expects($this->exactly(2)) + ->method('dispatchTyped') + ->willReturnCallback(function (Event $event) use (&$calls): void { + if (++$calls === 1) { + $this->assertInstanceOf(AppPasswordCreatedEvent::class, $event); + return; + } + $this->assertInstanceOf(CriticalActionPerformedEvent::class, $event); + $this->assertSame('auth.app_password.created', $event->getOperation()); + $this->assertSame('App password with ID "%s" and name "%s" was created', $event->getLogMessage()); + $this->assertSame(['tokenId', 'tokenName'], array_keys($event->getParameters())); + }); + } } diff --git a/tests/Core/Controller/LostControllerTest.php b/tests/Core/Controller/LostControllerTest.php index dc033d983f921..63a3ffdd2c5b3 100644 --- a/tests/Core/Controller/LostControllerTest.php +++ b/tests/Core/Controller/LostControllerTest.php @@ -26,6 +26,7 @@ use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; +use OCP\Log\Audit\CriticalActionPerformedEvent; use OCP\Mail\IEMailTemplate; use OCP\Mail\IMailer; use OCP\Security\VerificationToken\InvalidTokenException; @@ -238,6 +239,8 @@ public function testEmailUnsuccessful(): void { $this->logger->expects($this->exactly(0)) ->method('error'); + $this->eventDispatcher->expects($this->never()) + ->method('dispatchTyped'); $this->logger->expects($this->exactly(2)) ->method('warning'); @@ -321,6 +324,12 @@ public function testEmailSuccessful(): void { ->method('send') ->with($message); + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(fn (CriticalActionPerformedEvent $event): bool => $event->getOperation() === 'auth.password_reset.requested' + && $event->getParameters() === ['userId' => 'ExistingUser'] + && $event->getLogMessage() === 'Password reset was requested for user "%s"')); + $response = $this->lostController->email('ExistingUser'); $expectedResponse = new JSONResponse(['status' => 'success']); $expectedResponse->throttle(); diff --git a/tests/Core/Service/LoginFlowV2ServiceUnitTest.php b/tests/Core/Service/LoginFlowV2ServiceUnitTest.php index 35f939a275eeb..300626b9bc5d4 100644 --- a/tests/Core/Service/LoginFlowV2ServiceUnitTest.php +++ b/tests/Core/Service/LoginFlowV2ServiceUnitTest.php @@ -20,7 +20,9 @@ use OC\Core\Service\LoginFlowV2Service; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; +use OCP\Log\Audit\CriticalActionPerformedEvent; use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; use PHPUnit\Framework\MockObject\MockObject; @@ -41,6 +43,7 @@ class LoginFlowV2ServiceUnitTest extends TestCase { private ISecureRandom&MockObject $secureRandom; private ITimeFactory&MockObject $timeFactory; private IProvider&MockObject $tokenProvider; + private IEventDispatcher&MockObject $eventDispatcher; #[\Override] public function setUp(): void { @@ -60,6 +63,7 @@ private function setupSubjectUnderTest(): void { $this->mapper = $this->createMock(LoginFlowV2Mapper::class); $this->logger = $this->createMock(LoggerInterface::class); $this->tokenProvider = $this->createMock(IProvider::class); + $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->secureRandom = $this->createMock(ISecureRandom::class); $this->timeFactory = $this->createMock(ITimeFactory::class); @@ -70,7 +74,8 @@ private function setupSubjectUnderTest(): void { $this->config, $this->crypto, $this->logger, - $this->tokenProvider + $this->tokenProvider, + $this->eventDispatcher, ); } @@ -370,6 +375,10 @@ public function testFlowDone(): void { ->method('getToken') ->willReturn($sessionToken); + $appToken = $this->createMock(IToken::class); + $appToken->method('getId')->willReturn(7); + $appToken->method('getName')->willReturn('client_name'); + $this->tokenProvider->expects($this->once()) ->method('generateToken') ->with( @@ -380,7 +389,13 @@ public function testFlowDone(): void { 'client_name', IToken::PERMANENT_TOKEN, IToken::DO_NOT_REMEMBER - ); + ) + ->willReturn($appToken); + + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(fn (CriticalActionPerformedEvent $event): bool => $event->getOperation() === 'auth.app_password.created' + && $event->getParameters() === ['tokenId' => 7, 'tokenName' => 'client_name'])); $result = $this->subjectUnderTest->flowDone( 'login_token', diff --git a/tests/lib/Authentication/Token/RemoteWipeTest.php b/tests/lib/Authentication/Token/RemoteWipeTest.php index a256c9d609456..4d2b2e2e2cd2a 100644 --- a/tests/lib/Authentication/Token/RemoteWipeTest.php +++ b/tests/lib/Authentication/Token/RemoteWipeTest.php @@ -18,6 +18,7 @@ use OC\Authentication\Token\RemoteWipe; use OCP\EventDispatcher\IEventDispatcher; use OCP\IUser; +use OCP\Log\Audit\CriticalActionPerformedEvent; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; @@ -152,4 +153,26 @@ public function startFinishWiping() { $this->assertTrue($result); } + + public function testFinishWiping(): void { + $token = $this->createMock(IToken::class); + $token->method('getId')->willReturn(42); + $token->method('getUID')->willReturn('user1'); + $this->getAutoMock(ITokenProvider::class)->expects($this->once()) + ->method('getToken') + ->with('tk1') + ->willThrowException(new WipeTokenException($token)); + $this->getAutoMock(ITokenProvider::class)->expects($this->once()) + ->method('invalidateToken') + ->with('tk1'); + $this->getAutoMock(IEventDispatcher::class)->expects($this->once()) + ->method('dispatch') + ->with(RemoteWipeFinished::class, $this->equalTo(new RemoteWipeFinished($token))); + $this->getAutoMock(IEventDispatcher::class)->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(fn (CriticalActionPerformedEvent $event): bool => $event->getOperation() === 'auth.remote_wipe.finished' + && $event->getParameters() === ['tokenId' => 42, 'userId' => 'user1'])); + + $this->assertTrue($this->remoteWipe->finish('tk1')); + } } diff --git a/tests/lib/Interaction/RestrictInteractionEventTest.php b/tests/lib/Interaction/RestrictInteractionEventTest.php index 0b30db0566627..61edf969bd888 100644 --- a/tests/lib/Interaction/RestrictInteractionEventTest.php +++ b/tests/lib/Interaction/RestrictInteractionEventTest.php @@ -97,11 +97,13 @@ public function testIsInteractionRestricted(bool $isRestricted): void { ? 'Interaction %s from user %s on %s to %s is restricted: my restriction message' : 'Interaction %s from user %s on %s to %s is allowed.', [ - $action::class, - 'my-uid', - 'my-resource1, my-resource2', - 'my-receiver1, my-receiver2', + 'action' => $action::class, + 'userId' => 'my-uid', + 'resources' => 'my-resource1, my-resource2', + 'receivers' => 'my-receiver1, my-receiver2', ], + false, + $isRestricted ? 'users.interaction.restricted' : 'users.interaction.allowed', ), ], $auditEvents); diff --git a/tests/lib/Security/Bruteforce/ThrottlerTest.php b/tests/lib/Security/Bruteforce/ThrottlerTest.php new file mode 100644 index 0000000000000..a9411ed986014 --- /dev/null +++ b/tests/lib/Security/Bruteforce/ThrottlerTest.php @@ -0,0 +1,81 @@ +throttler = $this->createInstanceWithMocks(Throttler::class); + } + + public function testRegisterAttemptFromAllowListedIp(): void { + $this->getAutoMock(IConfig::class)->method('getSystemValueBool') + ->with('auth.bruteforce.protection.enabled', true) + ->willReturn(true); + $this->getAutoMock(BruteforceAllowList::class)->method('isBypassListed') + ->with('10.10.10.10') + ->willReturn(true); + + $this->getAutoMock(IEventDispatcher::class)->expects($this->once()) + ->method('dispatchTyped') + ->with(new CriticalActionPerformedEvent( + 'Bruteforce attempt from "{ip}" detected for action "{action}" not throttled due to allow-list.', + ['ip' => '10.10.10.10', 'action' => 'login'], + false, + 'auth.bruteforce.bypassed', + )); + $this->getAutoMock(IBackend::class)->expects($this->never()) + ->method('registerAttempt'); + + $this->throttler->registerAttempt('login', '10.10.10.10'); + } + + public function testRegisterAttempt(): void { + $this->getAutoMock(IConfig::class)->method('getSystemValueBool') + ->with('auth.bruteforce.protection.enabled', true) + ->willReturn(true); + $this->getAutoMock(BruteforceAllowList::class)->method('isBypassListed') + ->with('10.10.10.10') + ->willReturn(false); + + $this->getAutoMock(IEventDispatcher::class)->expects($this->never()) + ->method('dispatchTyped'); + $this->getAutoMock(IBackend::class)->expects($this->once()) + ->method('registerAttempt') + ->with('10.10.10.10', '10.10.10.10/32', $this->anything(), 'login', []); + + $this->throttler->registerAttempt('login', '10.10.10.10'); + } + + public function testRegisterAttemptWithProtectionDisabled(): void { + $this->getAutoMock(IConfig::class)->method('getSystemValueBool') + ->with('auth.bruteforce.protection.enabled', true) + ->willReturn(false); + + $this->getAutoMock(IEventDispatcher::class)->expects($this->never()) + ->method('dispatchTyped'); + $this->getAutoMock(IBackend::class)->expects($this->never()) + ->method('registerAttempt'); + + $this->throttler->registerAttempt('login', '10.10.10.10'); + } +} diff --git a/tests/lib/Sharing/SharingManagerTest.php b/tests/lib/Sharing/SharingManagerTest.php index 2cdfeb6ec3102..7fb600c9825bc 100644 --- a/tests/lib/Sharing/SharingManagerTest.php +++ b/tests/lib/Sharing/SharingManagerTest.php @@ -19,9 +19,11 @@ use NCU\Sharing\ShareUserStatus; use NCU\Sharing\Source\ShareSource; use OC\Sharing\SharingManager; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IURLGenerator; use OCP\IUserManager; use OCP\L10N\IFactory; +use OCP\Log\Audit\CriticalActionPerformedEvent; use OCP\Server; use PHPUnit\Framework\Attributes\Group; @@ -303,4 +305,73 @@ protected function getShares(ShareAccessContext $accessContext, ?string $filterS throw $exception; } } + + public function testAuditLog(): void { + $this->registry->registerRecipientType(new TestShareRecipientTypePublicSecret(['recipient1' => 'Recipient 1'], [], true, true)); + + $auditEvents = []; + $listener = static function (CriticalActionPerformedEvent $event) use (&$auditEvents): void { + if (str_starts_with($event->getOperation() ?? '', 'unified_sharing.')) { + $auditEvents[] = $event; + } + }; + $eventDispatcher = Server::get(IEventDispatcher::class); + $eventDispatcher->addListener(CriticalActionPerformedEvent::class, $listener); + + $accessContext = new ShareAccessContext($this->owner); + $recipient = new ShareRecipient(TestShareRecipientType1::class, 'recipient1', null); + $secretRecipient = new ShareRecipient(TestShareRecipientTypePublicSecret::class, 'recipient1', null); + + try { + $this->dbConnection->beginTransaction(); + $share = $this->manager->createShare($accessContext); + $share = $this->manager->addShareSource($accessContext, $share, new ShareSource(TestShareSourceType1::class, 'source1')); + $share = $this->manager->addShareRecipient($accessContext, $share, $recipient); + $share = $this->manager->updateSharePermission($accessContext, $share, new SharePermission(TestSharePermissionType1::class, true)); + $share = $this->manager->updateShareProperty($accessContext, $share, new ShareProperty(TestSharePropertyType1::class, 'valid1')); + $share = $this->manager->updateShareState($accessContext, $share, ShareState::Active); + $share = $this->manager->addShareRecipient($accessContext, $share, $secretRecipient); + $share = $this->manager->updateShareRecipientSecret($accessContext, $share, $secretRecipient, 'mysecret'); + $share = $this->manager->updateShareRecipientPermission($accessContext, $share, $recipient, new SharePermission(TestSharePermissionType1::class, false)); + $share = $this->manager->selectSharePermissionPreset($accessContext, $share, TestSharePermissionPreset1::class); + $recipientContext = new ShareAccessContext($this->user1); + $this->manager->updateShareUserStatus($recipientContext, $this->manager->getShare($recipientContext, $share->id), ShareUserStatus::Accepted); + $share = $this->manager->removeShareRecipient($accessContext, $share, $secretRecipient); + $share = $this->manager->removeShareSource($accessContext, $share, new ShareSource(TestShareSourceType1::class, 'source1')); + $this->manager->deleteShare($accessContext, $share); + $this->dbConnection->commit(); + } finally { + $eventDispatcher->removeListener(CriticalActionPerformedEvent::class, $listener); + } + + $this->assertSame([ + 'unified_sharing.share.created', + 'unified_sharing.source.added', + 'unified_sharing.recipient.added', + 'unified_sharing.permission.updated', + 'unified_sharing.property.updated', + 'unified_sharing.share.state_updated', + 'unified_sharing.recipient.added', + 'unified_sharing.recipient_secret.updated', + 'unified_sharing.recipient_permission.updated', + 'unified_sharing.permission_preset.selected', + 'unified_sharing.share.user_status_updated', + 'unified_sharing.recipient.removed', + 'unified_sharing.source.removed', + 'unified_sharing.share.deleted', + ], array_map(static fn (CriticalActionPerformedEvent $event): ?string => $event->getOperation(), $auditEvents)); + + $this->assertSame([ + 'shareId' => $share->id, + 'recipientClass' => TestShareRecipientType1::class, + 'recipientValue' => 'recipient1', + 'instance' => 'local', + ], $auditEvents[2]->getParameters()); + $this->assertSame(['shareId' => $share->id, 'userStatus' => 'accepted', 'user' => 'user1'], $auditEvents[10]->getParameters()); + $this->assertSame(['shareId' => $share->id, 'owner' => 'owner'], $auditEvents[13]->getParameters()); + + foreach ($auditEvents as $event) { + $this->assertNotContains('mysecret', $event->getParameters(), 'The recipient secret must not be logged'); + } + } }