Skip to content

Add reset MFA button for admin s on user profile edit #6056

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
clauvaldez wants to merge 1 commit into
base: development
Choose a base branch
from
+39 −0
Open

Add reset MFA button for admin s on user profile edit #6056

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 13 additions & 0 deletions app/Users/Controllers/UserController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,4 +208,17 @@ public function destroy(Request $request, int $id)

return redirect('/settings/users');
}

/**
* Reset MFA for the specified user.
*/
public function resetMfa(Request $request, int $id)
{
$this->checkPermission(Permission::UsersManage);
$user = $this->userRepo->getById($id);
// Resetear el 2FA del usuario
Copy link
Copy Markdown
Member

@ssddanbrown ssddanbrown Apr 19, 2026

Choose a reason for hiding this comment

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

Remove this comment, I think it's a bit redundant based on context and I'd prefer to keep comments in one language.

$user->mfaValues()->delete();
session()->flash('success', trans('settings.users_mfa_reset_success', ['userName' => $user->name]));
Copy link
Copy Markdown
Member

@ssddanbrown ssddanbrown Apr 19, 2026

Choose a reason for hiding this comment

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

Please can you use the $this->showSuccessNotification method instead to show a notification

return redirect()->back();
Copy link
Copy Markdown
Member

@ssddanbrown ssddanbrown Apr 19, 2026

Choose a reason for hiding this comment

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

Can we redirect directly to the user edit view?
Redirect backs like this can go wonky in a range of scenarios, so best to specify a location where known.

}
}
5 changes: 5 additions & 0 deletions lang/en/settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,11 @@
'users_mfa_desc' => 'Setup multi-factor authentication as an extra layer of security for your user account.',
'users_mfa_x_methods' => ':count method configured|:count methods configured',
'users_mfa_configure' => 'Configure Methods',
'users_mfa_reset' => 'Reset 2FA',
'users_mfa_reset_desc' => 'Reset and clear all configured MFA methods for :userName. They will be prompted to reconfigure on next login.',
Copy link
Copy Markdown
Member

@ssddanbrown ssddanbrown Apr 19, 2026

Choose a reason for hiding this comment

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

This is kind of incorrect. They will only be prompted to reconfigure on login if MFA is required for their role.

'users_mfa_reset_confirm' => 'Are you sure you want to reset 2FA for :userName?',
'users_mfa_reset_success' => '2FA has been reset for :userName',
'users_mfa_reset_error' => 'Failed to reset 2FA for :userName',
Comment on lines +266 to +270
Copy link
Copy Markdown
Member

@ssddanbrown ssddanbrown Apr 19, 2026

Choose a reason for hiding this comment

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

Please can you use "multi-factor authentication" instead of "2FA" to keep the language used consistent in the app.

Also, avoid passing through the username here. We only do that where necessary, to keep translations simpler and avoid other issues, but it can be implied by context here so I don't think it's needed.


// API Tokens
'user_api_token_create' => 'Create API Token',
Expand Down
20 changes: 20 additions & 0 deletions resources/views/users/edit.blade.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,26 @@ class="button outline">{{ trans('settings.users_mfa_configure') }}</a>
</div>
</div>

@if(user()->hasSystemRole('admin'))
Copy link
Copy Markdown
Member

@ssddanbrown ssddanbrown Apr 19, 2026

Choose a reason for hiding this comment

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

This does not align with the actual permission used in the controller, and we're already checking the relevant permissions before providing the view, so a permission check here is redundant.

Instead, We should probably only show this section if the user has at least one MFA option configured.

<div class="mt-xl">
<hr class="my-m">
<div class="grid half gap-xl v-center">
<div>
<strong class="text-neg">{{ trans('settings.users_mfa_reset') }}</strong>
<p class="text-small text-muted">{{ trans('settings.users_mfa_reset_desc', ['userName' => $user->name]) }}</p>
</div>
<div class="text-m-right">
<form action="{{ url("/settings/users/{$user->id}/reset-mfa") }}" method="POST" style="display: inline;">
@csrf
<button type="submit" class="button neg"
onclick="return confirm('{{ trans('settings.users_mfa_reset_confirm', ['userName' => $user->name]) }}')">
Copy link
Copy Markdown
Member

@ssddanbrown ssddanbrown Apr 19, 2026

Choose a reason for hiding this comment

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

This won't work due to CSP (security) blocking. We avoid any inline JavaScript.

Making the initial a dropdown, with a second button for confirmation, may be better. We do this in a select few other areas I think.

{{ trans('settings.users_mfa_reset') }}
</button>
</form>
</div>
</div>
</div>
@endif
</section>

@if(count($activeSocialDrivers) > 0)
Expand Down
1 change: 1 addition & 0 deletions routes/web.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@
Route::get('/settings/users/{id}', [UserControllers\UserController::class, 'edit']);
Route::put('/settings/users/{id}', [UserControllers\UserController::class, 'update']);
Route::delete('/settings/users/{id}', [UserControllers\UserController::class, 'destroy']);
Route::post('/settings/users/{id}/reset-mfa', [UserControllers\UserController::class, 'resetMfa']);

// User Account
Route::get('/my-account', [UserControllers\UserAccountController::class, 'redirect']);
Expand Down