Skip to content

Feat | Add Device Trust Service#133

Open
matiasperrone-exo wants to merge 1 commit into
feat/mfa-challenge-strategy-patternfrom
feat/mfa-device-trust-service
Open

Feat | Add Device Trust Service#133
matiasperrone-exo wants to merge 1 commit into
feat/mfa-challenge-strategy-patternfrom
feat/mfa-device-trust-service

Conversation

@matiasperrone-exo
Copy link
Copy Markdown
Contributor

@matiasperrone-exo matiasperrone-exo commented May 21, 2026

Task:

Ref: https://app.clickup.com/t/86ba0nah4

Blocked by:

PR: Feature | MFA Challenge Strategy Pattern (Interface, Abstract, Factory, EmailOTP) #129

Changes:

New service layer (app/Services/Auth/)

  • IDeviceTrustService.php — Interface defining the contract: trustDevice, isDeviceTrusted, removeTrustedDevices, generateDeviceIdentifier.
  • DeviceTrustService.php — Implementation:
    • trustDevice: generates a 128-char random hex token, hashes it with SHA-256, persists a UserTrustedDevice record with configurable expiry (default 30 days), returns the raw token
      for cookie storage.
    • isDeviceTrusted: hashes the cookie token, looks up the record, returns false if not found / revoked / expired; updates last_seen_at on a valid hit.
    • removeTrustedDevices: bulk-revokes all devices for a user (sets is_revoked = true).
  • TwoFactorServiceProvider.php — Deferred service provider that binds IDeviceTrustService → DeviceTrustService as a singleton; registered in config/app.php.

Repository changes (IUserTrustedDeviceRepository / DoctrineUserTrustedDeviceRepository)

Two new methods added:

  • getByUserAndDeviceIdentifier — looks up a device by user + hashed identifier with no revoked/expiry filter (used by the service to check all states).
  • revokeAllForUser — bulk DQL UPDATE setting is_revoked = true for all devices belonging to a user.

Entity updates (UserTrustedDevice)

  • Constructor now initializes last_seen_at to now().
  • Added isExpired() helper (compares expires_at against now).
  • Fixed targetEntity reference from FQCN string to User::class.

Config

  • config/two_factor.php — Added device_trust_lifetime_days (default 30) and cookie_name (default 'device_trust_token') settings, driven by env vars.

Tests (tests/DeviceTrustServiceTest.php)

264-line unit test suite using Mockery covering:

  • isDeviceTrusted: null/empty cookie, unknown token, revoked device, expired device, valid device, last_seen_at update.
  • trustDevice: returns 128-char hex token, stores SHA-256 hash (not raw token), persists exactly one record.
  • removeTrustedDevices: delegates to revokeAllForUser.
  • generateDeviceIdentifier: SHA-256 correctness.

Requested GOAL

Current state

No device trust mechanism exists. Users would need to complete 2FA on every single login, even from the same browser and device they used yesterday.

Target state

IDeviceTrustService and its implementation allow the system to remember trusted devices via a secure cookie mechanism. A SHA-256 hash of a cryptographically random token is stored
in the user_trusted_devices table with a configurable TTL (default 30 days). The raw token is stored in a long-lived HttpOnly cookie , On subsequent logins, the cookie token is hashed and compared against stored records to bypass 2FA.

TASKS

  • Create IDeviceTrustService interface with methods: isDeviceTrusted(User, ?string cookieToken): bool, trustDevice(User, string userAgent, string ipAddress): string (returns raw cookie token), removeTrustedDevices(User): void, generateDeviceIdentifier(string token): string (returns SHA-256 hash)
  • Implement DeviceTrustService: isDeviceTrusted() hashes the cookie token via generateDeviceIdentifier(), queries IUserTrustedDeviceRepository for a matching non-revoked, non-expired record for the user, updates last_seen_at on match.
  • trustDevice() generates a 64-byte random token via bin2hex(random_bytes(32)), stores SHA-256 hash in user_trusted_devices with TTL from config, extracts User-Agent for device_name, returns raw token.
  • removeTrustedDevices() revokes all devices for the user (sets is_revoked=true).
  • Implement IUserTrustedDeviceRepository::getByUserAndDeviceIdentifier() query method
  • Register IDeviceTrustService in TwoFactorServiceProvider
  • Write unit tests for isDeviceTrusted() with: valid trusted device, expired device, revoked device, no cookie, wrong cookie
  • Write unit tests for trustDevice(): verify token generation, hash storage, and record creation
  • Write unit tests for removeTrustedDevices(): verify all devices for user are revoked
  • DeviceTrustService must depend on IUserTrustedDeviceRepository, not EntityManager directly.
  • trustDevice() must instantiate a UserTrustedDevice entity, associate it with the User, set device_identifier, device_name, ip_address, user_agent, trusted_at, expires_at, last_seen_at, is_revoked=false, then persist it through the repository.
  • isDeviceTrusted() must call IUserTrustedDeviceRepository::getByUserAndDeviceIdentifier($user, $deviceIdentifier) and only return true for a non-revoked, non-expired record.
  • On successful match, isDeviceTrusted() must update last_seen_at and persist the change.
  • removeTrustedDevices() must call a repository method that revokes all trusted devices for the user.

