Test that a provider account's first password needs its inbox

GHSA-4r8h-mwfm-f5f4

tests/Feature/Identity/ProviderAccountPasswordTest.php

@@ -5,8 +5,12 @@ declare(strict_types=1);
 use App\Models\User;
 use App\Modules\Identity\AuthSource;
 use App\Modules\Platform\Settings\Setting;
+use App\Modules\Identity\Notifications\ResetPasswordNotification;
+use App\Modules\Identity\Social\SocialAccount;
 use App\Modules\Platform\Settings\Settings;
 use Illuminate\Support\Facades\Hash;
+use Illuminate\Support\Facades\Notification;
+use Illuminate\Support\Facades\Password;
 use Inertia\Testing\AssertableInertia;
 
 /**
@@ -21,32 +25,118 @@ use Inertia\Testing\AssertableInertia;
  * confirm a password they do not have, and every other screen, including
  * the one that would have given them one, redirected back. Reported by
  * Ricardo Cazati.
+ *
+ * The first answer let the signed-in session choose the password with no
+ * proof at all, which made a stolen session permanent: the thief set a
+ * password, confirmed it, enrolled their own second factor and removed the
+ * owner's provider (GHSA-4r8h-mwfm-f5f4). The password now comes from a
+ * link emailed to the account's own address, through the same reset flow
+ * every account already has. The session asks for the link; the inbox
+ * answers it.
  */
 function providerAccount(array $overrides = []): User
 {
     return User::factory()->create(array_merge(['auth_source' => AuthSource::Social], $overrides));
 }
 
