浏览代码

dev: update handling of duplicate emails

KernelDeimos 7 月之前
父节点
当前提交
196531d118

+ 8 - 3
notes.md

@@ -8,9 +8,7 @@
   an email already exists on an account with a confirmed email.
   an email already exists on an account with a confirmed email.
   Then, upon confirming the update, Ensure that in the meanwhile no
   Then, upon confirming the update, Ensure that in the meanwhile no
   new account came up with that email set.
   new account came up with that email set.
-  
-NEXT: figure out where email change is handled on backend
-(prior to confirmation) and see what checks are performed.
+- ensure `clean_email` is updated whenever the email is updated
 
 
 ### Email duplicate check on confirmation
 ### Email duplicate check on confirmation
 
 
@@ -67,3 +65,10 @@ When does `pseudo_user` have an email?
   was already confirmed since the time the link was sent, we need to display
   was already confirmed since the time the link was sent, we need to display
   an error message to the user.
   an error message to the user.
 
 
+### Find places where (on backend) email change process is triggered
+
+Right now there are two handlers:
+- `/user-protected/change-email` (UserProtectedEndpointsService)
+  - Invokes the process (sends confirmation email)
+- `/change_email/confirm` (PuterAPIService)
+  - Endpoint that the email link points to

+ 5 - 2
src/backend/src/routers/_default.js

