From fbe174adda7251acc8c738652bf4231104a7863a Mon Sep 17 00:00:00 2001 From: marvelefe Date: Sat, 18 Dec 2021 00:17:34 +0100 Subject: [PATCH 1/8] Added check inside the create method of the RegisterController to find duplicate github ID --- app/Http/Controllers/Auth/RegisterController.php | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/app/Http/Controllers/Auth/RegisterController.php b/app/Http/Controllers/Auth/RegisterController.php index 8472691ed..6686b206f 100644 --- a/app/Http/Controllers/Auth/RegisterController.php +++ b/app/Http/Controllers/Auth/RegisterController.php @@ -11,6 +11,7 @@ use Illuminate\Contracts\Validation\Validator as ValidatorContract; use Illuminate\Foundation\Auth\RegistersUsers; use Illuminate\Support\Facades\Validator; +use Illuminate\Database\Eloquent\ModelNotFoundException; class RegisterController extends Controller { @@ -56,7 +57,15 @@ protected function validator(array $data): ValidatorContract * Create a new user instance after a valid registration. */ protected function create(array $data): User - { - return $this->dispatchNow(RegisterUser::fromRequest(app(RegisterRequest::class))); - } + { + try { + $user = User::findByGithubId($data['github_id']); + if ($user instanceof User) { + $this->error('errors.github_account_exists'); + return redirect()->route('login'); + } + } catch (ModelNotFoundException $exception) { + return $this->dispatchNow(RegisterUser::fromRequest(app(RegisterRequest::class))); + } + } } From 29812ecd747ca93dacd531c80fbe909fd26361a3 Mon Sep 17 00:00:00 2001 From: marvelefe Date: Sat, 18 Dec 2021 00:18:30 +0100 Subject: [PATCH 2/8] Added error message saying "We already found a user with the given GitHub account....... --- resources/lang/en/errors.php | 1 + 1 file changed, 1 insertion(+) diff --git a/resources/lang/en/errors.php b/resources/lang/en/errors.php index 6d2dc3c01..e5e9e5e67 100644 --- a/resources/lang/en/errors.php +++ b/resources/lang/en/errors.php @@ -5,4 +5,5 @@ 'fields' => 'Something went wrong. Please review the fields below.', 'github_invalid_state' => 'The request timed out. Please try again.', 'github_account_too_young' => 'Your Github account needs to be older than 2 weeks in order to register.', + 'github_account_exists' => 'We already found a user with the given GitHub account (https://github/'.session('githubData')['username'].'). Would you like to login instead?' ]; From d2c9ca98203b2a02c37e56b09abc69f2cc33c1ab Mon Sep 17 00:00:00 2001 From: marvelefe Date: Sat, 18 Dec 2021 00:40:57 +0100 Subject: [PATCH 3/8] fixed failing test --- resources/lang/en/errors.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/resources/lang/en/errors.php b/resources/lang/en/errors.php index e5e9e5e67..a86efadd5 100644 --- a/resources/lang/en/errors.php +++ b/resources/lang/en/errors.php @@ -1,9 +1,10 @@ - 'This account is banned.', 'fields' => 'Something went wrong. Please review the fields below.', 'github_invalid_state' => 'The request timed out. Please try again.', 'github_account_too_young' => 'Your Github account needs to be older than 2 weeks in order to register.', - 'github_account_exists' => 'We already found a user with the given GitHub account (https://github/'.session('githubData')['username'].'). Would you like to login instead?' + 'github_account_exists' => 'We already found a user with the given GitHub account (https://github/'.$githubUsername ?? ''.'). Would you like to login instead?' ]; From 1d989ea9908d441857b7bd1e37e2138992cdd337 Mon Sep 17 00:00:00 2001 From: marvelefe Date: Sat, 18 Dec 2021 00:52:06 +0100 Subject: [PATCH 4/8] fixed stycli errors --- app/Http/Controllers/Auth/RegisterController.php | 6 +++++- resources/lang/en/errors.php | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/Auth/RegisterController.php b/app/Http/Controllers/Auth/RegisterController.php index 6686b206f..4b2aa499b 100644 --- a/app/Http/Controllers/Auth/RegisterController.php +++ b/app/Http/Controllers/Auth/RegisterController.php @@ -9,9 +9,9 @@ use App\Models\User; use App\Providers\RouteServiceProvider; use Illuminate\Contracts\Validation\Validator as ValidatorContract; +use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Foundation\Auth\RegistersUsers; use Illuminate\Support\Facades\Validator; -use Illuminate\Database\Eloquent\ModelNotFoundException; class RegisterController extends Controller { @@ -57,15 +57,19 @@ protected function validator(array $data): ValidatorContract * Create a new user instance after a valid registration. */ protected function create(array $data): User + { try { $user = User::findByGithubId($data['github_id']); if ($user instanceof User) { + $this->error('errors.github_account_exists'); + return redirect()->route('login'); } } catch (ModelNotFoundException $exception) { return $this->dispatchNow(RegisterUser::fromRequest(app(RegisterRequest::class))); } + } } diff --git a/resources/lang/en/errors.php b/resources/lang/en/errors.php index a86efadd5..5a799dba0 100644 --- a/resources/lang/en/errors.php +++ b/resources/lang/en/errors.php @@ -1,4 +1,5 @@ 'The request timed out. Please try again.', 'github_account_too_young' => 'Your Github account needs to be older than 2 weeks in order to register.', 'github_account_exists' => 'We already found a user with the given GitHub account (https://github/'.$githubUsername ?? ''.'). Would you like to login instead?' + ]; From d67304ed53d8da4f9e63b574950a043c528d52f9 Mon Sep 17 00:00:00 2001 From: marvelefe Date: Sat, 18 Dec 2021 00:59:31 +0100 Subject: [PATCH 5/8] styleci fixes --- resources/lang/en/errors.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/lang/en/errors.php b/resources/lang/en/errors.php index 5a799dba0..8c122b634 100644 --- a/resources/lang/en/errors.php +++ b/resources/lang/en/errors.php @@ -7,6 +7,6 @@ 'fields' => 'Something went wrong. Please review the fields below.', 'github_invalid_state' => 'The request timed out. Please try again.', 'github_account_too_young' => 'Your Github account needs to be older than 2 weeks in order to register.', - 'github_account_exists' => 'We already found a user with the given GitHub account (https://github/'.$githubUsername ?? ''.'). Would you like to login instead?' - + 'github_account_exists' => 'We already found a user with the given GitHub account (https://github/'.$githubUsername ?? ''.'). Would you like to login instead?', + ]; From c0c692eaf149b3c110453ecafff85b88d4d21228 Mon Sep 17 00:00:00 2001 From: marvelefe Date: Sat, 18 Dec 2021 01:02:09 +0100 Subject: [PATCH 6/8] styleci fixes --- app/Http/Controllers/Auth/RegisterController.php | 12 +++++------- resources/lang/en/errors.php | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/app/Http/Controllers/Auth/RegisterController.php b/app/Http/Controllers/Auth/RegisterController.php index 4b2aa499b..e37925eac 100644 --- a/app/Http/Controllers/Auth/RegisterController.php +++ b/app/Http/Controllers/Auth/RegisterController.php @@ -57,19 +57,17 @@ protected function validator(array $data): ValidatorContract * Create a new user instance after a valid registration. */ protected function create(array $data): User - - { + { try { $user = User::findByGithubId($data['github_id']); if ($user instanceof User) { - $this->error('errors.github_account_exists'); + $this->error('errors.github_account_exists'); return redirect()->route('login'); - } + } } catch (ModelNotFoundException $exception) { return $this->dispatchNow(RegisterUser::fromRequest(app(RegisterRequest::class))); - } - - } + } + } } diff --git a/resources/lang/en/errors.php b/resources/lang/en/errors.php index 8c122b634..a8f5602ac 100644 --- a/resources/lang/en/errors.php +++ b/resources/lang/en/errors.php @@ -1,4 +1,4 @@ - Date: Sat, 18 Dec 2021 01:03:11 +0100 Subject: [PATCH 7/8] more styleci fixes --- app/Http/Controllers/Auth/RegisterController.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/Http/Controllers/Auth/RegisterController.php b/app/Http/Controllers/Auth/RegisterController.php index e37925eac..5880fe12c 100644 --- a/app/Http/Controllers/Auth/RegisterController.php +++ b/app/Http/Controllers/Auth/RegisterController.php @@ -61,13 +61,12 @@ protected function create(array $data): User try { $user = User::findByGithubId($data['github_id']); if ($user instanceof User) { - $this->error('errors.github_account_exists'); return redirect()->route('login'); } } catch (ModelNotFoundException $exception) { return $this->dispatchNow(RegisterUser::fromRequest(app(RegisterRequest::class))); - } + } } } From 83c0e3d876c3fd47431490cc767640a9b592e078 Mon Sep 17 00:00:00 2001 From: Dries Vints Date: Sat, 25 Dec 2021 15:23:54 +0100 Subject: [PATCH 8/8] wip --- .../Controllers/Auth/RegisterController.php | 12 +------ app/Http/Requests/RegisterRequest.php | 3 +- app/Rules/UniqueGitHubUser.php | 34 +++++++++++++++++++ resources/lang/en/errors.php | 5 +-- tests/Feature/AuthTest.php | 19 +++++++++++ 5 files changed, 57 insertions(+), 16 deletions(-) create mode 100644 app/Rules/UniqueGitHubUser.php diff --git a/app/Http/Controllers/Auth/RegisterController.php b/app/Http/Controllers/Auth/RegisterController.php index 5880fe12c..8472691ed 100644 --- a/app/Http/Controllers/Auth/RegisterController.php +++ b/app/Http/Controllers/Auth/RegisterController.php @@ -9,7 +9,6 @@ use App\Models\User; use App\Providers\RouteServiceProvider; use Illuminate\Contracts\Validation\Validator as ValidatorContract; -use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Foundation\Auth\RegistersUsers; use Illuminate\Support\Facades\Validator; @@ -58,15 +57,6 @@ protected function validator(array $data): ValidatorContract */ protected function create(array $data): User { - try { - $user = User::findByGithubId($data['github_id']); - if ($user instanceof User) { - $this->error('errors.github_account_exists'); - - return redirect()->route('login'); - } - } catch (ModelNotFoundException $exception) { - return $this->dispatchNow(RegisterUser::fromRequest(app(RegisterRequest::class))); - } + return $this->dispatchNow(RegisterUser::fromRequest(app(RegisterRequest::class))); } } diff --git a/app/Http/Requests/RegisterRequest.php b/app/Http/Requests/RegisterRequest.php index ad09dd436..c776caca3 100644 --- a/app/Http/Requests/RegisterRequest.php +++ b/app/Http/Requests/RegisterRequest.php @@ -2,6 +2,7 @@ namespace App\Http\Requests; +use App\Rules\UniqueGitHubUser; use Illuminate\Foundation\Http\FormRequest; class RegisterRequest extends FormRequest @@ -19,7 +20,7 @@ public function rules() 'username' => 'required|alpha_dash|max:40|unique:users', 'rules' => 'accepted', 'terms' => 'accepted', - 'github_id' => 'required', + 'github_id' => ['required', new UniqueGitHubUser], ]; } diff --git a/app/Rules/UniqueGitHubUser.php b/app/Rules/UniqueGitHubUser.php new file mode 100644 index 000000000..e326d2d65 --- /dev/null +++ b/app/Rules/UniqueGitHubUser.php @@ -0,0 +1,34 @@ +user = User::findByGithubId($value); + } catch (ModelNotFoundException) { + return true; + } + + return false; + } + + public function message() + { + $this->error('errors.github_account_exists', [ + 'username' => '@'.$this->user->githubUsername(), + 'login' => route('login'), + ]); + } +} diff --git a/resources/lang/en/errors.php b/resources/lang/en/errors.php index a8f5602ac..55c8844e8 100644 --- a/resources/lang/en/errors.php +++ b/resources/lang/en/errors.php @@ -1,12 +1,9 @@ 'This account is banned.', 'fields' => 'Something went wrong. Please review the fields below.', 'github_invalid_state' => 'The request timed out. Please try again.', 'github_account_too_young' => 'Your Github account needs to be older than 2 weeks in order to register.', - 'github_account_exists' => 'We already found a user with the given GitHub account (https://github/'.$githubUsername ?? ''.'). Would you like to login instead?', - + 'github_account_exists' => 'We already found a user with the given GitHub account (:username). Would you like to login instead?', ]; diff --git a/tests/Feature/AuthTest.php b/tests/Feature/AuthTest.php index 44dc9a1dd..d70e5296d 100644 --- a/tests/Feature/AuthTest.php +++ b/tests/Feature/AuthTest.php @@ -1,5 +1,6 @@ see('The username must only contain letters, numbers, dashes and underscores.'); }); +test('registration fails with a duplicate github id', function () { + User::factory()->create(['github_id' => 123, 'github_username' => 'johndoe']); + + session(['githubData' => ['id' => 123, 'username' => 'johndoe']]); + + $this->visit('/register') + ->type('John Doe', 'name') + ->type('john.doe@example.com', 'email') + ->type('johndoe', 'username') + ->type('123', 'github_id') + ->type('johndoe', 'github_username') + ->check('rules') + ->check('terms') + ->press('Register') + ->seePageIs('/register') + ->see('We already found a user with the given GitHub account (@johndoe). Would you like to login instead?'); +}); + test('users can resend the email verification', function () { $this->login(['email_verified_at' => null]);