From 0694b93b4a26ae308b22b8b5eeb993b7d7eea457 Mon Sep 17 00:00:00 2001 From: Jamie Sykes Date: Wed, 10 Jul 2024 12:31:09 +0100 Subject: [PATCH 1/2] feat: implements error handling when username is required --- src/controllers/MagicLoginController.php | 8 +++++- test-config/general.php | 1 + tests/_craft/config/general.php | 1 + tests/functional/LoginFormTest.php | 2 +- tests/functional/SignupFormCest.php | 36 ++++++++++++++++++++++++ 5 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 tests/functional/SignupFormCest.php diff --git a/src/controllers/MagicLoginController.php b/src/controllers/MagicLoginController.php index 150ebaf..407abc5 100755 --- a/src/controllers/MagicLoginController.php +++ b/src/controllers/MagicLoginController.php @@ -11,10 +11,10 @@ namespace creode\magiclogin\controllers; use Craft; +use DateTime; use craft\elements\User; use craft\web\Controller; use creode\magiclogin\MagicLogin; -use DateTime; /** * MagicLogin Controller @@ -191,6 +191,12 @@ public function actionRegister() ->getRequest() ->getRequiredParam('email'); + $generalConfig = Craft::$app->getConfig()->getGeneral(); + if (! $generalConfig->useEmailAsUsername) { + $this->setFailFlash(\Craft::t('magic-login', 'Please enter a valid username.')); + return; + } + if (!filter_var($email, FILTER_VALIDATE_EMAIL)) { // TODO: Maybe set this to be configurable in future. $this->setFailFlash(\Craft::t('magic-login', 'Please enter a valid email address.')); diff --git a/test-config/general.php b/test-config/general.php index 6b398e9..9e1affa 100644 --- a/test-config/general.php +++ b/test-config/general.php @@ -2,4 +2,5 @@ return [ 'postLoginRedirect' => '', + 'useEmailAsUsername' => true, ]; diff --git a/tests/_craft/config/general.php b/tests/_craft/config/general.php index 8684f17..c7774f1 100644 --- a/tests/_craft/config/general.php +++ b/tests/_craft/config/general.php @@ -8,4 +8,5 @@ 'requireUserAgentAndIpForSession' => false, 'securityKey' => App::env('SECURITY_KEY'), 'enableCsrfProtection' => false, + 'useEmailAsUsername' => true, ]; diff --git a/tests/functional/LoginFormTest.php b/tests/functional/LoginFormTest.php index 5382479..eccae5b 100644 --- a/tests/functional/LoginFormTest.php +++ b/tests/functional/LoginFormTest.php @@ -100,7 +100,7 @@ public function testUnregisteredUserSignup() $this->tester->submitForm( '#magic-login-form', [ - 'email' => 'test@example.com', + 'email' => 'test-2@example.com', ], 'submitButton' ); diff --git a/tests/functional/SignupFormCest.php b/tests/functional/SignupFormCest.php new file mode 100644 index 0000000..c645436 --- /dev/null +++ b/tests/functional/SignupFormCest.php @@ -0,0 +1,36 @@ +getConfig()->getGeneral(); + $generalConfig->useEmailAsUsername = false; + } + + public function testSignupFormWithoutRequiredUsername(FunctionalTester $I) + { + $I->amOnPage('/magic-login/register'); + + // Attempt to submit the form with only an email address + $I->fillField('email', 'test@example.com'); + $I->click('Submit'); + + // Expect error messages and no redirection + $I->dontSeeInCurrentUrl('/confirmation'); + $I->see('Please enter a valid username'); // Adjust according to the actual error message expected + } + + public function _after(FunctionalTester $I) + { + // Reset useEmailAsUsername to true + $generalConfig = Craft::$app->getConfig()->getGeneral(); + $generalConfig->useEmailAsUsername = true; + } +} From ea38abd65348978c7feacd113bdb5a3229534dfc Mon Sep 17 00:00:00 2001 From: Jamie Sykes Date: Wed, 10 Jul 2024 12:31:33 +0100 Subject: [PATCH 2/2] feat: add improved checks when public registration is disabled. --- src/controllers/MagicLoginController.php | 11 ++++++++ tests/functional/RegistrationFormTest.php | 31 +++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/src/controllers/MagicLoginController.php b/src/controllers/MagicLoginController.php index 407abc5..999cfc6 100755 --- a/src/controllers/MagicLoginController.php +++ b/src/controllers/MagicLoginController.php @@ -15,6 +15,7 @@ use craft\elements\User; use craft\web\Controller; use creode\magiclogin\MagicLogin; +use yii\web\NotFoundHttpException; /** * MagicLogin Controller @@ -162,6 +163,11 @@ public function actionRegisterForm() $this->redirect($generalConfig->postLoginRedirect); } + $userConfig = Craft::$app->getProjectConfig()->get('users') ?? []; + if (! $userConfig['allowPublicRegistration']) { + throw new NotFoundHttpException(); + } + return $this->renderTemplate('magic-login/_register-form'); } @@ -181,6 +187,11 @@ public function actionRegister() ) ); + $userSettings = Craft::$app->getProjectConfig()->get('users'); + if (!$userSettings['allowPublicRegistration']) { + throw new NotFoundHttpException(); + } + if (Craft::$app->getUser()->getIdentity()) { $generalConfig = Craft::$app->getConfig()->getGeneral(); $this->setSuccessFlash(\Craft::t('magic-login', 'You are already logged in.')); diff --git a/tests/functional/RegistrationFormTest.php b/tests/functional/RegistrationFormTest.php index 80a5d67..05c00ad 100644 --- a/tests/functional/RegistrationFormTest.php +++ b/tests/functional/RegistrationFormTest.php @@ -365,6 +365,37 @@ public function testWhenRegistrationErrorOccursUserIsNotCreated() $this->assertEquals($userCount, count(User::find()->all())); } + /** + * Test that we return a 404, not the user registration page when public registration is disabled. + */ + public function testWhenPublicRegistrationDisabledUserCannotRegister() + { + $userSettings = Craft::$app->getProjectConfig()->get('users') ?? []; + $userSettings['allowPublicRegistration'] = false; + Craft::$app->projectConfig->set('users', $userSettings); + + $this->tester->amOnPage('/magic-login/register'); + $this->tester->seeResponseCodeIs(404); + } + + public function testWhenPublicRegistrationDisabledUserCannotRegisterWithMagicLink() + { + $this->tester->amOnPage('/magic-login/register'); + + $userSettings = Craft::$app->getProjectConfig()->get('users') ?? []; + $userSettings['allowPublicRegistration'] = false; + + Craft::$app->projectConfig->set('users', $userSettings); + $this->tester->submitForm( + '#magic-login-register', + [ + 'email' => 'creode-test@example.com', + ], + ); + + $this->tester->seeResponseCodeIs(404); + } + /** * Tests that when a user is successfully registered the magic * login group is attached.