Skip to content

Commit edb79f2

Browse files
authored
Fix/remove b csv admin only setting (#7926)
## Summary Remove deprecated CSV export permission flags (`bCSVAdminOnly` and `bExportCSV`) that provided no real security value since users could already export data via DataTables buttons on every table. ## Why These settings created a false sense of security: - `bCSVAdminOnly` - Restricted CSV option in financial report forms, but users could still export via DataTables - `bExportCSV` - User permission that only hid some CSV buttons, while DataTables export remained available Both settings added UI complexity without actual data protection. Removing them simplifies the codebase and user management. ## Changes ### System Configuration - Removed `bCSVAdminOnly` from `SystemConfig.php` - Removed conditional logic from all financial report forms ### User Permissions - Removed `isCSVExport()` and `isCSVExportEnabled()` methods from `User.php` - Removed permission check from `CSVExport.php` - Updated `Header.php` to always show DataTables export buttons - Updated cart functions to always show CSV export button - Removed `csvExport` from admin user API response - Removed `CSVExport` from access-denied page role descriptions ### Database - Created `src/mysql/upgrade/6.8.0.sql` to delete both settings from existing installations ## Files Changed | File | Change | |------|--------| | `src/ChurchCRM/dto/SystemConfig.php` | Remove bCSVAdminOnly config | | `src/ChurchCRM/model/ChurchCRM/User.php` | Remove CSV export methods | | `src/CSVExport.php` | Remove permission check | | `src/Include/Header.php` | Always show DataTables buttons | | `src/FinancialReports.php` | Simplify CSV option logic | | `src/ReminderReport.php` | Remove bCSVAdminOnly check | | `src/TaxReport.php` | Remove bCSVAdminOnly check | | `src/Reports/*.php` (6 files) | Remove bCSVAdminOnly checks | | `src/finance/views/reports.php` | Remove warning message | | `src/admin/routes/api/user-admin.php` | Remove csvExport from response | | `src/v2/templates/cart/cartfunctions.php` | Always show CSV button | | `src/v2/templates/common/access-denied.php` | Remove CSVExport role | | `src/mysql/install/Install.sql` | Remove default setting | | `src/mysql/upgrade/6.8.0.sql` | New upgrade script | ## Testing - ✅ CSV reports test (6 passing) - ✅ Finance reports test (10 passing) - ✅ Cart tests (4 passing) - ✅ Admin reports test (1 passing) - ✅ Admin user test (3 passing) - ✅ No PHP errors in logs
2 parents 4b73796 + 04e3b22 commit edb79f2

File tree

21 files changed

+30
-102
lines changed

21 files changed

+30
-102
lines changed

src/CSVExport.php

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,7 @@
33
require_once __DIR__ . '/Include/Config.php';
44
require_once __DIR__ . '/Include/Functions.php';
55

6-
use ChurchCRM\Authentication\AuthenticationManager;
76
use ChurchCRM\dto\SystemURLs;
8-
use ChurchCRM\Utils\RedirectUtils;
9-
10-
// If user does not have CSV Export permission, redirect to the menu.
11-
if (!AuthenticationManager::getCurrentUser()->isCSVExport()) {
12-
RedirectUtils::securityRedirect("CSVExport");
13-
}
147

158
// Get Classifications for the drop-down
169
$sSQL = 'SELECT * FROM list_lst WHERE lst_ID = 1 ORDER BY lst_OptionSequence';

src/ChurchCRM/dto/SystemConfig.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,6 @@ private static function buildConfigs(): array
101101
'sDirRoleChild' => new ConfigItem(8, 'sDirRoleChild', 'choice', '3', gettext('These are the family role numbers designated as child'), '', json_encode(SystemConfig::getFamilyRoleChoices(), JSON_THROW_ON_ERROR)),
102102
'iSessionTimeout' => new ConfigItem(9, 'iSessionTimeout', 'number', '3600', gettext("Session timeout length in seconds\nSet to zero to disable session timeouts.")),
103103
'aFinanceQueries' => new ConfigItem(10, 'aFinanceQueries', 'text', '28,30', gettext('Queries for which user must have finance permissions to use') . ':'),
104-
'bCSVAdminOnly' => new ConfigItem(11, 'bCSVAdminOnly', 'boolean', '1', gettext('Should only administrators have access to the CSV export system and directory report?')),
105104
'iMinPasswordLength' => new ConfigItem(13, 'iMinPasswordLength', 'number', '6', gettext('Minimum length a user may set their password to')),
106105
'iMinPasswordChange' => new ConfigItem(14, 'iMinPasswordChange', 'number', '4', gettext("Minimum amount that a new password must differ from the old one (# of characters changed)\nSet to zero to disable this feature")),
107106
'aDisallowedPasswords' => new ConfigItem(15, 'aDisallowedPasswords', 'text', 'password,god,jesus,church,christian', gettext('A comma-separated list of disallowed (too obvious) passwords.')),
@@ -274,7 +273,7 @@ private static function buildCategories(): array
274273
gettext('Church Services') => ['iPersonConfessionFatherCustomField', 'iPersonConfessionDateCustomField'],
275274
gettext('Backup') => ['sLastBackupTimeStamp', 'bEnableExternalBackupTarget', 'sExternalBackupType', 'sExternalBackupAutoInterval', 'sExternalBackupEndpoint', 'sExternalBackupUsername', 'sExternalBackupPassword'],
276275
gettext('Two-Factor Authentication') => ['bEnable2FA', 'bRequire2FA', 's2FAApplicationName', 'sTwoFASecretKey'],
277-
gettext('System Settings') => ['sLogLevel', 'bCSVAdminOnly', 'bEnforceCSP', 'bHSTSEnable', 'iDashboardServiceIntervalTime', 'bAllowPrereleaseUpgrade'],
276+
gettext('System Settings') => ['sLogLevel', 'bEnforceCSP', 'bHSTSEnable', 'iDashboardServiceIntervalTime', 'bAllowPrereleaseUpgrade'],
278277
];
279278
}
280279

