-
Notifications
You must be signed in to change notification settings - Fork 15
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Security Fix: reset password functionality Merge pull request #981 fr…
…om pli888/fix-reset-password Re-implementation of reset password functionality with improved security based on good standard practices. Essentially, the token consists of 2 parts: a selector and a verifier, and the token is only valid for use for 1 hour. The token validation process involves calculating the hash of the verifier and comparing it with the hash that has been saved in the database when the token was first created. If the hashes match then the token is valid if it has not expired.
- Loading branch information
Showing
37 changed files
with
1,075 additions
and
300 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| id,email,password,first_name,last_name,affiliation,role,is_activated,newsletter,previous_newsletter_state,facebook_id,twitter_id,linkedin_id,google_id,username,orcid_id,preferred_link | ||
| 8,[email protected],5a4f75053077a32e681f81daa8792f95,Guojie,Zhang,BGI,"",f,t,t,,,,,[email protected],,EBI | ||
| 14,[email protected],5a4f75053077a32e681f81daa8792f95,Shifeng,Cheng,BGI,"",f,t,t,,,,,[email protected],,EBI | ||
| 22,[email protected],5a4f75053077a32e681f81daa8792f95,John,Doe,BGI,user,true,false,true,,,,,[email protected],,EBI | ||
| 38,[email protected],bcf839ae349d13cb0c3c4a9ec0cc243e,C,H,BGI,admin,t,t,t,,,,114715812788877222335,[email protected],,EBI | ||
| 163,[email protected],5a4f75053077a32e681f81daa8792f95,Carson,Chow,National Institutes of Health,user,t,f,t,,,,,[email protected],,EBI | ||
| 336,[email protected],5a4f75053077a32e681f81daa8792f95,Hugh,Shanahan,"Department of Computer Science, Royal Holloway, University of London, Egham, TW20 0EX, UK","",f,t,f,,,,,[email protected],,EBI | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| selector,hashed_token,requested_at,expires_at,gigadb_user_id | ||
| 6_WVbmz1e-nm6YPm2sTZ,IBDO6gfgkejy3XF9H5lgVqs4pe0oZf1Li1Fm7DaGPvk=,2022-03-01 01:53:23.000000,9999-03-01 02:53:23.000000,22 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
26 changes: 26 additions & 0 deletions
26
ops/configuration/yii-conf/triple-stash.migration.php.dist
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| {{! A handlebars template to generate Yii migration scripts}} | ||
| <?php | ||
|
|
||
| class {{ class_name }} extends CDbMigration | ||
| { | ||
| public function safeUp() | ||
| { | ||
| {{#each safeup_data}} | ||
| $this->insert('{{ ../table_name }}', array( | ||
| {{#each this}} | ||
| {{#if this}} | ||
| '{{@key}}' => '{{{this}}}', | ||
| {{/if}} | ||
| {{/each}} | ||
| )); | ||
| {{/each}} | ||
| } | ||
|
|
||
| public function safeDown() | ||
| { | ||
| $ids = array({{#each safedown_data}}'{{this}}',{{/each}}); | ||
| foreach ($ids as $id) { | ||
| $this->delete('{{table_name}}', 'id=:id', array(':id' => $id)); | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| <?php | ||
|
|
||
| /** | ||
| * Service to provide tokens for password reset functionality | ||
| */ | ||
| class CryptoService extends yii\base\Component | ||
| { | ||
| const RANDOM_STRING_LENGTH = 20; | ||
|
|
||
| /** | ||
| * Initializes application component. | ||
| * This method overrides the parent implementation by setting default cache | ||
| * key prefix. | ||
| */ | ||
| public function init() | ||
| { | ||
| parent::init(); | ||
| } | ||
|
|
||
| /** | ||
| * Returns the hash of a token. Used to generate a hash for the verifier. | ||
| * | ||
| * @param string $signingKey Unique, random, cryptographically secure string | ||
| * @param string $data Token to be hashed | ||
| * @return string | ||
| */ | ||
| public static function getHashedToken(string $signingKey, string $data) | ||
| { | ||
| $hash = base64_encode(hash_hmac('sha256', $data, $signingKey, true)); | ||
| return $hash; | ||
| } | ||
|
|
||
| /** | ||
| * Uses method in Yii2 to generate random string | ||
| * | ||
| * String length is 20 characters | ||
| */ | ||
| public static function getRandomString() | ||
| { | ||
| return Yii::$app->security->generateRandomString(self::RANDOM_STRING_LENGTH); | ||
| } | ||
| } | ||
| ?> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| <?php | ||
| /** | ||
| * Class to handle the identity of user and how they authenticate | ||
| * | ||
| * @see https://www.yiiframework.com/doc/guide/1.1/en/topics.auth | ||
| */ | ||
| class PasswordResetTokenUserIdentity extends UserIdentity { | ||
|
|
||
| public $_id; | ||
| private $random_string_length; | ||
| const ERROR_SELECTOR_NOT_ASSOCIATED_WITH_A_USER = 4; | ||
| const ERROR_RECALCULATED_HASH_OF_VERIFIER_DOES_NOT_MATCH_HASH_IN_DATABASE = 5; | ||
| const ERROR_TOKEN_HAS_EXPIRED = 6; | ||
|
|
||
| /** | ||
| * @var string Token from URL sent by email | ||
| */ | ||
| public $urlToken; | ||
|
|
||
| /** | ||
| * Constructor | ||
| */ | ||
| public function __construct($urlToken) | ||
| { | ||
| $this->urlToken = $urlToken; | ||
| $this->random_string_length = CryptoService::RANDOM_STRING_LENGTH; | ||
| } | ||
|
|
||
| /** | ||
| * Provides the authentication process for an anonymous user with a password | ||
| * reset token that consists of two parts: a selector and verifier. | ||
| * | ||
| * @return boolean whether authentication succeeds. True if successful, False otherwise | ||
| */ | ||
| public function authenticate() | ||
| { | ||
| Yii::log("[INFO] [".__CLASS__.".php] ".__FUNCTION__.": In PasswordResetTokenUserIdentity::authenticate()", 'info'); | ||
|
|
||
| // Find user associated with selector part in URL | ||
| $selectorFromURL = substr($this->urlToken, 0, $this->random_string_length); | ||
| $resetPasswordRequest = ResetPasswordRequest::model()->findByAttributes(array('selector' => $selectorFromURL)); | ||
| $user = User::model()->findByAttributes(array('id' => $resetPasswordRequest->gigadb_user_id)); | ||
|
|
||
| if ($user === null) // User not found | ||
| { | ||
| $this->errorCode = self::ERROR_SELECTOR_NOT_ASSOCIATED_WITH_A_USER; | ||
| } | ||
| else // User found | ||
| { | ||
| // Check re-calculated hash from verifier matches hash in reset_password_request database table | ||
| $signingKey = Yii::app()->params['signing_key']; | ||
| $verifierFromURL = substr($this->urlToken, $this->random_string_length, $this->random_string_length); | ||
| $hashedTokenFromURLVerifier = Yii::app()->cryptoService->getHashedToken($signingKey, $verifierFromURL); | ||
| if($hashedTokenFromURLVerifier === $resetPasswordRequest->hashed_token) | ||
| { | ||
| // Check if token has expired | ||
| if($resetPasswordRequest->isExpired()) { | ||
| Yii::log("[INFO] [".__CLASS__.".php] ".__FUNCTION__.": Token has expired: ", 'info'); | ||
| $this->errorCode = self::ERROR_TOKEN_HAS_EXPIRED; | ||
| } | ||
| else { | ||
| $this->_id = $user->id; | ||
| $this->errorCode = self::ERROR_NONE; | ||
| } | ||
| } | ||
| else | ||
| { | ||
| $this->errorCode = self::ERROR_RECALCULATED_HASH_OF_VERIFIER_DOES_NOT_MATCH_HASH_IN_DATABASE; | ||
| } | ||
| } | ||
| return !$this->errorCode; | ||
| } | ||
|
|
||
| public function getId() | ||
| { | ||
| return $this->_id; | ||
| } | ||
| } | ||
|
|
||
| ?> |
Oops, something went wrong.