Provide proper error for updating to an email that's already in use (#…

…1770) provide proper error for updating to an email already in use
bluesky-social · Nov 21, 2023 · f232371 · f232371
1 parent b532c50
commit f232371
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 9 deletions.
diff --git a/packages/pds/src/api/com/atproto/server/updateEmail.ts b/packages/pds/src/api/com/atproto/server/updateEmail.ts
@@ -1,7 +1,8 @@
+import disposable from 'disposable-email'
+import { InvalidRequestError } from '@atproto/xrpc-server'
 import { Server } from '../../../../lexicon'
 import AppContext from '../../../../context'
-import { InvalidRequestError } from '@atproto/xrpc-server'
-import disposable from 'disposable-email'
+import { UserAlreadyExistsError } from '../../../../services/account'
 
 export default function (server: Server, ctx: AppContext) {
   server.com.atproto.server.updateEmail({
@@ -38,7 +39,17 @@ export default function (server: Server, ctx: AppContext) {
           await accntSrvce.deleteEmailToken(did, 'update_email')
         }
         if (user.email !== email) {
-          await accntSrvce.updateEmail(did, email)
+          try {
+            await accntSrvce.updateEmail(did, email)
+          } catch (err) {
+            if (err instanceof UserAlreadyExistsError) {
+              throw new InvalidRequestError(
+                'This email address is already in use, please use a different email.',
+              )
+            } else {
+              throw err
+            }
+          }
         }
       })
     },

diff --git a/packages/pds/src/db/util.ts b/packages/pds/src/db/util.ts
@@ -57,3 +57,11 @@ export const dummyDialect = {
 export type DbRef = RawBuilder | ReturnType<DynamicModule['ref']>
 
 export type AnyQb = SelectQueryBuilder<any, any, any>
+
+export const isErrUniqueViolation = (err: unknown) => {
+  const code = err?.['code']
+  return (
+    code === '23505' || // postgres, see https://www.postgresql.org/docs/current/errcodes-appendix.html
+    code === 'SQLITE_CONSTRAINT_UNIQUE' // sqlite, see https://www.sqlite.org/rescode.html#constraint_unique
+  )
+}
diff --git a/packages/pds/src/services/account/index.ts b/packages/pds/src/services/account/index.ts
@@ -8,7 +8,11 @@ import * as scrypt from '../../db/scrypt'
 import { UserAccountEntry } from '../../db/tables/user-account'
 import { DidHandle } from '../../db/tables/did-handle'
 import { RepoRoot } from '../../db/tables/repo-root'
-import { countAll, notSoftDeletedClause } from '../../db/util'
+import {
+  countAll,
+  isErrUniqueViolation,
+  notSoftDeletedClause,
+} from '../../db/util'
 import { paginate, TimeCidKeyset } from '../../db/pagination'
 import * as sequencer from '../../sequencer'
 import { AppPassword } from '../../lexicon/types/com/atproto/server/createAppPassword'
@@ -189,11 +193,19 @@ export class AccountService {
   }
 
   async updateEmail(did: string, email: string) {
-    await this.db.db
-      .updateTable('user_account')
-      .set({ email: email.toLowerCase(), emailConfirmedAt: null })
-      .where('did', '=', did)
-      .executeTakeFirst()
+    try {
+      await this.db.db
+        .updateTable('user_account')
+        .set({ email: email.toLowerCase(), emailConfirmedAt: null })
+        .where('did', '=', did)
+        .executeTakeFirst()
+    } catch (err) {
+      if (isErrUniqueViolation(err)) {
+        throw new UserAlreadyExistsError()
+      } else {
+        throw err
+      }
+    }
   }
 
   async updateUserPassword(did: string, password: string) {

diff --git a/packages/pds/tests/email-confirmation.test.ts b/packages/pds/tests/email-confirmation.test.ts
@@ -194,6 +194,19 @@ describe('email confirmation', () => {
     )
   })
 
+  it('fails email update with in-use email', async () => {
+    const attempt = agent.api.com.atproto.server.updateEmail(
+      {
+        email: '[email protected]',
+        token: updateToken,
+      },
+      { headers: sc.getHeaders(alice.did), encoding: 'application/json' },
+    )
+    await expect(attempt).rejects.toThrow(
+      'This email address is already in use, please use a different email.',
+    )
+  })
+
   it('updates email', async () => {
     await agent.api.com.atproto.server.updateEmail(
       {