Skip to content

Commit 59f585e

Browse files
authored
Fix forced password change not redirecting on MVC pages (#8101)
## Problem When a user had (first login after setup), the MVC (legacy) pages would not redirect to the password change form. The user could bypass the forced password change by directly accessing any MVC page, allowing them to continue with a password that must be changed. ## Root Cause The authentication check in `AuthMiddleware` only called `ensureAuthentication()` (which checks and calls `header()` redirects) when the user was **not** authenticated. For authenticated users with an active session, it did nothing, meaning any required redirect steps (like forced password change) were skipped. ## Solution ### 1. AuthMiddleware Enhancement ([AuthMiddleware.php](src/ChurchCRM/Slim/Middleware/AuthMiddleware.php#L51-L57)) - For authenticated browser requests (not background/API), call `validateUserSessionIsActive(true)` to check for required redirect steps - Use PSR-15 response redirect (302 to `nextSte When a user had (first login after setup), the MVC (lenon ## Root Cause The authentication check in `AuthMiddleware` only called `ensureAuthentication()` (which checks and calls `header()` redirects) when the user was **not** authenticated. For authenticated users with an active session, it did nothing, meaning any required redirect sted
2 parents 7ca8d65 + 73eee8c commit 59f585e

File tree

8 files changed

+195
-36
lines changed

8 files changed

+195
-36
lines changed

cypress/e2e/new-system/01-setup-wizard.spec.js

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ describe('01 - Setup Wizard', () => {
130130
cy.get('input[name=Password]').type(adminCredentials.password + '{enter}');
131131

132132
// Should redirect away from login
133-
cy.url({ timeout: 15000 }).should('not.include', '/login');
133+
cy.url({ timeout: 15000 }).should('not.include', '/session/begin');
134134

135135
// Should be on some page (dashboard or admin)
136136
cy.get('body').should('exist');
@@ -140,22 +140,24 @@ describe('01 - Setup Wizard', () => {
140140
cy.visit('/login');
141141
cy.get('input[name=User]').type(adminCredentials.username);
142142
cy.get('input[name=Password]').type(adminCredentials.password + '{enter}');
143-
cy.url({ timeout: 15000 }).should('not.include', '/login');
143+
cy.url({ timeout: 15000 }).should('not.include', '/session/begin');
144144

145145
// Fresh system should show admin dashboard (no people yet)
146146
// The system may redirect to admin or v2/dashboard
147+
// If the admin was created with NeedPasswordChange=true, may land on forced change-password page
147148
cy.url().then((url) => {
148149
cy.log('Redirected to: ' + url);
149-
// Just verify we're logged in and not on login page
150-
cy.get('.main-sidebar, .wrapper, .content-wrapper').should('exist');
150+
// Verify we're logged in and not on login page.
151+
// Accept either the full dashboard layout or the forced change-password minimal layout (.login-box).
152+
cy.get('.main-sidebar, .wrapper, .content-wrapper, .login-box').should('exist');
151153
});
152154
});
153155

154156
it('should verify system is empty (no people/families)', () => {
155157
cy.visit('/login');
156158
cy.get('input[name=User]').type(adminCredentials.username);
157159
cy.get('input[name=Password]').type(adminCredentials.password + '{enter}');
158-
cy.url({ timeout: 15000 }).should('not.include', '/login');
160+
cy.url({ timeout: 15000 }).should('not.include', '/session/begin');
159161

160162
// Check people API - should return mostly empty (only admin user)
161163
cy.request({

cypress/e2e/new-system/02-demo-import.spec.js

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,26 @@ describe('02 - Demo Data Import', () => {
1717
password: 'changeme'
1818
};
1919

20-
// Helper function to login
20+
// Helper function to login, handling forced password-change redirect on first login
2121
const loginAsAdmin = () => {
22+
const password = Cypress.env('newSystemAdminPassword') || adminCredentials.password;
2223
cy.visit('/login');
2324
cy.get('input[name=User]').type(adminCredentials.username);
24-
cy.get('input[name=Password]').type(adminCredentials.password + '{enter}');
25-
cy.url({ timeout: 15000 }).should('not.include', '/login');
25+
cy.get('input[name=Password]').type(password + '{enter}');
26+
cy.url({ timeout: 15000 }).should('not.include', '/session/begin');
27+
28+
// Fresh-install admin has NeedPasswordChange=true; complete the forced form if needed
29+
cy.url().then((url) => {
30+
if (url.includes('/changepassword')) {
31+
const newPassword = 'Cypress@01!';
32+
cy.get('#OldPassword').type(password);
33+
cy.get('#NewPassword1').type(newPassword);
34+
cy.get('#NewPassword2').type(newPassword);
35+
cy.get('button[type=submit]').click();
36+
cy.contains('Password Changed', { timeout: 10000 }).should('be.visible');
37+
Cypress.env('newSystemAdminPassword', newPassword);
38+
}
39+
});
2640
};
2741

2842
describe('Import Demo Data via Admin UI', () => {
@@ -226,5 +240,23 @@ describe('02 - Demo Data Import', () => {
226240
// Should see the finance dashboard title
227241
cy.contains('Finance Dashboard').should('be.visible');
228242
});
243+
244+
it('should reset admin password back to changeme for subsequent tests', () => {
245+
// Tests 03-04 expect 'changeme' as the default password
246+
// Change password from 'Cypress@01!' back to 'changeme'
247+
cy.visit('/v2/user/current/changepassword');
248+
249+
const currentPassword = 'Cypress@01!';
250+
const newPassword = 'changeme';
251+
252+
// Fill in change password form
253+
cy.get('#OldPassword').type(currentPassword);
254+
cy.get('#NewPassword1').type(newPassword);
255+
cy.get('#NewPassword2').type(newPassword);
256+
cy.get('input[type=submit]').click();
257+
258+
// Should show success message
259+
cy.contains('Password Change Successful', { timeout: 10000 }).should('be.visible');
260+
});
229261
});
230262
});

cypress/e2e/new-system/03-backup-restore.spec.js

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@ describe('03 - Backup and Restore', () => {
1919
password: 'changeme'
2020
};
2121

22-
// Helper function to login
22+
// Helper function to login, handling forced password-change redirect on first login
2323
const loginAsAdmin = () => {
24+
// Test 02 resets password back to 'changeme' after testing forced change
25+
const password = adminCredentials.password;
2426
cy.visit('/login');
2527
cy.get('input[name=User]').type(adminCredentials.username);
26-
cy.get('input[name=Password]').type(adminCredentials.password + '{enter}');
27-
cy.url({ timeout: 15000 }).should('not.include', '/login');
28+
cy.get('input[name=Password]').type(password + '{enter}');
29+
cy.url({ timeout: 15000 }).should('not.include', '/session/begin');
2830
};
2931

3032
describe('Step 6: Create Backup', () => {
@@ -104,12 +106,12 @@ describe('03 - Backup and Restore', () => {
104106
it('should restore seed.sql file', () => {
105107
cy.visit('/admin/system/restore');
106108

107-
// The seed SQL file path (relative to cypress project root)
108-
const seedSqlPath = 'cypress/data/seed.sql';
109+
// The demo SQL file path (relative to cypress project root)
110+
const demoSqlPath = 'cypress/data/seed.sql';
109111

110112
// Use cy.selectFile to upload the file
111113
// The file input is hidden, so we need to force the action
112-
cy.get('#restoreFile').selectFile(seedSqlPath, { force: true });
114+
cy.get('#restoreFile').selectFile(demoSqlPath, { force: true });
113115

114116
// File info should show
115117
cy.get('#fileInfo').should('be.visible');

cypress/e2e/new-system/04-system-reset.spec.js

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,32 @@ describe('04 - System Reset', () => {
2121
password: 'changeme'
2222
};
2323

24-
// Helper to manually login (skip session caching for this test)
24+
// Helper to manually login, handling forced password-change redirect after a DB reset
2525
const manualLogin = () => {
2626
cy.clearCookies();
2727
cy.clearLocalStorage();
28+
// Admin password is 'changeme'. After a DB reset NeedPasswordChange=true,
29+
// which forces a redirect to /changepassword on first login.
30+
const password = adminCredentials.password;
2831
cy.visit('/login');
2932
cy.get('input[name=User]', { timeout: 15000 }).type(adminCredentials.username);
30-
cy.get('input[name=Password]').type(adminCredentials.password);
33+
cy.get('input[name=Password]').type(password);
3134
cy.get('input[name=Password]').type('{enter}');
32-
cy.url({ timeout: 30000 }).should('not.include', '/login');
35+
cy.url({ timeout: 30000 }).should('not.include', '/session/begin');
3336
// Give the session time to establish
3437
cy.wait(1000);
38+
39+
// After a DB reset the admin has NeedPasswordChange=true; complete the forced form if needed.
40+
// The forced form uses button[type=submit] (login-box layout, not card layout).
41+
cy.url().then((url) => {
42+
if (url.includes('/changepassword')) {
43+
cy.get('#OldPassword').type(password);
44+
cy.get('#NewPassword1').type('Cypress@01!');
45+
cy.get('#NewPassword2').type('Cypress@01!');
46+
cy.get('button[type=submit]').click();
47+
cy.contains('Password Changed', { timeout: 10000 }).should('be.visible');
48+
}
49+
});
3550
};
3651

3752
describe('Step 10a: Navigate to Reset Page', () => {
@@ -69,7 +84,7 @@ describe('04 - System Reset', () => {
6984
describe('Step 10b: Perform System Reset', () => {
7085
it('should reset the database via API', () => {
7186
manualLogin();
72-
87+
7388
// Perform reset via API
7489
cy.request({
7590
method: 'DELETE',
@@ -117,11 +132,18 @@ describe('04 - System Reset', () => {
117132
});
118133

119134
it('should login with default credentials after reset', () => {
120-
// Fresh login after reset (no session caching)
135+
// manualLogin() uses 'changeme', detects forced /changepassword redirect,
136+
// and completes it — leaving password as 'Cypress@01!' with NeedPasswordChange=false.
121137
manualLogin();
122-
123-
// Should be redirected to admin dashboard (blank system)
124-
cy.url().should('include', '/admin');
138+
139+
// Now change it back to 'changeme' so Steps 10d/10e can login cleanly.
140+
// NeedPasswordChange=false so this shows the voluntary form (input[type=submit]).
141+
cy.visit('/v2/user/current/changepassword');
142+
cy.get('#OldPassword').type('Cypress@01!');
143+
cy.get('#NewPassword1').type('changeme');
144+
cy.get('#NewPassword2').type('changeme');
145+
cy.get('input[type=submit]').click();
146+
cy.contains('Password Change Successful', { timeout: 10000 }).should('be.visible');
125147
});
126148
});
127149

src/ChurchCRM/Slim/Middleware/AuthMiddleware.php

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,14 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
4848
// since /background operations do not connotate user activity.
4949

5050
// User with an active browser session is still authenticated.
51-
// don't really need to do anything here...
51+
// For browser requests (non-background), enforce any required redirect steps (e.g. forced password change).
52+
// Use a PSR-15 response redirect rather than calling ensureAuthentication() which exits via header().
53+
if ($this->isBrowserRequest($request) && !$this->isPath($request, 'background')) {
54+
$result = AuthenticationManager::getAuthenticationProvider()->validateUserSessionIsActive(true);
55+
if ($result->nextStepURL !== null) {
56+
return (new Response())->withStatus(302)->withHeader('Location', $result->nextStepURL);
57+
}
58+
}
5259
} else {
5360
$logger = LoggerUtils::getAppLogger();
5461
$logger->warning('No authenticated user or session', [
@@ -73,12 +80,10 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
7380

7481
private function isPath(ServerRequestInterface $request, string $pathPart): bool
7582
{
83+
// explode produces an empty string at index 0 for paths starting with '/',
84+
// so use in_array to check if the segment exists anywhere in the path
7685
$pathAry = explode('/', $request->getUri()->getPath());
77-
if ($pathAry[0] === $pathPart) {
78-
return true;
79-
}
80-
81-
return false;
86+
return in_array($pathPart, $pathAry, true);
8287
}
8388

8489
/**

src/v2/routes/user-current.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ function changepassword(Request $request, Response $response, array $args): Resp
4141
$pageArgs = [
4242
'sRootPath' => SystemURLs::getRootPath(),
4343
'user' => $curUser,
44+
'isForced' => $curUser->getNeedPasswordChange(),
4445
];
4546

4647
if ($authenticationProvider instanceof LocalAuthentication) {

src/v2/templates/common/success-changepassword.php

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,54 @@
22

33
use ChurchCRM\dto\SystemURLs;
44

5+
$isForced = $isForced ?? false;
56
$sPageTitle = gettext("Change Password") . ": " . $user->getFullName();
6-
require SystemURLs::getDocumentRoot() . '/Include/Header.php';
7+
if ($isForced) {
8+
require SystemURLs::getDocumentRoot() . '/Include/HeaderNotLoggedIn.php';
9+
} else {
10+
require SystemURLs::getDocumentRoot() . '/Include/Header.php';
11+
}
712
?>
8-
Password Change Successful
13+
14+
<?php if ($isForced): ?>
15+
<div class="login-box">
16+
<div class="card card-outline card-success">
17+
<div class="card-header text-center">
18+
<a href="<?= SystemURLs::getRootPath() ?>" class="h1">
19+
<img src="<?= SystemURLs::getRootPath() ?>/Images/logo-churchcrm-350.jpg" alt="ChurchCRM" style="max-width:280px; height:auto;" />
20+
</a>
21+
</div>
22+
<div class="card-body text-center">
23+
<p class="login-box-msg">
24+
<i class="fa fa-check-circle text-success" style="font-size:2rem;"></i><br>
25+
<strong><?= gettext('Password Changed') ?></strong><br>
26+
<small class="text-muted"><?= gettext('Your password has been updated successfully.') ?></small>
27+
</p>
28+
<a href="<?= SystemURLs::getRootPath() ?>/v2/dashboard" class="btn btn-success btn-block">
29+
<?= gettext('Continue to Dashboard') ?>
30+
</a>
31+
</div>
32+
</div>
33+
</div>
34+
<?php else: ?>
35+
<div class="row">
36+
<div class="col-md-6">
37+
<div class="card card-success">
38+
<div class="card-header">
39+
<h3 class="card-title"><?= gettext('Password Change Successful') ?></h3>
40+
</div>
41+
<div class="card-body">
42+
<p><?= sprintf(gettext('The password for %s has been updated.'), $user->getFullName()) ?></p>
43+
<a href="<?= SystemURLs::getRootPath() ?>/v2/dashboard" class="btn btn-success"><?= gettext('Go to Dashboard') ?></a>
44+
</div>
45+
</div>
46+
</div>
47+
</div>
48+
<?php endif; ?>
949

1050
<?php
11-
require SystemURLs::getDocumentRoot() . '/Include/Footer.php';
51+
if ($isForced) {
52+
require SystemURLs::getDocumentRoot() . '/Include/FooterNotLoggedIn.php';
53+
} else {
54+
require SystemURLs::getDocumentRoot() . '/Include/Footer.php';
55+
}

src/v2/templates/user/changepassword.php

Lines changed: 56 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,55 @@
55
use ChurchCRM\Utils\CSRFUtils;
66

77
$sPageTitle = gettext("Change Password") . ": " . $user->getFullName();
8-
require SystemURLs::getDocumentRoot() . '/Include/Header.php';
8+
if ($isForced) {
9+
require SystemURLs::getDocumentRoot() . '/Include/HeaderNotLoggedIn.php';
10+
} else {
11+
require SystemURLs::getDocumentRoot() . '/Include/Header.php';
12+
}
913
?>
1014

15+
<?php if ($isForced): ?>
16+
<div class="login-box">
17+
<div class="card card-outline card-warning">
18+
<div class="card-header text-center">
19+
<a href="<?= SystemURLs::getRootPath() ?>" class="h1">
20+
<img src="<?= SystemURLs::getRootPath() ?>/Images/logo-churchcrm-350.jpg" alt="ChurchCRM" style="max-width:280px; height:auto;" />
21+
</a>
22+
</div>
23+
<div class="card-body">
24+
<p class="login-box-msg">
25+
<strong><?= gettext('Password Change Required') ?></strong><br>
26+
<small class="text-muted"><?= gettext('You must set a new password before continuing.') ?></small>
27+
</p>
28+
<form method="post" action="<?= SystemURLs::getRootPath() ?>/v2/user/current/changepassword" id="passwordChangeForm">
29+
<?= CSRFUtils::getTokenInputField('user_change_password') ?>
30+
<div class="input-group mb-3">
31+
<input type="password" name="OldPassword" id="OldPassword" class="form-control" placeholder="<?= gettext('Current Password') ?>" autofocus>
32+
<div class="input-group-append"><div class="input-group-text"><i class="fa fa-lock"></i></div></div>
33+
<?php if (!empty($sOldPasswordError ?? '')): ?>
34+
<span class="form-field-error"><?= $sOldPasswordError ?></span>
35+
<?php endif; ?>
36+
</div>
37+
<div class="input-group mb-3">
38+
<input type="password" name="NewPassword1" id="NewPassword1" class="form-control" placeholder="<?= gettext('New Password') ?>">
39+
<div class="input-group-append"><div class="input-group-text"><i class="fa fa-key"></i></div></div>
40+
</div>
41+
<div class="input-group mb-3">
42+
<input type="password" name="NewPassword2" id="NewPassword2" class="form-control" placeholder="<?= gettext('Confirm New Password') ?>">
43+
<div class="input-group-append"><div class="input-group-text"><i class="fa fa-key"></i></div></div>
44+
<?php if (!empty($sNewPasswordError ?? '')): ?>
45+
<span class="form-field-error"><?= $sNewPasswordError ?></span>
46+
<?php endif; ?>
47+
</div>
48+
<p class="text-muted small mb-3">
49+
<?= gettext('Passwords must be at least') ?> <?= SystemConfig::getValue('iMinPasswordLength') ?> <?= gettext('characters in length.') ?>
50+
</p>
51+
<button type="submit" name="Submit" class="btn btn-warning btn-block"><?= gettext('Set New Password') ?></button>
52+
</form>
53+
</div>
54+
</div>
55+
</div>
56+
<?php else: ?>
1157
<div class="row">
1258
<!-- left column -->
1359
<div class="col-md-8">
@@ -22,15 +68,15 @@
2268
<div class="card-body">
2369
<div class="form-group">
2470
<label for="OldPassword"><?= gettext('Old Password') ?>:</label>
25-
<input type="password" name="OldPassword" id="OldPassword" class="form-control" value="<?= $sOldPassword ?>" autofocus><span id="oldPasswordError" class="form-field-error"><?= $sOldPasswordError ?></span>
71+
<input type="password" name="OldPassword" id="OldPassword" class="form-control" autofocus><span id="oldPasswordError" class="form-field-error"><?= $sOldPasswordError ?? '' ?></span>
2672
</div>
2773
<div class="form-group">
2874
<label for="NewPassword1"><?= gettext('New Password') ?>:</label>
29-
<input type="password" name="NewPassword1" id="NewPassword1" class="form-control" value="<?= $sNewPassword1 ?>">
75+
<input type="password" name="NewPassword1" id="NewPassword1" class="form-control">
3076
</div>
3177
<div class="form-group">
3278
<label for="NewPassword2"><?= gettext('Confirm New Password') ?>:</label>
33-
<input type="password" name="NewPassword2" id="NewPassword2" class="form-control" value="<?= $sNewPassword2 ?>"><span id="NewPasswordError" class="form-field-error"><?= $sNewPasswordError ?></span>
79+
<input type="password" name="NewPassword2" id="NewPassword2" class="form-control"><span id="NewPasswordError" class="form-field-error"><?= $sNewPasswordError ?? '' ?></span>
3480
</div>
3581
</div>
3682

@@ -41,6 +87,11 @@
4187
</div>
4288
</div>
4389
</div>
90+
<?php endif; ?>
4491
<script src="<?= SystemURLs::assetVersioned('/skin/js/PasswordChange.js') ?>"></script>
4592
<?php
46-
require SystemURLs::getDocumentRoot() . '/Include/Footer.php';
93+
if ($isForced) {
94+
require SystemURLs::getDocumentRoot() . '/Include/FooterNotLoggedIn.php';
95+
} else {
96+
require SystemURLs::getDocumentRoot() . '/Include/Footer.php';
97+
}

0 commit comments

Comments
 (0)