diff --git a/app/Http/Controllers/Api/UserApiController.php b/app/Http/Controllers/Api/UserApiController.php index 12fc34aa..b32c7307 100644 --- a/app/Http/Controllers/Api/UserApiController.php +++ b/app/Http/Controllers/Api/UserApiController.php @@ -247,6 +247,15 @@ public function updateMe() return $this->update(Auth::user()->getId()); } + public function revokeAllMyTokens() + { + if (!Auth::check()) + return $this->error403(); + + $this->service->revokeAllGrantsOnSessionRevocation(Auth::user()->getId()); + return $this->deleted(); + } + public function updateMyPic(){ if (!Auth::check()) return $this->error403(); diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 69368891..5c9dabf2 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -13,6 +13,7 @@ **/ use App\Http\Controllers\OpenId\DiscoveryController; +use App\Jobs\RevokeUserGrantsOnExplicitLogout; use App\Http\Controllers\OpenId\OpenIdController; use App\Http\Controllers\Traits\JsonResponses; use App\Http\Utils\CountryList; @@ -673,7 +674,7 @@ public function getIdentity($identifier) public function logout() { $user = $this->auth_service->getCurrentUser(); - //RevokeUserGrantsOnExplicitLogout::dispatch($user)->afterResponse(); + // RevokeUserGrantsOnExplicitLogout::dispatch($user)->afterResponse(); $this->auth_service->logout(); Session::flush(); Session::regenerate(); diff --git a/app/Jobs/RevokeUserGrants.php b/app/Jobs/RevokeUserGrants.php new file mode 100644 index 00000000..13a3dc23 --- /dev/null +++ b/app/Jobs/RevokeUserGrants.php @@ -0,0 +1,122 @@ +user_id = $user->getId(); + $this->client_id = $client_id; + $this->reason = $reason; + $this->client_ip = IPHelper::getUserIp(); + Log::debug(sprintf( + "RevokeUserGrants::constructor user %s client_id %s reason %s", + $this->user_id, + $client_id ?? 'N/A', + $reason + )); + } + + public function handle(ITokenService $service): void + { + Log::debug("RevokeUserGrants::handle"); + + try { + $service->revokeUsersToken($this->user_id, $this->client_id); + } catch (\Exception $ex) { + Log::warning(sprintf("RevokeUserGrants::handle attempt %d failed: %s", + $this->attempts(), $ex->getMessage())); + throw $ex; + } + + $scope = !empty($this->client_id) + ? sprintf("client %s", $this->client_id) + : "all clients"; + + $action = sprintf( + "Revoking all grants for user %s on %s due to %s.", + $this->user_id, + $scope, + $this->reason + ); + + AddUserAction::dispatch($this->user_id, $this->client_ip, $action); + + // Emit to OTEL audit log (Elasticsearch) if enabled + if (config('opentelemetry.enabled', false)) { + EmitAuditLogJob::dispatch('audit.security.tokens_revoked', [ + 'audit.action' => 'revoke_tokens', + 'audit.entity' => 'User', + 'audit.entity_id' => (string) $this->user_id, + 'audit.timestamp' => now()->toISOString(), + 'audit.description' => $action, + 'audit.reason' => $this->reason, + 'audit.scope' => $scope, + 'auth.user.id' => $this->user_id, + 'client.ip' => $this->client_ip, + 'elasticsearch.index' => config('opentelemetry.logs.elasticsearch_index', 'logs-audit'), + ]); + } + } + + public function failed(\Throwable $exception): void + { + Log::error(sprintf("RevokeUserGrants::failed %s", $exception->getMessage())); + } +} diff --git a/app/Jobs/RevokeUserGrantsOnExplicitLogout.php b/app/Jobs/RevokeUserGrantsOnExplicitLogout.php index 33f139e8..965eb20d 100644 --- a/app/Jobs/RevokeUserGrantsOnExplicitLogout.php +++ b/app/Jobs/RevokeUserGrantsOnExplicitLogout.php @@ -13,71 +13,18 @@ **/ use Auth\User; -use Illuminate\Bus\Queueable; -use Illuminate\Contracts\Queue\ShouldQueue; -use Illuminate\Foundation\Bus\Dispatchable; -use Illuminate\Queue\InteractsWithQueue; -use Illuminate\Queue\SerializesModels; -use Illuminate\Support\Facades\Log; -use OAuth2\Services\ITokenService; -use Utils\IPHelper; /** - * Class RevokeUserGrants + * Class RevokeUserGrantsOnExplicitLogout + * Revokes all OAuth2 grants for a user when they explicitly log out. * @package App\Jobs */ -class RevokeUserGrantsOnExplicitLogout implements ShouldQueue +class RevokeUserGrantsOnExplicitLogout extends RevokeUserGrants { - use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; + const REASON = 'explicit logout'; - public $tries = 5; - - public $timeout = 0; - - /** - * @var int - */ - private $user_id; - - /** - * @var string - */ - private $client_id; - - - /** - * @param User $user - * @param string|null $client_id - */ - public function __construct(User $user, ?string $client_id = null){ - $this->user_id = $user->getId(); - $this->client_id = $client_id; - Log::debug(sprintf("RevokeUserGrants::constructor user %s client id %s", $this->user_id, !empty($client_id)? $client_id :"N/A")); - } - - public function handle(ITokenService $service){ - Log::debug(sprintf("RevokeUserGrants::handle")); - - if(empty($this->client_id)) { - return; - } - try{ - $action = sprintf - ( - "Revoking all grants for user %s on %s due explicit Log out.", - $this->user_id, sprintf("Client %s", $this->client_id) - ); - - AddUserAction::dispatch($this->user_id, IPHelper::getUserIp(), $action); - $service->revokeUsersToken($this->user_id, $this->client_id); - } - catch (\Exception $ex) { - Log::error($ex); - } - } - - public function failed(\Throwable $exception) + public function __construct(User $user, ?string $client_id = null) { - Log::error(sprintf( "RevokeUserGrants::failed %s", $exception->getMessage())); + parent::__construct($user, $client_id, self::REASON); } -} \ No newline at end of file +} diff --git a/app/Jobs/RevokeUserGrantsOnPasswordChange.php b/app/Jobs/RevokeUserGrantsOnPasswordChange.php new file mode 100644 index 00000000..bb059ac8 --- /dev/null +++ b/app/Jobs/RevokeUserGrantsOnPasswordChange.php @@ -0,0 +1,30 @@ + [ ], 'Illuminate\Auth\Events\Logout' => [ - //'App\Listeners\OnUserLogout', + // 'App\Listeners\OnUserLogout', ], 'Illuminate\Auth\Events\Login' => [ 'App\Listeners\OnUserLogin', @@ -169,6 +170,8 @@ public function boot() if(is_null($user)) return; if(!$user instanceof User) return; Mail::queue(new UserPasswordResetMail($user)); + // Revoke all OAuth2 tokens for this user across all clients + RevokeUserGrantsOnPasswordChange::dispatch($user)->afterResponse(); }); Event::listen(OAuth2ClientLocked::class, function($event){ diff --git a/app/Services/OpenId/UserService.php b/app/Services/OpenId/UserService.php index 2e09d8c5..cb5f5654 100644 --- a/app/Services/OpenId/UserService.php +++ b/app/Services/OpenId/UserService.php @@ -15,6 +15,7 @@ use App\Events\UserEmailUpdated; use App\Events\UserPasswordResetSuccessful; use App\Jobs\AddUserAction; +use App\Jobs\RevokeUserGrantsOnSessionRevocation; use App\Jobs\PublishUserDeleted; use App\Jobs\PublishUserUpdated; use App\libs\Auth\Factories\UserFactory; @@ -33,6 +34,7 @@ use Illuminate\Support\Facades\Config; use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Log; +use Illuminate\Support\Facades\Session; use Illuminate\Support\Facades\Mail; use Illuminate\Support\Facades\Storage; use models\exceptions\EntityNotFoundException; @@ -292,7 +294,9 @@ public function create(array $payload): IEntity */ public function update(int $id, array $payload): IEntity { - $user = $this->tx_service->transaction(function () use ($id, $payload) { + $password_changed = false; + + $user = $this->tx_service->transaction(function () use ($id, $payload, &$password_changed) { $user = $this->repository->getById($id); @@ -372,12 +376,17 @@ public function update(int $id, array $payload): IEntity if ($former_password != $user->getPassword()) { Log::warning(sprintf("UserService::update use id %s - password changed", $id)); - Event::dispatch(new UserPasswordResetSuccessful($user->getId())); + $password_changed = true; } return $user; }); + if ($password_changed) { + Session::regenerate(); + Event::dispatch(new UserPasswordResetSuccessful($user->getId())); + } + try { if (Config::get("queue.enable_message_broker", false) == true) PublishUserUpdated::dispatch($user)->onConnection('message_broker'); @@ -473,6 +482,21 @@ public function updateProfilePhoto($user_id, UploadedFile $file, $max_file_size return $user; } + public function revokeAllGrantsOnSessionRevocation(int $user_id): void + { + $user = $this->tx_service->transaction(function () use ($user_id) { + $user = $this->repository->getById($user_id); + if (!$user instanceof User) + throw new EntityNotFoundException("User not found."); + + $user->setRememberToken(\Illuminate\Support\Str::random(60)); + + return $user; + }); + + RevokeUserGrantsOnSessionRevocation::dispatch($user)->afterResponse(); + } + public function notifyMonitoredSecurityGroupActivity ( string $action, diff --git a/app/libs/Auth/Models/User.php b/app/libs/Auth/Models/User.php index 919a0efa..c1d70d96 100644 --- a/app/libs/Auth/Models/User.php +++ b/app/libs/Auth/Models/User.php @@ -1578,6 +1578,8 @@ public function setPassword(string $password): void $this->password_salt = AuthHelper::generateSalt(self::SaltLen, $this->password_enc); $this->password = AuthHelper::encrypt_password($password, $this->password_salt, $this->password_enc); + $this->setRememberToken(\Illuminate\Support\Str::random(60)); + $action = 'User set new password.'; $current_user = Auth::user(); if($current_user instanceof User) { diff --git a/app/libs/OpenId/Services/IUserService.php b/app/libs/OpenId/Services/IUserService.php index 1ed21a0a..1d301610 100644 --- a/app/libs/OpenId/Services/IUserService.php +++ b/app/libs/OpenId/Services/IUserService.php @@ -105,4 +105,12 @@ public function notifyMonitoredSecurityGroupActivity( string $action_by, ): void; + /** + * Rotates the user's remember token (persisted) and schedules revocation + * of all OAuth2 grants for that user. + * @param int $user_id + * @throws EntityNotFoundException + */ + public function revokeAllGrantsOnSessionRevocation(int $user_id): void; + } \ No newline at end of file diff --git a/resources/js/profile/actions.js b/resources/js/profile/actions.js index b7fa2ba1..c1c6c2f0 100644 --- a/resources/js/profile/actions.js +++ b/resources/js/profile/actions.js @@ -83,6 +83,10 @@ export const revokeToken = async (value, hint) => { )({'X-CSRF-TOKEN': window.CSFR_TOKEN}); } +export const revokeAllTokens = async () => { + return deleteRawRequest(window.REVOKE_ALL_TOKENS_ENDPOINT)({'X-CSRF-TOKEN': window.CSFR_TOKEN}); +} + const normalizeEntity = (entity) => { entity.public_profile_show_photo = entity.public_profile_show_photo ? 1 : 0; entity.public_profile_show_fullname = entity.public_profile_show_fullname ? 1 : 0; diff --git a/resources/js/profile/profile.js b/resources/js/profile/profile.js index c9e89c87..a530d04a 100644 --- a/resources/js/profile/profile.js +++ b/resources/js/profile/profile.js @@ -20,7 +20,7 @@ import RichTextEditor from "../components/rich_text_editor"; import FormControlLabel from "@material-ui/core/FormControlLabel"; import UserAccessTokensGrid from "../components/user_access_tokens_grid"; import UserActionsGrid from "../components/user_actions_grid"; -import {getUserActions, getUserAccessTokens, PAGE_SIZE, revokeToken, save} from "./actions"; +import {getUserActions, getUserAccessTokens, PAGE_SIZE, revokeToken, revokeAllTokens, save} from "./actions"; import ProfileImageUploader from "./components/profile_image_uploader/profile_image_uploader"; import Navbar from "../components/navbar/navbar"; import Divider from "@material-ui/core/Divider"; @@ -82,6 +82,30 @@ const ProfilePage = ({ }, }); + const confirmRevokeAll = () => { + Swal({ + title: 'Sign out all other devices?', + html: '', + showCancelButton: true, + confirmButtonColor: '#3085d6', + cancelButtonColor: '#d33', + confirmButtonText: 'Yes, sign out all devices!' + }).then((result) => { + if (result.value) { + revokeAllTokens().then(() => { + Swal('Signed out', 'All other sessions and tokens have been revoked.', 'success'); + setAccessTokensListRefresh(!accessTokensListRefresh); + }).catch((err) => { + handleErrorResponse(err); + }); + } + }); + }; + const confirmRevocation = (value) => { Swal({ title: 'Are you sure to revoke this token?', @@ -840,6 +864,15 @@ const ProfilePage = ({ onRevoke={confirmRevocation} /> + + + diff --git a/resources/views/profile.blade.php b/resources/views/profile.blade.php index 811648d0..335094e7 100644 --- a/resources/views/profile.blade.php +++ b/resources/views/profile.blade.php @@ -109,6 +109,7 @@ window.GET_USER_ACTIONS_ENDPOINT = '{{URL::action("Api\UserActionApiController@getActionsByCurrentUser")}}'; window.GET_USER_ACCESS_TOKENS_ENDPOINT = '{{URL::action("Api\ClientApiController@getAccessTokensByCurrentUser")}}'; window.REVOKE_ACCESS_TOKENS_ENDPOINT = '{!!URL::action("Api\UserApiController@revokeMyToken", ["value"=>"@value", "hint"=>"@hint"])!!}'; + window.REVOKE_ALL_TOKENS_ENDPOINT = '{{URL::action("Api\UserApiController@revokeAllMyTokens")}}'; window.SAVE_PROFILE_ENDPOINT = '{!!URL::action("Api\UserApiController@updateMe")!!}'; window.SAVE_PIC_ENDPOINT = '{!!URL::action("Api\UserApiController@updateMyPic")!!}'; window.CSFR_TOKEN = document.head.querySelector('meta[name="csrf-token"]').content; diff --git a/routes/web.php b/routes/web.php index f8473c4c..b49a3547 100644 --- a/routes/web.php +++ b/routes/web.php @@ -187,6 +187,7 @@ Route::post('', ['middleware' => ['openstackid.currentuser.serveradmin.json'], 'uses' => "UserApiController@create"]); Route::group(['prefix' => 'me'], function () { + Route::delete('tokens', "UserApiController@revokeAllMyTokens"); Route::delete('tokens/{value}', "UserApiController@revokeMyToken"); Route::put('', "UserApiController@updateMe"); Route::put('pic', "UserApiController@updateMyPic"); diff --git a/tests/PasswordChangeRevokeTokenTest.php b/tests/PasswordChangeRevokeTokenTest.php new file mode 100644 index 00000000..ad54021b --- /dev/null +++ b/tests/PasswordChangeRevokeTokenTest.php @@ -0,0 +1,277 @@ +test_user = $user_repository->findOneBy(['identifier' => 'sebastian.marcet']); + $this->be($this->test_user, 'web'); + } + + protected function tearDown(): void + { + $this->addToAssertionCount(Mockery::getContainer()->mockery_getExpectationCount()); + Mockery::close(); + parent::tearDown(); + } + + // ------------------------------------------------------------------------- + // Test 1: Dispatching UserPasswordResetSuccessful fires RevokeUserGrantsOnPasswordChange + // ------------------------------------------------------------------------- + + /** + * When a UserPasswordResetSuccessful event is fired the EventServiceProvider + * listener must schedule a RevokeUserGrantsOnPasswordChange job for all clients. + */ + public function testPasswordResetEventDispatchesRevokeUserGrantsJob(): void + { + Event::dispatch(new UserPasswordResetSuccessful($this->test_user->getId())); + + // afterResponse() registers a terminating callback; fire it now. + app()->terminate(); + + Queue::assertPushed(RevokeUserGrantsOnPasswordChange::class, function (RevokeUserGrantsOnPasswordChange $job) { + $ref = new \ReflectionClass(\App\Jobs\RevokeUserGrants::class); + $userId = $ref->getProperty('user_id'); + $clientId = $ref->getProperty('client_id'); + $userId->setAccessible(true); + $clientId->setAccessible(true); + + return $userId->getValue($job) === $this->test_user->getId() + && $clientId->getValue($job) === null; + }); + } + + // ------------------------------------------------------------------------- + // Test 2: PUT /admin/api/v1/users/me with password schedules RevokeUserGrantsOnPasswordChange + // ------------------------------------------------------------------------- + + /** + * Posting a new password via the profile API must schedule a + * RevokeUserGrantsOnPasswordChange job so tokens from other sessions are revoked. + */ + public function testProfilePasswordChangePutsRevokeUserGrantsJobOnQueue(): void + { + $payload = [ + 'first_name' => $this->test_user->getFirstName(), + 'last_name' => $this->test_user->getLastName(), + 'email' => $this->test_user->getEmail(), + 'current_password' => '1Qaz2wsx!', + 'password' => 'NewP@ssw0rd!99', + 'password_confirmation' => 'NewP@ssw0rd!99', + ]; + + $this->put('/admin/api/v1/users/me', $payload) + ->assertResponseStatus(201); + + // The listener for UserPasswordResetSuccessful uses afterResponse(). + app()->terminate(); + + Queue::assertPushed(RevokeUserGrantsOnPasswordChange::class, function (RevokeUserGrantsOnPasswordChange $job) { + $ref = new \ReflectionClass(\App\Jobs\RevokeUserGrants::class); + $clientId = $ref->getProperty('client_id'); + $clientId->setAccessible(true); + + return $clientId->getValue($job) === null; + }); + } + + // ------------------------------------------------------------------------- + // Test 3: setPassword() rotates the remember_token + // ------------------------------------------------------------------------- + + /** + * Calling User::setPassword() must rotate the remember_token so that + * "remember me" cookies issued to other devices become invalid. + */ + public function testSetPasswordRotatesRememberToken(): void + { + $original_token = $this->test_user->getRememberToken(); + + $this->test_user->setPassword('AnotherS3cur3!Pass'); + + $new_token = $this->test_user->getRememberToken(); + + $this->assertNotEmpty($new_token); + $this->assertNotEquals($original_token, $new_token); + } + + // ------------------------------------------------------------------------- + // Test 4: Current session is preserved (regenerated) after password change + // ------------------------------------------------------------------------- + + /** + * After a profile password change the API response must be successful and + * the user must remain authenticated (session is regenerated, not destroyed). + */ + public function testCurrentSessionPreservedAfterPasswordChange(): void + { + $payload = [ + 'first_name' => $this->test_user->getFirstName(), + 'last_name' => $this->test_user->getLastName(), + 'email' => $this->test_user->getEmail(), + 'current_password' => '1Qaz2wsx!', + 'password' => 'Preserved@Sess10n!', + 'password_confirmation' => 'Preserved@Sess10n!', + ]; + + $this->put('/admin/api/v1/users/me', $payload) + ->assertResponseStatus(201); + + // The currently authenticated user must still be set after the call. + $this->assertTrue(\Illuminate\Support\Facades\Auth::check()); + $this->assertEquals($this->test_user->getId(), \Illuminate\Support\Facades\Auth::id()); + } + + // ------------------------------------------------------------------------- + // Test 5: DELETE /admin/api/v1/users/me/tokens schedules RevokeUserGrantsOnSessionRevocation + // ------------------------------------------------------------------------- + + /** + * The "sign out all other devices" endpoint must respond with 204 and + * schedule a RevokeUserGrantsOnSessionRevocation job for all clients. + */ + public function testBulkRevokeEndpointSchedulesRevokeUserGrantsJob(): void + { + $this->delete('/admin/api/v1/users/me/tokens') + ->assertResponseStatus(204); + + app()->terminate(); + + Queue::assertPushed(RevokeUserGrantsOnSessionRevocation::class, function (RevokeUserGrantsOnSessionRevocation $job) { + $ref = new \ReflectionClass(\App\Jobs\RevokeUserGrants::class); + $userId = $ref->getProperty('user_id'); + $clientId = $ref->getProperty('client_id'); + $userId->setAccessible(true); + $clientId->setAccessible(true); + + return $userId->getValue($job) === $this->test_user->getId() + && $clientId->getValue($job) === null; + }); + } + + // ------------------------------------------------------------------------- + // Test 6: RevokeUserGrantsOnExplicitLogout passes client_id to token service + // ------------------------------------------------------------------------- + + /** + * When constructed with a specific client_id the job must call + * ITokenService::revokeUsersToken($user_id, $client_id). + */ + public function testRevokeUserGrantsJobPassesClientIdToTokenService(): void + { + $client_id = 'test-client-id'; + + $mock_service = Mockery::mock(ITokenService::class); + $mock_service->shouldReceive('revokeUsersToken') + ->once() + ->with($this->test_user->getId(), $client_id); + + $job = new RevokeUserGrantsOnExplicitLogout($this->test_user, $client_id); + $job->handle($mock_service); + } + + // ------------------------------------------------------------------------- + // Test 7: RevokeUserGrantsOnPasswordChange passes null client_id to token service + // ------------------------------------------------------------------------- + + /** + * RevokeUserGrantsOnPasswordChange must call + * ITokenService::revokeUsersToken($user_id, null), revoking across all clients. + */ + public function testRevokeUserGrantsJobPassesNullClientIdToTokenService(): void + { + $mock_service = Mockery::mock(ITokenService::class); + $mock_service->shouldReceive('revokeUsersToken') + ->once() + ->with($this->test_user->getId(), null); + + $job = new RevokeUserGrantsOnPasswordChange($this->test_user); + $job->handle($mock_service); + } + + // ------------------------------------------------------------------------- + // Test 8a: OTEL audit job is dispatched when opentelemetry is enabled + // ------------------------------------------------------------------------- + + /** + * When opentelemetry.enabled is true, the job must dispatch an EmitAuditLogJob + * with the correct log message and audit fields. + */ + public function testOtelAuditJobDispatchedWhenOpentelemetryEnabled(): void + { + Config::set('opentelemetry.enabled', true); + + $mock_service = Mockery::mock(ITokenService::class); + $mock_service->shouldReceive('revokeUsersToken')->once(); + + $job = new RevokeUserGrantsOnPasswordChange($this->test_user); + $job->handle($mock_service); + + Queue::assertPushed(EmitAuditLogJob::class, function (EmitAuditLogJob $emitted) { + return $emitted->logMessage === 'audit.security.tokens_revoked' + && $emitted->auditData['audit.action'] === 'revoke_tokens' + && $emitted->auditData['audit.entity'] === 'User' + && $emitted->auditData['audit.entity_id'] === (string) $this->test_user->getId() + && $emitted->auditData['audit.reason'] === 'password change' + && $emitted->auditData['auth.user.id'] === $this->test_user->getId(); + }); + } + + // ------------------------------------------------------------------------- + // Test 8b: OTEL audit job is NOT dispatched when opentelemetry is disabled + // ------------------------------------------------------------------------- + + /** + * When opentelemetry.enabled is false, the job must not dispatch any EmitAuditLogJob. + */ + public function testOtelAuditJobNotDispatchedWhenOpentelemetryDisabled(): void + { + Config::set('opentelemetry.enabled', false); + + $mock_service = Mockery::mock(ITokenService::class); + $mock_service->shouldReceive('revokeUsersToken')->once(); + + $job = new RevokeUserGrantsOnPasswordChange($this->test_user); + $job->handle($mock_service); + + Queue::assertNotPushed(EmitAuditLogJob::class); + } +}