Skip to content

Commit 2465d20

Browse files
Don't overwrite an already two factor secret unless force = true (#518)
* Update EnableTwoFactorAuthentication.php Add a force parameter that only overwrites existing two factor information if it's null / has been disabled. * Update TwoFactorAuthenticationController.php Allow users to pass force = true to the post request to enable forcing overwriting of current secret keys. * Update TwoFactorAuthenticationControllerTest.php Add tests for 'force' request parameter * Passing tests. * StyleCI * formatting * formatting --------- Co-authored-by: Taylor Otwell <[email protected]>
1 parent 502fc61 commit 2465d20

File tree

3 files changed

+94
-9
lines changed

3 files changed

+94
-9
lines changed

src/Actions/EnableTwoFactorAuthentication.php

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,17 +31,20 @@ public function __construct(TwoFactorAuthenticationProvider $provider)
3131
* Enable two factor authentication for the user.
3232
*
3333
* @param mixed $user
34+
* @param bool $force
3435
* @return void
3536
*/
36-
public function __invoke($user)
37+
public function __invoke($user, $force = false)
3738
{
38-
$user->forceFill([
39-
'two_factor_secret' => encrypt($this->provider->generateSecretKey()),
40-
'two_factor_recovery_codes' => encrypt(json_encode(Collection::times(8, function () {
41-
return RecoveryCode::generate();
42-
})->all())),
43-
])->save();
39+
if (empty($user->two_factor_secret) || $force === true) {
40+
$user->forceFill([
41+
'two_factor_secret' => encrypt($this->provider->generateSecretKey()),
42+
'two_factor_recovery_codes' => encrypt(json_encode(Collection::times(8, function () {
43+
return RecoveryCode::generate();
44+
})->all())),
45+
])->save();
4446

45-
TwoFactorAuthenticationEnabled::dispatch($user);
47+
TwoFactorAuthenticationEnabled::dispatch($user);
48+
}
4649
}
4750
}

src/Http/Controllers/TwoFactorAuthenticationController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class TwoFactorAuthenticationController extends Controller
2020
*/
2121
public function store(Request $request, EnableTwoFactorAuthentication $enable)
2222
{
23-
$enable($request->user());
23+
$enable($request->user(), $request->boolean('force', false));
2424

2525
return app(TwoFactorEnabledResponse::class);
2626
}

tests/TwoFactorAuthenticationControllerTest.php

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,88 @@ public function test_two_factor_authentication_can_be_enabled()
4646
$this->assertNotNull($user->twoFactorQrCodeSvg());
4747
}
4848

49+
#[ResetRefreshDatabaseState]
50+
public function test_calling_two_factor_authentication_endpoint_will_not_overwrite_without_force_parameter()
51+
{
52+
Event::fake();
53+
54+
$user = TestTwoFactorAuthenticationUser::forceCreate([
55+
'name' => 'Taylor Otwell',
56+
'email' => '[email protected]',
57+
'password' => bcrypt('secret'),
58+
]);
59+
60+
$response = $this->withoutExceptionHandling()->actingAs($user)->postJson(
61+
'/user/two-factor-authentication'
62+
);
63+
64+
$response->assertStatus(200);
65+
66+
Event::assertDispatched(TwoFactorAuthenticationEnabled::class);
67+
68+
$user = $user->fresh();
69+
70+
$old_value = $user->two_factor_secret;
71+
72+
$response = $this->withoutExceptionHandling()->actingAs($user)->postJson(
73+
'/user/two-factor-authentication'
74+
);
75+
76+
$response->assertStatus(200);
77+
78+
$this->assertNotNull($user->two_factor_secret);
79+
$this->assertNotNull($user->two_factor_recovery_codes);
80+
$this->assertEquals($old_value, $user->fresh()->two_factor_secret);
81+
$this->assertNull($user->two_factor_confirmed_at);
82+
$this->assertIsArray(json_decode(decrypt($user->two_factor_recovery_codes), true));
83+
$this->assertNotNull($user->twoFactorQrCodeSvg());
84+
}
85+
86+
#[ResetRefreshDatabaseState]
87+
public function test_calling_two_factor_authentication_endpoint_will_overwrite_with_force_parameter()
88+
{
89+
Event::fake();
90+
91+
$user = TestTwoFactorAuthenticationUser::forceCreate([
92+
'name' => 'Taylor Otwell',
93+
'email' => '[email protected]',
94+
'password' => bcrypt('secret'),
95+
]);
96+
97+
$response = $this->withoutExceptionHandling()->actingAs($user)->postJson(
98+
'/user/two-factor-authentication',
99+
[
100+
'force' => true,
101+
]
102+
);
103+
104+
$response->assertStatus(200);
105+
106+
Event::assertDispatched(TwoFactorAuthenticationEnabled::class);
107+
108+
$user = $user->fresh();
109+
110+
$old_value = $user->two_factor_secret;
111+
112+
$response = $this->withoutExceptionHandling()->actingAs($user)->postJson(
113+
'/user/two-factor-authentication',
114+
[
115+
'force' => true,
116+
]
117+
);
118+
119+
$response->assertStatus(200);
120+
121+
$user = $user->fresh();
122+
123+
$this->assertNotNull($user->two_factor_secret);
124+
$this->assertNotNull($user->two_factor_recovery_codes);
125+
$this->assertNotEquals($old_value, $user->fresh()->two_factor_secret);
126+
$this->assertNull($user->two_factor_confirmed_at);
127+
$this->assertIsArray(json_decode(decrypt($user->two_factor_recovery_codes), true));
128+
$this->assertNotNull($user->twoFactorQrCodeSvg());
129+
}
130+
49131
public function test_two_factor_authentication_secret_key_can_be_retrieved()
50132
{
51133
Event::fake();

0 commit comments

Comments
 (0)