diff --git a/CHANGELOG.md b/CHANGELOG.md index d671af15b..3bb803342 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Added - Support Nextcloud 35 +- Log integration settings saved through the setup API and every new Nextcloud OAuth client for OpenProject to the audit log +- Log the project folder setup and terms of service signed for the OpenProject user to the audit log +- Log users connecting and disconnecting their OpenProject account to the audit log +- Log files linked to and unlinked from OpenProject work packages to the audit log ### Fixed - Keep the admin settings when Nextcloud replays the migrations to check the database schema (`occ db:schema:check`, every `occ upgrade` on Nextcloud 35) @@ -22,6 +26,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Changed - Make OAuth2 Client class usage compatible with older Nextcloud versions [#1148](https://github.com/nextcloud/integration_openproject/pull/1148) - Add owner_file_name property in filesInfo endpoint response [#1189](https://github.com/nextcloud/integration_openproject/pull/1189) +- Write the audit log entries through the admin_audit app, with an operation and named parameters, whenever the app is enabled ### Removed diff --git a/bootstrap.php b/bootstrap.php index 050e0e925..c78427811 100644 --- a/bootstrap.php +++ b/bootstrap.php @@ -29,4 +29,5 @@ $classLoader = new ClassLoader(); $classLoader->addPsr4("OCA\\OpenProject\\", $rootDir . '/lib', true); +$classLoader->addPsr4("OCA\\OpenProject\\Tests\\", $rootDir . '/tests/lib', true); $classLoader->register(); diff --git a/lib/AuditLog/Operation.php b/lib/AuditLog/Operation.php new file mode 100644 index 000000000..dc303873b --- /dev/null +++ b/lib/AuditLog/Operation.php @@ -0,0 +1,50 @@ + 'An admin changed the OpenProject integration settings, such as the OpenProject URL or the OpenProject OAuth client', + self::IntegrationReset => 'An admin reset the OpenProject integration, which disconnects all users from OpenProject', + self::OauthClientCreated => 'An admin created or replaced the Nextcloud OAuth client used by OpenProject', + self::AppPasswordCreated => 'An app password was created or replaced for the OpenProject system user', + self::AppPasswordDeleted => 'The app password of the OpenProject system user was deleted', + self::ProjectFolderCreated => 'The project folder setup created the OpenProject system user and groups, made the user their group admin and created the project groupfolder', + self::ProjectFolderDeactivated => 'An admin deactivated the project folder setup', + self::TermsOfServiceSigned => 'Terms of service were signed on behalf of the OpenProject system user, by an admin, during the project folder setup or automatically when the terms change', + self::AccountConnected => 'A user connected their OpenProject account through OAuth or a personal access token', + self::AccountDisconnected => 'A user disconnected their OpenProject account', + self::FileLinkCreated => 'A user linked a file to an OpenProject work package, which shares the file ID, name and type with OpenProject', + self::FileLinkDeleted => 'A user removed the link between a file and an OpenProject work package', + }; + } +} diff --git a/lib/Controller/ConfigController.php b/lib/Controller/ConfigController.php index a0cc0db8b..ce8b1522f 100755 --- a/lib/Controller/ConfigController.php +++ b/lib/Controller/ConfigController.php @@ -14,6 +14,7 @@ use OCA\OAuth2\Exceptions\ClientNotFoundException; use OCA\OAuth2\Service\ClientService; use OCA\OpenProject\AppInfo\Application; +use OCA\OpenProject\AuditLog\Operation; use OCA\OpenProject\Exception\OpenprojectGroupfolderSetupConflictException; use OCA\OpenProject\Service\OauthService; use OCA\OpenProject\Service\OpenProjectAPIService; @@ -113,6 +114,20 @@ public function clearUserInfo(?string $userId = null) { $this->config->deleteUserValue($userId, Application::APP_ID, 'token_expires_at'); } + /** + * Only the OpenProject user ID is logged, never the access or refresh token + */ + private function logAccountConnected(): void { + $this->openprojectAPIService->logToAuditLog( + Operation::AccountConnected, + 'User %s has connected the OpenProject account %s in application ' . Application::APP_ID, + [ + 'userId' => $this->userId, + 'openprojectUserId' => (string)$this->config->getUserValue($this->userId, Application::APP_ID, 'user_id'), + ], + ); + } + /** * @return void */ @@ -148,8 +163,16 @@ public function setConfig(array $values): DataResponse { if (isset($values['token'])) { if ($values['token']) { $result = $this->openprojectAPIService->initUserInfo($this->userId, $values['token']); + if (!isset($result['error'])) { + $this->logAccountConnected(); + } } else { $this->clearUserInfo(); + $this->openprojectAPIService->logToAuditLog( + Operation::AccountDisconnected, + 'User %s has disconnected the OpenProject account in application ' . Application::APP_ID, + ['userId' => $this->userId], + ); $result = [ 'user_name' => '', ]; @@ -185,16 +208,7 @@ private function setIntegrationConfig(array $values): array { if (!$this->userManager->userExists(Application::OPEN_PROJECT_ENTITIES_NAME)) { throw new NoUserException('User "' . Application::OPEN_PROJECT_ENTITIES_NAME . '" does not exists to create application password'); } - $appPassword = $this->openprojectAPIService->generateAppPasswordTokenForUser(); - if ($isAppPasswordBeingReplaced) { - $this->openprojectAPIService->logToAuditFile( - "Application password for user 'OpenProject has been replaced' with new password in application " . Application::APP_ID - ); - } else { - $this->openprojectAPIService->logToAuditFile( - "New application password for user 'OpenProject' has been created in application " . Application::APP_ID - ); - } + $appPassword = $this->openprojectAPIService->generateAppPasswordTokenForUser($isAppPasswordBeingReplaced); } $oldClientId = $oldClientSecret = ''; $oldOpenProjectOauthUrl = $this->config->getAppValue( @@ -293,8 +307,9 @@ private function setIntegrationConfig(array $values): array { if (key_exists('setup_app_password', $values) && $values['setup_app_password'] === false) { $this->openprojectAPIService->deleteAppPassword(); if (!$runningFullReset) { - $this->openprojectAPIService->logToAuditFile( - "Project folder setup has been deactivated in application " . Application::APP_ID + $this->openprojectAPIService->logToAuditLog( + Operation::ProjectFolderDeactivated, + 'Project folder setup has been deactivated in application ' . Application::APP_ID, ); } } @@ -302,10 +317,12 @@ private function setIntegrationConfig(array $values): array { $resetOpenProjectClient = ((key_exists('openproject_client_id', $values) && $values['openproject_client_id'] !== $oldClientId) || (key_exists('openproject_client_secret', $values) && $values['openproject_client_secret'] !== $oldClientSecret)); + $usersDisconnected = false; if ($resetOpenProjectClient || $runningFullResetWithOAuth2Auth || $switchingOAuthToOIDC) { $this->userManager->callForAllUsers(function (IUser $user) { $this->clearUserInfo($user->getUID()); }); + $usersDisconnected = true; if ($switchingOAuthToOIDC) { $this->resetOauth2Configs(); @@ -315,14 +332,17 @@ private function setIntegrationConfig(array $values): array { $this->userManager->callForAllUsers(function (IUser $user) { $this->clearUserInfo($user->getUID()); }); + $usersDisconnected = true; } // whenever doing full reset we want the project folder switch state to be "on" in the UI // so setting `fresh_project_folder_setup` as true if ($runningFullReset) { $this->config->setAppValue(Application::APP_ID, 'fresh_project_folder_setup', "1"); - $this->openprojectAPIService->logToAuditFile( - "OpenProject Integration admin configuration has been reset in application " . Application::APP_ID + $this->openprojectAPIService->logToAuditLog( + Operation::IntegrationReset, + 'OpenProject Integration admin configuration has been reset in application ' . Application::APP_ID, + ['usersDisconnected' => $usersDisconnected], ); } elseif (key_exists('setup_app_password', $values) && key_exists('setup_project_folder', $values)) { // for other cases when api has key 'setup_app_password' and 'setup_project_folder' we set it to false @@ -330,6 +350,21 @@ private function setIntegrationConfig(array $values): array { $this->config->setAppValue(Application::APP_ID, 'fresh_project_folder_setup', "0"); } + $savedSettings = array_values(array_diff(array_keys($values), ['setup_project_folder', 'setup_app_password'])); + if (!$runningFullReset && $savedSettings !== []) { + // only the names of the settings are logged, never the OpenProject client secret + $this->openprojectAPIService->logToAuditLog( + Operation::IntegrationConfigured, + "OpenProject integration settings %s have been saved for the OpenProject instance '%s' in application " . Application::APP_ID, + [ + 'settings' => implode(', ', $savedSettings), + 'openprojectUrl' => key_exists('openproject_instance_url', $values) ? (string)$values['openproject_instance_url'] : $oldOpenProjectOauthUrl, + 'authorizationMethod' => key_exists('authorization_method', $values) ? (string)$values['authorization_method'] : $oldAuthMethod, + 'usersDisconnected' => $usersDisconnected, + ], + ); + } + return [ "status" => OpenProjectAPIService::isAdminConfigOk($this->config), "oPUserAppPassword" => $appPassword, @@ -355,23 +390,6 @@ public function setAdminConfig(array $values): DataResponse { } } $result = $this->setIntegrationConfig($values); - $isOPOAuthCrdentialSet = key_exists('openproject_client_id', $values) && - key_exists('openproject_client_secret', $values) && - $values['openproject_client_id'] && - $values['openproject_client_secret']; - - if (key_exists('openproject_instance_url', $values) && - $values['openproject_instance_url'] && !$isOPOAuthCrdentialSet) { - // sending admin audit log if admin has changed or added the openproject host url - $this->openprojectAPIService->logToAuditFile( - "OpenProject host url has been set to '" . $values['openproject_instance_url'] . "' in application " . Application::APP_ID - ); - } - if ($isOPOAuthCrdentialSet) { - $this->openprojectAPIService->logToAuditFile( - "OpenProject OAuth credential 'openproject_client_id' and 'openproject_client_secret' has been set in application " . Application::APP_ID - ); - } return new DataResponse($result); } catch (OpenprojectGroupfolderSetupConflictException $e) { return new DataResponse([ @@ -476,6 +494,7 @@ public function oauthRedirect(string $code = '', string $state = ''): RedirectRe $this->config->setUserValue( $this->userId, Application::APP_ID, 'oauth_connection_result', 'success' ); + $this->logAccountConnected(); } return new RedirectResponse($newUrl); } @@ -510,13 +529,13 @@ public function oauthRedirect(string $code = '', string $state = ''): RedirectRe */ public function autoOauthCreation(): DataResponse { $result = $this->recreateOauthClientInformation(); - $this->openprojectAPIService->logToAuditFile( - "Nextcloud OAuth credential has been set in application " . Application::APP_ID - ); return new DataResponse($result); } - private function deleteOauthClient(): void { + /** + * @return bool whether an OAuth client was configured + */ + private function deleteOauthClient(): bool { $oauthClientInternalId = $this->config->getAppValue( Application::APP_ID, 'nc_oauth_client_id', '' ); @@ -527,7 +546,9 @@ private function deleteOauthClient(): void { } catch (ClientNotFoundException $e) { } $this->config->deleteAppValue(Application::APP_ID, 'nc_oauth_client_id'); + return true; } + return false; } /** @@ -701,10 +722,17 @@ public function resetIntegration(): DataResponse { * @return array */ private function recreateOauthClientInformation(): array { - $this->deleteOauthClient(); + $replaced = $this->deleteOauthClient(); $opUrl = $this->config->getAppValue(Application::APP_ID, 'openproject_instance_url', ''); $clientInfo = $this->oauthService->createNcOauthClient('OpenProject client', rtrim($opUrl, '/') .'/oauth_clients/%s/callback'); $this->config->setAppValue(Application::APP_ID, 'nc_oauth_client_id', $clientInfo['id']); + $this->openprojectAPIService->logToAuditLog( + Operation::OauthClientCreated, + $replaced + ? 'Nextcloud OAuth client %s for OpenProject has replaced the previous one in application ' . Application::APP_ID + : 'Nextcloud OAuth client %s for OpenProject has been created in application ' . Application::APP_ID, + ['oauthClientId' => (int)$clientInfo['id'], 'replaced' => $replaced], + ); unset($clientInfo['id']); return $clientInfo; } diff --git a/lib/Service/OpenProjectAPIService.php b/lib/Service/OpenProjectAPIService.php index 080ea97cb..a09a5af95 100644 --- a/lib/Service/OpenProjectAPIService.php +++ b/lib/Service/OpenProjectAPIService.php @@ -18,10 +18,10 @@ use OC\Authentication\Events\AppPasswordCreatedEvent; use OC\Authentication\Token\IProvider; use OC\User\NoUserException; -use OCA\AdminAudit\AuditLogger; use OCA\GroupFolders\Folder\FolderManager; use OCA\OIDCIdentityProvider\Exceptions\ClientNotFoundException; use OCA\OpenProject\AppInfo\Application; +use OCA\OpenProject\AuditLog\Operation; use OCA\OpenProject\Exception\OpenprojectAvatarErrorException; use OCA\OpenProject\Exception\OpenprojectErrorException; use OCA\OpenProject\Exception\OpenprojectGroupfolderSetupConflictException; @@ -56,7 +56,7 @@ use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; -use OCP\Log\ILogFactory; +use OCP\Log\Audit\CriticalActionPerformedEvent; use OCP\PreConditionNotMetException; use OCP\Security\ISecureRandom; use OCP\Server; @@ -126,7 +126,6 @@ class OpenProjectAPIService { */ private ISubAdmin $subAdminManager; private IDBConnection $db; - private ILogFactory $logFactory; private IManager $encryptionManager; /** @@ -143,7 +142,6 @@ class OpenProjectAPIService { private IProvider $tokenProvider; private ISecureRandom $random; private IEventDispatcher $eventDispatcher; - private AuditLogger $auditLogger; private TokenEventFactory $tokenEventFactory; public function __construct( @@ -164,7 +162,6 @@ public function __construct( IEventDispatcher $eventDispatcher, ISubAdmin $subAdminManager, IDBConnection $db, - ILogFactory $logFactory, IManager $encryptionManager, TokenEventFactory $tokenEventFactory, IUserSession $userSession, @@ -187,7 +184,6 @@ public function __construct( $this->random = $random; $this->eventDispatcher = $eventDispatcher; $this->db = $db; - $this->logFactory = $logFactory; $this->encryptionManager = $encryptionManager; $this->tokenEventFactory = $tokenEventFactory; $this->userSession = $userSession; @@ -820,6 +816,18 @@ public function linkWorkPackageToFile( } $fileIds [] = $result['_embedded']['elements'][$i]['id']; } + foreach (array_values($fileInfos) as $i => $fileInfo) { + $this->logToAuditLog( + Operation::FileLinkCreated, + 'File %s has been linked to OpenProject work package %s by user %s in application ' . Application::APP_ID, + [ + 'fileId' => $fileInfo['id'], + 'workPackageId' => $values['workpackageId'], + 'userId' => $userId, + 'fileLinkId' => $fileIds[$i], + ], + ); + } return $fileIds; } @@ -890,6 +898,11 @@ public function deleteFileLink(int $fileLinkId, string $userId): array { if (!isset($result['success'])) { throw new OpenprojectResponseException('Malformed response'); } + $this->logToAuditLog( + Operation::FileLinkDeleted, + 'OpenProject file link %s has been deleted by user %s in application ' . Application::APP_ID, + ['fileLinkId' => $fileLinkId, 'userId' => $userId], + ); return $result; } @@ -1076,11 +1089,12 @@ public function getProjectFolderSetupInformation(): array { } /** + * @return int the ID of the created groupfolder * @throws NotPermittedException * @throws NotFoundException * @throws NoUserException */ - public function createGroupfolder(): void { + public function createGroupfolder(): int { $groupfoldersFolderManager = Server::get(FolderManager::class); $folderId = $groupfoldersFolderManager->createFolder( Application::OPEN_PROJECT_ENTITIES_NAME @@ -1100,6 +1114,7 @@ public function createGroupfolder(): void { Application::OPEN_PROJECT_ENTITIES_NAME, true ); + return $folderId; } public function getGroupFolderManager(): FolderManager { @@ -1162,30 +1177,23 @@ public function isGroupfoldersAppSupported(): bool { } /** - * @param $auditLogMessage - * @return void + * Writes an entry to the admin audit log, if the admin_audit app is enabled + * + * The operation is only passed on to servers whose CriticalActionPerformedEvent supports it + * + * @param string $message format string whose %s placeholders are filled by the first parameters, in order + * @param array $parameters */ - public function logToAuditFile($auditLogMessage): void { - if ($this->isAdminAuditConfigSetCorrectly()) { - $this->auditLogger = new AuditLogger($this->logFactory, $this->config); - $this->auditLogger->info($auditLogMessage, - ['app' => 'admin_audit'] - ); + public function logToAuditLog(Operation $operation, string $message, array $parameters = []): void { + $arguments = [$message, $parameters, false]; + if (method_exists(CriticalActionPerformedEvent::class, 'getOperation')) { + $arguments[] = $operation->value; } + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent(...$arguments)); } public function isAdminAuditConfigSetCorrectly(): bool { - $logLevel = $this->config->getSystemValue('loglevel'); - $configAuditFile = $this->config->getSystemValue('logfile_audit'); - $logCondition = $this->config->getSystemValue('log.condition'); - // All the above config should be satisfied in order for admin audit log for the integration application - // if any of the config is failed to be set then we are not able to send the admin audit logging in the audit.log file - return ( - $this->appManager->isInstalled('admin_audit') && - $configAuditFile === $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data') . '/audit.log' && - (isset($logCondition["apps"]) && in_array('admin_audit', $logCondition["apps"])) && - $logLevel >= 1 - ); + return $this->appManager->isInstalled('admin_audit'); } public function isTermsOfServiceAppEnabled(): bool { @@ -1258,8 +1266,10 @@ public function isAllTermsOfServiceSignedForUserOpenProject(?SignatoryMapper $si /** * @throws \OCP\DB\Exception */ - public function signTermsOfServiceForUserOpenProject(): void { - $signatoryMapper = new SignatoryMapper($this->db); + public function signTermsOfServiceForUserOpenProject(?SignatoryMapper $signatoryMapper = null): void { + if ($signatoryMapper === null) { + $signatoryMapper = new SignatoryMapper($this->db); + } // get all the available terms of services $terms = $this->getAllTermsOfServiceAvailable(); $alreadySignedTermsIdForUserOpenProject = $this->getAllTermsOfServiceSignedByUserOpenProject($signatoryMapper); @@ -1267,15 +1277,27 @@ public function signTermsOfServiceForUserOpenProject(): void { $termId = $term->id; // sign only not signed TOS for user "OpenProject" if (!in_array($termId, $alreadySignedTermsIdForUserOpenProject)) { - $signatory = new Signatory(); - $signatory->setUserId(Application::OPEN_PROJECT_ENTITIES_NAME); - $signatory->setTermsId($termId); - $signatory->setTimestamp(time()); - $signatoryMapper->insert($signatory); + $signatoryMapper->insert($this->createSignatoryForUserOpenProject($termId)); + $this->logToAuditLog( + Operation::TermsOfServiceSigned, + "Terms of service %s have been signed for user '%s' in application " . Application::APP_ID, + ['termsId' => $termId, 'userId' => Application::OPEN_PROJECT_ENTITIES_NAME], + ); } } } + /** + * @param int $termId + */ + protected function createSignatoryForUserOpenProject($termId) { + $signatory = new Signatory(); + $signatory->setUserId(Application::OPEN_PROJECT_ENTITIES_NAME); + $signatory->setTermsId($termId); + $signatory->setTimestamp(time()); + return $signatory; + } + public function isUserPartOfAndAdminOfGroup():bool { if ($this->groupManager->isInGroup( Application::OPEN_PROJECT_ENTITIES_NAME, @@ -1330,9 +1352,10 @@ public function getPasswordLength(): int { } /** + * @param bool $replacesExisting whether an existing app password was deleted to create this one * @return string */ - public function generateAppPasswordTokenForUser(): string { + public function generateAppPasswordTokenForUser(bool $replacesExisting = false): string { $user = $this->userManager->get(Application::OPEN_PROJECT_ENTITIES_NAME); $userID = $user->getUID(); $token = $this->random->generate(self::getPasswordLength(), ISecureRandom::CHAR_ALPHANUMERIC.ISecureRandom::CHAR_SYMBOLS); @@ -1347,6 +1370,13 @@ public function generateAppPasswordTokenForUser(): string { $this->eventDispatcher->dispatchTyped( new AppPasswordCreatedEvent($generatedToken) ); + $this->logToAuditLog( + Operation::AppPasswordCreated, + $replacesExisting + ? "Application password for user '%s' has been replaced with new password in application " . Application::APP_ID + : "New application password for user '%s' has been created in application " . Application::APP_ID, + ['userId' => $userID, 'replaced' => $replacesExisting], + ); return $token; } @@ -1361,8 +1391,10 @@ public function deleteAppPassword(): void { foreach ($tokens as $token) { if ($token->getName() === Application::OPEN_PROJECT_ENTITIES_NAME) { $this->tokenProvider->invalidateTokenById(Application::OPEN_PROJECT_ENTITIES_NAME, $token->getId()); - $this->logToAuditFile( - "Application password for user 'OpenProject has been deleted' in application " . Application::APP_ID + $this->logToAuditLog( + Operation::AppPasswordDeleted, + "Application password for user '%s' has been deleted in application " . Application::APP_ID, + ['userId' => Application::OPEN_PROJECT_ENTITIES_NAME], ); } } diff --git a/lib/Service/SettingsService.php b/lib/Service/SettingsService.php index 6ef591bf8..861d26540 100644 --- a/lib/Service/SettingsService.php +++ b/lib/Service/SettingsService.php @@ -9,6 +9,7 @@ use InvalidArgumentException; use OCA\OpenProject\AppInfo\Application; +use OCA\OpenProject\AuditLog\Operation; use OCP\Group\ISubAdmin; use OCP\IGroupManager; use OCP\IUserManager; @@ -257,7 +258,16 @@ public function setupProjectFolder(): void { $allGroup->addUser($user); $this->subAdmin->createSubAdmin($user, $group); $this->subAdmin->createSubAdmin($user, $allGroup); - $this->openprojectAPIService->createGroupfolder(); + $groupfolderId = $this->openprojectAPIService->createGroupfolder(); + $this->openprojectAPIService->logToAuditLog( + Operation::ProjectFolderCreated, + "Project folder %s has been set up for user '%s', who manages the groups %s, in application " . Application::APP_ID, + [ + 'groupfolderId' => $groupfolderId, + 'userId' => Application::OPEN_PROJECT_ENTITIES_NAME, + 'groupIds' => Application::OPEN_PROJECT_ENTITIES_NAME . ', ' . Application::OPENPROJECT_ALL_GROUP_NAME, + ], + ); if ($this->openprojectAPIService->isTermsOfServiceAppEnabled() && $this->userManager->userExists(Application::OPEN_PROJECT_ENTITIES_NAME)) { $this->openprojectAPIService->signTermsOfServiceForUserOpenProject(); } diff --git a/psalm.xml b/psalm.xml index b954657c1..9030a1c5f 100644 --- a/psalm.xml +++ b/psalm.xml @@ -65,7 +65,6 @@ - diff --git a/tests/lib/AuditLog/AuditEvents.php b/tests/lib/AuditLog/AuditEvents.php new file mode 100644 index 000000000..1f8a0beb4 --- /dev/null +++ b/tests/lib/AuditLog/AuditEvents.php @@ -0,0 +1,22 @@ +getOperation() === $operation->value; + } +} diff --git a/tests/lib/AuditLog/OperationTest.php b/tests/lib/AuditLog/OperationTest.php new file mode 100644 index 000000000..25104ae24 --- /dev/null +++ b/tests/lib/AuditLog/OperationTest.php @@ -0,0 +1,25 @@ + [$operation], Operation::cases()); + } + + /** + * @dataProvider dataOperations + */ + public function testOperationFormat(Operation $operation): void { + $this->assertMatchesRegularExpression('/^integration_openproject\.[a-z][a-z0-9_]*\.[a-z][a-z0-9_]*$/', $operation->value); + $this->assertNotSame('', $operation->description()); + } +} diff --git a/tests/lib/Controller/ConfigControllerTest.php b/tests/lib/Controller/ConfigControllerTest.php index dc53c90f1..180d893be 100644 --- a/tests/lib/Controller/ConfigControllerTest.php +++ b/tests/lib/Controller/ConfigControllerTest.php @@ -13,6 +13,7 @@ use OCA\OAuth2\Db\ClientMapper; use OCA\OAuth2\Service\ClientService; use OCA\OpenProject\AppInfo\Application; +use OCA\OpenProject\AuditLog\Operation; use OCA\OpenProject\Service\OauthService; use OCA\OpenProject\Service\OpenProjectAPIService; use OCA\OpenProject\Service\SettingsService; @@ -98,6 +99,7 @@ private function getConfigMock( ['testUser', 'integration_openproject', 'code_verifier', false, $codeVerifier], ['testUser', 'integration_openproject', 'oauth_journey_starting_page', '', $startingPage], ['testUser', 'integration_openproject', 'refresh_token', false, 'oAuthRefreshToken'], + ['testUser', 'integration_openproject', 'user_id', '', '7'], ]); return $configMock; } @@ -237,6 +239,13 @@ public function testOauthRedirectSuccess():void { $apiServiceMock ->method('requestOAuthAccessToken') ->willReturn(['access_token' => 'oAuthAccessToken', 'refresh_token' => 'oAuthRefreshToken']); + $apiServiceMock->expects($this->once()) + ->method('logToAuditLog') + ->with( + Operation::AccountConnected, + 'User %s has connected the OpenProject account %s in application integration_openproject', + ['userId' => 'testUser', 'openprojectUserId' => '7'], + ); $constructArgs = $this->getConfigControllerConstructArgs([ 'config' => $configMock, @@ -321,8 +330,12 @@ public function testOauthRedirectWrongState() { [['testUser', 'integration_openproject', 'oauth_connection_error_message', 'Error during OAuth exchanges', null], null], ]); + $apiServiceMock = $this->createMock(OpenProjectAPIService::class); + $apiServiceMock->expects($this->never())->method('logToAuditLog'); + $constructArgs = $this->getConfigControllerConstructArgs([ 'config' => $configMock, + 'openprojectAPIService' => $apiServiceMock, ]); $configController = new ConfigController(...$constructArgs); @@ -481,6 +494,7 @@ public function testOauthNoAccessTokenInResponse($oauthResponse, $expectedErrorM $apiServiceMock ->method('requestOAuthAccessToken') ->willReturn($oauthResponse); + $apiServiceMock->expects($this->never())->method('logToAuditLog'); $configMock = $this->getConfigMock( str_repeat("A", 128), str_repeat("S", 50) ); @@ -1852,4 +1866,337 @@ public function testUpdateIntegrationSuccess(string $authMethod, int $oauthClien $this->assertArrayHasKey('openproject_redirect_uri', $data); } } + + /** + * @param list}> $auditEntries filled with the audit log entries written through the service + */ + private function getApiServiceMockCollectingAuditEntries(array &$auditEntries): OpenProjectAPIService&MockObject { + $apiService = $this->createMock(OpenProjectAPIService::class); + $apiService->method('logToAuditLog') + ->willReturnCallback(function (Operation $operation, string $message, array $parameters = []) use (&$auditEntries): void { + $auditEntries[] = [$operation, $message, $parameters]; + }); + return $apiService; + } + + /** + * @param array $appValues + */ + private function getConfigMockWithAppValues(array $appValues): IConfig&MockObject { + $configMock = $this->createMock(IConfig::class); + $configMock->method('getAppValue') + ->willReturnCallback(fn (string $app, string $key, string $default = '') => $appValues[$key] ?? $default); + return $configMock; + } + + /** + * @param array $constructParams + */ + private function getConfigControllerForAuditLog(array &$auditEntries, array $appValues, array $constructParams = []): ConfigController { + $constructArgs = $this->getConfigControllerConstructArgs(array_merge([ + 'config' => $this->getConfigMockWithAppValues($appValues), + 'userManager' => $this->createMock(IUserManager::class), + 'openprojectAPIService' => $this->getApiServiceMockCollectingAuditEntries($auditEntries), + ], $constructParams)); + return new ConfigController(...$constructArgs); + } + + public function testSetAdminConfigLogsSavedSettings(): void { + $auditEntries = []; + $controller = $this->getConfigControllerForAuditLog($auditEntries, [ + 'openproject_instance_url' => 'https://old.example.com', + 'authorization_method' => Application::AUTH_METHOD_OAUTH, + 'nc_oauth_client_id' => '12', + ]); + + $result = $controller->setAdminConfig(['openproject_instance_url' => 'https://openproject.example.com']); + + $this->assertSame(Http::STATUS_OK, $result->getStatus()); + $this->assertSame([[ + Operation::IntegrationConfigured, + "OpenProject integration settings %s have been saved for the OpenProject instance '%s' in application integration_openproject", + [ + 'settings' => 'openproject_instance_url', + 'openprojectUrl' => 'https://openproject.example.com', + 'authorizationMethod' => Application::AUTH_METHOD_OAUTH, + 'usersDisconnected' => false, + ], + ]], $auditEntries); + } + + public function testSetAdminConfigLogsOpenProjectOauthCredentialsWithoutSecret(): void { + $auditEntries = []; + $userManagerMock = $this->createMock(IUserManager::class); + $userManagerMock->expects($this->once())->method('callForAllUsers'); + $controller = $this->getConfigControllerForAuditLog($auditEntries, [ + 'openproject_instance_url' => 'https://openproject.example.com', + 'authorization_method' => Application::AUTH_METHOD_OAUTH, + 'openproject_client_id' => 'old-client-id', + 'openproject_client_secret' => 'old-client-secret', + ], ['userManager' => $userManagerMock]); + + $result = $controller->setAdminConfig([ + 'openproject_instance_url' => 'https://openproject.example.com', + 'openproject_client_id' => 'op-client-id', + 'openproject_client_secret' => 'op-client-secret', + ]); + + $this->assertSame(Http::STATUS_OK, $result->getStatus()); + $this->assertSame([[ + Operation::IntegrationConfigured, + "OpenProject integration settings %s have been saved for the OpenProject instance '%s' in application integration_openproject", + [ + 'settings' => 'openproject_instance_url, openproject_client_id, openproject_client_secret', + 'openprojectUrl' => 'https://openproject.example.com', + 'authorizationMethod' => Application::AUTH_METHOD_OAUTH, + 'usersDisconnected' => true, + ], + ]], $auditEntries); + $this->assertStringNotContainsString('op-client-secret', json_encode($auditEntries)); + } + + public function testSetAdminConfigLogsProjectFolderDeactivation(): void { + $auditEntries = []; + $controller = $this->getConfigControllerForAuditLog($auditEntries, [ + 'openproject_instance_url' => 'https://openproject.example.com', + 'authorization_method' => Application::AUTH_METHOD_OAUTH, + ]); + + // only the project folder flag is sent, so no settings are logged as saved + $result = $controller->setAdminConfig(['setup_app_password' => false]); + + $this->assertSame(Http::STATUS_OK, $result->getStatus()); + $this->assertSame([[ + Operation::ProjectFolderDeactivated, + 'Project folder setup has been deactivated in application integration_openproject', + [], + ]], $auditEntries); + } + + public function testSetAdminConfigWithUnknownSettingDoesNotLog(): void { + $auditEntries = []; + $controller = $this->getConfigControllerForAuditLog($auditEntries, []); + + $result = $controller->setAdminConfig(['openproject_instance_url' => 'https://openproject.example.com', 'unknown' => 'value']); + + $this->assertSame(Http::STATUS_BAD_REQUEST, $result->getStatus()); + $this->assertSame([], $auditEntries); + } + + public function testResetIntegrationLogsReset(): void { + $auditEntries = []; + $controller = $this->getConfigControllerForAuditLog($auditEntries, [ + 'openproject_instance_url' => 'https://openproject.example.com', + 'authorization_method' => Application::AUTH_METHOD_OAUTH, + ]); + + $result = $controller->resetIntegration(); + + $this->assertSame(Http::STATUS_OK, $result->getStatus()); + $this->assertSame([[ + Operation::IntegrationReset, + 'OpenProject Integration admin configuration has been reset in application integration_openproject', + ['usersDisconnected' => true], + ]], $auditEntries); + } + + public function testSetUpIntegrationLogsSettingsAndOauthClient(): void { + $auditEntries = []; + $oauthServiceMock = $this->createMock(OauthService::class); + $oauthServiceMock->method('createNcOauthClient')->willReturn([ + 'id' => 34, + 'nextcloud_oauth_client_name' => 'OpenProject client', + 'openproject_redirect_uri' => 'https://openproject.example.com/oauth_clients/client-id/callback', + 'nextcloud_client_id' => 'client-id', + 'nextcloud_client_secret' => 'nc-client-secret', + ]); + $settingsServiceMock = $this->createMock(SettingsService::class); + $controller = $this->getConfigControllerForAuditLog($auditEntries, [], [ + 'oauthService' => $oauthServiceMock, + 'settingsService' => $settingsServiceMock, + ]); + + $result = $controller->setUpIntegration([ + 'openproject_instance_url' => 'https://openproject.example.com', + 'authorization_method' => Application::AUTH_METHOD_OAUTH, + 'openproject_client_id' => 'op-client-id', + 'openproject_client_secret' => 'op-client-secret', + 'default_enable_navigation' => false, + 'default_enable_unified_search' => false, + 'setup_project_folder' => false, + 'setup_app_password' => false, + ]); + + $this->assertSame(Http::STATUS_OK, $result->getStatus()); + $this->assertSame([ + [ + Operation::ProjectFolderDeactivated, + 'Project folder setup has been deactivated in application integration_openproject', + [], + ], + [ + Operation::IntegrationConfigured, + "OpenProject integration settings %s have been saved for the OpenProject instance '%s' in application integration_openproject", + [ + 'settings' => 'openproject_instance_url, authorization_method, openproject_client_id, openproject_client_secret, default_enable_navigation, default_enable_unified_search', + 'openprojectUrl' => 'https://openproject.example.com', + 'authorizationMethod' => Application::AUTH_METHOD_OAUTH, + 'usersDisconnected' => true, + ], + ], + [ + Operation::OauthClientCreated, + 'Nextcloud OAuth client %s for OpenProject has been created in application integration_openproject', + ['oauthClientId' => 34, 'replaced' => false], + ], + ], $auditEntries); + $encodedEntries = json_encode($auditEntries); + $this->assertStringNotContainsString('op-client-secret', $encodedEntries); + $this->assertStringNotContainsString('nc-client-secret', $encodedEntries); + } + + public function testUpdateIntegrationLogsSavedSettings(): void { + $auditEntries = []; + $oauthServiceMock = $this->createMock(OauthService::class); + $oauthServiceMock->method('getClientInfo')->willReturn(['id' => 12]); + $oauthServiceMock->expects($this->never())->method('createNcOauthClient'); + $controller = $this->getConfigControllerForAuditLog($auditEntries, [ + 'openproject_instance_url' => 'https://openproject.example.com', + 'authorization_method' => Application::AUTH_METHOD_OAUTH, + 'nc_oauth_client_id' => '12', + ], [ + 'oauthService' => $oauthServiceMock, + 'settingsService' => $this->createMock(SettingsService::class), + ]); + + $result = $controller->updateIntegration(['default_enable_navigation' => true]); + + $this->assertSame(Http::STATUS_OK, $result->getStatus()); + $this->assertSame([[ + Operation::IntegrationConfigured, + "OpenProject integration settings %s have been saved for the OpenProject instance '%s' in application integration_openproject", + [ + 'settings' => 'default_enable_navigation', + 'openprojectUrl' => 'https://openproject.example.com', + 'authorizationMethod' => Application::AUTH_METHOD_OAUTH, + 'usersDisconnected' => false, + ], + ]], $auditEntries); + } + + public function testUpdateIntegrationWithInvalidSettingsDoesNotLog(): void { + $auditEntries = []; + $settingsServiceMock = $this->createMock(SettingsService::class); + $settingsServiceMock->method('validateAdminSettingsForm')->willThrowException(new \InvalidArgumentException('invalid')); + $controller = $this->getConfigControllerForAuditLog($auditEntries, [], ['settingsService' => $settingsServiceMock]); + + $result = $controller->updateIntegration(['openproject_instance_url' => 'invalid']); + + $this->assertSame(Http::STATUS_BAD_REQUEST, $result->getStatus()); + $this->assertSame([], $auditEntries); + } + + public function testAutoOauthCreationLogsOauthClientCreation(): void { + $auditEntries = []; + $oauthServiceMock = $this->createMock(OauthService::class); + $oauthServiceMock->method('createNcOauthClient')->willReturn([ + 'id' => 34, + 'nextcloud_oauth_client_name' => 'OpenProject client', + 'openproject_redirect_uri' => 'https://openproject.example.com/oauth_clients/client-id/callback', + 'nextcloud_client_id' => 'client-id', + 'nextcloud_client_secret' => 'client-secret', + ]); + $controller = $this->getConfigControllerForAuditLog($auditEntries, [ + 'openproject_instance_url' => 'https://openproject.example.com', + 'nc_oauth_client_id' => '12', + ], ['oauthService' => $oauthServiceMock]); + + $controller->autoOauthCreation(); + + $this->assertSame([[ + Operation::OauthClientCreated, + 'Nextcloud OAuth client %s for OpenProject has replaced the previous one in application integration_openproject', + ['oauthClientId' => 34, 'replaced' => true], + ]], $auditEntries); + $this->assertStringNotContainsString('client-secret', json_encode($auditEntries)); + } + + public function testSetAdminConfigPassesAppPasswordReplacementToService(): void { + $auditEntries = []; + $apiService = $this->getApiServiceMockCollectingAuditEntries($auditEntries); + $apiService->method('hasAppPassword')->willReturn(true); + $apiService->expects($this->once()) + ->method('generateAppPasswordTokenForUser') + ->with(true) + ->willReturn('app-password'); + $userManagerMock = $this->createMock(IUserManager::class); + $userManagerMock->method('userExists')->willReturn(true); + $controller = $this->getConfigControllerForAuditLog($auditEntries, [ + 'openproject_instance_url' => 'https://openproject.example.com', + 'authorization_method' => Application::AUTH_METHOD_OAUTH, + ], ['openprojectAPIService' => $apiService, 'userManager' => $userManagerMock]); + + $result = $controller->setAdminConfig(['setup_app_password' => true]); + + $this->assertSame(Http::STATUS_OK, $result->getStatus()); + } + + public function testSetConfigWithTokenLogsConnectedAccountWithoutToken(): void { + $auditEntries = []; + $apiService = $this->getApiServiceMockCollectingAuditEntries($auditEntries); + $apiService->method('initUserInfo')->with('testUser', 'personal-token')->willReturn(['user_name' => 'Some User']); + $configMock = $this->createMock(IConfig::class); + $configMock->method('getUserValue')->willReturnMap([ + ['testUser', Application::APP_ID, 'user_id', '', '7'], + ]); + $controller = $this->getConfigControllerForAuditLog($auditEntries, [], [ + 'config' => $configMock, + 'openprojectAPIService' => $apiService, + ]); + + $result = $controller->setConfig(['token' => 'personal-token']); + + $this->assertSame(Http::STATUS_OK, $result->getStatus()); + $this->assertSame([[ + Operation::AccountConnected, + 'User %s has connected the OpenProject account %s in application integration_openproject', + ['userId' => 'testUser', 'openprojectUserId' => '7'], + ]], $auditEntries); + $this->assertStringNotContainsString('personal-token', json_encode($auditEntries)); + } + + public function testSetConfigWithInvalidTokenDoesNotLog(): void { + $auditEntries = []; + $apiService = $this->getApiServiceMockCollectingAuditEntries($auditEntries); + $apiService->method('initUserInfo')->willReturn(['error' => 'Unauthorized']); + $controller = $this->getConfigControllerForAuditLog($auditEntries, [], ['openprojectAPIService' => $apiService]); + + $result = $controller->setConfig(['token' => 'invalid-token']); + + $this->assertSame(Http::STATUS_UNAUTHORIZED, $result->getStatus()); + $this->assertSame([], $auditEntries); + } + + public function testSetConfigWithEmptyTokenLogsDisconnectedAccount(): void { + $auditEntries = []; + $controller = $this->getConfigControllerForAuditLog($auditEntries, []); + + $result = $controller->setConfig(['token' => '']); + + $this->assertSame(Http::STATUS_OK, $result->getStatus()); + $this->assertSame([[ + Operation::AccountDisconnected, + 'User %s has disconnected the OpenProject account in application integration_openproject', + ['userId' => 'testUser'], + ]], $auditEntries); + } + + public function testSetConfigWithoutTokenDoesNotLog(): void { + $auditEntries = []; + $controller = $this->getConfigControllerForAuditLog($auditEntries, []); + + $controller->setConfig(['search_enabled' => '1']); + + $this->assertSame([], $auditEntries); + } } diff --git a/tests/lib/Service/OpenProjectAPIServiceTest.php b/tests/lib/Service/OpenProjectAPIServiceTest.php index 7abae88f7..65b53e6ed 100644 --- a/tests/lib/Service/OpenProjectAPIServiceTest.php +++ b/tests/lib/Service/OpenProjectAPIServiceTest.php @@ -20,9 +20,11 @@ use OCA\GroupFolders\Folder\FolderManager; use OCA\OIDCIdentityProvider\Db\Client as OIDCClient; use OCA\OpenProject\AppInfo\Application; +use OCA\OpenProject\AuditLog\Operation; use OCA\OpenProject\Exception\OpenprojectErrorException; use OCA\OpenProject\Exception\OpenprojectResponseException; use OCA\OpenProject\OIDCClientMapper; +use OCA\OpenProject\Tests\AuditLog\AuditEvents; use OCA\OpenProject\TokenEventFactory; use OCA\TermsOfService\Db\Mapper\SignatoryMapper; use OCA\UserOIDC\Event\ExchangedTokenRequestedEvent; @@ -33,6 +35,7 @@ use OCP\App\IAppManager; use OCP\AppFramework\Http; use OCP\Encryption\IManager; +use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Folder; use OCP\Files\IRootFolder; @@ -56,7 +59,7 @@ use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; -use OCP\Log\ILogFactory; +use OCP\Log\Audit\CriticalActionPerformedEvent; use OCP\Security\ISecureRandom; use OCP\UserInterface; use phpmock\phpunit\PHPMock; @@ -690,7 +693,6 @@ private function getOpenProjectAPIServiceConstructArgs(array $constructParams = 'eventDispatcher' => $this->createMock(IEventDispatcher::class), 'subAdmin' => $this->createMock(ISubAdmin::class), 'dbConnection' => $this->createMock(IDBConnection::class), - 'logFactory' => $this->createMock(ILogFactory::class), 'manager' => $this->createMock(IManager::class), 'tokenEventFactory' => $this->createMock(TokenEventFactory::class), 'userSession' => $this->createMock(IUserSession::class), @@ -1922,6 +1924,42 @@ public function testLinkWorkPackageToFileRequest(): void { $this->assertSame([2456], $result); } + public function testLinkWorkPackageToFileLogsEachLinkedFile(): void { + $auditEvents = []; + $service = $this->getOpenProjectAPIServiceMock(['request', 'getNode'], [ + 'eventDispatcher' => $this->getEventDispatcherMockCollectingAuditEvents($auditEvents), + ]); + $service->method('getNode')->willReturn($this->getNodeMock()); + $service->method('request') + ->willReturn(['_type' => 'Collection', '_embedded' => ['elements' => [['id' => 2456], ['id' => 2457], ['id' => 2458]]]]); + + $service->linkWorkPackageToFile($this->multipleFileInformation, 'user'); + + $message = 'File %s has been linked to OpenProject work package %s by user %s in application integration_openproject'; + $this->assertAuditEvents([ + [Operation::FileLinkCreated, $message, ['fileId' => 5503, 'workPackageId' => 123, 'userId' => 'user', 'fileLinkId' => 2456]], + [Operation::FileLinkCreated, $message, ['fileId' => 5504, 'workPackageId' => 123, 'userId' => 'user', 'fileLinkId' => 2457]], + [Operation::FileLinkCreated, $message, ['fileId' => 5505, 'workPackageId' => 123, 'userId' => 'user', 'fileLinkId' => 2458]], + ], $auditEvents); + } + + public function testLinkWorkPackageToFileWithMalformedResponseDoesNotLog(): void { + $auditEvents = []; + $service = $this->getOpenProjectAPIServiceMock(['request', 'getNode'], [ + 'eventDispatcher' => $this->getEventDispatcherMockCollectingAuditEvents($auditEvents), + ]); + $service->method('getNode')->willReturn($this->getNodeMock()); + $service->method('request') + ->willReturn(['_type' => 'Collection', '_embedded' => ['elements' => [['id' => 2456]]]]); + + try { + $service->linkWorkPackageToFile($this->multipleFileInformation, 'user'); + $this->fail('Expected OpenprojectResponseException'); + } catch (OpenprojectResponseException $e) { + } + $this->assertSame([], $auditEvents); + } + /** * @return void */ @@ -2499,6 +2537,91 @@ public function testIsGroupFolderNotSetup( $this->assertFalse($service->isProjectFoldersSetupComplete()); } + /** + * @param list $auditEvents filled with the audit log events dispatched through the returned mock + */ + private function getEventDispatcherMockCollectingAuditEvents(array &$auditEvents): IEventDispatcher&MockObject { + $eventDispatcherMock = $this->createMock(IEventDispatcher::class); + $eventDispatcherMock->method('dispatchTyped') + ->willReturnCallback(function (Event $event) use (&$auditEvents): void { + if ($event instanceof CriticalActionPerformedEvent) { + $auditEvents[] = $event; + } + }); + return $eventDispatcherMock; + } + + /** + * @param list}> $expected + * @param list $auditEvents + */ + private function assertAuditEvents(array $expected, array $auditEvents): void { + $this->assertSame( + array_map(static fn (array $entry): array => [$entry[1], $entry[2]], $expected), + array_map( + static fn (CriticalActionPerformedEvent $event): array => [$event->getLogMessage(), $event->getParameters()], + $auditEvents, + ), + ); + foreach ($auditEvents as $index => $event) { + $this->assertTrue(AuditEvents::hasOperation($event, $expected[$index][0])); + $this->assertFalse($event->getObfuscateParameters()); + } + } + + public function testLogToAuditLog(): void { + $auditEvents = []; + $service = $this->getOpenProjectAPIServiceMock([], [ + 'eventDispatcher' => $this->getEventDispatcherMockCollectingAuditEvents($auditEvents), + ]); + + $service->logToAuditLog(Operation::IntegrationReset, 'Message with %s', ['value' => 'parameter', 'extra' => true]); + + $this->assertAuditEvents([ + [Operation::IntegrationReset, 'Message with %s', ['value' => 'parameter', 'extra' => true]], + ], $auditEvents); + } + + public function generateAppPasswordAuditLogDataProvider(): array { + return [ + 'new app password' => [ + false, + "New application password for user '%s' has been created in application integration_openproject", + ], + 'replaced app password' => [ + true, + "Application password for user '%s' has been replaced with new password in application integration_openproject", + ], + ]; + } + + /** + * @dataProvider generateAppPasswordAuditLogDataProvider + */ + public function testGenerateAppPasswordTokenForUserLogsWithoutPassword(bool $replacesExisting, string $expectedMessage): void { + $userMock = $this->createMock(IUser::class); + $userMock->method('getUID')->willReturn(Application::OPEN_PROJECT_ENTITIES_NAME); + $userManagerMock = $this->createMock(IUserManager::class); + $userManagerMock->method('get')->willReturn($userMock); + $secureRandomMock = $this->createMock(ISecureRandom::class); + $secureRandomMock->method('generate')->willReturn('generated-app-password'); + $tokenProviderMock = $this->createMock(IProvider::class); + $tokenProviderMock->method('generateToken')->willReturn($this->createMock(IToken::class)); + $auditEvents = []; + $service = $this->getOpenProjectAPIServiceMock([], [ + 'userManager' => $userManagerMock, + 'secureRandom' => $secureRandomMock, + 'provider' => $tokenProviderMock, + 'eventDispatcher' => $this->getEventDispatcherMockCollectingAuditEvents($auditEvents), + ]); + + $this->assertSame('generated-app-password', $service->generateAppPasswordTokenForUser($replacesExisting)); + + $this->assertAuditEvents([ + [Operation::AppPasswordCreated, $expectedMessage, ['userId' => Application::OPEN_PROJECT_ENTITIES_NAME, 'replaced' => $replacesExisting]], + ], $auditEvents); + } + public function testGenerateAppPasswordTokenForUser(): void { $userMock = $this->getMockBuilder(IUser::class)->getMock(); $userMock @@ -2812,10 +2935,12 @@ public function testProjectFolderDeleteAppPassword(): void { ->method('getTokenByUser') ->with(Application::OPEN_PROJECT_ENTITIES_NAME) ->willReturn([$tokenMock1,$tokenMock2,$tokenMock3,$tokenMock4,$tokenMock5]); + $auditEvents = []; $service = $this->getOpenProjectAPIServiceMock( ['hasAppPassword'], [ 'provider' => $tokenProviderMock, + 'eventDispatcher' => $this->getEventDispatcherMockCollectingAuditEvents($auditEvents), ], ); $service->method('hasAppPassword')->willReturn(true); @@ -2828,6 +2953,30 @@ public function testProjectFolderDeleteAppPassword(): void { ] ); $service->deleteAppPassword(); + $deletedEntry = [ + Operation::AppPasswordDeleted, + "Application password for user '%s' has been deleted in application integration_openproject", + ['userId' => Application::OPEN_PROJECT_ENTITIES_NAME], + ]; + $this->assertAuditEvents([$deletedEntry, $deletedEntry], $auditEvents); + } + + public function testDeleteAppPasswordWithoutAppPasswordDoesNotLog(): void { + $tokenProviderMock = $this->createMock(IProvider::class); + $tokenProviderMock->expects($this->never())->method('invalidateTokenById'); + $auditEvents = []; + $service = $this->getOpenProjectAPIServiceMock( + ['hasAppPassword'], + [ + 'provider' => $tokenProviderMock, + 'eventDispatcher' => $this->getEventDispatcherMockCollectingAuditEvents($auditEvents), + ], + ); + $service->method('hasAppPassword')->willReturn(false); + + $service->deleteAppPassword(); + + $this->assertSame([], $auditEvents); } /** @@ -3747,13 +3896,36 @@ public function testDeleteFileLinkResponse(): void { * @return void */ public function testDeleteFileLinkErrorResponse(): void { - $service = $this->getOpenProjectAPIServiceMock(); + $auditEvents = []; + $service = $this->getOpenProjectAPIServiceMock(['request'], [ + 'eventDispatcher' => $this->getEventDispatcherMockCollectingAuditEvents($auditEvents), + ]); $service->method('request') ->willReturn([ 'error' => 'something went wrong', ]); - $this->expectException(OpenprojectErrorException::class); + try { + $service->deleteFileLink(7, 'user'); + $this->fail('Expected OpenprojectErrorException'); + } catch (OpenprojectErrorException $e) { + } + $this->assertSame([], $auditEvents); + } + + public function testDeleteFileLinkLogsDeletion(): void { + $auditEvents = []; + $service = $this->getOpenProjectAPIServiceMock(['request'], [ + 'eventDispatcher' => $this->getEventDispatcherMockCollectingAuditEvents($auditEvents), + ]); + $service->method('request')->willReturn(['success' => true]); + $service->deleteFileLink(7, 'user'); + + $this->assertAuditEvents([[ + Operation::FileLinkDeleted, + 'OpenProject file link %s has been deleted by user %s in application integration_openproject', + ['fileLinkId' => 7, 'userId' => 'user'], + ]], $auditEvents); } /** @@ -4273,6 +4445,44 @@ public function testIsAllTermsOfServiceSignedForUserOpenProject(array $available $this->assertSame($expectedResult, $result); } + public function signTermsOfServiceDataProvider(): array { + return [ + 'one term not signed yet' => [[1], [2]], + 'no term signed yet' => [[], [1, 2]], + 'all terms already signed' => [[1, 2], []], + ]; + } + + /** + * @dataProvider signTermsOfServiceDataProvider + * @param list $alreadySignedTermIds + * @param list $expectedSignedTermIds + */ + public function testSignTermsOfServiceForUserOpenProjectLogsSignedTerms(array $alreadySignedTermIds, array $expectedSignedTermIds): void { + $signatoryMapperBuilder = $this->getMockBuilder(SignatoryMapper::class)->disableOriginalConstructor(); + // the terms_of_service app is optional, so its mapper might not exist + $signatoryMapperMock = method_exists(SignatoryMapper::class, 'insert') + ? $signatoryMapperBuilder->onlyMethods(['insert'])->getMock() + : $signatoryMapperBuilder->addMethods(['insert'])->getMock(); + $signatoryMapperMock->expects($this->exactly(count($expectedSignedTermIds)))->method('insert'); + $auditEvents = []; + $service = $this->getOpenProjectAPIServiceMock( + ['getAllTermsOfServiceAvailable', 'getAllTermsOfServiceSignedByUserOpenProject', 'createSignatoryForUserOpenProject'], + ['eventDispatcher' => $this->getEventDispatcherMockCollectingAuditEvents($auditEvents)], + ); + $service->method('getAllTermsOfServiceAvailable')->willReturn([(object)['id' => 1], (object)['id' => 2]]); + $service->method('getAllTermsOfServiceSignedByUserOpenProject')->willReturn($alreadySignedTermIds); + $service->method('createSignatoryForUserOpenProject')->willReturn(new \stdClass()); + + $service->signTermsOfServiceForUserOpenProject($signatoryMapperMock); + + $this->assertAuditEvents(array_map(static fn (int $termId): array => [ + Operation::TermsOfServiceSigned, + "Terms of service %s have been signed for user '%s' in application integration_openproject", + ['termsId' => $termId, 'userId' => Application::OPEN_PROJECT_ENTITIES_NAME], + ], $expectedSignedTermIds), $auditEvents); + } + public function testGetSubline(): void { $configMock = $this->getMockBuilder(IConfig::class)->getMock(); $configMock->method('getUserValue') @@ -4389,99 +4599,25 @@ public function testGetWorkPackageInfoForNoUserAccessToken(): void { public function auditLogDataProvider(): array { return [ - [ - // log level less than 1 - 0, - \OC::$SERVERROOT . '/data/audit.log', - [], - true, - false - ], - [ - // wrong path to audit file - 1, - 'wrong-path-to-audit-log/audit.log', - [], - true, - false - ], - [ - // no audit apps in apps section - 1, - '/audit.log', - ['apps' => ['no_audit_app']], - true, - false - ], - [ - // multiple values including apps with no audit apps in apps section - 1, - '/audit.log', - ['apps' => ['no_audit_app'], 'others' => []], - true, - false - ], - [ - // all values configured correctly - 1, - '/audit.log', - ['apps' => ['admin_audit']], - true, - true - ], - [ - // admin_audit app not installed - 1, - '/audit.log', - ['apps' => ['admin_audit']], - false, - false - ], + 'admin_audit app enabled' => [true], + 'admin_audit app not enabled' => [false], ]; } /** * @dataProvider auditLogDataProvider - * @param int $logLevel - * @param string $pathToAuditLog - * @param array $logCondition - * @param bool $isAdminAuditAppInstalled - * @param bool $expectedResult - * - * @return void */ - - public function testIsAdminAuditConfigSetCorrectly( - int $logLevel, - string $pathToAuditLog, - array $logCondition, - bool $isAdminAuditAppInstalled, - bool $expectedResult - ): void { - $configMock = $this->getMockBuilder(IConfig::class)->getMock(); + public function testIsAdminAuditConfigSetCorrectly(bool $isAdminAuditAppInstalled): void { $iAppManagerMock = $this->getMockBuilder(IAppManager::class)->getMock(); - $configMock - ->method('getSystemValue') - ->willReturnMap([ - ['loglevel', '', $logLevel], - ['logfile_audit', '', $pathToAuditLog], - ['log.condition', '', $logCondition] - ] - ); - $userManagerMock = $this->getMockBuilder(IUserManager::class) - ->getMock(); $iAppManagerMock->method('isInstalled')->with('admin_audit') ->willReturn($isAdminAuditAppInstalled); $service = $this->getOpenProjectAPIServiceMock( [], [ - 'userManager' => $userManagerMock, 'appManager' => $iAppManagerMock, - 'config' => $configMock, ], ); - $actualResult = $service->isAdminAuditConfigSetCorrectly(); - $this->assertEquals($expectedResult, $actualResult); + $this->assertSame($isAdminAuditAppInstalled, $service->isAdminAuditConfigSetCorrectly()); } public function serverSideEncryptionEnabledDataProvider(): array { diff --git a/tests/lib/Service/SettingsServiceTest.php b/tests/lib/Service/SettingsServiceTest.php index 19f4b1b15..f5666ff18 100644 --- a/tests/lib/Service/SettingsServiceTest.php +++ b/tests/lib/Service/SettingsServiceTest.php @@ -9,6 +9,7 @@ use InvalidArgumentException; use OCA\OpenProject\AppInfo\Application; +use OCA\OpenProject\AuditLog\Operation; use OCP\Group\ISubAdmin; use OCP\IGroup; use OCP\IGroupManager; @@ -437,7 +438,20 @@ public function testSetupProjectFolder(bool $systemReady, bool $tosEnabled): voi $opApiService ->expects($this->once()) - ->method('createGroupfolder'); + ->method('createGroupfolder') + ->willReturn(7); + $opApiService + ->expects($this->once()) + ->method('logToAuditLog') + ->with( + Operation::ProjectFolderCreated, + "Project folder %s has been set up for user '%s', who manages the groups %s, in application integration_openproject", + [ + 'groupfolderId' => 7, + 'userId' => Application::OPEN_PROJECT_ENTITIES_NAME, + 'groupIds' => Application::OPEN_PROJECT_ENTITIES_NAME . ', ' . Application::OPENPROJECT_ALL_GROUP_NAME, + ], + ); $opApiService ->expects($this->once()) ->method('isTermsOfServiceAppEnabled') @@ -458,6 +472,9 @@ public function testSetupProjectFolder(bool $systemReady, bool $tosEnabled): voi $opApiService ->expects($this->never()) ->method('createGroupfolder'); + $opApiService + ->expects($this->never()) + ->method('logToAuditLog'); $opApiService ->expects($this->never()) ->method('isTermsOfServiceAppEnabled');