Skip to content

Commit 7e4e133

Browse files
nolanpronolanpro
authored
Merge pull request #8757 from ProcessMaker/bugfix/FOUR-30040-A
Account Lock Bypass via Password Reset Flow
2 parents e910706 + e41939c commit 7e4e133

File tree

6 files changed

+256
-4
lines changed

6 files changed

+256
-4
lines changed

‎ProcessMaker/Http/Controllers/Auth/ForgotPasswordController.php‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@
33
namespace ProcessMaker\Http\Controllers\Auth;
44

55
use Illuminate\Foundation\Auth\SendsPasswordResetEmails;
6+
use Illuminate\Http\Request;
7+
use Illuminate\Support\Facades\Password;
68
use ProcessMaker\Http\Controllers\Controller;
9+
use ProcessMaker\Models\User;
710

811
class ForgotPasswordController extends Controller
912
{
@@ -29,4 +32,30 @@ public function __construct()
2932
{
3033
$this->middleware('guest');
3134
}
35+
36+
/**
37+
* Send a reset link to the given user.
38+
* Blocked or inactive users will not receive the reset email for security reasons.
39+
*
40+
* @param Request $request
41+
* @return \Illuminate\Http\RedirectResponse|\Illuminate\Http\JsonResponse
42+
*/
43+
public function sendResetLinkEmail(Request $request)
44+
{
45+
$this->validateEmail($request);
46+
47+
$user = User::where('email', $request->input('email'))->first();
48+
49+
if ($user && ($user->status === 'BLOCKED' || $user->status === 'INACTIVE')) {
50+
return $this->sendResetLinkResponse($request, Password::RESET_LINK_SENT);
51+
}
52+
53+
$response = $this->broker()->sendResetLink(
54+
$this->credentials($request)
55+
);
56+
57+
return $response == Password::RESET_LINK_SENT
58+
? $this->sendResetLinkResponse($request, $response)
59+
: $this->sendResetLinkFailedResponse($request, $response);
60+
}
3261
}

‎ProcessMaker/Http/Controllers/Auth/ResetPasswordController.php‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@ class ResetPasswordController extends Controller
2020
|
2121
*/
2222

23-
use ResetsPasswords;
23+
use ResetsPasswords {
24+
reset as protected performPasswordReset;
25+
}
2426

2527
/**
2628
* Where to redirect users after resetting their password.
@@ -46,8 +48,44 @@ public function __construct()
4648
*/
4749
public function showResetForm(Request $request, $token)
4850
{
49-
$username = User::where('email', $request->input('email'))->firstOrFail()->username;
51+
$user = User::where('email', $request->input('email'))->firstOrFail();
52+
53+
if ($user->status === 'BLOCKED') {
54+
return redirect()->route('password.request')
55+
->withErrors(['email' => __('passwords.blocked')]);
56+
}
57+
58+
if ($user->status === 'INACTIVE') {
59+
return redirect()->route('password.request')
60+
->withErrors(['email' => __('passwords.inactive')]);
61+
}
62+
63+
return view('auth.passwords.reset', [
64+
'username' => $user->username,
65+
'token' => $token,
66+
'email' => $request->input('email'),
67+
]);
68+
}
69+
70+
/**
71+
* Reset the given user's password.
72+
* Blocked or inactive users cannot reset their password.
73+
*
74+
* @param Request $request
75+
* @return \Illuminate\Http\RedirectResponse|\Illuminate\Http\JsonResponse
76+
*/
77+
public function reset(Request $request)
78+
{
79+
$user = User::where('email', $request->input('email'))->first();
80+
81+
if ($user && $user->status === 'BLOCKED') {
82+
return $this->sendResetFailedResponse($request, 'passwords.blocked');
83+
}
84+
85+
if ($user && $user->status === 'INACTIVE') {
86+
return $this->sendResetFailedResponse($request, 'passwords.inactive');
87+
}
5088

51-
return view('auth.passwords.reset', compact('username', 'token'));
89+
return $this->performPasswordReset($request);
5290
}
5391
}

