Showing AI reviews for safee-meet-backend
Project AI Score ?
72%
Quality Avg ?
76%
Security Avg ?
67%
Reviews ?
16

Review Result ?

jattin01/safee-meet-backend · 02455bb4
The commit includes commands for migrating the user IDs to BIGINT and decrypting previously encrypted emails. It has good thoroughness in handling the migration of foreign keys and audit columns and performs staged steps for safer migration. However, it drops email/phone encryption, which risks reducing security. Error handling exists for email decryption failures, but there is no indication of security controls to protect the plaintext email column. Quality is good with namespace usage and structured command classes, but also missing testing references or rollback plans. Business value is high as migrating to BIGINT allows scalability and system consistency. Bug risk remains due to the complexity and critical nature of ID remapping and potential data integrity issues. Security is moderate due to the removal of encryption and unclear protection of sensitive data after migration.
Quality ?
85%
Security ?
50%
Business Value ?
90%
Maintainability ?
73%
Issues & Suggested Fixes ?
1AI detects the issue
2Shows file, line, or evidence
3Suggests the fix to apply
Missing Validation
Where app/Console/Commands/BackfillPlainEmailCommand.php:28
Issue / Evidence no validation or sanitization of decrypted email
Suggested Fix add email format validation before saving to prevent invalid data
Storing Emails In Plaintext After Decrypti...
Where app/Console/Commands/BackfillPlainEmailCommand.php:38
Issue / Evidence storing emails in plaintext after decrypting encrypted emails
Suggested Fix consider alternative approaches to protect email confidentiality and comply with data protection
Missing Test Coverage
Where app/Console/Commands/MigrateUsersToBigintId.php:11-25
Issue / Evidence no automated tests or rollback provided for migration steps
Suggested Fix create tests and rollback procedures to ensure safe migration and easier recovery from failures
Security Issue
Where commit message
Issue / Evidence no mention of security implications or rollback strategy
Suggested Fix add details on security considerations and rollback plans in the commit message to improve clarity and risk management
Multiple Steps But No Logging Or Progress...
Where app/Console/Commands/MigrateUsersToBigintId.php:144-151
Issue / Evidence multiple steps but no logging or progress reporting
Suggested Fix add detailed logging and progress indicators for each migration step to aid troubleshooting and monitoring
Code Change Preview · app/Console/Commands/BackfillPlainEmailCommand.php ?
Removed / Before Commit

                                                
Added / After Commit
+ <?php
+ 
+ namespace App\Console\Commands;
+ 
+ use App\Models\User;
+ use Illuminate\Console\Command;
+ use Throwable;
+ 
+ class BackfillPlainEmailCommand extends Command
+ {
+     protected $signature = 'users:backfill-plain-email {--dry-run : Report what would change without saving}';
+ 
+     protected $description = 'Decrypts each user\'s email_encrypted into the new plain email column';
+ 
+     public function handle(): int
+     {
+         $dryRun = (bool) $this->option('dry-run');
+ 
Removed / Before Commit

                                                
Added / After Commit
+ <?php
+ 
+ namespace App\Console\Commands;
+ 
+ use App\Models\User;
+ use Illuminate\Console\Command;
+ use Illuminate\Support\Facades\DB;
+ use Illuminate\Support\Facades\Schema;
+ 
+ /**
+  * Staged, safe conversion of users.id from CHAR(26) ULID to BIGINT
+  * AUTO_INCREMENT, remapping every dependent foreign-key and audit column
+  * across the schema (see safee_meet_database.sql, repo root, for the
+  * authoritative list this command's config arrays were generated from).
+  *
+  * Run in this exact order, verifying output at each step:
+  *   php artisan users:migrate-to-bigint-id --step=shadow
+  *   php artisan users:migrate-to-bigint-id --step=backfill
Removed / Before Commit
- 'kycStatus'          => $this->kyc_status,
- 'trustScore'         => $this->trust_score,
- 'trustTier'          => VerificationLevelResolver::fromUser($this->kyc_status, $this->trust_tier),
-             'email'              => $this->decryptValue($this->email_encrypted),
-             'phone'              => $this->decryptValue($this->phone_encrypted),
- 'isChatEnabled'      => $this->is_chat_enabled,
- 'isMeetingEnabled'   => $this->is_meeting_enabled,
- 'isSosEnabled'       => $this->is_sos_enabled,
- 'createdAt'          => $this->created_at,
- ];
- }
- 
-     private function decryptValue(?string $value): ?string
-     {
-         if (!$value) {
-             return null;
-         }
- 
Added / After Commit
+ 'kycStatus'          => $this->kyc_status,
+ 'trustScore'         => $this->trust_score,
+ 'trustTier'          => VerificationLevelResolver::fromUser($this->kyc_status, $this->trust_tier),
+             'email'              => $this->email,
+             'phone'              => $this->phone,
+ 'isChatEnabled'      => $this->is_chat_enabled,
+ 'isMeetingEnabled'   => $this->is_meeting_enabled,
+ 'isSosEnabled'       => $this->is_sos_enabled,
+ 'createdAt'          => $this->created_at,
+ ];
+ }
+ }
Removed / Before Commit
- protected function casts(): array
- {
- return [
- 'meeting_date' => 'date',
- 'scheduled_start_at' => 'datetime',
- 'arrived_at' => 'datetime',
Added / After Commit
+ protected function casts(): array
+ {
+ return [
+             // Keep these string-shaped in API responses regardless of the
+             // underlying users.id column type (char ULID or bigint) — the
+             // mobile client expects string ids everywhere.
+             'id' => 'string',
+             'host_user_id' => 'string',
+             'guest_user_id' => 'string',
+ 'meeting_date' => 'date',
+ 'scheduled_start_at' => 'datetime',
+ 'arrived_at' => 'datetime',
Removed / Before Commit
- 
- protected function casts(): array
- {
-         return ['recorded_at' => 'datetime'];
- }
- 
- protected static function booted(): void
Added / After Commit
+ 
+ protected function casts(): array
+ {
+         return [
+             'id' => 'string',
+             'user_id' => 'string',
+             'recorded_at' => 'datetime',
+         ];
+ }
+ 
+ protected static function booted(): void
Removed / Before Commit
- use Illuminate\Database\Eloquent\SoftDeletes;
- use Illuminate\Foundation\Auth\User as Authenticatable;
- use Illuminate\Notifications\Notifiable;
- use Illuminate\Support\Str;
- use Laravel\Sanctum\HasApiTokens;
- use App\Models\EmergencyContact;
- {
- use HasApiTokens, HasFactory, Notifiable, SoftDeletes;
- 
-     // users.id is a char(26) ULID (Firebase-based accounts), not an
-     // auto-increment integer — without this, Eloquent overwrites the id
-     // we set on create() with a bogus lastInsertId() after every insert.
-     protected $keyType = 'string';
-     public $incrementing = false;
- 
- protected static function booted(): void
- {
- // 'id' isn't mass-assignable (see $fillable below), so a plain
Added / After Commit
+ use Illuminate\Database\Eloquent\SoftDeletes;
+ use Illuminate\Foundation\Auth\User as Authenticatable;
+ use Illuminate\Notifications\Notifiable;
+ use Illuminate\Support\Facades\Schema;
+ use Illuminate\Support\Str;
+ use Laravel\Sanctum\HasApiTokens;
+ use App\Models\EmergencyContact;
+ {
+ use HasApiTokens, HasFactory, Notifiable, SoftDeletes;
+ 
+     // users.id was historically a char(26) ULID (Firebase-based accounts) on
+     // some deployments, and is being migrated to a bigint auto-increment.
+     // Detect the live column type at runtime — same pattern as
+     // Meeting::usesUlidKey()/EmergencyContact::usesUlidKey() — so this model
+     // works correctly before, during, and after that migration.
+     public function getIncrementing()
+     {
+         return !static::usesUlidKey();
Removed / Before Commit
- 'auth_provider'   => $identity->provider,
- 'display_name'    => $payload['name'] ?? $identity->name ?? 'SAFEE User',
- 'avatar_url'      => $identity->avatarUrl,
-                 'email_encrypted' => ($payload['email'] ?? $identity->email) ? encrypt($payload['email'] ?? $identity->email) : null,
-                 'email_hash'      => ($payload['email'] ?? $identity->email) ? hash_hmac('sha256', strtolower(trim($payload['email'] ?? $identity->email)), config('app.key')) : null,
-                 'phone_encrypted' => $identity->phone ? encrypt($identity->phone) : null,
-                 'phone_hash'      => $identity->phone ? hash_hmac('sha256', $identity->phone, config('app.key')) : null,
- 'status'          => 'active',
- 'onboarding_status' => 'completed',
- 'kyc_status'      => 'not_started',
- }
- 
- if ($email) {
-             $hash = hash_hmac('sha256', strtolower(trim($email)), config('app.key'));
-             $user = User::where('email_hash', $hash)->first();
- if ($user) return $user;
- }
- 
Added / After Commit
+ 'auth_provider'   => $identity->provider,
+ 'display_name'    => $payload['name'] ?? $identity->name ?? 'SAFEE User',
+ 'avatar_url'      => $identity->avatarUrl,
+                 // Plain columns going forward — email_encrypted/email_hash/
+                 // phone_encrypted/phone_hash are left in place (unused) for
+                 // existing rows; no encryption-at-rest at this stage.
+                 'email'           => ($payload['email'] ?? $identity->email) ? strtolower(trim($payload['email'] ?? $identity->email)) : null,
+                 'phone'           => $identity->phone,
+ 'status'          => 'active',
+ 'onboarding_status' => 'completed',
+ 'kyc_status'      => 'not_started',
+ }
+ 
+ if ($email) {
+             $user = User::where('email', strtolower(trim($email)))->first();
+ if ($user) return $user;
+ }
+ 
Removed / Before Commit

                                                
Added / After Commit
+ <?php
+ 
+ use Illuminate\Database\Migrations\Migration;
+ use Illuminate\Database\Schema\Blueprint;
+ use Illuminate\Support\Facades\Schema;
+ 
+ return new class extends Migration
+ {
+     /**
+      * Adds a plain lookup `email` column (replacing email_encrypted/email_hash
+      * as the primary lookup going forward — those stay in place, unused, not
+      * dropped) plus a few lifecycle/account-state columns. Additive only.
+      */
+     public function up(): void
+     {
+         Schema::table('users', function (Blueprint $table): void {
+             if (!Schema::hasColumn('users', 'email')) {
+                 $table->string('email')->nullable()->unique();