Skip to content

Commit 29e0314

Browse files
amnahamnah
committed
remove user.new_email and use user_token.data instead
1 parent 2a8bb11 commit 29e0314

File tree

12 files changed

+98
-106
lines changed

12 files changed

+98
-106
lines changed

‎controllers/AuthController.php‎

Lines changed: 10 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,10 @@ protected function attemptLogin($client)
113113
{
114114
/** @var \amnah\yii2\user\models\User $user */
115115
/** @var \amnah\yii2\user\models\UserAuth $userAuth */
116+
/** @var \amnah\yii2\user\models\UserToken $userToken */
116117
$user = Yii::$app->getModule("user")->model("User");
117118
$userAuth = Yii::$app->getModule("user")->model("UserAuth");
119+
$userToken = Yii::$app->getModule("user")->model("UserToken");
118120

119121
// attempt to find userAuth in database by id and name
120122
$attributes = $client->getUserAttributes();
@@ -123,9 +125,9 @@ protected function attemptLogin($client)
123125
"provider_id" => (string)$attributes["id"],
124126
]);
125127
if ($userAuth) {
126-
$user = $user::findOne($userAuth->user_id);
127128

128-
// check if user is banned
129+
// check if user is banned, otherwise log in
130+
$user = $user::findOne($userAuth->user_id);
129131
if ($user && $user->banned_at) {
130132
return false;
131133
}
@@ -143,9 +145,13 @@ protected function attemptLogin($client)
143145
// attempt to find user by email
144146
if (!empty($user["email"])) {
145147

146-
// check if any user has `new_email` set and clear it
148+
// check if any user has has tried to change their email
149+
// if so, delete it
147150
$email = trim($user["email"]);
148-
$this->clearNewEmail($email);
151+
$userToken = $userToken::findByData($email, $userToken::TYPE_EMAIL_CHANGE);
152+
if ($userToken) {
153+
$userToken->delete();
154+
}
149155

150156
// find user and create user provider for match
151157
$user = $user::findOne(["email" => $email]);
@@ -160,30 +166,6 @@ protected function attemptLogin($client)
160166
return false;
161167
}
162168

163-
/**
164-
* Check to see if any user has changed their email but not yet confirmed it. It will
165-
* thus be in `user.new_email`, so we need to clear it in case they try to actually
166-
* confirm the change (which would result in an unique constraint error for email)
167-
* @param string $email
168-
*/
169-
protected function clearNewEmail($email)
170-
{
171-
/** @var \amnah\yii2\user\models\User $user */
172-
/** @var \amnah\yii2\user\models\UserToken $userToken */
173-
$user = Yii::$app->getModule("user")->model("User");
174-
$userToken = Yii::$app->getModule("user")->model("UserToken");
175-
176-
// attempt to find user with new_email. remove it and the associated userToken
177-
$user = $user::findOne(["new_email" => $email]);
178-
if ($user) {
179-
$user->clearNewEmail();
180-
$userToken = $userToken::findByUser($user->id, $userToken::TYPE_EMAIL_CHANGE);
181-
if ($userToken) {
182-
$userToken->delete();
183-
}
184-
}
185-
}
186-
187169
/**
188170
* Register a new user using client attributes and then associate userAuth
189171
* @param \yii\authclient\BaseClient $client

‎controllers/DefaultController.php‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,6 @@ protected function afterRegister($user)
171171

172172
// check if we have a userToken type to process, or just log user in directly
173173
if ($userTokenType) {
174-
175-
// generate userToken and send email
176174
$userToken = $userToken::generate($user->id, $userTokenType);
177175
if (!$numSent = $user->sendEmailConfirmation($userToken)) {
178176

@@ -203,12 +201,13 @@ public function actionConfirm($token)
203201
// for example, user registered another account before confirming change of email
204202
$user = Yii::$app->getModule("user")->model("User");
205203
$user = $user::findOne($userToken->user_id);
206-
$email = $user->new_email ?: $user->email;;
207-
if ($user->confirm()) {
204+
$newEmail = $userToken->data;
205+
if ($user->confirm($newEmail)) {
208206
$success = true;
209207
}
210208

211-
// delete token either way
209+
// set email and delete token
210+
$email = $newEmail ?: $user->email;
212211
$userToken->delete();
213212
}
214213

@@ -235,13 +234,13 @@ public function actionAccount()
235234
}
236235

237236
// validate for normal request
237+
$userToken = Yii::$app->getModule("user")->model("UserToken");
238238
if ($loadedPost && $user->validate()) {
239239

240-
// generate userToken and send email if user changed his email
241-
if (Yii::$app->getModule("user")->emailChangeConfirmation && $user->checkAndPrepEmailChange()) {
242-
243-
$userToken = Yii::$app->getModule("user")->model("UserToken");
244-
$userToken = $userToken::generate($user->id, $userToken::TYPE_EMAIL_CHANGE);
240+
// check if user changed his email
241+
$newEmail = $user->checkEmailChange();
242+
if ($newEmail) {
243+
$userToken = $userToken::generate($user->id, $userToken::TYPE_EMAIL_CHANGE, $newEmail);
245244
if (!$numSent = $user->sendEmailConfirmation($userToken)) {
246245

247246
// handle email error
@@ -253,9 +252,11 @@ public function actionAccount()
253252
$user->save(false);
254253
Yii::$app->session->setFlash("Account-success", Yii::t("user", "Account updated"));
255254
return $this->refresh();
255+
} else {
256+
$userToken = $userToken::findByUser($user->id, $userToken::TYPE_EMAIL_CHANGE);
256257
}
257258

258-
return $this->render("account", compact("user"));
259+
return $this->render("account", compact("user", "userToken"));
259260
}
260261

261262
/**
@@ -338,9 +339,6 @@ public function actionCancel()
338339
$userToken = Yii::$app->getModule("user")->model("UserToken");
339340
$userToken = $userToken::findByUser($user->id, $userToken::TYPE_EMAIL_CHANGE);
340341
if ($userToken) {
341-
342-
// clear new_email, delete userToken, and set flash message
343-
$user->clearNewEmail();
344342
$userToken->delete();
345343
Yii::$app->session->setFlash("Cancel-success", Yii::t("user", "Email change cancelled"));
346344
}

‎migrations/m150214_044831_init_user.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ public function safeUp()
2525
'role_id' => Schema::TYPE_INTEGER . ' not null',
2626
'status' => Schema::TYPE_SMALLINT . ' not null',
2727
'email' => Schema::TYPE_STRING . ' null default null',
28-
'new_email' => Schema::TYPE_STRING . ' null default null',
2928
'username' => Schema::TYPE_STRING . ' null default null',
3029
'password' => Schema::TYPE_STRING . ' null default null',
3130
'auth_key' => Schema::TYPE_STRING . ' null default null',
@@ -43,6 +42,7 @@ public function safeUp()
4342
'user_id' => Schema::TYPE_INTEGER . ' not null',
4443
'type' => Schema::TYPE_SMALLINT . ' not null',
4544
'token' => Schema::TYPE_STRING . ' not null',
45+
'data' => Schema::TYPE_STRING . ' null default null',
4646
'created_at' => Schema::TYPE_TIMESTAMP . ' null default null',
4747
'expired_at' => Schema::TYPE_TIMESTAMP . ' null default null',
4848
], $tableOptions);

‎models/User.php‎

Lines changed: 26 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
* @property string $role_id
1818
* @property integer $status
1919
* @property string $email
20-
* @property string $new_email
2120
* @property string $username
2221
* @property string $password
2322
* @property string $auth_key
@@ -136,7 +135,6 @@ public function attributeLabels()
136135
'role_id' => Yii::t('user', 'Role ID'),
137136
'status' => Yii::t('user', 'Status'),
138137
'email' => Yii::t('user', 'Email'),
139-
'new_email' => Yii::t('user', 'New Email'),
140138
'username' => Yii::t('user', 'Username'),
141139
'password' => Yii::t('user', 'Password'),
142140
'auth_key' => Yii::t('user', 'Auth Key'),
@@ -334,30 +332,31 @@ public function setRegisterAttributes($roleId, $userIp, $status = null)
334332
}
335333

336334
/**
337-
* Check and prepare for email change
338-
* @return bool True if user set a `new_email`
335+
* Check for email change
336+
* @return string|bool
339337
*/
340-
public function checkAndPrepEmailChange()
338+
public function checkEmailChange()
341339
{
342-
// check if user is removing email address (only if Module::$requireEmail = false)
343-
if (trim($this->email) === "") {
340+
// check if user didn't change email
341+
if ($this->email == $this->getOldAttribute("email")) {
344342
return false;
345343
}
346344

347-
// check for change in email
348-
if ($this->email != $this->getOldAttribute("email")) {
349-
350-
// change status
351-
$this->status = static::STATUS_UNCONFIRMED_EMAIL;
352-
353-
// set `new_email` attribute and restore old one
354-
$this->new_email = $this->email;
355-
$this->email = $this->getOldAttribute("email");
345+
// check if we need to confirm email change
346+
if (!Yii::$app->getModule("user")->emailChangeConfirmation) {
347+
return false;
348+
}
356349

357-
return true;
350+
// check if user is removing email address (only valid if Module::$requireEmail = false)
351+
if (!$this->email) {
352+
return false;
358353
}
359354

360-
return false;
355+
// update status and email before returning new email
356+
$newEmail = $this->email;
357+
$this->status = static::STATUS_UNCONFIRMED_EMAIL;
358+
$this->email = $this->getOldAttribute("email");
359+
return $newEmail;
361360
}
362361

363362
/**
@@ -371,42 +370,29 @@ public function updateLoginMeta()
371370
return $this->save(false, ["logged_in_ip", "logged_in_at"]);
372371
}
373372

374-
/**
375-
* Clear new_email field
376-
* @param bool $save
377-
* @return static
378-
*/
379-
public function clearNewEmail($save = true)
380-
{
381-
$this->new_email = null;
382-
if ($save) {
383-
$this->save(false, ["new_email"]);
384-
}
385-
return $this;
386-
}
387-
388373
/**
389374
* Confirm user email
375+
* @param string $newEmail
390376
* @return bool
391377
*/
392-
public function confirm()
378+
public function confirm($newEmail)
393379
{
394380
// update status
395381
$this->status = static::STATUS_ACTIVE;
396382

397-
// check new_email if set and check if another user already has it
383+
// process $newEmail from a userToken
384+
// check if another user already has that email
398385
$success = true;
399-
if ($this->new_email) {
400-
$checkUser = static::findOne(["email" => $this->new_email]);
386+
if ($newEmail) {
387+
$checkUser = static::findOne(["email" => $newEmail]);
401388
if ($checkUser) {
402389
$success = false;
403390
} else {
404-
$this->email = $this->new_email;
391+
$this->email = $newEmail;
405392
}
406-
$this->new_email = null;
407393
}
408394

409-
$this->save(false, ["email", "new_email", "status"]);
395+
$this->save(false, ["email", "status"]);
410396
return $success;
411397
}
412398

@@ -479,7 +465,7 @@ public function sendEmailConfirmation($userToken)
479465
// send email
480466
$user = $this;
481467
$profile = $user->profile;
482-
$email = $user->new_email ?: $user->email;
468+
$email = $userToken->data ?: $user->email;
483469
$subject = Yii::$app->id . " - " . Yii::t("user", "Email Confirmation");
484470
$message = $mailer->compose('confirmEmail', compact("subject", "user", "profile", "userToken"))
485471
->setTo($email)

‎models/UserToken.php‎

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ public function attributeLabels()
4545
'user_id' => Yii::t('user', 'User ID'),
4646
'type' => Yii::t('user', 'Type'),
4747
'token' => Yii::t('user', 'Token'),
48+
'data' => Yii::t('user', 'Data'),
4849
'created_at' => Yii::t('user', 'Created At'),
4950
'expired_at' => Yii::t('user', 'Expired At'),
5051
];
@@ -79,10 +80,11 @@ public function getUser()
7980
* Generate/reuse a userToken
8081
* @param int $userId
8182
* @param int $type
83+
* @param string $data
8284
* @param string $expireTime
8385
* @return static
8486
*/
85-
public static function generate($userId, $type, $expireTime = null)
87+
public static function generate($userId, $type, $data = null, $expireTime = null)
8688
{
8789
// attempt to find existing record
8890
// otherwise create new
@@ -95,6 +97,7 @@ public static function generate($userId, $type, $expireTime = null)
9597
// set/update data
9698
$model->user_id = $userId;
9799
$model->type = $type;
100+
$model->data = $data;
98101
$model->created_at = date("Y-m-d H:i:s");
99102
$model->expired_at = $expireTime;
100103
$model->token = Yii::$app->security->generateRandomString();
@@ -103,15 +106,16 @@ public static function generate($userId, $type, $expireTime = null)
103106
}
104107

105108
/**
106-
* Find an active userToken by userId
107-
* @param int $userId
109+
* Find a userToken by specified field/value
110+
* @param string $field
111+
* @param string $value
108112
* @param array|int $type
109113
* @param bool $checkExpiration
110114
* @return static
111115
*/
112-
public static function findByUser($userId, $type, $checkExpiration = true)
116+
public static function findBy($field, $value, $type, $checkExpiration)
113117
{
114-
$query = static::find()->where(["user_id" => $userId, "type" => $type ]);
118+
$query = static::find()->where([$field => $value, "type" => $type ]);
115119
if ($checkExpiration) {
116120
$now = date("Y-m-d H:i:s");
117121
$query->andWhere("([[expired_at]] >= '$now' or [[expired_at]] is NULL)");
@@ -120,19 +124,38 @@ public static function findByUser($userId, $type, $checkExpiration = true)
120124
}
121125

122126
/**
123-
* Find an active userToken by token
127+
* Find a userToken by userId
128+
* @param int $userId
129+
* @param array|int $type
130+
* @param bool $checkExpiration
131+
* @return static
132+
*/
133+
public static function findByUser($userId, $type, $checkExpiration = true)
134+
{
135+
return static::findBy("user_id", $userId, $type, $checkExpiration);
136+
}
137+
138+
/**
139+
* Find a userToken by token
124140
* @param string $token
125141
* @param array|int $type
126142
* @param bool $checkExpiration
127143
* @return static
128144
*/
129145
public static function findByToken($token, $type, $checkExpiration = true)
130146
{
131-
$query = static::find()->where(["token" => $token, "type" => $type ]);
132-
if ($checkExpiration) {
133-
$now = date("Y-m-d H:i:s");
134-
$query->andWhere("([[expired_at]] >= '$now' or [[expired_at]] is NULL)");
135-
}
136-
return $query->one();
147+
return static::findBy("token", $token, $type, $checkExpiration);
148+
}
149+
150+
/**
151+
* Find a userToken by data
152+
* @param string $data
153+
* @param array|int $type
154+
* @param bool $checkExpiration
155+
* @return static
156+
*/
157+
public static function findByData($data, $type, $checkExpiration = true)
158+
{
159+
return static::findBy("data", $data, $type, $checkExpiration);
137160
}
138161
}

‎models/forms/ForgotForm.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ public function sendForgotEmail()
9292

9393
// create userToken
9494
$userToken = Yii::$app->getModule("user")->model("UserToken");
95-
$userToken = $userToken::generate($user->id, $userToken::TYPE_PASSWORD_RESET, $expireTime);
95+
$userToken = $userToken::generate($user->id, $userToken::TYPE_PASSWORD_RESET, null, $expireTime);
9696

9797
// modify view path to module views
9898
$mailer = Yii::$app->mailer;

0 commit comments

Comments
 (0)