diff --git a/app/Http/Controllers/Api/UserApiController.php b/app/Http/Controllers/Api/UserApiController.php index b334959b..3bc1f5c6 100644 --- a/app/Http/Controllers/Api/UserApiController.php +++ b/app/Http/Controllers/Api/UserApiController.php @@ -13,9 +13,13 @@ **/ use App\Http\Controllers\APICRUDController; +use App\Http\Controllers\Traits\MFACookieManager; use App\Http\Controllers\Traits\RequestProcessor; use App\Http\Controllers\UserValidationRulesFactory; +use App\libs\Auth\Models\UserTrustedDevice; +use App\ModelSerializers\Auth\UserTrustedDeviceSerializer; use App\ModelSerializers\SerializerRegistry; +use App\Services\Auth\IDeviceTrustService; use App\Services\Auth\IRecoveryCodeService; use Auth\Repositories\IUserRepository; use Auth\User; @@ -40,6 +44,8 @@ final class UserApiController extends APICRUDController use RequestProcessor; + use MFACookieManager; + /** * @var ITokenService */ @@ -50,6 +56,11 @@ final class UserApiController extends APICRUDController */ private $recovery_code_service; + /** + * @var IDeviceTrustService + */ + private $device_trust_service; + /** * UserApiController constructor. * @param IUserRepository $user_repository @@ -57,6 +68,7 @@ final class UserApiController extends APICRUDController * @param IUserService $user_service * @param ITokenService $token_service * @param IRecoveryCodeService $recovery_code_service + * @param IDeviceTrustService $device_trust_service */ public function __construct ( @@ -64,12 +76,14 @@ public function __construct ILogService $log_service, IUserService $user_service, ITokenService $token_service, - IRecoveryCodeService $recovery_code_service + IRecoveryCodeService $recovery_code_service, + IDeviceTrustService $device_trust_service ) { parent::__construct($user_repository, $user_service, $log_service); $this->token_service = $token_service; $this->recovery_code_service = $recovery_code_service; + $this->device_trust_service = $device_trust_service; } /** @@ -319,6 +333,98 @@ public function regenerateRecoveryCodes() }); } + /** + * Lists the current user's active trusted devices. "is_current" flags the + * device whose device-trust cookie came with this request. + * + * @return \Illuminate\Http\JsonResponse|mixed + */ + public function getMyTrustedDevices() + { + if (!Auth::check()) + return $this->error403(); + + return $this->processRequest(function () { + $params = [ + UserTrustedDeviceSerializer::ParamCurrentDeviceIdentifier => $this->getCurrentDeviceIdentifier(), + ]; + + $data = array_map( + fn(UserTrustedDevice $device) => SerializerRegistry::getInstance() + ->getSerializer($device) + ->serialize(null, [], [], $params), + $this->device_trust_service->getActiveTrustedDevices(Auth::user()) + ); + + return $this->ok(['data' => array_values($data)]); + }); + } + + /** + * Revokes one of the current user's trusted devices. Only the MFA bypass is + * removed; the current session stays active. + * + * @param $id + * @return \Illuminate\Http\JsonResponse|mixed + */ + public function revokeMyTrustedDevice($id) + { + if (!Auth::check()) + return $this->error403(); + + return $this->processRequest(function () use ($id) { + $device = $this->device_trust_service->revokeTrustedDevice(Auth::user(), intval($id)); + + if ($this->isCurrentDevice($device)) { + $this->expireDeviceTrustCookie(); + } + + return $this->deleted(); + }); + } + + /** + * Revokes all of the current user's active trusted devices. Only the MFA + * bypass is removed; the current session stays active. + * + * @return \Illuminate\Http\JsonResponse|mixed + */ + public function revokeAllMyTrustedDevices() + { + if (!Auth::check()) + return $this->error403(); + + return $this->processRequest(function () { + $devices = $this->device_trust_service->removeTrustedDevices(Auth::user()); + + foreach ($devices as $device) { + if ($this->isCurrentDevice($device)) { + $this->expireDeviceTrustCookie(); + break; + } + } + + return $this->deleted(); + }); + } + + /** + * Hashed identifier of the device-trust cookie sent with this request, or + * null when there is none. + */ + private function getCurrentDeviceIdentifier(): ?string + { + $token = $this->getCookieToken(); + if (empty($token)) return null; + return $this->device_trust_service->generateDeviceIdentifier($token); + } + + private function isCurrentDevice(UserTrustedDevice $device): bool + { + $current = $this->getCurrentDeviceIdentifier(); + return !is_null($current) && hash_equals($device->getDeviceIdentifier(), $current); + } + public function revokeAllMyTokens() { if (!Auth::check()) diff --git a/app/Http/Controllers/Traits/MFACookieManager.php b/app/Http/Controllers/Traits/MFACookieManager.php index 718b9305..65efb65d 100644 --- a/app/Http/Controllers/Traits/MFACookieManager.php +++ b/app/Http/Controllers/Traits/MFACookieManager.php @@ -83,4 +83,36 @@ protected function queueDeviceTrustCookie(User $user): void ); } + + /** + * Queues an already-expired trusted-device cookie so the browser drops it. + * Name, path, domain and flags must match queueDeviceTrustCookie() or the + * browser treats it as a different cookie and keeps the original. + * + * @return void + */ + protected function expireDeviceTrustCookie(): void + { + $name = Config::get('two_factor.cookie_name', 'device_trust_token'); + $path = Config::get('session.path'); + $domain = Config::get('session.domain'); + $secure = true; + $httpOnly = true; + $raw = false; + $sameSite = 'lax'; + + // Negative lifetime, same as \Illuminate\Cookie\CookieJar::forget() + Cookie::queue + ( + $name, + '', // value + -2628000, + $path, + $domain, + $secure, + $httpOnly, + $raw, + $sameSite + ); + } } diff --git a/app/ModelSerializers/Auth/UserTrustedDeviceSerializer.php b/app/ModelSerializers/Auth/UserTrustedDeviceSerializer.php new file mode 100644 index 00000000..134b18fc --- /dev/null +++ b/app/ModelSerializers/Auth/UserTrustedDeviceSerializer.php @@ -0,0 +1,58 @@ + 'device_name:json_string', + 'IpAddress' => 'ip_address:json_string', + 'TrustedAt' => 'trusted_at:datetime_epoch', + 'ExpiresAt' => 'expires_at:datetime_epoch', + 'LastSeenAt' => 'last_seen_at:datetime_epoch', + ]; + + /** + * @param null $expand + * @param array $fields + * @param array $relations + * @param array $params + * @return array + */ + public function serialize($expand = null, array $fields = [], array $relations = [], array $params = []) + { + $device = $this->object; + if (!$device instanceof UserTrustedDevice) return []; + $values = parent::serialize($expand, $fields, $relations, $params); + $current = $params[self::ParamCurrentDeviceIdentifier] ?? null; + $values['is_current'] = is_string($current) && hash_equals($device->getDeviceIdentifier(), $current); + return $values; + } +} diff --git a/app/ModelSerializers/SerializerRegistry.php b/app/ModelSerializers/SerializerRegistry.php index 12250286..e65b716c 100644 --- a/app/ModelSerializers/SerializerRegistry.php +++ b/app/ModelSerializers/SerializerRegistry.php @@ -16,6 +16,7 @@ use App\ModelSerializers\Auth\PublicUserSerializer; use App\ModelSerializers\Auth\UserActionSerializer; use App\ModelSerializers\Auth\UserRegistrationRequestSerializer; +use App\ModelSerializers\Auth\UserTrustedDeviceSerializer; use App\ModelSerializers\OAuth2\AccessTokenSerializer; use App\ModelSerializers\OAuth2\ApiEndpointSerializer; use App\ModelSerializers\OAuth2\ApiScopeGroupSerializer; @@ -83,6 +84,8 @@ private function __construct() $this->registry["UserAction"] = UserActionSerializer::class; + $this->registry["UserTrustedDevice"] = UserTrustedDeviceSerializer::class; + $this->registry["UserRegistrationRequest"] = UserRegistrationRequestSerializer::class; $this->registry["Group"] = [ diff --git a/app/Repositories/DoctrineUserTrustedDeviceRepository.php b/app/Repositories/DoctrineUserTrustedDeviceRepository.php index 37267066..b3953940 100644 --- a/app/Repositories/DoctrineUserTrustedDeviceRepository.php +++ b/app/Repositories/DoctrineUserTrustedDeviceRepository.php @@ -42,17 +42,15 @@ public function getByUserAndDeviceIdentifier(User $user, string $deviceIdentifie return $result instanceof UserTrustedDevice ? $result : null; } - public function revokeAllForUser(User $user): void + public function getByIdAndUser(int $id, User $user): ?UserTrustedDevice { - $this->getEntityManager() - ->createQueryBuilder() - ->update($this->getBaseEntity(), 'd') - ->set('d.is_revoked', ':revoked') - ->where('d.user = :user') - ->setParameter('revoked', true) - ->setParameter('user', $user) - ->getQuery() - ->execute(); + $criteria = Criteria::create() + ->where(Criteria::expr()->eq('id', $id)) + ->andWhere(Criteria::expr()->eq('user', $user)) + ->setMaxResults(1); + + $result = $this->matching($criteria)->first(); + return $result instanceof UserTrustedDevice ? $result : null; } public function getActiveByUserAndIdentifier(User $user, string $deviceIdentifier): ?UserTrustedDevice diff --git a/app/Services/Auth/DeviceTrustService.php b/app/Services/Auth/DeviceTrustService.php index d4242c2f..4057cee8 100644 --- a/app/Services/Auth/DeviceTrustService.php +++ b/app/Services/Auth/DeviceTrustService.php @@ -20,6 +20,8 @@ use DateTime; use DateInterval; use DateTimeZone; +use Illuminate\Support\Facades\Log; +use models\exceptions\EntityNotFoundException; use Utils\IPHelper; use Utils\Db\ITransactionService; @@ -96,15 +98,72 @@ public function isDeviceTrusted(User $user, ?string $cookieToken): bool return true; } - public function removeTrustedDevices(User $user): void + public function getActiveTrustedDevices(User $user): array { - $this->repository->revokeAllForUser($user); + return $this->repository->getActiveByUser($user); + } - $this->audit_service->log( - $user, - TwoFactorAuditLog::EventDeviceRevoked, - $user->getTwoFactorMethod(), - IPHelper::getUserIp() - ); + public function revokeTrustedDevice(User $user, int $deviceId): UserTrustedDevice + { + // Scoped by owner: another user's device id is indistinguishable from a + // missing one, so the caller cannot probe for ids that exist. + $device = $this->repository->getByIdAndUser($deviceId, $user); + if (!$device instanceof UserTrustedDevice) { + throw new EntityNotFoundException('Trusted device not found.'); + } + + // Idempotent: a device that no longer bypasses the challenge has nothing + // to revoke, and logging it again would only add noise to the audit trail. + if ($device->isRevoked() || $device->isExpired()) { + return $device; + } + + $this->tx_service->transaction(function () use ($device) { + $device->setIsRevoked(true); + $this->repository->add($device, false); + }); + + $this->logDeviceRevoked($user, $device); + + return $device; + } + + public function removeTrustedDevices(User $user): array + { + $devices = $this->tx_service->transaction(function () use ($user) { + $devices = $this->repository->getActiveByUser($user); + foreach ($devices as $device) { + $device->setIsRevoked(true); + $this->repository->add($device, false); + } + return $devices; + }); + + foreach ($devices as $device) { + $this->logDeviceRevoked($user, $device); + } + + return $devices; + } + + /** + * Best-effort, after the revocation is committed: an audit failure must not + * 500 a request whose device is already revoked, and audit_service->log() + * opens its own transaction, which cannot be nested inside ours (see + * RecoveryCodeService::enableTwoFactorAndGenerateCodes()). + */ + private function logDeviceRevoked(User $user, UserTrustedDevice $device): void + { + try { + $this->audit_service->log( + $user, + TwoFactorAuditLog::EventDeviceRevoked, + $user->getTwoFactorMethod(), + IPHelper::getUserIp(), + ['device_id' => $device->getId()] + ); + } catch (\Throwable $ex) { + Log::warning($ex); + } } } diff --git a/app/Services/Auth/IDeviceTrustService.php b/app/Services/Auth/IDeviceTrustService.php index 2750335c..e629cf74 100644 --- a/app/Services/Auth/IDeviceTrustService.php +++ b/app/Services/Auth/IDeviceTrustService.php @@ -12,7 +12,9 @@ * limitations under the License. **/ +use App\libs\Auth\Models\UserTrustedDevice; use Auth\User; +use models\exceptions\EntityNotFoundException; /** * Interface IDeviceTrustService @@ -34,9 +36,28 @@ public function isDeviceTrusted(User $user, ?string $cookieToken): bool; public function trustDevice(User $user, string $userAgent, string $ipAddress): string; /** - * Revokes all trusted devices for the given user. + * Returns the user's active (non-revoked, non-expired) trusted devices. + * Read-only: unlike isDeviceTrusted() it never touches last_seen_at. + * + * @return UserTrustedDevice[] */ - public function removeTrustedDevices(User $user): void; + public function getActiveTrustedDevices(User $user): array; + + /** + * Revokes one of the user's trusted devices. Idempotent: an already revoked + * or expired device is returned untouched and no audit event is logged. + * + * @throws EntityNotFoundException if the device does not exist or belongs to another user + */ + public function revokeTrustedDevice(User $user, int $deviceId): UserTrustedDevice; + + /** + * Revokes all active trusted devices for the given user, logging one + * device_revoked audit event per revoked device. + * + * @return UserTrustedDevice[] the devices that were revoked by this call + */ + public function removeTrustedDevices(User $user): array; /** * Returns the SHA-256 hash of the given token used as the stored device identifier. diff --git a/app/libs/Auth/Repositories/IUserTrustedDeviceRepository.php b/app/libs/Auth/Repositories/IUserTrustedDeviceRepository.php index 04e86edf..60f43477 100644 --- a/app/libs/Auth/Repositories/IUserTrustedDeviceRepository.php +++ b/app/libs/Auth/Repositories/IUserTrustedDeviceRepository.php @@ -23,9 +23,10 @@ interface IUserTrustedDeviceRepository extends IBaseRepository public function getByUserAndDeviceIdentifier(User $user, string $deviceIdentifier): ?UserTrustedDevice; /** - * Revoke all trusted devices for the given user (sets is_revoked = true). + * Look up a trusted device record by id, scoped to its owner (no revoked/expiry filter). + * Returns null when the id does not exist or belongs to another user. */ - public function revokeAllForUser(User $user): void; + public function getByIdAndUser(int $id, User $user): ?UserTrustedDevice; /** * Look up an active (non-revoked, non-expired) trusted device for a user by its hashed identifier. diff --git a/resources/js/components/trusted_devices_section.js b/resources/js/components/trusted_devices_section.js new file mode 100644 index 00000000..f7bab60e --- /dev/null +++ b/resources/js/components/trusted_devices_section.js @@ -0,0 +1,139 @@ +import React, {useEffect, useState} from "react"; +import Box from "@material-ui/core/Box"; +import Button from "@material-ui/core/Button"; +import Chip from "@material-ui/core/Chip"; +import Table from "@material-ui/core/Table"; +import TableBody from "@material-ui/core/TableBody"; +import TableCell from "@material-ui/core/TableCell"; +import TableHead from "@material-ui/core/TableHead"; +import TableRow from "@material-ui/core/TableRow"; +import Typography from "@material-ui/core/Typography"; +import moment from "moment"; +import Swal from "sweetalert2"; +import {getTrustedDevices, revokeAllTrustedDevices, revokeTrustedDevice} from "../profile/actions"; +import {handleErrorResponse} from "../utils"; + +const formatEpoch = (value) => value ? moment.utc(value * 1000).format("DD/MM/YYYY hh:mm A") : ""; + +const TrustedDevicesSection = () => { + const [devices, setDevices] = useState([]); + const [loaded, setLoaded] = useState(false); + const [busy, setBusy] = useState(false); + + useEffect(() => { + getTrustedDevices().then(({response}) => { + setDevices(response?.data ?? []); + setLoaded(true); + }).catch((err) => { + setLoaded(true); + handleErrorResponse(err); + }); + }, []); + + // Buttons are plain onClick handlers: the whole profile page is one