diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 3d7c1213..cfbca0bf 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -676,7 +676,27 @@ public function logout() { $user = $this->auth_service->getCurrentUser(); // RevokeUserGrantsOnExplicitLogout::dispatch($user)->afterResponse(); + + // A web logout issued while a relying party request is pending (the user is switching + // accounts from the consent/profile pages) must not drop that request: AuthService::logout + // flushes the whole session, so keep the pending OAuth2 / OpenID memento aside and put it + // back afterwards. The next login then keeps the OAUTH2 / OIDC strategy and returns the + // user to the relying party instead of their identity page. + $pending_oauth2_request = $this->oauth2_memento_service->exists() ? $this->oauth2_memento_service->load() : null; + $pending_openid_request = $this->openid_memento_service->exists() ? $this->openid_memento_service->load() : null; + $this->auth_service->logout(); + + if (!is_null($pending_oauth2_request)) { + Log::debug("UserController::logout restoring pending OAuth2 request after logout"); + $this->oauth2_memento_service->serialize($pending_oauth2_request); + } + + if (!is_null($pending_openid_request)) { + Log::debug("UserController::logout restoring pending OpenID request after logout"); + $this->openid_memento_service->serialize($pending_openid_request); + } + return Redirect::action("UserController@getLogin"); } diff --git a/tests/UserLogoutPreservesPendingOAuth2RequestTest.php b/tests/UserLogoutPreservesPendingOAuth2RequestTest.php new file mode 100644 index 00000000..89b06107 --- /dev/null +++ b/tests/UserLogoutPreservesPendingOAuth2RequestTest.php @@ -0,0 +1,186 @@ + 'test-apple-client-id', + 'client_secret' => 'test-apple-client-secret', + 'redirect' => sprintf('/auth/login/%s/callback', self::Provider), + ]); + $user = EntityManager::getRepository(User::class)->findOneBy(['email' => self::LoggedUserEmail]); + Session::start(); + $this->be($user); + } + + protected function tearDown(): void + { + Mockery::close(); + parent::tearDown(); + } + + // ------------------------------------------------------------------------- + // Helpers + // ------------------------------------------------------------------------- + + private function oauth2RequestParams(): array + { + return [ + 'client_id' => self::ClientId, + 'redirect_uri' => self::RedirectUri, + 'response_type' => 'code', + 'scope' => sprintf('%s/resource-server/read', Config::get('app.url')), + ]; + } + + private function memento(): IMementoOAuth2SerializerService + { + return app(IMementoOAuth2SerializerService::class); + } + + /** + * The relying party starts an authorization while the IDP session already belongs to a user: + * the IDP stores the request in session and sends the user to the consent page. + */ + private function startOAuth2FlowAsLoggedUser(): void + { + $this->action('POST', "OAuth2\OAuth2ProviderController@auth", $this->oauth2RequestParams()); + $this->assertResponseStatus(302); + $this->assertTrue($this->memento()->exists(), 'The authorization request should be pending in session.'); + } + + private function webLogout(): void + { + $response = $this->call('GET', '/accounts/user/logout'); + $this->assertResponseStatus(302); + $this->assertEquals(url()->action('UserController@getLogin'), $response->getTargetUrl()); + $this->assertTrue(Auth::guest(), 'The web logout should end the IDP session.'); + } + + private function startSocialLogin(): string + { + $this->call('GET', sprintf('/auth/login/%s', self::Provider)); + $this->assertResponseStatus(302); + $state = Session::get('state'); + $this->assertNotEmpty($state, 'Socialite should have stored its state in session.'); + return $state; + } + + private function mockProviderUser(string $email): void + { + $social_user = (new SocialiteUser())->map([ + 'id' => '001234.abcdef.5678', + 'nickname' => null, + 'name' => 'Tipit Llc', + 'email' => $email, + 'avatar' => null, + ]); + + $driver = Mockery::mock(AppleProvider::class); + $driver->shouldReceive('user')->once()->andReturn($social_user); + Socialite::shouldReceive('driver')->with(self::Provider)->andReturn($driver); + } + + private function postSocialCallback(string $state) + { + return $this->call('POST', sprintf('/auth/login/%s/callback', self::Provider), [ + 'state' => $state, + 'code' => 'c0de', + ]); + } + + // ------------------------------------------------------------------------- + // Tests + // ------------------------------------------------------------------------- + + /** + * Regression: the web logout must not drop the relying party's pending request. + */ + public function testWebLogoutKeepsPendingOAuth2Request() + { + $this->startOAuth2FlowAsLoggedUser(); + + $this->webLogout(); + + $this->assertTrue($this->memento()->exists(), 'The pending OAuth2 request was lost on logout.'); + $state = $this->memento()->load()->getState(); + $this->assertEquals(self::ClientId, $state['client_id']); + $this->assertEquals(self::RedirectUri, $state['redirect_uri']); + } + + /** + * Regression (JP's report, 2026-09-25): logged in as user A, the relying party starts an + * authorization, the user logs out from the IDP page and signs in with Apple as a brand-new + * account. The callback has to send them back into the OAuth2 flow, not to their identity page. + */ + public function testSocialLoginAsNewUserAfterWebLogoutReturnsToRelyingParty() + { + $this->startOAuth2FlowAsLoggedUser(); + $this->webLogout(); + + $state = $this->startSocialLogin(); + $this->mockProviderUser(self::NewUserEmail); + $response = $this->postSocialCallback($state); + + $this->assertResponseStatus(302); + $this->assertEquals(url()->action('OAuth2\OAuth2ProviderController@auth'), $response->getTargetUrl()); + $this->assertTrue(Auth::check()); + $this->assertEquals(self::NewUserEmail, Auth::user()->getEmail()); + } + + /** + * Baseline: a logout outside any authorization flow leaves no OAuth2 state behind. + */ + public function testWebLogoutWithoutPendingRequestLeavesNoOAuth2State() + { + $this->assertFalse($this->memento()->exists()); + + $this->webLogout(); + + $this->assertFalse($this->memento()->exists()); + } +}