Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
ca21138
Add access-denied page for security redirects
DawoudIO Nov 29, 2025
7c3d8a6
Add role parameter to redirectHomeIfFalse for security feedback
DawoudIO Nov 29, 2025
ed5c7fe
Update securityRedirect to use access-denied page
DawoudIO Nov 29, 2025
f25fe2f
RedirectUtils::securityRedirect
DawoudIO Nov 29, 2025
b28586e
Merge branch 'master' into feature-securityRedirect
DawoudIO Nov 29, 2025
ca41095
Update role middleware to redirect browsers to access-denied page
DawoudIO Nov 29, 2025
7e69902
Merge branch 'feature-securityRedirect' of https://github.com/ChurchC…
DawoudIO Nov 29, 2025
8e69bf9
Initial plan
Copilot Nov 30, 2025
e95dc5d
Initial plan
Copilot Nov 30, 2025
188b21e
Initial plan
Copilot Nov 30, 2025
fe59db0
Initial plan
Copilot Nov 30, 2025
698908f
Merge branch 'master' into feature-securityRedirect
DawoudIO Nov 30, 2025
4edb49a
Fix undefined variable $bCSVAdminOnly in PrintDeposit.php
Copilot Nov 30, 2025
953a523
[WIP] WIP addressing review feedback for security redirect page featu…
DawoudIO Nov 30, 2025
aec3fb9
Fix incorrect CSVAdminOnly logic in TaxReport.php
Copilot Nov 30, 2025
cb7c8d7
Add role parameter validation to prevent XSS/injection attacks
Copilot Nov 30, 2025
bd0ad5d
Add Cypress UI tests for access-denied page
Copilot Nov 30, 2025
2e42a59
Fix incorrect CSVAdminOnly logic in TaxReport.php (#7679)
DawoudIO Nov 30, 2025
00182ef
Add Cypress UI tests for access-denied security redirect page (#7677)
DawoudIO Nov 30, 2025
51b2e30
Add role parameter validation to prevent XSS/injection attacks (#7678)
DawoudIO Nov 30, 2025
b476de3
Merge branch 'master' into feature-securityRedirect
DawoudIO Nov 30, 2025
12d6d20
fixed unknown role
DawoudIO Nov 30, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/CartToEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
use ChurchCRM\Utils\RedirectUtils;

// Security: User must have Manage Groups & Roles permission
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isManageGroupsEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isManageGroupsEnabled(), 'ManageGroups');

// Was the form submitted?
if (isset($_POST['Submit']) && count($_SESSION['aPeopleCart']) > 0 && isset($_POST['EventID'])) {
Expand Down
2 changes: 1 addition & 1 deletion src/CartToFamily.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use ChurchCRM\Utils\RedirectUtils;

// Security: User must have add records permission
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isAddRecordsEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isAddRecordsEnabled(), 'AddRecords');

// Was the form submitted?
if (isset($_POST['Submit']) && count($_SESSION['aPeopleCart']) > 0) {
Expand Down
2 changes: 1 addition & 1 deletion src/CartToGroup.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use ChurchCRM\Utils\RedirectUtils;

// Security: User must have Manage Groups & Roles permission
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isManageGroupsEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isManageGroupsEnabled(), 'ManageGroups');

// Was the form submitted?
if ((isset($_GET['groupeCreationID']) || isset($_POST['Submit'])) && count($_SESSION['aPeopleCart']) > 0) {
Expand Down
8 changes: 6 additions & 2 deletions src/ChurchCRM/Authentication/AuthenticationManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -247,10 +247,14 @@ public static function getForgotPasswordURL(): string
// but rather redirect users to some other password reset mechanism.
return SystemURLs::getRootPath() . '/session/forgot-password/reset-request';
}
public static function redirectHomeIfFalse(bool $hasAccess): void
public static function redirectHomeIfFalse(bool $hasAccess, string $missingRole = ''): void
{
if (!$hasAccess) {
RedirectUtils::redirect('v2/dashboard');
if ($missingRole !== '') {
RedirectUtils::securityRedirect($missingRole);
} else {
RedirectUtils::redirect('v2/dashboard');
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ protected function hasRole(): bool

protected function noRoleMessage(): string
{
return gettext('User must have bAddEvent permissions');
return gettext('User must have Add Event permission');
}

protected function getRoleName(): string
{
return 'AddEvent';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

class AdminRoleAuthMiddleware extends BaseAuthRoleMiddleware
{
protected function hasRole()
protected function hasRole(): bool
{
return $this->user->isAdmin();
}
Expand All @@ -13,4 +13,9 @@ protected function noRoleMessage(): string
{
return gettext('User must be an Admin');
}

protected function getRoleName(): string
{
return 'Admin';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace ChurchCRM\Slim\Middleware\Request\Auth;

use ChurchCRM\Authentication\AuthenticationManager;
use ChurchCRM\dto\SystemURLs;
use ChurchCRM\model\ChurchCRM\User;
use ChurchCRM\Utils\LoggerUtils;
use Laminas\Diactoros\Response;
Expand All @@ -28,6 +29,12 @@ public function process(Request $request, RequestHandlerInterface $handler): Res
'method' => $request->getMethod(),
'exception' => $ex->getMessage()
]);

// Check if this is a browser request (not API)
if ($this->isBrowserRequest($request)) {
return $this->redirectToAccessDenied('Authentication');
}

$response = new Response();
$errorBody = json_encode(['error' => gettext('No logged in user'), 'code' => 401]);
$response->getBody()->write($errorBody);
Expand All @@ -42,6 +49,12 @@ public function process(Request $request, RequestHandlerInterface $handler): Res
'user' => $this->user->getUserName(),
'required_role' => $this->noRoleMessage()
]);

// Check if this is a browser request (not API)
if ($this->isBrowserRequest($request)) {
return $this->redirectToAccessDenied($this->getRoleName());
}

$response = new Response();
$errorBody = json_encode(['error' => $this->noRoleMessage(), 'code' => 403]);
$response->getBody()->write($errorBody);
Expand All @@ -51,7 +64,59 @@ public function process(Request $request, RequestHandlerInterface $handler): Res
return $handler->handle($request);
}

/**
* Check if request is from a browser (expects HTML) vs API client (expects JSON)
*/
protected function isBrowserRequest(Request $request): bool
{
$path = $request->getUri()->getPath();

// API routes should always return JSON
if (str_contains($path, '/api/')) {
return false;
}

// Check Accept header - browsers typically send text/html
$acceptHeader = $request->getHeaderLine('Accept');
if (!empty($acceptHeader)) {
// If client explicitly wants JSON, it's an API request
if (str_contains($acceptHeader, 'application/json') && !str_contains($acceptHeader, 'text/html')) {
return false;
}
// If client accepts HTML, treat as browser
if (str_contains($acceptHeader, 'text/html')) {
return true;
}
}

// Check X-Requested-With header (AJAX requests)
if ($request->getHeaderLine('X-Requested-With') === 'XMLHttpRequest') {
return false;
}

// Default to browser for non-API routes
return true;
}

/**
* Redirect to the access-denied page
*/
protected function redirectToAccessDenied(string $role): ResponseInterface
{
$response = new Response();
$redirectUrl = SystemURLs::getRootPath() . '/v2/access-denied?role=' . urlencode($role);
return $response->withStatus(302)->withHeader('Location', $redirectUrl);
}

abstract protected function hasRole();

abstract protected function noRoleMessage();
abstract protected function noRoleMessage(): string;

/**
* Get the role name for redirect (without "User must be" prefix)
*/
protected function getRoleName(): string
{
return 'Admin'; // Default, subclasses should override
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,9 @@ protected function noRoleMessage(): string
{
return gettext('User must have Delete Records permission');
}

protected function getRoleName(): string
{
return 'DeleteRecords';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,9 @@ protected function noRoleMessage(): string
{
return gettext('User must have Edit Records permission');
}

protected function getRoleName(): string
{
return 'EditRecords';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,9 @@ protected function noRoleMessage(): string
{
return gettext('User must have Finance permission');
}

protected function getRoleName(): string
{
return 'Finance';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,9 @@ protected function noRoleMessage(): string
{
return gettext('User must have Manage Groups permission');
}

protected function getRoleName(): string
{
return 'ManageGroups';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ protected function hasRole(): bool

protected function noRoleMessage(): string
{
return gettext('User must have MenuOptions permission');
return gettext('User must have Menu Options permission');
}

protected function getRoleName(): string
{
return 'MenuOptions';
}
}
2 changes: 1 addition & 1 deletion src/ChurchCRM/utils/RedirectUtils.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ public static function absoluteRedirect(string $sTargetURL): void
public static function securityRedirect(string $missingRole): void
{
LoggerUtils::getAppLogger()->warning('Security Redirect Request due to Role: ' . $missingRole);
self::Redirect('v2/dashboard');
self::redirect('v2/access-denied?role=' . urlencode($missingRole));
}
}
2 changes: 1 addition & 1 deletion src/DepositSlipEditor.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

// Security: User must have finance permission or be the one who created this deposit
if (!(AuthenticationManager::getCurrentUser()->isFinanceEnabled() || AuthenticationManager::getCurrentUser()->getId() === $thisDeposit->getEnteredby())) {
RedirectUtils::redirect('v2/dashboard');
RedirectUtils::securityRedirect('Finance');
}

$iDepositSlipID = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/DirectoryReports.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use ChurchCRM\Utils\RedirectUtils;

// Check for Create Directory user permission.
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isCreateDirectoryEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isCreateDirectoryEnabled(), 'CreateDirectory');

$sPageTitle = gettext('Directory reports');
require_once 'Include/Header.php';
Expand Down
4 changes: 2 additions & 2 deletions src/FamilyEditor.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@
// Clean error handling: (such as somebody typing an incorrect URL ?PersonID= manually)
if ($iFamilyID > 0) {
if (!(AuthenticationManager::getCurrentUser()->isEditRecordsEnabled() || (AuthenticationManager::getCurrentUser()->isEditSelfEnabled() && $iFamilyID == AuthenticationManager::getCurrentUser()->getPerson()->getFamId()))) {
RedirectUtils::redirect('v2/dashboard');
RedirectUtils::securityRedirect('EditRecords');
}

$family = FamilyQuery::create()->findOneById($iFamilyID);
if ($family === null) {
RedirectUtils::redirect('v2/dashboard');
}
} elseif (!AuthenticationManager::getCurrentUser()->isAddRecordsEnabled()) {
RedirectUtils::redirect('v2/dashboard');
RedirectUtils::securityRedirect('AddRecords');
}

// Get the list of custom person fields
Expand Down
2 changes: 1 addition & 1 deletion src/FinancialReports.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
use ChurchCRM\Utils\RedirectUtils;

// Security
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isFinanceEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isFinanceEnabled(), 'Finance');

$sReportType = '';

Expand Down
2 changes: 1 addition & 1 deletion src/GroupEditor.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
use ChurchCRM\Utils\RedirectUtils;

// Security: User must have Manage Groups permission
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isManageGroupsEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isManageGroupsEnabled(), 'ManageGroups');

$sPageTitle = gettext('Group Editor');
$groupService = new GroupService();
Expand Down
2 changes: 1 addition & 1 deletion src/GroupPropsEditor.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use ChurchCRM\Utils\RedirectUtils;

// Security: user must be allowed to edit records to use this page.
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isEditRecordsEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isEditRecordsEnabled(), 'EditRecords');

$sPageTitle = gettext('Group Member Properties Editor');

Expand Down
2 changes: 1 addition & 1 deletion src/GroupPropsFormEditor.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use ChurchCRM\Utils\RedirectUtils;

// Security: user must be allowed to edit records to use this page.
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isManageGroupsEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isManageGroupsEnabled(), 'ManageGroups');

// Get the Group from the querystring
$iGroupID = InputUtils::legacyFilterInput($_GET['GroupID'], 'int');
Expand Down
2 changes: 1 addition & 1 deletion src/GroupPropsFormRowOps.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
use ChurchCRM\Utils\RedirectUtils;

// Security: user must be allowed to edit records to use this page.
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isManageGroupsEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isManageGroupsEnabled(), 'ManageGroups');