‎resources/lang/en.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1546,6 +1546,8 @@
15461546
"passwords.sent": "We have e-mailed your password reset link!",
15471547
"passwords.token": "This password reset token is invalid.",
15481548
"passwords.user": "We can't find a user with that e-mail address.",
1549+
"passwords.blocked": "Your account has been blocked. Please contact your administrator.",
1550+
"passwords.inactive": "Your account is inactive. Please contact your administrator.",
15491551
"Pause Start Timer Events": "Pause Start Timer Events",
15501552
"Pause Timer Start Events": "Pause Timer Start Events",
15511553
"per page": "per page",

‎resources/lang/en/passwords.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,7 @@
1818
'throttled' => 'Please wait before retrying.',
1919
'token' => 'This password reset token is invalid.',
2020
'user' => "We can't find a user with that email address.",
21+
'blocked' => 'Your account has been blocked. Please contact your administrator.',
22+
'inactive' => 'Your account is inactive. Please contact your administrator.',
2123

2224
];

‎resources/views/auth/passwords/reset.blade.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
<input type="hidden" name="token" value="{{ $token }}">
1616
<div class="form-group">
1717
<label for="email">{{__('Email Address')}}</label>
18-
<input id="email" type="email" class="form-control{{ $errors->has('email') ? ' is-invalid' : '' }}" name="email">
18+
<input id="email" type="email" class="form-control{{ $errors->has('email') ? ' is-invalid' : '' }}" name="email" value="{{ $email ?? old('email') }}">
1919
@if ($errors->has('email'))
2020
<span class="
2121
invalid-feedback" role="alert">

