Skip to content

Commit 3d6cd09

Browse files
committed
fix(security): revoke tokens and invalidate sessions on password change
- Introduce RevokeUserGrants job (replaces RevokeUserGrantsOnExplicitLogout) with an optional client_id and a reason parameter; remove the guard clause that silently prevented all-client revocation. - Wire RevokeUserGrants to UserPasswordResetSuccessful so all OAuth2 tokens are revoked whenever a password is reset or changed. - Rotate remember_token in User::setPassword() to invalidate remember-me cookies on other devices. - Regenerate the session in UserApiController::updateMe() after a password change to protect against session fixation. - Add DELETE /admin/api/v1/users/me/tokens endpoint with a corresponding "Sign Out All Other Devices" button on the profile page. - Add PasswordChangeRevokeTokenTest covering all eight scenarios from the security specification.
1 parent 34d0c3c commit 3d6cd09

File tree

13 files changed

+469
-93
lines changed

13 files changed

+469
-93
lines changed

app/Http/Controllers/Api/UserApiController.php

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
**/
1414

1515
use App\Http\Controllers\APICRUDController;
16+
use App\Jobs\RevokeUserGrants;
1617
use App\Http\Controllers\Traits\RequestProcessor;
1718
use App\Http\Controllers\UserValidationRulesFactory;
1819
use App\ModelSerializers\SerializerRegistry;
@@ -244,7 +245,23 @@ public function updateMe()
244245
if (!Auth::check())
245246
return $this->error403();
246247

247-
return $this->update(Auth::user()->getId());
248+
$password_changed = request()->filled('password');
249+
$response = $this->update(Auth::user()->getId());
250+
if ($password_changed) {
251+
request()->session()->regenerate();
252+
}
253+
return $response;
254+
}
255+
256+
public function revokeAllMyTokens()
257+
{
258+
if (!Auth::check())
259+
return $this->error403();
260+
261+
$user = Auth::user();
262+
RevokeUserGrants::dispatch($user, null, 'user-initiated session revocation')->afterResponse();
263+
$user->setRememberToken(\Illuminate\Support\Str::random(60));
264+
return $this->deleted();
248265
}
249266

250267
public function updateMyPic(){

app/Http/Controllers/UserController.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
**/
1414

1515
use App\Http\Controllers\OpenId\DiscoveryController;
16+
use App\Jobs\RevokeUserGrants;
1617
use App\Http\Controllers\OpenId\OpenIdController;
1718
use App\Http\Controllers\Traits\JsonResponses;
1819
use App\Http\Utils\CountryList;
@@ -673,7 +674,7 @@ public function getIdentity($identifier)
673674
public function logout()
674675
{
675676
$user = $this->auth_service->getCurrentUser();
676-
//RevokeUserGrantsOnExplicitLogout::dispatch($user)->afterResponse();
677+
RevokeUserGrants::dispatch($user, null, 'explicit logout')->afterResponse();
677678
$this->auth_service->logout();
678679
Session::flush();
679680
Session::regenerate();

app/Jobs/RevokeUserGrants.php

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
<?php namespace App\Jobs;
2+
/*
3+
* Copyright 2024 OpenStack Foundation
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
**/
14+
15+
use Auth\User;
16+
use Illuminate\Bus\Queueable;
17+
use Illuminate\Contracts\Queue\ShouldQueue;
18+
use Illuminate\Foundation\Bus\Dispatchable;
19+
use Illuminate\Queue\InteractsWithQueue;
20+
use Illuminate\Queue\SerializesModels;
21+
use Illuminate\Support\Facades\Log;
22+
use OAuth2\Services\ITokenService;
23+
use Utils\IPHelper;
24+
25+
/**
26+
* Class RevokeUserGrants
27+
* @package App\Jobs
28+
*/
29+
class RevokeUserGrants implements ShouldQueue
30+
{
31+
use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
32+
33+
public $tries = 5;
34+
35+
public $timeout = 0;
36+
37+
/**
38+
* @var int
39+
*/
40+
private int $user_id;
41+
42+
/**
43+
* @var string|null
44+
*/
45+
private ?string $client_id;
46+
47+
/**
48+
* @var string
49+
*/
50+
private string $reason;
51+
52+
/**
53+
* @param User $user
54+
* @param string|null $client_id null = revoke across all clients
55+
* @param string $reason audit message suffix
56+
*/
57+
public function __construct(User $user, ?string $client_id = null, string $reason = 'explicit logout')
58+
{
59+
$this->user_id = $user->getId();
60+
$this->client_id = $client_id;
61+
$this->reason = $reason;
62+
Log::debug(sprintf(
63+
"RevokeUserGrants::constructor user %s client_id %s reason %s",
64+
$this->user_id,
65+
$client_id ?? 'N/A',
66+
$reason
67+
));
68+
}
69+
70+
public function handle(ITokenService $service): void
71+
{
72+
Log::debug("RevokeUserGrants::handle");
73+
74+
try {
75+
$scope = !empty($this->client_id)
76+
? sprintf("client %s", $this->client_id)
77+
: "all clients";
78+
79+
$action = sprintf(
80+
"Revoking all grants for user %s on %s due to %s.",
81+
$this->user_id,
82+
$scope,
83+
$this->reason
84+
);
85+
86+
AddUserAction::dispatch($this->user_id, IPHelper::getUserIp(), $action);
87+
88+
// Emit to OTEL audit log (Elasticsearch) if enabled
89+
if (config('opentelemetry.enabled', false)) {
90+
EmitAuditLogJob::dispatch('audit.security.tokens_revoked', [
91+
'audit.action' => 'revoke_tokens',
92+
'audit.entity' => 'User',
93+
'audit.entity_id' => (string) $this->user_id,
94+
'audit.timestamp' => now()->toISOString(),
95+
'audit.description' => $action,
96+
'audit.reason' => $this->reason,
97+
'audit.scope' => $scope,
98+
'auth.user.id' => $this->user_id,
99+
'client.ip' => IPHelper::getUserIp(),
100+
'elasticsearch.index' => config('opentelemetry.logs.elasticsearch_index', 'logs-audit'),
101+
]);
102+
}
103+
104+
$service->revokeUsersToken($this->user_id, $this->client_id);
105+
} catch (\Exception $ex) {
106+
Log::error($ex);
107+
}
108+
}
109+
110+
public function failed(\Throwable $exception): void
111+
{
112+
Log::error(sprintf("RevokeUserGrants::failed %s", $exception->getMessage()));
113+
}
114+
}

app/Jobs/RevokeUserGrantsOnExplicitLogout.php

Lines changed: 0 additions & 83 deletions
This file was deleted.

app/Listeners/OnUserLogout.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
* limitations under the License.
1313
**/
1414

15-
use App\Jobs\RevokeUserGrantsOnExplicitLogout;
15+
use App\Jobs\RevokeUserGrants;
1616
use Illuminate\Auth\Events\Logout;
1717
use Illuminate\Support\Facades\Log;
1818

@@ -33,6 +33,6 @@ public function handle(Logout $event)
3333
{
3434
$user = $event->user;
3535
Log::debug(sprintf("OnUserLogout::handle user %s (%s)", $user->getEmail(), $user->getId()));
36-
RevokeUserGrantsOnExplicitLogout::dispatch($user);
36+
RevokeUserGrants::dispatch($user, null, 'explicit logout');
3737
}
3838
}

app/Providers/EventServiceProvider.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use App\Events\UserLocked;
2020
use App\Events\UserPasswordResetRequestCreated;
2121
use App\Events\UserPasswordResetSuccessful;
22+
use App\Jobs\RevokeUserGrants;
2223
use App\Events\UserSpamStateUpdated;
2324
use App\Audit\AuditContext;
2425
use App\libs\Auth\Repositories\IUserPasswordResetRequestRepository;
@@ -57,7 +58,7 @@ final class EventServiceProvider extends ServiceProvider
5758
'Illuminate\Database\Events\QueryExecuted' => [
5859
],
5960
'Illuminate\Auth\Events\Logout' => [
60-
//'App\Listeners\OnUserLogout',
61+
'App\Listeners\OnUserLogout',
6162
],
6263
'Illuminate\Auth\Events\Login' => [
6364
'App\Listeners\OnUserLogin',
@@ -169,6 +170,8 @@ public function boot()
169170
if(is_null($user)) return;
170171
if(!$user instanceof User) return;
171172
Mail::queue(new UserPasswordResetMail($user));
173+
// Revoke all OAuth2 tokens for this user across all clients
174+
RevokeUserGrants::dispatch($user, null, 'password change')->afterResponse();
172175
});
173176

