From 2fb564d438515daf8465048453f59c58731f7fcf Mon Sep 17 00:00:00 2001 From: Ushindi Gedeon Date: Mon, 20 Jul 2026 02:50:31 +0200 Subject: [PATCH] FIX accounting export: enhance file metadata return and secure access checks (#39218) Co-authored-by: gedeonts --- .../class/api_accountancy.class.php | 21 +++++++++- htdocs/core/lib/files.lib.php | 2 +- test/phpunit/FilesLibTest.php | 39 +++++++++++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/htdocs/accountancy/class/api_accountancy.class.php b/htdocs/accountancy/class/api_accountancy.class.php index 0b201108f5b..17093546ec9 100644 --- a/htdocs/accountancy/class/api_accountancy.class.php +++ b/htdocs/accountancy/class/api_accountancy.class.php @@ -78,7 +78,7 @@ class Accountancy extends DolibarrApi * @param int $alreadyexport [=0] by default export data only if it's not yet exported or 1 already exported (always export data even if 'date_export" is set) * @param int $notnotifiedasexport [=0] by default notified as exported or 1 not notified as exported (when the export is done, notified or not the column 'date_export') * - * @return string + * @return array{modulepart:string,relative_path:string,filename:string,mimetype:string} Generated file metadata * * @url GET exportdata * @@ -271,7 +271,24 @@ class Accountancy extends DolibarrApi throw new RestException(500, 'Error accountancy export : '.implode(',', $accountancyexport->errors)); } else { $this->db->commit(); - exit(); + + $filedata = $accountancyexport->generatedfiledata; + if (empty($filedata['downloadFilePath']) || empty($filedata['downloadFileFullName'])) { + throw new RestException(500, 'Accounting export generated no downloadable file'); + } + + $outputdir = !empty($conf->accounting->multidir_output[$conf->entity]) ? $conf->accounting->multidir_output[$conf->entity] : $conf->accounting->dir_output; + $outputdir = rtrim($outputdir, '/').'/'; + if (strpos($filedata['downloadFilePath'], $outputdir) !== 0) { + throw new RestException(500, 'Accounting export generated a file outside the accounting output directory'); + } + + return array( + 'modulepart' => 'export_compta', + 'relative_path' => substr($filedata['downloadFilePath'], strlen($outputdir)), + 'filename' => basename($filedata['downloadFileFullName']), + 'mimetype' => $filedata['downloadFileMimeType'], + ); } } } diff --git a/htdocs/core/lib/files.lib.php b/htdocs/core/lib/files.lib.php index 5e5e434209b..de458fa87f1 100644 --- a/htdocs/core/lib/files.lib.php +++ b/htdocs/core/lib/files.lib.php @@ -3570,7 +3570,7 @@ function dol_check_secure_access_document($modulepart, $original_file, $entity, } } elseif ($modulepart == 'export_compta' && !empty($conf->accounting->dir_output)) { // Wrapping for accounting exports - if ($fuser->hasRight('accounting', 'bind', 'write') || preg_match('/^specimen/i', $original_file)) { + if ($fuser->hasRight('accounting', 'bind', 'write') || $fuser->hasRight('accounting', 'mouvements', 'export') || preg_match('/^specimen/i', $original_file)) { $accessallowed = 1; } $original_file = $conf->accounting->dir_output.'/'.$original_file; diff --git a/test/phpunit/FilesLibTest.php b/test/phpunit/FilesLibTest.php index 706f9320a36..964257558b2 100644 --- a/test/phpunit/FilesLibTest.php +++ b/test/phpunit/FilesLibTest.php @@ -488,6 +488,45 @@ class FilesLibTest extends CommonClassTest $user->rights->facture->creer = $savpermcreer; } + /** + * Check that a user allowed to export the ledger can download the generated accounting export. + * + * @return void + */ + public function testDolCheckSecureAccessAccountingExport() + { + global $conf, $user, $langs, $db; + $conf = $this->savconf; + $user = $this->savuser; + $langs = $this->savlangs; + $db = $this->savdb; + + $savpermbindwrite = $user->hasRight('accounting', 'bind', 'write'); + $savpermexport = $user->hasRight('accounting', 'mouvements', 'export'); + + if (empty($user->rights->accounting)) { + $user->rights->accounting = new stdClass(); + } + if (empty($user->rights->accounting->bind)) { + $user->rights->accounting->bind = new stdClass(); + } + if (empty($user->rights->accounting->mouvements)) { + $user->rights->accounting->mouvements = new stdClass(); + } + + $user->rights->accounting->bind->write = 0; + $user->rights->accounting->mouvements->export = 1; + $result = dol_check_secure_access_document('export_compta', 'export/1/general_ledger.csv', 0, $user, '', 'read'); + $this->assertEquals(1, $result['accessallowed']); + + $user->rights->accounting->mouvements->export = 0; + $result = dol_check_secure_access_document('export_compta', 'export/1/general_ledger.csv', 0, $user, '', 'read'); + $this->assertEquals(0, $result['accessallowed']); + + $user->rights->accounting->bind->write = $savpermbindwrite; + $user->rights->accounting->mouvements->export = $savpermexport; + } + /** * testDolDirMove *