Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion app/Http/Controllers/Api/UserApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
**/

use App\Http\Controllers\APICRUDController;
use App\Jobs\RevokeUserGrants;
use App\Http\Controllers\Traits\RequestProcessor;
use App\Http\Controllers\UserValidationRulesFactory;
use App\ModelSerializers\SerializerRegistry;
Expand Down Expand Up @@ -244,7 +245,23 @@ public function updateMe()
if (!Auth::check())
return $this->error403();

return $this->update(Auth::user()->getId());
$password_changed = request()->filled('password');
$response = $this->update(Auth::user()->getId());
if ($password_changed) {
request()->session()->regenerate();
Copy link
Copy Markdown
Collaborator

@smarcet smarcet Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mulldug move the session regeneration ( Session::regenerate() ) into the service layer where the password is actually changed

Event::dispatch(new UserPasswordResetSuccessful($user->getId()));

}
return $response;
}

public function revokeAllMyTokens()
{
if (!Auth::check())
return $this->error403();

$user = Auth::user();
RevokeUserGrants::dispatch($user, null, 'user-initiated session revocation')->afterResponse();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace RevokeUserGrants
with RevokeUserGrantsOnUserInitiatedRequest::dispatch($user)
see rationale here https://github.com/OpenStackweb/openstackid/pull/117/changes#diff-c9a5faef38fdfdc240cc8783a852d995bb99a0640daa0f5de44c7ab04d7103b2R29

$user->setRememberToken(\Illuminate\Support\Str::random(60));
return $this->deleted();
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

public function updateMyPic(){
Expand Down
3 changes: 2 additions & 1 deletion app/Http/Controllers/UserController.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
**/

use App\Http\Controllers\OpenId\DiscoveryController;
use App\Jobs\RevokeUserGrants;
use App\Http\Controllers\OpenId\OpenIdController;
use App\Http\Controllers\Traits\JsonResponses;
use App\Http\Utils\CountryList;
Expand Down Expand Up @@ -673,7 +674,7 @@ public function getIdentity($identifier)
public function logout()
{
$user = $this->auth_service->getCurrentUser();
//RevokeUserGrantsOnExplicitLogout::dispatch($user)->afterResponse();
RevokeUserGrants::dispatch($user, null, 'explicit logout')->afterResponse();
Comment thread
smarcet marked this conversation as resolved.
Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@this reintroduce this bug
please revert this change
1d8461d

$this->auth_service->logout();
Session::flush();
Session::regenerate();
Expand Down
114 changes: 114 additions & 0 deletions app/Jobs/RevokeUserGrants.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
<?php namespace App\Jobs;
/*
* Copyright 2024 OpenStack Foundation
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
**/

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
* @package App\Jobs
*/
class RevokeUserGrants implements ShouldQueue
Copy link
Copy Markdown
Collaborator

@smarcet smarcet Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mulldug please make this class abstract
and create:

  • RevokeUserGrantsOnExplicitLogout that inherits from this and set the reason at constructor
  • also do the same and create an specialization called RevokeUserGrantsOnPasswordChange and
    RevokeUserGrantsOnUserInitiatedRequest
abstract class RevokeUserGrants implements ShouldQueue {
//....
protected string $reason
}

final class RevokeUserGrantsOnExplicitLogout extends 
RevokeUserGrants {

public function __construct($user, $client_id){
    parent::__construct($user, $client_id, 'explicit logout');
}

}

final class RevokeUserGrantsOnPasswordChange extends 
RevokeUserGrants {

public function __construct($user, $client_id){
    parent::__construct($user, null, ''password change'');
}

}

final class RevokeUserGrantsOnUserInitiatedRequest extends 
RevokeUserGrants {

public function __construct($user, $client_id){
    parent::__construct($user, null, 'user-initiated session revocation');
}

}

flags are antipatterns ( code smell )

{
use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;

public $tries = 5;

public $timeout = 0;

/**
* @var int
*/
private int $user_id;

/**
* @var string|null
*/
private ?string $client_id;

/**
* @var string
*/
private string $reason;

/**
* @param User $user
* @param string|null $client_id null = revoke across all clients
* @param string $reason audit message suffix
*/
public function __construct(User $user, ?string $client_id = null, string $reason = 'explicit logout')
{
$this->user_id = $user->getId();
$this->client_id = $client_id;
$this->reason = $reason;
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 {
$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, IPHelper::getUserIp(), $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' => IPHelper::getUserIp(),
'elasticsearch.index' => config('opentelemetry.logs.elasticsearch_index', 'logs-audit'),
]);
}

$service->revokeUsersToken($this->user_id, $this->client_id);
} catch (\Exception $ex) {
Log::error($ex);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mulldug exception should be propagated in order to activate the retry mechanism

}
}

public function failed(\Throwable $exception): void
{
Log::error(sprintf("RevokeUserGrants::failed %s", $exception->getMessage()));
}
}
83 changes: 0 additions & 83 deletions app/Jobs/RevokeUserGrantsOnExplicitLogout.php

This file was deleted.

4 changes: 2 additions & 2 deletions app/Listeners/OnUserLogout.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* limitations under the License.
**/

use App\Jobs\RevokeUserGrantsOnExplicitLogout;
use App\Jobs\RevokeUserGrants;
use Illuminate\Auth\Events\Logout;
use Illuminate\Support\Facades\Log;

Expand All @@ -33,6 +33,6 @@ public function handle(Logout $event)
{
$user = $event->user;
Log::debug(sprintf("OnUserLogout::handle user %s (%s)", $user->getEmail(), $user->getId()));
RevokeUserGrantsOnExplicitLogout::dispatch($user);
RevokeUserGrants::dispatch($user, null, 'explicit logout');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace here RevokeUserGrants with

 RevokeUserGrantsOnExplicitLogout::dispatch($user, $client_id')->afterResponse();

see https://github.com/OpenStackweb/openstackid/pull/117/changes#r2913356525 for rationale

}
}
5 changes: 4 additions & 1 deletion app/Providers/EventServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use App\Events\UserLocked;
use App\Events\UserPasswordResetRequestCreated;
use App\Events\UserPasswordResetSuccessful;
use App\Jobs\RevokeUserGrants;
use App\Events\UserSpamStateUpdated;
use App\Audit\AuditContext;
use App\libs\Auth\Repositories\IUserPasswordResetRequestRepository;
Expand Down Expand Up @@ -57,7 +58,7 @@ final class EventServiceProvider extends ServiceProvider
'Illuminate\Database\Events\QueryExecuted' => [
],
'Illuminate\Auth\Events\Logout' => [
//'App\Listeners\OnUserLogout',
'App\Listeners\OnUserLogout',
Copy link
Copy Markdown
Collaborator

@smarcet smarcet Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mulldug please rollback this
OnUserLogout listener revokes tokens on logout, contradicting PR objectives.

],
Comment thread
coderabbitai[bot] marked this conversation as resolved.
'Illuminate\Auth\Events\Login' => [
'App\Listeners\OnUserLogin',
Expand Down Expand Up @@ -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
RevokeUserGrants::dispatch($user, null, 'password change')->afterResponse();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace here RevokeUserGrants with

 RevokeUserGrantsOnPasswordChange::dispatch($user)->afterResponse();

see https://github.com/OpenStackweb/openstackid/pull/117/changes#r2913356525 for rationale

});

Event::listen(OAuth2ClientLocked::class, function($event){
Expand Down
2 changes: 2 additions & 0 deletions app/libs/Auth/Models/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 2 additions & 4 deletions app/libs/OAuth2/OAuth2Protocol.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
**/

use App\Http\Utils\UserIPHelperProvider;
use App\Jobs\RevokeUserGrantsOnExplicitLogout;
use App\Jobs\RevokeUserGrants;
use Exception;
use Illuminate\Support\Facades\Log;
use jwa\JSONWebSignatureAndEncryptionAlgorithms;
Expand Down Expand Up @@ -1562,11 +1562,9 @@ public function endSession(OAuth2Request $request = null)
);
}

/*
if(!is_null($user)){
RevokeUserGrantsOnExplicitLogout::dispatch($user, $client_id)->afterResponse();
RevokeUserGrants::dispatch($user, $client_id, 'explicit logout')->afterResponse();
Comment thread
smarcet marked this conversation as resolved.
Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace here RevokeUserGrants with

 RevokeUserGrantsOnExplicitLogout::dispatch($user, $client_id')->afterResponse();

see https://github.com/OpenStackweb/openstackid/pull/117/changes#r2913356525 for rationale

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mulldug please rever this change it breaks this fix
1d8461d

}
*/

if (!is_null($logged_user)) {
$this->auth_service->logout();
Expand Down
4 changes: 4 additions & 0 deletions resources/js/profile/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
35 changes: 34 additions & 1 deletion resources/js/profile/profile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -82,6 +82,30 @@ const ProfilePage = ({
},
});

const confirmRevokeAll = () => {
Swal({
title: 'Sign out all other devices?',
html: '<ul style="text-align:left">' +
'<li>All OAuth2 access and refresh tokens will be revoked</li>' +
'<li>All other browser sessions will need to re-authenticate</li>' +
'<li>Your current session will remain active</li>' +
'</ul>',
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?',
Expand Down Expand Up @@ -840,6 +864,15 @@ const ProfilePage = ({
onRevoke={confirmRevocation}
/>
</Grid>
<Grid item container justifyContent="flex-end">
<Button
variant="outlined"
color="secondary"
onClick={confirmRevokeAll}
>
Sign Out All Other Devices
</Button>
</Grid>
<Divider/>
<Grid item container>
<Typography component="h1" variant="h5">
Expand Down
1 change: 1 addition & 0 deletions resources/views/profile.blade.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading
Loading