ACCEPTANCE CRITERIA

  • trustDevice() returns a 128-character hex string (64 bytes as hex)
  • The stored device_identifier is a SHA-256 hash of the returned token (not the raw token itself)
  • isDeviceTrusted() returns true when a matching non-revoked, non-expired record exists
  • isDeviceTrusted() returns false for expired records (expires_at < now)
  • isDeviceTrusted() returns false for revoked records (is_revoked = true)
  • isDeviceTrusted() returns false when cookieToken is null or empty
  • isDeviceTrusted() updates last_seen_at on a valid match
  • removeTrustedDevices() sets is_revoked=true on all records for the user
  • DeviceTrustService does not query Doctrine directly; all UserTrustedDevice access goes through IUserTrustedDeviceRepository.
  • trustDevice() creates exactly one UserTrustedDevice record per call.
  • The raw cookie token is returned to the caller but is never stored in UserTrustedDevice.
  • UserTrustedDevice::device_identifier stores hash('sha256', $rawToken).
  • isDeviceTrusted() updates UserTrustedDevice::last_seen_at when a valid device is found.
  • Device trust TTL defaults to 30 days, configurable via config('two_factor.device_trust_lifetime_days')
  • cookie name is configuravle via a config('two_factor.cookie_name')
  • All unit tests pass

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 21, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 063f03d0-5774-45a5-82a7-21bbc80ea3fe

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/mfa-device-trust-service

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@matiasperrone-exo matiasperrone-exo self-assigned this May 21, 2026
@github-actions
Copy link
Copy Markdown

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-133/

This page is automatically updated on each push to this PR.

@matiasperrone-exo matiasperrone-exo marked this pull request as ready for review May 21, 2026 17:32
@matiasperrone-exo matiasperrone-exo marked this pull request as draft May 21, 2026 17:33
@matiasperrone-exo matiasperrone-exo force-pushed the feat/mfa-device-trust-service branch from 09750ff to 9faa517 Compare May 21, 2026 17:42
@github-actions
Copy link
Copy Markdown

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-133/

This page is automatically updated on each push to this PR.

@matiasperrone-exo matiasperrone-exo force-pushed the feat/mfa-challenge-strategy-pattern branch from d9beb67 to b1ef3fa Compare May 21, 2026 20:04
@matiasperrone-exo matiasperrone-exo force-pushed the feat/mfa-device-trust-service branch from 9faa517 to 236b644 Compare May 21, 2026 20:19
@matiasperrone-exo matiasperrone-exo marked this pull request as ready for review May 21, 2026 20:19
@github-actions
Copy link
Copy Markdown

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-133/

This page is automatically updated on each push to this PR.

@matiasperrone-exo matiasperrone-exo force-pushed the feat/mfa-device-trust-service branch from 236b644 to f1e8af5 Compare May 21, 2026 20:56
@github-actions
Copy link
Copy Markdown

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-133/

This page is automatically updated on each push to this PR.

@matiasperrone-exo matiasperrone-exo force-pushed the feat/mfa-challenge-strategy-pattern branch from b1ef3fa to 2c99a62 Compare May 21, 2026 21:25
@matiasperrone-exo matiasperrone-exo force-pushed the feat/mfa-device-trust-service branch from f1e8af5 to e096f94 Compare May 21, 2026 21:25
@github-actions
Copy link
Copy Markdown

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-133/

This page is automatically updated on each push to this PR.

@matiasperrone-exo matiasperrone-exo force-pushed the feat/mfa-device-trust-service branch from e096f94 to 16c4945 Compare May 21, 2026 21:40
@github-actions
Copy link
Copy Markdown

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-133/

This page is automatically updated on each push to this PR.

@matiasperrone-exo matiasperrone-exo force-pushed the feat/mfa-device-trust-service branch from 16c4945 to 4d99419 Compare May 21, 2026 22:05
@github-actions
Copy link
Copy Markdown

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-133/

This page is automatically updated on each push to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant