Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
24 changes: 24 additions & 0 deletions ProcessMaker/Http/Controllers/Api/UserController.php
Original file line number Diff line number Diff line change
Expand Up @@ -1150,4 +1150,28 @@ public function updateLanguage(Request $request)

return response([], 204);
}

public function resetAuthApp(User $user)
{
if (!Auth::user()->can('edit', $user)) {
throw new AuthorizationException(__('Not authorized to update this user.'));
}

if (!$user->hasAuthAppConfigured()) {
return response([
'message' => __('Authenticator app is not configured for this user.'),
], 422);
}

$original = $user->getOriginal();
$user->auth_app_configured_at = null;
$user->saveOrFail();

UserUpdated::dispatch($user, $user->getChanges(), $original);

return response([
'message' => __('Authenticator app reset successfully.'),
'auth_app_configured_at' => null,
]);
}
}
12 changes: 11 additions & 1 deletion ProcessMaker/Http/Controllers/Auth/TwoFactorAuthController.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ public function displayTwoFactorAuthForm(Request $request)
}

// Display view
return view('auth.2fa.otp');
return view('auth.2fa.otp', [
'showAuthAppSetup' => $this->twoFactorAuthentication->userCanSetUpAuthApp($user),
]);
}

public function validateTwoFactorAuthCode(Request $request)
Expand Down Expand Up @@ -89,6 +91,10 @@ public function validateTwoFactorAuthCode(Request $request)
session()->put(self::TFA_VALIDATED, $validated);

if ($validated) {
if ($this->twoFactorAuthentication->isAuthAppCode($code)) {
$this->twoFactorAuthentication->markAuthAppConfigured($user);
}

// Remove 2fa values in session
session()->remove(self::TFA_MESSAGE);
session()->remove(self::TFA_ERROR);
Expand Down Expand Up @@ -133,6 +139,10 @@ public function displayAuthAppQr(Request $request)
return redirect()->route('login');
}

if (!$this->twoFactorAuthentication->userCanSetUpAuthApp($user)) {
return redirect()->route('2fa');
}

// Generate QR code
$qrCode = $this->twoFactorAuthentication->generateQr($user);

Expand Down
7 changes: 7 additions & 0 deletions ProcessMaker/Models/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ class User extends Authenticatable implements HasMedia
'password_changed_at',
'connected_accounts',
'preferences_2fa',
'auth_app_configured_at',
'email_task_notification',
];

Expand All @@ -144,6 +145,7 @@ class User extends Authenticatable implements HasMedia
'loggedin_at' => 'datetime',
'schedule' => 'array',
'preferences_2fa' => 'array',
'auth_app_configured_at' => 'datetime',
];