// Get the Group, Property, and Action from the querystring
$iGroupID = InputUtils::legacyFilterInput($_GET['GroupID'], 'int');
Expand Down
2 changes: 1 addition & 1 deletion src/ManageEnvelopes.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
$sPageTitle = gettext('Envelope Manager');

// Security: User must have finance permission to use this form
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isFinanceEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isFinanceEnabled(), 'Finance');

$iClassification = 0;
if (isset($_POST['Classification'])) {
Expand Down
2 changes: 1 addition & 1 deletion src/MemberRoleChange.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use ChurchCRM\Utils\RedirectUtils;

// Security: User must have Manage Groups & Roles permission
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isManageGroupsEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isManageGroupsEnabled(), 'ManageGroups');

$sPageTitle = gettext('Member Role Change');

Expand Down
2 changes: 1 addition & 1 deletion src/NoteDelete.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

// Security: User must have Notes permission
// Otherwise, re-direct them to the main menu.
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isNotesEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isNotesEnabled(), 'Notes');

$sPageTitle = gettext('Note Delete Confirmation');

Expand Down
2 changes: 1 addition & 1 deletion src/NoteEditor.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

// Security: User must have Notes permission
// Otherwise, re-direct them to the main menu.
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isNotesEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isNotesEnabled(), 'Notes');

