diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bb3008..6873134 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,16 @@ All notable changes to this project will be documented in this file. +## 3.1.2 - 2024-08-30 + +* Backports several features to 3.x + +### Features + +* Add in functionality to allow a configurable amount of times a link can be accessed before it is expired. (65c274) +* Add migrations to count how many times a link has been accessed (fa28ee) +* Disable settings field and display message to user if being overridden in config (1a5dbf) + ## 3.1.1 - 2023-04-12 ### Bug Fixes diff --git a/composer.json b/composer.json index d1a7519..d253fb8 100755 --- a/composer.json +++ b/composer.json @@ -77,5 +77,5 @@ }, "class": "creode\\magiclogin\\MagicLogin" }, - "version": "3.1.1" + "version": "3.1.2" } \ No newline at end of file diff --git a/src/MagicLogin.php b/src/MagicLogin.php index ed117b8..b306f87 100755 --- a/src/MagicLogin.php +++ b/src/MagicLogin.php @@ -70,7 +70,7 @@ class MagicLogin extends Plugin * * @var string */ - public string $schemaVersion = '2.0.0'; + public string $schemaVersion = '2.1.0'; /** * Set to `true` if the plugin should have a settings view in the control panel. @@ -423,10 +423,14 @@ public function getSettingsResponse(): mixed /** @var Controller $controller */ $controller = Craft::$app->controller; + $overrides = Craft::$app->getConfig()->getConfigFromFile(strtolower($this->handle)); + + return $controller->renderTemplate('magic-login/settings', [ 'plugin' => $this, 'settingsHtml' => $settingsHtml, 'settings' => $this->getSettings(), + 'overrides' => array_keys($overrides), ]); } diff --git a/src/config.php b/src/config.php index e7e77d7..af1d450 100755 --- a/src/config.php +++ b/src/config.php @@ -31,4 +31,6 @@ 'authenticationEmailSubject' => 'Magic Login Link', // Rate Limit for how frequently a magic login email can be sent (in minutes). 'emailRateLimit' => 5, + // Amount of times a link can be accessed before it expires. + 'linkAccessLimit' => 1, ]; diff --git a/src/controllers/MagicLoginController.php b/src/controllers/MagicLoginController.php index ba5e7b9..9d75754 100755 --- a/src/controllers/MagicLoginController.php +++ b/src/controllers/MagicLoginController.php @@ -326,8 +326,14 @@ public function actionAuth($publicKey, $timestamp, $signature) return $this->redirect($loginUrl); } - // Remove the auth record since we are logged in now. - $authRecord->delete(); + // Increment the access count. + $authRecord->accessCount++; + $authRecord->save(); + + if ($authRecord->hasExpired()) { + // Remove the auth record since we are logged in now. + $authRecord->delete(); + } // Redirect user to the url provided by the login page. return $this->redirect($authRecord->redirectUrl); diff --git a/src/migrations/Install.php b/src/migrations/Install.php index 3e4dc6a..32a8cfa 100755 --- a/src/migrations/Install.php +++ b/src/migrations/Install.php @@ -111,6 +111,7 @@ protected function createTables() 'publicKey' => $this->string()->notNull(), 'privateKey' => $this->string()->notNull(), 'redirectUrl' => $this->string()->null(), + 'accessCount' => $this->integer()->notNull()->defaultValue(0), 'dateCreated' => $this->dateTime()->notNull(), 'dateUpdated' => $this->dateTime()->notNull(), // 'siteId' => $this->integer()->notNull(), - I don't think this is required right now but may be in future. diff --git a/src/migrations/m240615_120115_add_access_count_field.php b/src/migrations/m240615_120115_add_access_count_field.php new file mode 100644 index 0000000..70a53e2 --- /dev/null +++ b/src/migrations/m240615_120115_add_access_count_field.php @@ -0,0 +1,35 @@ +addColumn('{{%magiclogin_authrecord}}', 'accessCount', $this->integer()->defaultValue(0)); + return true; + } + + /** + * @inheritdoc + */ + public function safeDown(): bool + { + $this->dropColumn('{{%magiclogin_authrecord}}', 'accessCount'); + return true; + } +} diff --git a/src/models/AuthModel.php b/src/models/AuthModel.php index 9c7d377..2adb6c8 100755 --- a/src/models/AuthModel.php +++ b/src/models/AuthModel.php @@ -61,6 +61,13 @@ class AuthModel extends Model */ public $redirectUrl; + /** + * Amount of times the link has been accessed. + * + * @var integer + */ + public $accessCount = 0; + /** * Creation date of the record. * @@ -83,6 +90,12 @@ class AuthModel extends Model */ public function isExpired() { + // Access count. + $linkAccessLimit = MagicLogin::getInstance()->getSettings()->linkAccessLimit; + if ($linkAccessLimit !== null && $this->accessCount >= $linkAccessLimit) { + return true; + } + // Check if timestamp is within bounds set by plugin configuration $linkExpiryAmount = MagicLogin::getInstance()->getSettings()->linkExpiry; $dateCreated = new \DateTime($this->dateCreated->format('Y-m-d H:i:s'), new \DateTimeZone('UTC')); @@ -121,7 +134,7 @@ public function rules(): array { $rules = parent::rules(); $rules[] = [['publicKey', 'privateKey', 'redirectUrl'], 'string']; - $rules[] = [['userId'], 'number']; + $rules[] = [['userId', 'accessCount'], 'number']; $rules[] = [['dateCreated', 'nextEmailSend'], DateTimeValidator::class]; return $rules; diff --git a/src/models/Settings.php b/src/models/Settings.php index f765d1c..145b148 100755 --- a/src/models/Settings.php +++ b/src/models/Settings.php @@ -59,6 +59,13 @@ class Settings extends Model */ public ?int $emailRateLimit = 5; + /** + * How many times a login link can be accessed before it expires. + * + * @var integer + */ + public ?int $linkAccessLimit = 1; + // TODO: Add a setting to say if magic login click should also verify a user. // Grey out the option if verification is disabled on the website. @@ -78,7 +85,7 @@ class Settings extends Model public function rules(): array { return [ - [['linkExpiry', 'passwordLength', 'emailRateLimit'], 'number'], + [['linkExpiry', 'passwordLength', 'emailRateLimit', 'linkAccessLimit'], 'number'], [['authenticationEmailSubject'], 'string'], ]; } diff --git a/src/records/AuthRecord.php b/src/records/AuthRecord.php index bff0b5f..75c5e9b 100755 --- a/src/records/AuthRecord.php +++ b/src/records/AuthRecord.php @@ -52,4 +52,34 @@ public static function tableName() { return '{{%magiclogin_authrecord}}'; } + + /** + * Determine if we have hit the access limit for the auth record. + * + * @return boolean + */ + public function hasHitAccessLimit() { + $linkAccessLimit = MagicLogin::$plugin->getSettings()->linkAccessLimit; + if ($linkAccessLimit !== null && $this->accessCount >= $linkAccessLimit) { + return true; + } + + return false; + } + + public function hasExpired() { + // Check if timestamp is within bounds set by plugin configuration + $linkExpiryAmount = MagicLogin::getInstance()->getSettings()->linkExpiry; + $dateCreated = new \DateTime($this->dateCreated, new \DateTimeZone('UTC')); + $expiryTimestamp = $dateCreated->getTimestamp() + ($linkExpiryAmount * 60); + if ($expiryTimestamp < time()) { + return true; + } + + if ($this->hasHitAccessLimit()) { + return true; + } + + return false; + } } diff --git a/src/templates/settings.twig b/src/templates/settings.twig index 97f8e72..ceeb482 100755 --- a/src/templates/settings.twig +++ b/src/templates/settings.twig @@ -24,6 +24,15 @@ {% import "_includes/forms" as forms %} +{% macro configWarning(setting) -%} + {% set setting = ''~setting~'' %} + {{ "This is being overridden by the {setting} config setting in your {file} config file."|t('magic-login', { + setting: setting, + file: 'magin-login.php' + })|raw }} +{%- endmacro %} +{% from _self import configWarning %} + {% block content %} {{ actionInput('plugins/save-plugin-settings') }} @@ -39,7 +48,9 @@ id: 'linkExpiry', name: 'linkExpiry', min: 1, - value: settings.linkExpiry}) + value: settings.linkExpiry, + disabled: 'linkExpiry' in overrides, + warning: 'linkExpiry' in overrides ? configWarning('linkExpiry')}) }} {{ forms.textField({ @@ -49,7 +60,9 @@ name: 'passwordLength', min: 16, max: 50, - value: settings.passwordLength}) + value: settings.passwordLength, + disabled: 'passwordLength' in overrides, + warning: 'passwordLength' in overrides ? configWarning('passwordLength')}) }} {{ forms.textField({ @@ -58,7 +71,20 @@ id: 'emailRateLimit', name: 'emailRateLimit', min: 1, - value: settings.emailRateLimit}) + value: settings.emailRateLimit, + disabled: 'emailRateLimit' in overrides, + warning: 'emailRateLimit' in overrides ? configWarning('emailRateLimit')}) + }} + + {{ forms.textField({ + label: 'Link Access Limit', + instructions: 'When sending magic login links, this number states how many times the link can be accessed before it expires.', + id: 'linkAccessLimit', + name: 'linkAccessLimit', + min: 1, + value: settings.linkAccessLimit, + disabled: 'linkAccessLimit' in overrides, + warning: 'linkAccessLimit' in overrides ? configWarning('linkAccessLimit')}) }} @@ -69,7 +95,9 @@ instructions: 'What to display on the subject line for Magic Link emails.', id: 'authenticationEmailSubject', name: 'authenticationEmailSubject', - value: settings.authenticationEmailSubject}) + value: settings.authenticationEmailSubject, + disabled: 'authenticationEmailSubject' in overrides, + warning: 'authenticationEmailSubject' in overrides ? configWarning('authenticationEmailSubject')}) }} {% endnamespace %} diff --git a/tests/_data/config/multiple-link.php b/tests/_data/config/multiple-link.php new file mode 100644 index 0000000..50749bd --- /dev/null +++ b/tests/_data/config/multiple-link.php @@ -0,0 +1,6 @@ + 'Here is your magic login link', + 'linkAccessLimit' => 3, +]; diff --git a/tests/_data/config/single-link.php b/tests/_data/config/single-link.php new file mode 100644 index 0000000..57c34e1 --- /dev/null +++ b/tests/_data/config/single-link.php @@ -0,0 +1,5 @@ + 'Here is your magic login link', +]; diff --git a/tests/_data/magiclogin_authrecord.php b/tests/_data/magiclogin_authrecord.php index 1013e32..29170bb 100644 --- a/tests/_data/magiclogin_authrecord.php +++ b/tests/_data/magiclogin_authrecord.php @@ -23,7 +23,7 @@ 'nextEmailSend' => null, ], 'test_user_4_auth_record' => [ - 'userId' => 6, + 'userId' => 7, 'publicKey' => 'nq37y47rn9qq1v753q85daa96oh35zpx0okrpcnn9806pzhy18guyytr3mdhtg5x', 'privateKey' => 'vgpcgc8yh8q2f7m9p7xxj8b8zd9jnp3m58ydp5w4a4dukk3ubcqwjp6zbef5582nkc5eew5ann86emmtk9kf8mkvg9yj4zfhbzz9ur5juw5h7d92zk55wnurx3q4twmd', 'redirectUrl' => '/', @@ -32,4 +32,14 @@ 'uid' => '1fc4a27f-7615-4d7a-9248-760b1099711b', 'nextEmailSend' => null, ], + 'test_login_expiry' => [ + 'userId' => 4, + 'publicKey' => 'kwnyvg7fxbbyg5v2tdesyahryu3u73ykxhad4z2u732239u2q4e995vjdjxsj3yz', + 'privateKey' => 'mgcewe6nnxm9g798kys3fdy9cprv4dpsx9xqymcwq2595gzx7wqvp2a6z4k3p7nzn4kn7x3zycqw3neybx98x3ezgrqba357j2hb7bcqsbwnjqeyxmpyvua23a4cuxnu', + 'redirectUrl' => '/', + 'dateCreated' => $date->format('Y-m-d H:i:s'), + 'dateUpdated' => $date->format('Y-m-d H:i:s'), + 'uid' => 'f46424ab-ffcf-47da-9a2c-f6fae94fd202', + 'nextEmailSend' => null, + ], ]; diff --git a/tests/_data/magiclogin_userrecord.php b/tests/_data/magiclogin_userrecord.php index e9f4574..d00a736 100644 --- a/tests/_data/magiclogin_userrecord.php +++ b/tests/_data/magiclogin_userrecord.php @@ -2,15 +2,24 @@ return [ 'pending_user' => [ - 'id' => 2, + // 'id' => 1, 'username' => 'test', 'email' => 'test@example.com', - 'pending' => 1, + 'active' => false, + 'uid' => '929e0997-03fa-401e-b745-99480e53bf97', ], 'creode_test_user' => [ - 'id' => 2, + // 'id' => 2, 'username' => 'test', 'email' => 'creode-test@example.com', - 'pending' => 1, + 'active' => false, + 'uid' => 'fe432693-99c9-44f6-9902-f09e246e2ca8', + ], + 'magic_link_expiry_user' => [ + // 'id' => 10, + 'username' => 'magic-login-expiry', + 'email' => 'magic-login-expiry@example.com', + 'uid' => '16b52ee5-4386-4fed-991e-63c4a06a7fd5', + 'active' => false, ], ]; diff --git a/tests/functional/LinkExpiryTest.php b/tests/functional/LinkExpiryTest.php new file mode 100644 index 0000000..4e42533 --- /dev/null +++ b/tests/functional/LinkExpiryTest.php @@ -0,0 +1,95 @@ +getConfig()->configDir . '/magic-login.php' + ); + } + + public function _after() { + // Copy the config file to the right folder + copy( + codecept_data_dir() . 'config/single-link.php', + Craft::$app->getConfig()->configDir . '/magic-login.php' + ); + } + + /** + * Sets up any fixtures used in this class. + * + * @return array + */ + public function _fixtures() + { + return [ + 'user_records' => [ + 'class' => UserRecordFixture::class, + // fixture data located in tests/_data/magiclogin_authrecord.php + 'dataFile' => codecept_data_dir() . 'magiclogin_userrecord.php' + ], + 'auth_records' => [ + 'class' => AuthRecordFixture::class, + 'dataFile' => codecept_data_dir() . 'magiclogin_authrecord.php' + ], + ]; + } + + /** + * Test that a link can only be used a certain number of times before it expires. + * + * @return void + */ + public function testALinkCanHaveAConfigurableNumberOfTimesBeforeItExpires() { + $user = User::findOne(['email' => $this->test_user_email]); + $authRecord = AuthRecord::findOne(['userId' => $user->id]); + $link = $this->generateValidMagicLink($authRecord); + + $linkAccessLimit = MagicLogin::$plugin->getSettings()->linkAccessLimit; + + for ($i = 0; $i < 3; $i++) { + $this->tester->amOnPage($link); + $this->tester->seeCurrentUrlEquals($authRecord->redirectUrl); + // Logout user. + $this->tester->amOnPage('/logout'); + } + + // Ensure that we are redirected to the login page. + $this->tester->amOnPage($link); + $this->tester->seeCurrentUrlEquals('/index.php?p=login'); + } + + // TODO: Test that the link expires after a certain amount of time. + + // TODO: Test that the link can be used multiple times before expiry. + + // TODO: Test the link expiry overall. +}