/**
Expand Down Expand Up @@ -550,6 +552,11 @@ public function sessions(): HasMany
return $this->hasMany(UserSession::class);
}

public function hasAuthAppConfigured(): bool
{
return $this->auth_app_configured_at !== null;
}

public function getValid2FAPreferences(): array
{
// Get global and user values
Expand Down
26 changes: 22 additions & 4 deletions ProcessMaker/TwoFactorAuthentication.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,18 +80,36 @@ private function getCodeForEmailSms(User $user): string
return $otp->now();
}

public function validateCode(User $user, string $code)
public function isAuthAppCode(string $code): bool
{
// The code is for Google Authenticator app?
$forGoogleAuthApp = strlen($code) === 6;
return strlen($code) === 6;
}

public function validateCode(User $user, string $code)
{
// Create OTP instance
$otp = $this->createOtpInstance($user, $forGoogleAuthApp);
$otp = $this->createOtpInstance($user, $this->isAuthAppCode($code));

// Validate code
return $otp->verify($code);
}

public function markAuthAppConfigured(User $user): void
{
if ($user->hasAuthAppConfigured()) {
return;
}

$user->auth_app_configured_at = now();
$user->save();
}

public function userCanSetUpAuthApp(User $user): bool
{
return in_array(self::AUTH_APP, $user->getValid2FAPreferences(), true)
&& !$user->hasAuthAppConfigured();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Username change locks authenticator users

High Severity

userCanSetUpAuthApp hides QR setup once auth_app_configured_at is set, but the TOTP secret is derived from username. A username change invalidates existing codes and leaves auth_app_configured_at set, so users with only Authenticator App cannot re-enroll. Self-service profile updates allow username changes, and reset is admin-only, so those users cannot recover on their own.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 72809c4. Configure here.


/**
* @param User $user
* @param string $code
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

return new class extends Migration
{
public function up(): void
{
Schema::table('users', function (Blueprint $table) {
$table->timestamp('auth_app_configured_at')->nullable()->after('preferences_2fa');
});
}

public function down(): void
{
Schema::table('users', function (Blueprint $table) {
$table->dropColumn('auth_app_configured_at');
});
}
};
22 changes: 22 additions & 0 deletions resources/views/admin/users/edit.blade.php
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@
originalEmail: '',
emailHasChanged: false,
canCreateTokens: @json($canCreateTokens),
resettingAuthApp: false,
}
},
created() {
Expand Down Expand Up @@ -555,6 +556,27 @@
this.errors = error.response.data.errors;
});
},
resetAuthApp() {
if (!confirm(this.$t('Reset the authenticator app for this user?'))) {
return;
}

this.resettingAuthApp = true;

ProcessMaker.apiClient.put(`users/${this.formData.id}/reset_auth_app`)
.then(() => {
this.formData.auth_app_configured_at = null;
ProcessMaker.alert(this.$t('Authenticator app reset successfully.'), 'success');
})
.catch(error => {
const message = error.response?.data?.message
|| this.$t('Unable to reset authenticator app.');
ProcessMaker.alert(message, 'danger');
})
.finally(() => {
this.resettingAuthApp = false;
});
},
loadGroups(filter) {
filter = typeof filter === 'string' ? '?filter=' + filter + '&' : '?';
ProcessMaker.apiClient
Expand Down
3 changes: 1 addition & 2 deletions resources/views/auth/2fa/otp.blade.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ class="form-control{{ $errors->has('code') ? ' is-invalid' : '' }}"
{{ __('Send Again') }}
</a>
</div>
@if (in_array(\ProcessMaker\TwoFactorAuthentication::AUTH_APP,
config('password-policies.2fa_method', [])))
@if ($showAuthAppSetup ?? false)
<div class="form-group">
<a href="{{ route('2fa.auth_app_qr') }}">
{{ __('Authenticator app') }}
Expand Down
15 changes: 15 additions & 0 deletions resources/views/shared/users/sidebar.blade.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,21 @@
>
</b-form-checkbox-group>
</div>
@if (!\Request::is('profile/edit') && in_array(\ProcessMaker\TwoFactorAuthentication::AUTH_APP, $global2FAEnabled))
<div class="form-group mb-0" v-if="formData.auth_app_configured_at">
<button
type="button"
class="btn btn-outline-danger btn-sm w-100"
@click="resetAuthApp"
:disabled="resettingAuthApp"
>
{{ __('Reset Authenticator App') }}
</button>
<small class="form-text text-muted">
{{ __('The user will configure a new authenticator on next login.') }}
</small>
</div>
@endif
@endif
</div>

Expand Down
1 change: 1 addition & 0 deletions routes/api.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@

// User Groups
Route::put('users/{user}/groups', [UserController::class, 'updateGroups'])->name('users.groups.update')->middleware('can:edit-users');
Route::put('users/{user}/reset_auth_app', [UserController::class, 'resetAuthApp'])->name('users.reset_auth_app')->middleware('can:edit-users');
// User personal access tokens
Route::get('users/{user}/tokens', [UserTokenController::class, 'index'])->name('users.tokens.index'); // Permissions handled in the controller
Route::get('users/{user}/tokens/{tokenId}', [UserTokenController::class, 'show'])->name('users.tokens.show'); // Permissions handled in the controller
Expand Down
37 changes: 37 additions & 0 deletions tests/Feature/Api/ResetAuthAppTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php

declare(strict_types=1);

namespace Tests\Feature\Api;

use ProcessMaker\Models\User;
use Tests\Feature\Shared\RequestHelper;
use Tests\TestCase;

class ResetAuthAppTest extends TestCase
{
use RequestHelper;

public function test_admin_can_reset_authenticator_app_for_a_user(): void
{
$targetUser = User::factory()->create([
'auth_app_configured_at' => now(),
]);

$response = $this->apiCall('PUT', route('api.users.reset_auth_app', $targetUser));

$response->assertStatus(200);
$this->assertNull($targetUser->fresh()->auth_app_configured_at);
}

public function test_reset_returns_error_when_authenticator_is_not_configured(): void
{
$targetUser = User::factory()->create([
'auth_app_configured_at' => null,
]);

$response = $this->apiCall('PUT', route('api.users.reset_auth_app', $targetUser));

$response->assertStatus(422);
}
}
80 changes: 80 additions & 0 deletions tests/Feature/Auth/TwoFactorAuthAppTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
<?php

declare(strict_types=1);

namespace Tests\Feature\Auth;

use OTPHP\TOTP;
use ParagonIE\ConstantTime\Base32;
use ProcessMaker\Models\User;
use ProcessMaker\TwoFactorAuthentication;
use Tests\Feature\Shared\RequestHelper;
use Tests\TestCase;

class TwoFactorAuthAppTest extends TestCase
{
use RequestHelper {
setUp as requestHelperSetUp;
}

protected function setUp(): void
{
$this->requestHelperSetUp();

config([
'password-policies.2fa_enabled' => true,
'password-policies.2fa_method' => [TwoFactorAuthentication::AUTH_APP],
]);
}

public function test_otp_shows_authenticator_link_before_setup(): void
{
$this->user->update(['auth_app_configured_at' => null]);

$response = $this->webGet(route('2fa'));

$response->assertStatus(200);
$response->assertSee('Authenticator app', false);
}

public function test_otp_hides_authenticator_link_after_setup(): void
{
$this->user->update(['auth_app_configured_at' => now()]);

$response = $this->webGet(route('2fa'));

$response->assertStatus(200);
$response->assertDontSee('>Authenticator app<', false);
}

public function test_auth_app_qr_is_blocked_after_setup(): void
{
$this->user->update(['auth_app_configured_at' => now()]);

$response = $this->webGet(route('2fa.auth_app_qr'));

$response->assertRedirect(route('2fa'));
}

public function test_valid_auth_app_code_marks_user_as_configured(): void
{
$this->user->update(['auth_app_configured_at' => null]);

$code = $this->generateAuthAppCode($this->user);

$response = $this->webCall('POST', route('2fa.validate'), ['code' => $code]);

$response->assertRedirect(route('login'));
$this->assertNotNull($this->user->fresh()->auth_app_configured_at);
}

private function generateAuthAppCode(User $user): string
{
$secret = trim(Base32::encodeUpper($user->uuid . '_' . $user->username), '=');
$otp = TOTP::createFromSecret($secret);
$otp->setIssuer('ProcessMaker');
$otp->setLabel($user->username);

return $otp->now();
}
}
Loading