$sPageTitle = gettext('Note Editor');

Expand Down
4 changes: 2 additions & 2 deletions src/OptionManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@
switch ($mode) {
case 'famroles':
case 'classes':
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isMenuOptionsEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isMenuOptionsEnabled(), 'MenuOptions');
break;

case 'grptypes':
case 'grproles':
case 'groupcustom':
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isManageGroupsEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isManageGroupsEnabled(), 'ManageGroups');
break;

case 'custom':
Expand Down
4 changes: 2 additions & 2 deletions src/OptionManagerRowOps.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@
switch ($mode) {
case 'famroles':
case 'classes':
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isMenuOptionsEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isMenuOptionsEnabled(), 'MenuOptions');
break;

case 'grptypes':
case 'grproles':
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isManageGroupsEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isManageGroupsEnabled(), 'ManageGroups');
break;

case 'custom':
Expand Down
2 changes: 1 addition & 1 deletion src/PledgeDelete.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

// Security: User must have Add or Edit Records permission to use this form in those manners
// Clean error handling: (such as somebody typing an incorrect URL ?PersonID= manually)
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isDeleteRecordsEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isDeleteRecordsEnabled(), 'DeleteRecords');

// Is this the second pass?
if (isset($_POST['Delete'])) {
Expand Down
2 changes: 1 addition & 1 deletion src/PledgeDetails.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

// Security: User must have Finance permission to use this form.
// Clean error handling: (such as somebody typing an incorrect URL ?PersonID= manually)
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isFinanceEnabled());
AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isFinanceEnabled(), 'Finance');

// Is this the second pass?
if (isset($_POST['Back'])) {
Expand Down
Loading
Loading