@@ -230,13 +230,16 @@ router.all('*', async function(req, res, next) {
                     // avoid further nested branching. This is a temporary
                     // avoid further nested branching. This is a temporary
                     // solution; next time this code should be refactored.
                     // solution; next time this code should be refactored.
                     await (async () => {
                     await (async () => {
+                        const svc_cleanEmail = req.services.get('clean-email');
+                        const clean_email = svc_cleanEmail.clean(user.email);
                         // If other users have the same CONFIRMED email, display an error
                         // If other users have the same CONFIRMED email, display an error
                         const maybe_rows = await db.read(
                         const maybe_rows = await db.read(
                             `SELECT EXISTS(
                             `SELECT EXISTS(
-                                SELECT 1 FROM user WHERE email=?
+                                SELECT 1 FROM user WHERE (email=? OR clean_email=?)
                                 AND email_confirmed=1
                                 AND email_confirmed=1
                                 AND password IS NOT NULL
                                 AND password IS NOT NULL
-                            ) AS email_exists`
+                            ) AS email_exists`,
+                            [user.email, clean_email]
                         );
                         );
                         if ( maybe_rows[0]?.email_exists ) {
                         if ( maybe_rows[0]?.email_exists ) {
                             // TODO: maybe display the username of that account
                             // TODO: maybe display the username of that account

+ 7 - 4
src/backend/src/routers/change_email.js

@@ -53,10 +53,13 @@ const CHANGE_EMAIL_CONFIRM = eggspress('/change_email/confirm', {
         throw APIError.create('token_invalid');
         throw APIError.create('token_invalid');
     }
     }
 
 
+    const svc_cleanEmail = req.services.get('clean-email');
+    const clean_email = svc_cleanEmail.clean(rows[0].unconfirmed_change_email);
+
     // Scenario: email was confirmed on another account already
     // Scenario: email was confirmed on another account already
     const rows2 = await db.read(
     const rows2 = await db.read(
-        'SELECT `id` FROM `user` WHERE `email` = ?',
-        [rows[0].unconfirmed_change_email]
+        'SELECT `id` FROM `user` WHERE `email` = ? OR `clean_email` = ?',
+        [rows[0].unconfirmed_change_email, clean_email]
     );
     );
     if ( rows2.length > 0 ) {
     if ( rows2.length > 0 ) {
         throw APIError.create('email_already_in_use');
         throw APIError.create('email_already_in_use');
@@ -71,8 +74,8 @@ const CHANGE_EMAIL_CONFIRM = eggspress('/change_email/confirm', {
     const new_email = rows[0].unconfirmed_change_email;
     const new_email = rows[0].unconfirmed_change_email;
 
 
     await db.write(
     await db.write(
-        'UPDATE `user` SET `email` = ?, `unconfirmed_change_email` = NULL, `change_email_confirm_token` = NULL, `pass_recovery_token` = NULL WHERE `id` = ?',
-        [new_email, user_id]
+        'UPDATE `user` SET `email` = ?, `clean_email` = ?, `unconfirmed_change_email` = NULL, `change_email_confirm_token` = NULL, `pass_recovery_token` = NULL WHERE `id` = ?',
+        [new_email, clean_email, user_id]
     );
     );
 
 
     invalidate_cached_user_by_id(user_id);
     invalidate_cached_user_by_id(user_id);

+ 10 - 2
src/backend/src/routers/signup.js

@@ -143,12 +143,18 @@ module.exports = eggspress(['/signup'], {
     else if(!req.body.is_temp && req.body.password.length < config.min_pass_length)
     else if(!req.body.is_temp && req.body.password.length < config.min_pass_length)
         return res.status(400).send(`Password must be at least ${config.min_pass_length} characters long.`);
         return res.status(400).send(`Password must be at least ${config.min_pass_length} characters long.`);
 
 
+    const svc_cleanEmail = req.services.get('clean-email');
+    const clean_email = svc_cleanEmail.clean(req.body.email);
+
     // duplicate username check
     // duplicate username check
     if(await username_exists(req.body.username))
     if(await username_exists(req.body.username))
         return res.status(400).send('This username already exists in our database. Please use another one.');
         return res.status(400).send('This username already exists in our database. Please use another one.');
     // Email check is here :: Add condition for email_confirmed=1
     // Email check is here :: Add condition for email_confirmed=1
     // duplicate email check (pseudo-users don't count)
     // duplicate email check (pseudo-users don't count)
-    let rows2 = await db.read(`SELECT EXISTS(SELECT 1 FROM user WHERE email=? AND email_confirmed=1 AND password IS NOT NULL) AS email_exists`, [req.body.email]);
+    let rows2 = await db.read(
+        `SELECT EXISTS(
+            SELECT 1 FROM user WHERE (email=? OR clean_email=?) AND email_confirmed=1 AND password IS NOT NULL
+        ) AS email_exists`, [req.body.email, clean_email]);
     if(rows2[0].email_exists)
     if(rows2[0].email_exists)
         return res.status(400).send('This email already exists in our database. Please use another one.');
         return res.status(400).send('This email already exists in our database. Please use another one.');
     // get pseudo user, if exists
     // get pseudo user, if exists
@@ -188,12 +194,14 @@ module.exports = eggspress(['/signup'], {
     if(pseudo_user === undefined){
     if(pseudo_user === undefined){
         insert_res = await db.write(
         insert_res = await db.write(
             `INSERT INTO user
             `INSERT INTO user
-            (username, email, password, uuid, referrer, email_confirm_code, email_confirm_token, free_storage, referred_by) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`,
+            (username, email, clean_email, password, uuid, referrer, email_confirm_code, email_confirm_token, free_storage, referred_by) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
             [
             [
                 // username
                 // username
                 req.body.username,
                 req.body.username,
                 // email
                 // email
                 req.body.is_temp ? null : req.body.email,
                 req.body.is_temp ? null : req.body.email,
+                // normalized email
+                req.body.is_temp ? null : clean_email,
                 // password
                 // password
                 req.body.is_temp ? null : await bcrypt.hash(req.body.password, 8),
                 req.body.is_temp ? null : await bcrypt.hash(req.body.password, 8),
                 // uuid
                 // uuid

+ 5 - 2
src/backend/src/routers/user-protected/change-email.js

@@ -45,12 +45,15 @@ module.exports = {
             throw APIError.create('field_invalid', null, {
             throw APIError.create('field_invalid', null, {
                 key: 'new_email', expected: 'a valid email address' });
                 key: 'new_email', expected: 'a valid email address' });
         }
         }
+
+        const svc_cleanEmail = req.services.get('clean-email');
+        const clean_email = svc_cleanEmail.clean(new_email);
         
         
         // check if email is already in use
         // check if email is already in use
         const db = req.services.get('database').get(DB_WRITE, 'auth');
         const db = req.services.get('database').get(DB_WRITE, 'auth');
         const rows = await db.read(
         const rows = await db.read(
-            'SELECT COUNT(*) AS `count` FROM `user` WHERE `email` = ?',
-            [new_email]
+            'SELECT COUNT(*) AS `count` FROM `user` WHERE (`email` = ? OR `clean_email` = ?) AND `email_confirmed` = 1',
+            [new_email, clean_email]
         );
         );
         if ( rows[0].count > 0 ) {
         if ( rows[0].count > 0 ) {
             throw APIError.create('email_already_in_use', null, { email: new_email });
             throw APIError.create('email_already_in_use', null, { email: new_email });

+ 2 - 2
src/backend/src/services/CleanEmailService.js

@@ -60,7 +60,7 @@ class CleanEmailService extends BaseService {
         this.domain_nondistinct = this.constructor.DOMAIN_NONDISTINCT;
         this.domain_nondistinct = this.constructor.DOMAIN_NONDISTINCT;
     }
     }
 
 
-    clean_email (email) {
+    clean (email) {
         const eml = (() => {
         const eml = (() => {
             const [local, domain] = email.split('@');
             const [local, domain] = email.split('@');
             return { local, domain };
             return { local, domain };
@@ -117,7 +117,7 @@ class CleanEmailService extends BaseService {
         ];
         ];
 
 
         for ( const { email, expected } of cases ) {
         for ( const { email, expected } of cases ) {
-            const cleaned = this.clean_email(email);
+            const cleaned = this.clean(email);
             assert.equal(cleaned, expected, `clean_email(${email}) === ${expected}`);
             assert.equal(cleaned, expected, `clean_email(${email}) === ${expected}`);
         }
         }
     }
     }