From e9e0d4fdeb41a1ebc8c2a14f12f9877fbb070235 Mon Sep 17 00:00:00 2001 From: ilestis Date: Mon, 19 Feb 2024 09:42:31 -0600 Subject: [PATCH] Change logic, bind token to routing to avoid exposing user ids --- .../Settings/SubscriptionController.php | 1 - .../User/EmailValidationController.php | 19 +++-------- .../Subscriptions/EmailValidationJob.php | 12 ++++--- app/Models/Relations/UserRelations.php | 2 +- app/Models/UserValidation.php | 9 +++++ app/Providers/RouteServiceProvider.php | 2 ++ app/Services/Users/EmailValidationService.php | 34 +++++++++---------- app/User.php | 2 ++ lang/en/emails/validation.php | 4 +-- .../views/components/dialog/close.blade.php | 7 +--- .../settings/subscription/change.blade.php | 2 +- .../settings/subscription/index.blade.php | 2 +- routes/web.php | 2 +- 13 files changed, 48 insertions(+), 50 deletions(-) diff --git a/app/Http/Controllers/Settings/SubscriptionController.php b/app/Http/Controllers/Settings/SubscriptionController.php index e5e83ed97f..e4b80c2f89 100644 --- a/app/Http/Controllers/Settings/SubscriptionController.php +++ b/app/Http/Controllers/Settings/SubscriptionController.php @@ -116,7 +116,6 @@ public function change(Request $request, Tier $tier) 'hasPromo', 'limited', 'isYearly', - 'validationService', )); } diff --git a/app/Http/Controllers/User/EmailValidationController.php b/app/Http/Controllers/User/EmailValidationController.php index 65b80ef965..dafbd0f383 100644 --- a/app/Http/Controllers/User/EmailValidationController.php +++ b/app/Http/Controllers/User/EmailValidationController.php @@ -20,25 +20,14 @@ public function __construct() $this->middleware('auth'); } - public function validateEmail(Request $request, User $user) + public function validateEmail(Request $request, UserValidation $userValidation) { - if (auth()->user()->id != $user->id) { + if (auth()->user()->id != $userValidation->user_id) { return response()->redirectTo(route('settings.subscription'))->withError(__('emails/validation.error')); } - $token = $request->get('token'); - - /** @var UserValidation $validation */ - $validation = UserValidation::where('user_id', $user->id) - ->where('token', $token) - ->first(); - - if ($validation->exists) { - $validation->is_valid = true; - $validation->saveQuietly(); - } else { - response()->redirectTo(route('settings.subscription'))->withError(__('emails/validation.error')); - } + $userValidation->is_valid = true; + $userValidation->saveQuietly(); return response()->redirectTo(route('settings.subscription'))->withSuccess(__('emails/validation.success')); } diff --git a/app/Jobs/Emails/Subscriptions/EmailValidationJob.php b/app/Jobs/Emails/Subscriptions/EmailValidationJob.php index 8ac4cf6b32..636f4bfd5c 100644 --- a/app/Jobs/Emails/Subscriptions/EmailValidationJob.php +++ b/app/Jobs/Emails/Subscriptions/EmailValidationJob.php @@ -3,6 +3,7 @@ namespace App\Jobs\Emails\Subscriptions; use App\Mail\Subscription\User\ValidationEmail; +use App\Models\UserValidation; use App\User; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; @@ -23,10 +24,10 @@ class EmailValidationJob implements ShouldQueue /** */ - public function __construct(User $user, string $token) + public function __construct(User $user, UserValidation $token) { $this->user = $user->id; - $this->token = $token; + $this->token = $token->id; } /** @@ -34,14 +35,15 @@ public function __construct(User $user, string $token) */ public function handle() { - // User deleted their account already? Sure thing + // Small check in case the user deleted their account before the queue could get to them /** @var User|null $user */ $user = User::find($this->user); if (empty($user)) { return; } - $url = route('validation.email', ['user' => $user, 'token' => $this->token]); - // Send an email to the user + $userValidation = UserValidation::find($this->token); + $url = route('validation.email', ['userValidation' => $userValidation]); + Mail::to($user->email) ->locale($user->locale) ->send( diff --git a/app/Models/Relations/UserRelations.php b/app/Models/Relations/UserRelations.php index 64c130a269..7aeddfc4dd 100644 --- a/app/Models/Relations/UserRelations.php +++ b/app/Models/Relations/UserRelations.php @@ -202,7 +202,7 @@ public function upvotes(): HasMany return $this->hasMany(FeatureVote::class); } - public function userValidation(): HasOne + public function userValidation(): HasOne|UserValidation { return $this->hasOne(UserValidation::class, 'user_id', 'id'); } diff --git a/app/Models/UserValidation.php b/app/Models/UserValidation.php index 23ba6e58be..dc04a7740b 100644 --- a/app/Models/UserValidation.php +++ b/app/Models/UserValidation.php @@ -8,9 +8,13 @@ use Illuminate\Database\Eloquent\Builder; /** + * @property int $id * @property int $user_id + * @property string $token * @property bool $is_valid * @property User $user + * + * @method static self|Builder valid() */ class UserValidation extends Model { @@ -22,6 +26,11 @@ class UserValidation extends Model 'token' ]; + public function getRouteKeyName() + { + return 'token'; + } + /** * @return \Illuminate\Database\Eloquent\Relations\BelongsTo */ diff --git a/app/Providers/RouteServiceProvider.php b/app/Providers/RouteServiceProvider.php index 9c0c497465..0a42098e29 100644 --- a/app/Providers/RouteServiceProvider.php +++ b/app/Providers/RouteServiceProvider.php @@ -10,6 +10,7 @@ use App\Models\EntityType; use App\Models\Plugin; use App\Models\Tier; +use App\Models\UserValidation; use Illuminate\Support\Facades\Route; use Illuminate\Foundation\Support\Providers\RouteServiceProvider as ServiceProvider; @@ -42,6 +43,7 @@ public function boot(): void return Campaign::acl($value)->firstOrFail(); }); Route::model('entityType', EntityType::class); + Route::model('userValidation', UserValidation::class); } /** diff --git a/app/Services/Users/EmailValidationService.php b/app/Services/Users/EmailValidationService.php index b97963f84d..4e0e2106bb 100644 --- a/app/Services/Users/EmailValidationService.php +++ b/app/Services/Users/EmailValidationService.php @@ -18,26 +18,26 @@ public function requiresEmail(): void if ($token && $token->is_valid) { return; } - //Check for existing token - $flag = UserFlag::where('user_id', $this->user->id)->where('flag', UserFlag::FLAG_EMAIL)->first(); - - if (!$flag) { - $flag = new UserFlag(); - $flag->user_id = $this->user->id; - $flag->flag = UserFlag::FLAG_EMAIL; - $flag->save(); - } - if (!$token) { - $token = new UserValidation(); - $token->token = Str::uuid(); - $token->user_id = $this->user->id; - $token->is_valid = false; - $token->save(); + $flag = UserFlag::where('user_id', $this->user->id) + ->where('flag', UserFlag::FLAG_EMAIL) + ->first(); + // If we've already notified the user, no need to notify them again + if ($flag) { + return; } - EmailValidationJob::dispatch($this->user, $token->token); + $flag = new UserFlag(); + $flag->user_id = $this->user->id; + $flag->flag = UserFlag::FLAG_EMAIL; + $flag->save(); + + $token = new UserValidation(); + $token->token = Str::uuid(); + $token->user_id = $this->user->id; + $token->is_valid = false; + $token->save(); - return; + EmailValidationJob::dispatch($this->user, $token); } } diff --git a/app/User.php b/app/User.php index c7284968d0..d00f9df2ab 100644 --- a/app/User.php +++ b/app/User.php @@ -439,6 +439,8 @@ public function isFrauding(): bool return false; } + return true; + // If the account was created recently, add some small checks /*if ($this->created_at->isAfter(Carbon::now()->subHour())) { // User's name is directly in the campaign name diff --git a/lang/en/emails/validation.php b/lang/en/emails/validation.php index 53a784175e..e93d2aa636 100644 --- a/lang/en/emails/validation.php +++ b/lang/en/emails/validation.php @@ -1,7 +1,7 @@ 'Email validation required for subscription, please check your inbox to continue the validation process.', - 'success' => 'Email succesfully validated.', + 'modal' => 'An email validation is required for subscriptions. Please check your inbox for a link to confirm your email before continuing the subscription process.', + 'success' => 'Email successfully validated.', 'error' => 'Validation failed, please try again.', ]; diff --git a/resources/views/components/dialog/close.blade.php b/resources/views/components/dialog/close.blade.php index 6d0143a196..d66fdb8d70 100644 --- a/resources/views/components/dialog/close.blade.php +++ b/resources/views/components/dialog/close.blade.php @@ -1,9 +1,4 @@ -@if ($modal || $dismiss == 'alert')@else - -@endif diff --git a/resources/views/settings/subscription/change.blade.php b/resources/views/settings/subscription/change.blade.php index 02fe7fea1c..c49b86533a 100644 --- a/resources/views/settings/subscription/change.blade.php +++ b/resources/views/settings/subscription/change.blade.php @@ -1,4 +1,4 @@ - + {{ __('settings.subscription.change.title') }} diff --git a/resources/views/settings/subscription/index.blade.php b/resources/views/settings/subscription/index.blade.php index 11ef420218..e8e50f36ae 100644 --- a/resources/views/settings/subscription/index.blade.php +++ b/resources/views/settings/subscription/index.blade.php @@ -160,7 +160,7 @@ - + @endsection diff --git a/routes/web.php b/routes/web.php index b1222b5cb9..a56e289dc4 100644 --- a/routes/web.php +++ b/routes/web.php @@ -35,4 +35,4 @@ Route::post('roadmap/{feature}/upvote', [App\Http\Controllers\Roadmap\FeatureController::class, 'upvote'])->name('roadmap.upvote'); Route::post('roadmap/submit', [App\Http\Controllers\Roadmap\FeatureController::class, 'store'])->name('roadmap.store'); -Route::get('/users/{user}/validation', [App\Http\Controllers\User\EmailValidationController::class, 'validateEmail'])->name('validation.email'); +Route::get('/validation/{userValidation}', [App\Http\Controllers\User\EmailValidationController::class, 'validateEmail'])->name('validation.email');