-test('an account that signs in through a provider can set its first password', function () {
+test('a provider account cannot set its first password from the session alone', function () {
     $user = providerAccount();
 
     $this->actingAs($user)
         ->from('/settings/password')
         ->put('/settings/password', [
-            'password' => 'a-password-of-my-own',
-            'password_confirmation' => 'a-password-of-my-own',
+            'password' => 'chosen-by-whoever-holds-the-session',
+            'password_confirmation' => 'chosen-by-whoever-holds-the-session',
         ])
+        ->assertSessionHasErrors('password');
+
+    $user->refresh();
+
+    expect(Hash::check('chosen-by-whoever-holds-the-session', $user->password))->toBeFalse()
+        ->and($user->auth_source)->toBe(AuthSource::Social);
+});
+
+test('a provider account asks for a link, and it goes to its own address', function () {
+    Notification::fake();
+    $user = providerAccount();
+
+    $this->actingAs($user)->from('/settings/password')->post('/settings/password/link')
         ->assertSessionHasNoErrors()
         ->assertRedirect('/settings/password');
 
+    Notification::assertSentTo($user, ResetPasswordNotification::class);
+});
+
+test('the link sets the first password, signed in or not, and makes the account local', function () {
+    $user = providerAccount();
+    $token = Password::createToken($user);
+
+    // Opened in the same browser the person is signed in with.
+    $this->actingAs($user)->get("/reset-password/{$token}?email=".urlencode($user->email))->assertOk();
+
+    $this->actingAs($user)->post('/reset-password', [
+        'token' => $token,
+        'email' => $user->email,
+        'password' => 'a-password-of-my-own',
+        'password_confirmation' => 'a-password-of-my-own',
+    ])->assertSessionHasNoErrors();
+
     $user->refresh();
 
     expect(Hash::check('a-password-of-my-own', $user->password))->toBeTrue()
-        // The hash is now what signs this account in, and the settings
-        // screens read that off this column.
         ->and($user->auth_source)->toBe(AuthSource::Local);
 });
 
+test('using the link signs out the session that asked for it', function () {
+    $user = providerAccount();
+    $token = Password::createToken($user);
+
+    $this->actingAs($user)->post('/reset-password', [
+        'token' => $token,
+        'email' => $user->email,
+        'password' => 'a-password-of-my-own',
+        'password_confirmation' => 'a-password-of-my-own',
+    ]);
+
+    // Whoever held the session before has the password no longer: the
+    // next thing it asks for sends it to sign in. forgetRequestState() so
+    // that request reads the account afresh, as a real one would.
+    forgetRequestState();
+    $this->post('/confirm-password', ['password' => 'a-password-of-my-own'])->assertRedirect(route('login'));
+    $this->assertGuest();
+});
+
+test('an account that already has a password uses the form, not a link', function () {
+    Notification::fake();
+    $user = User::factory()->create();
+
+    $this->actingAs($user)->post('/settings/password/link')->assertForbidden();
+
+    Notification::assertNothingSent();
+});
+
+test('a directory account is not sent a link either', function () {
+    Notification::fake();
+
+    $this->actingAs(providerAccount(['auth_source' => AuthSource::Ldap]))->post('/settings/password/link')->assertForbidden();
+
+    Notification::assertNothingSent();
+});
+
+test('without a password, the last provider still cannot be removed', function () {
+    $user = providerAccount();
+    SocialAccount::query()->create(['user_id' => $user->id, 'provider' => 'google', 'provider_user_id' => 'g-1']);
+
+    $this->actingAs($user)->put('/settings/password', [
+        'password' => 'chosen-by-whoever-holds-the-session',
+        'password_confirmation' => 'chosen-by-whoever-holds-the-session',
+    ]);
+
+    $this->actingAs($user->refresh())->delete('/settings/connected-accounts/google')->assertSessionHasErrors('provider');
+
+    expect(SocialAccount::query()->where('user_id', $user->id)->exists())->toBeTrue();
+});
+
 test('an ordinary account still has to prove the password it is replacing', function () {
     $user = User::factory()->create();
 
@@ -106,20 +196,33 @@ test('compulsory two-factor leaves a provider account a way to get a password',
     // But not on the one screen that can end the loop.
     $this->actingAs($user)->get('/settings/password')->assertOk();
 
-    $this->actingAs($user)
-        ->put('/settings/password', [
-            'password' => 'a-password-of-my-own',
-            'password_confirmation' => 'a-password-of-my-own',
-        ])
-        ->assertSessionHasNoErrors();
+    // The link is asked for from that screen, and opened past the
+    // enforcement too: both are part of ending the loop.
+    Notification::fake();
+    $this->actingAs($user)->post('/settings/password/link')->assertSessionHasNoErrors();
+    Notification::assertSentTo($user, ResetPasswordNotification::class);
 
-    // And with one, the password confirmation in front of enrolling is
+    $token = Password::createToken($user);
+    $this->actingAs($user)->get("/reset-password/{$token}?email=".urlencode($user->email))->assertOk();
+    $this->actingAs($user)->post('/reset-password', [
+        'token' => $token,
+        'email' => $user->email,
+        'password' => 'a-password-of-my-own',
+        'password_confirmation' => 'a-password-of-my-own',
+    ])->assertSessionHasNoErrors();
+
+    // Setting it ends every session holding the old password, the one that
+    // asked for the link included, so the way on is to sign in again.
+    auth()->logout();
+    forgetRequestState();
+    $this->post('/login', ['email' => $user->email, 'password' => 'a-password-of-my-own']);
+    $this->assertAuthenticatedAs($user);
+
+    // And with a password, the confirmation in front of enrolling is
     // answerable, so two-factor can actually be turned on.
-    $this->actingAs($user->refresh())
-        ->post('/confirm-password', ['password' => 'a-password-of-my-own'])
-        ->assertSessionHasNoErrors();
+    $this->post('/confirm-password', ['password' => 'a-password-of-my-own'])->assertSessionHasNoErrors();
 
-    $this->actingAs($user)->post('/settings/two-factor')->assertSessionHasNoErrors();
+    $this->post('/settings/two-factor')->assertSessionHasNoErrors();
 
     expect($user->refresh()->two_factor_secret)->not->toBeNull();
 });