‎tests/Feature/Auth/PasswordResetTest.php‎

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
<?php
2+
3+
namespace Tests\Feature\Auth;
4+
5+
use Illuminate\Auth\Passwords\PasswordBroker as ConcretePasswordBroker;
6+
use Illuminate\Support\Facades\Hash;
7+
use Illuminate\Support\Facades\Notification;
8+
use Illuminate\Support\Facades\Password;
9+
use ProcessMaker\Models\User;
10+
use ProcessMaker\Notifications\ResetPassword as ResetPasswordNotification;
11+
use Tests\TestCase;
12+
13+
class PasswordResetTest extends TestCase
14+
{
15+
public function testForgotPasswordDoesNotNotifyBlockedUser(): void
16+
{
17+
Notification::fake();
18+
19+
$user = User::factory()->create([
20+
'email' => 'blocked-forgot@example.com',
21+
'status' => 'BLOCKED',
22+
]);
23+
24+
$response = $this->post(route('password.email'), [
25+
'email' => $user->email,
26+
]);
27+
28+
$response->assertSessionHas('status');
29+
Notification::assertNothingSent();
30+
}
31+
32+
public function testForgotPasswordDoesNotNotifyInactiveUser(): void
33+
{
34+
Notification::fake();
35+
36+
$user = User::factory()->create([
37+
'email' => 'inactive-forgot@example.com',
38+
'status' => 'INACTIVE',
39+
]);
40+
41+
$response = $this->post(route('password.email'), [
42+
'email' => $user->email,
43+
]);
44+
45+
$response->assertSessionHas('status');
46+
Notification::assertNothingSent();
47+
}
48+
49+
public function testForgotPasswordSendsNotificationToActiveUser(): void
50+
{
51+
Notification::fake();
52+
53+
$user = User::factory()->create([
54+
'email' => 'active-forgot@example.com',
55+
'status' => 'ACTIVE',
56+
]);
57+
58+
$response = $this->post(route('password.email'), [
59+
'email' => $user->email,
60+
]);
61+
62+
$response->assertSessionHas('status');
63+
Notification::assertSentTo($user, ResetPasswordNotification::class);
64+
}
65+
66+
public function testShowResetFormRedirectsBlockedUserToRequestForm(): void
67+
{
68+
$user = User::factory()->create([
69+
'email' => 'blocked-reset-form@example.com',
70+
'status' => 'BLOCKED',
71+
]);
72+
73+
$url = route('password.reset', ['token' => 'unused-token']);
74+
$response = $this->get($url . '?email=' . urlencode($user->email));
75+
76+
$response->assertRedirect(route('password.request'));
77+
$response->assertSessionHasErrors([
78+
'email' => __('passwords.blocked'),
79+
]);
80+
}
81+
82+
public function testShowResetFormRedirectsInactiveUserToRequestForm(): void
83+
{
84+
$user = User::factory()->create([
85+
'email' => 'inactive-reset-form@example.com',
86+
'status' => 'INACTIVE',
87+
]);
88+
89+
$url = route('password.reset', ['token' => 'unused-token']);
90+
$response = $this->get($url . '?email=' . urlencode($user->email));
91+
92+
$response->assertRedirect(route('password.request'));
93+
$response->assertSessionHasErrors([
94+
'email' => __('passwords.inactive'),
95+
]);
96+
}
97+
98+
public function testShowResetFormDisplaysForActiveUser(): void
99+
{
100+
$user = User::factory()->create([
101+
'email' => 'active-reset-form@example.com',
102+
'status' => 'ACTIVE',
103+
'username' => 'active_reset_user',
104+
]);
105+
106+
$url = route('password.reset', ['token' => 'some-token']);
107+
$response = $this->get($url . '?email=' . urlencode($user->email));
108+
109+
$response->assertOk();
110+
$response->assertViewIs('auth.passwords.reset');
111+
$response->assertViewHas('email', $user->email);
112+
$response->assertViewHas('username', $user->username);
113+
$response->assertViewHas('token', 'some-token');
114+
}
115+
116+
public function testResetPasswordRejectsBlockedUser(): void
117+
{
118+
$user = User::factory()->create([
119+
'email' => 'blocked-reset-post@example.com',
120+
'status' => 'BLOCKED',
121+
]);
122+
123+
$response = $this->from(route('password.request'))->post('/password/reset', [
124+
'token' => 'will-not-be-used',
125+
'email' => $user->email,
126+
'password' => 'NewPassword123!',
127+
'password_confirmation' => 'NewPassword123!',
128+
]);
129+
130+
$response->assertSessionHasErrors([
131+
'email' => __('passwords.blocked'),
132+
]);
133+
}
134+
135+
public function testResetPasswordRejectsInactiveUser(): void
136+
{
137+
$user = User::factory()->create([
138+
'email' => 'inactive-reset-post@example.com',
139+
'status' => 'INACTIVE',
140+
]);
141+
142+
$response = $this->from(route('password.request'))->post('/password/reset', [
143+
'token' => 'will-not-be-used',
144+
'email' => $user->email,
145+
'password' => 'NewPassword123!',
146+
'password_confirmation' => 'NewPassword123!',
147+
]);
148+
149+
$response->assertSessionHasErrors([
150+
'email' => __('passwords.inactive'),
151+
]);
152+
}
153+
154+
public function testResetPasswordUpdatesPasswordForActiveUser(): void
155+
{
156+
/** @var User $user */
157+
$user = User::factory()->create([
158+
'email' => 'active-reset-post@example.com',
159+
'status' => 'ACTIVE',
160+
]);
161+
162+
/** @var ConcretePasswordBroker $broker */
163+
$broker = Password::broker();
164+
$token = $broker->createToken($user);
165+
$plaintextSecret = 'NewSecurePass123!';
166+
167+
$response = $this->post('/password/reset', [
168+
'token' => $token,
169+
'email' => $user->email,
170+
'password' => $plaintextSecret,
171+
'password_confirmation' => $plaintextSecret,
172+
]);
173+
174+
$response->assertRedirect('/password/success');
175+
$response->assertSessionHas('status');
176+
177+
$user->refresh();
178+
$this->assertTrue(Hash::check($plaintextSecret, $user->password));
179+
$this->assertAuthenticatedAs($user, 'web');
180+
}
181+
}

0 commit comments

Comments
 (0)