174177
Event::listen(OAuth2ClientLocked::class, function($event){

app/libs/Auth/Models/User.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1578,6 +1578,8 @@ public function setPassword(string $password): void
15781578
$this->password_salt = AuthHelper::generateSalt(self::SaltLen, $this->password_enc);
15791579
$this->password = AuthHelper::encrypt_password($password, $this->password_salt, $this->password_enc);
15801580

1581+
$this->setRememberToken(\Illuminate\Support\Str::random(60));
1582+
15811583
$action = 'User set new password.';
15821584
$current_user = Auth::user();
15831585
if($current_user instanceof User) {

app/libs/OAuth2/OAuth2Protocol.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
**/
1414

1515
use App\Http\Utils\UserIPHelperProvider;
16-
use App\Jobs\RevokeUserGrantsOnExplicitLogout;
16+
use App\Jobs\RevokeUserGrants;
1717
use Exception;
1818
use Illuminate\Support\Facades\Log;
1919
use jwa\JSONWebSignatureAndEncryptionAlgorithms;
@@ -1562,11 +1562,9 @@ public function endSession(OAuth2Request $request = null)
15621562
);
15631563
}
15641564

1565-
/*
15661565
if(!is_null($user)){
1567-
RevokeUserGrantsOnExplicitLogout::dispatch($user, $client_id)->afterResponse();
1566+
RevokeUserGrants::dispatch($user, $client_id, 'explicit logout')->afterResponse();
15681567
}
1569-
*/
15701568

15711569
if (!is_null($logged_user)) {
15721570
$this->auth_service->logout();

resources/js/profile/actions.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@ export const revokeToken = async (value, hint) => {
8383
)({'X-CSRF-TOKEN': window.CSFR_TOKEN});
8484
}
8585

86+
export const revokeAllTokens = async () => {
87+
return deleteRawRequest(window.REVOKE_ALL_TOKENS_ENDPOINT)({'X-CSRF-TOKEN': window.CSFR_TOKEN});
88+
}
89+
8690
const normalizeEntity = (entity) => {
8791
entity.public_profile_show_photo = entity.public_profile_show_photo ? 1 : 0;
8892
entity.public_profile_show_fullname = entity.public_profile_show_fullname ? 1 : 0;

resources/js/profile/profile.js

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import RichTextEditor from "../components/rich_text_editor";
2020
import FormControlLabel from "@material-ui/core/FormControlLabel";
2121
import UserAccessTokensGrid from "../components/user_access_tokens_grid";
2222
import UserActionsGrid from "../components/user_actions_grid";
23-
import {getUserActions, getUserAccessTokens, PAGE_SIZE, revokeToken, save} from "./actions";
23+
import {getUserActions, getUserAccessTokens, PAGE_SIZE, revokeToken, revokeAllTokens, save} from "./actions";
2424
import ProfileImageUploader from "./components/profile_image_uploader/profile_image_uploader";
2525
import Navbar from "../components/navbar/navbar";
2626
import Divider from "@material-ui/core/Divider";
@@ -82,6 +82,30 @@ const ProfilePage = ({
8282
},
8383
});
8484

85+
const confirmRevokeAll = () => {
86+
Swal({
87+
title: 'Sign out all other devices?',
88+
html: '<ul style="text-align:left">' +
89+
'<li>All OAuth2 access and refresh tokens will be revoked</li>' +
90+
'<li>All other browser sessions will need to re-authenticate</li>' +
91+
'<li>Your current session will remain active</li>' +
92+
'</ul>',
93+
showCancelButton: true,
94+
confirmButtonColor: '#3085d6',
95+
cancelButtonColor: '#d33',
96+
confirmButtonText: 'Yes, sign out all devices!'
97+
}).then((result) => {
98+
if (result.value) {
99+
revokeAllTokens().then(() => {
100+
Swal('Signed out', 'All other sessions and tokens have been revoked.', 'success');
101+
setAccessTokensListRefresh(!accessTokensListRefresh);
102+
}).catch((err) => {
103+
handleErrorResponse(err);
104+
});
105+
}
106+
});
107+
};
108+
85109
const confirmRevocation = (value) => {
86110
Swal({
87111
title: 'Are you sure to revoke this token?',
@@ -840,6 +864,15 @@ const ProfilePage = ({
840864
onRevoke={confirmRevocation}
841865
/>
842866
</Grid>
867+
<Grid item container justifyContent="flex-end">
868+
<Button
869+
variant="outlined"
870+
color="secondary"
871+
onClick={confirmRevokeAll}
872+
>
873+
Sign Out All Other Devices
874+
</Button>
875+
</Grid>
843876
<Divider/>
844877
<Grid item container>
845878
<Typography component="h1" variant="h5">

0 commit comments

Comments
 (0)