Skip to content

Commit

Permalink
Change logic, bind token to routing to avoid exposing user ids
Browse files Browse the repository at this point in the history
  • Loading branch information
ilestis committed Feb 19, 2024
1 parent 7b87407 commit e9e0d4f
Show file tree
Hide file tree
Showing 13 changed files with 48 additions and 50 deletions.
1 change: 0 additions & 1 deletion app/Http/Controllers/Settings/SubscriptionController.php
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ public function change(Request $request, Tier $tier)
'hasPromo',
'limited',
'isYearly',
'validationService',
));
}

Expand Down
19 changes: 4 additions & 15 deletions app/Http/Controllers/User/EmailValidationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
}
Expand Down
12 changes: 7 additions & 5 deletions app/Jobs/Emails/Subscriptions/EmailValidationJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -23,25 +24,26 @@ 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;
}

/**
* @throws \Illuminate\Contracts\Container\BindingResolutionException
*/
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(
Expand Down
2 changes: 1 addition & 1 deletion app/Models/Relations/UserRelations.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
Expand Down
9 changes: 9 additions & 0 deletions app/Models/UserValidation.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -22,6 +26,11 @@ class UserValidation extends Model
'token'
];

public function getRouteKeyName()
{
return 'token';
}

/**
* @return \Illuminate\Database\Eloquent\Relations\BelongsTo
*/
Expand Down
2 changes: 2 additions & 0 deletions app/Providers/RouteServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -42,6 +43,7 @@ public function boot(): void
return Campaign::acl($value)->firstOrFail();
});
Route::model('entityType', EntityType::class);
Route::model('userValidation', UserValidation::class);
}

/**
Expand Down
34 changes: 17 additions & 17 deletions app/Services/Users/EmailValidationService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
2 changes: 2 additions & 0 deletions app/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lang/en/emails/validation.php
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?php

return [
'modal' => '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.',
];
7 changes: 1 addition & 6 deletions resources/views/components/dialog/close.blade.php
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
@if ($modal || $dismiss == 'alert')<button type="button" class="text-xl opacity-50 hover:opacity-100 focus:opacity-100 cursor-pointer text-decoration-none" data-dismiss="{{ $dismiss ?? 'modal' }}" aria-label="{{ __('crud.delete_modal.close') }}">
<button type="button" class="text-xl opacity-50 hover:opacity-100 focus:opacity-100 cursor-pointer text-decoration-none" data-dismiss="{{ $dismiss ?? 'modal' }}" aria-label="{{ __('crud.delete_modal.close') }}">
<x-icon class="fa-regular fa-circle-xmark"></x-icon>
<span class="sr-only">{{ __('crud.delete_modal.close') }}</span>
</button>@else
<button autofocus type="button" class="float-right text-xl rounded-full bg-base-300 opacity-80 hover:opacity-100" onclick="this.closest('dialog').close('close')" title="{{ __('crud.delete_modal.close') }}">
<x-icon class="fa-regular fa-times"></x-icon>
<span class="sr-only">{{ __('crud.delete_modal.close') }}</span>
</button>
@endif
2 changes: 1 addition & 1 deletion resources/views/settings/subscription/change.blade.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<x-dialog.header>
<x-dialog.header dismiss="alert">
{{ __('settings.subscription.change.title') }}
</x-dialog.header>

Expand Down
2 changes: 1 addition & 1 deletion resources/views/settings/subscription/index.blade.php
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@
</ul>
</x-dialog>

<x-dialog id="subscribe-confirm" :loading="true"></x-dialog>
<x-dialog id="subscribe-confirm" :loading="true" ></x-dialog>
@endsection


Expand Down
2 changes: 1 addition & 1 deletion routes/web.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');

0 comments on commit e9e0d4f

Please sign in to comment.