src/ChurchCRM/model/ChurchCRM/User.php

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -190,16 +190,6 @@ public function isAddEvent(): bool
190190
return $this->isAdmin() || $this->isEnabledSecurity('bAddEvent');
191191
}
192192

193-
public function isCSVExportEnabled(): bool
194-
{
195-
return $this->isCSVExport();
196-
}
197-
198-
public function isCSVExport(): bool
199-
{
200-
return $this->isAdmin() || $this->isEnabledSecurity('bExportCSV');
201-
}
202-
203193
public function isEmailEnabled(): bool
204194
{
205195
return $this->isAdmin() || $this->isEnabledSecurity('bEmailMailto');

src/FinancialReports.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -326,10 +326,8 @@
326326
echo '<td class=TextColumnWithBottomBorder><input name=RequireDonationYears type=text value=0 size=5></td></tr>';
327327
}
328328

329-
if (
330-
((AuthenticationManager::getCurrentUser()->isAdmin() && $bCSVAdminOnly) || !$bCSVAdminOnly)
331-
&& (in_array($sReportType, ['Pledge Summary', 'Giving Report', 'Individual Deposit Report', 'Advanced Deposit Report', 'Zero Givers']))
332-
) {
329+
// Show CSV output option for reports that support it
330+
if (in_array($sReportType, ['Pledge Summary', 'Giving Report', 'Individual Deposit Report', 'Advanced Deposit Report', 'Zero Givers'])) {
333331
echo '<tr><td class=LabelColumn>' . gettext('Output Method:') . '</td>';
334332
echo "<td class=TextColumnWithBottomBorder><input name=output type=radio checked value='pdf'>PDF";
335333
echo " <input name=output type=radio value='csv'>" . gettext('CSV') . '</tr>';

src/ReminderReport.php

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,11 @@
44
require_once __DIR__ . '/Include/Functions.php';
55

66
use ChurchCRM\Authentication\AuthenticationManager;
7-
use ChurchCRM\dto\SystemConfig;
87
use ChurchCRM\Utils\InputUtils;
98
use ChurchCRM\Utils\RedirectUtils;
109

11-
// If CSVAdminOnly option is enabled and user is not admin, redirect to the menu.
12-
if (!AuthenticationManager::getCurrentUser()->isAdmin() && SystemConfig::getValue('bCSVAdminOnly')) {
13-
RedirectUtils::securityRedirect('Admin');
14-
}
10+
// Security
11+
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isFinanceEnabled(), 'Finance');
1512

1613
$sPageTitle = gettext('Pledge Reminder Report');
1714
require_once __DIR__ . '/Include/Header.php';

src/Reports/AdvancedDeposit.php

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,9 @@
77

88
use ChurchCRM\Authentication\AuthenticationManager;
99
use ChurchCRM\dto\SystemConfig;
10+
use ChurchCRM\Service\FinancialService;
1011
use ChurchCRM\Utils\CsvExporter;
1112
use ChurchCRM\Utils\InputUtils;
12-
use ChurchCRM\Utils\RedirectUtils;
13-
use ChurchCRM\Service\FinancialService;
1413

1514
// Security
1615
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isFinanceEnabled(), 'Finance');
@@ -62,11 +61,6 @@
6261
$output = 'pdf';
6362
}
6463

65-
// If CSVAdminOnly option is enabled and user is not admin, redirect to the menu.
66-
if (!AuthenticationManager::getCurrentUser()->isAdmin() && SystemConfig::getValue('bCSVAdminOnly') && $output != 'pdf') {
67-
RedirectUtils::securityRedirect('Admin');
68-
}
69-
7064
// Normalize date range
7165
$today = date('Y-m-d');
7266
if (!$sDateEnd && $sDateStart) {

src/Reports/EnvelopeReport.php

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,9 @@
99
use ChurchCRM\dto\SystemConfig;
1010
use ChurchCRM\model\ChurchCRM\Family;
1111
use ChurchCRM\model\ChurchCRM\FamilyQuery;
12-
use ChurchCRM\Utils\RedirectUtils;
1312

14-
// If CSVAdminOnly option is enabled and user is not admin, redirect to the menu
15-
if (!AuthenticationManager::getCurrentUser()->isAdmin() && SystemConfig::getValue('bCSVAdminOnly')) {
16-
RedirectUtils::securityRedirect('Admin');
17-
}
13+
// Security
14+
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isFinanceEnabled(), 'Finance');
1815

1916
class PdfEnvelopeReport extends ChurchInfoReport
2017
{

src/Reports/FamilyPledgeSummary.php

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
use ChurchCRM\dto\SystemConfig;
1010
use ChurchCRM\Utils\FiscalYearUtils;
1111
use ChurchCRM\Utils\InputUtils;
12-
use ChurchCRM\Utils\RedirectUtils;
1312

1413
// Security
1514
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isFinanceEnabled(), 'Finance');
@@ -62,11 +61,6 @@
6261
$only_owe = InputUtils::legacyFilterInput($_POST['only_owe']);
6362
}
6463

65-
// If CSVAdminOnly option is enabled and user is not admin, redirect to the menu.
66-
if (!AuthenticationManager::getCurrentUser()->isAdmin() && SystemConfig::getValue('bCSVAdminOnly')) {
67-
RedirectUtils::securityRedirect('Admin');
68-
}
69-
7064
// Get all the families
7165
$sSQL = 'SELECT DISTINCT fam_ID, fam_Name FROM family_fam';
7266

src/Reports/PledgeSummary.php

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
use ChurchCRM\Utils\CsvExporter;
1111
use ChurchCRM\Utils\FiscalYearUtils;
1212
use ChurchCRM\Utils\InputUtils;
13-
use ChurchCRM\Utils\RedirectUtils;
1413

1514
// Security
1615
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isFinanceEnabled(), 'Finance');
@@ -24,11 +23,6 @@
2423
// Remember the chosen Fiscal Year ID
2524
$_SESSION['idefaultFY'] = $iFYID;
2625

27-
// If CSVAdminOnly option is enabled and user is not admin, redirect to the menu.
28-
if (!AuthenticationManager::getCurrentUser()->isAdmin() && SystemConfig::getValue('bCSVAdminOnly') && $output !== 'pdf') {
29-
RedirectUtils::securityRedirect('Admin');
30-
}
31-
3226
// Get the list of funds
3327
$sSQL = "SELECT fun_ID,fun_Name,fun_Description,fun_Active FROM donationfund_fun WHERE fun_Active = 'true' ORDER BY fun_Active, fun_Name";
3428
$rsFunds = RunQuery($sSQL);

src/Reports/PrintDeposit.php

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
use ChurchCRM\model\ChurchCRM\PledgeQuery;
88

99
use ChurchCRM\Authentication\AuthenticationManager;
10-
use ChurchCRM\dto\SystemConfig;
1110
use ChurchCRM\dto\SystemURLs;
1211
use ChurchCRM\Utils\InputUtils;
1312
use ChurchCRM\Utils\RedirectUtils;
@@ -42,11 +41,6 @@
4241
RedirectUtils::redirect('v2/dashboard');
4342
}
4443

45-
// If CSVAdminOnly option is enabled and user is not admin, redirect to access denied.
46-
if (!AuthenticationManager::getCurrentUser()->isAdmin() && SystemConfig::getValue('bCSVAdminOnly') && $output != 'pdf') {
47-
RedirectUtils::securityRedirect('Admin');
48-
}
49-
5044
if ($output === 'pdf') {
5145
// Server-side guard: if this deposit has no payments, show a friendly message instead of redirecting
5246
$paymentsCount = PledgeQuery::create()->filterByDepId($iDepositSlipID)->count();

0 commit comments

Comments
 (0)