Merge pull request #455 from near/funded-account-with-recovery-code-fix

Funded account with recovery code (lock fix)
near · Jul 1, 2021 · ea02a31 · ea02a31
2 parents c55e036 + 9f61292
commit ea02a31
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 20 deletions.
diff --git a/app.js b/app.js
@@ -59,7 +59,7 @@ const router = new Router();
 const {
     withNear,
     checkAccountOwnership,
-    checkAccountDoesNotExist,
+    createCheckAccountDoesNotExistMiddleware,
 } = require('./middleware/near');
 
 app.use(withNear);
@@ -103,7 +103,12 @@ const {
     clearFundedAccountNeedsDeposit,
     createFundedAccount
 } = require('./middleware/fundedAccount');
-router.post('/fundedAccount', fundedAccountCreateRatelimitMiddleware, createFundedAccount);
+router.post(
+    '/fundedAccount',
+    fundedAccountCreateRatelimitMiddleware,
+    createCheckAccountDoesNotExistMiddleware({ source: 'body', fieldName: 'newAccountId' }),
+    createFundedAccount
+);
 router.get('/checkFundedAccountAvailable', checkFundedAccountAvailable);
 router.post(
     '/fundedAccount/clearNeedsDeposit',
@@ -329,7 +334,7 @@ const completeRecoveryInit = async ctx => {
 };
 
 router.post('/account/initializeRecoveryMethodForTempAccount',
-    checkAccountDoesNotExist,
+    createCheckAccountDoesNotExistMiddleware({ source: 'body', fieldName: 'accountId' }),
     completeRecoveryInit
 );
 
@@ -384,7 +389,7 @@ router.post('/account/validateSecurityCode',
 );
 
 router.post('/account/validateSecurityCodeForTempAccount',
-    checkAccountDoesNotExist,
+    createCheckAccountDoesNotExistMiddleware({ source: 'body', fieldName: 'accountId' }),
     completeRecoveryValidation({ isNew: true })
 );
 

diff --git a/middleware/fundedAccount.js b/middleware/fundedAccount.js
@@ -71,14 +71,20 @@ const createFundedAccount = async (ctx) => {
     }
 
 
-    const [[sequelizeAccount], fundingAccount] = await Promise.all([
+    const [[sequelizeAccount, isAccountCreatedByThisCall], fundingAccount] = await Promise.all([
         models.Account.findOrCreate({
             where: { accountId: newAccountId },
             defaults: { fundedAccountNeedsDeposit: true }
         }),
         ctx.near.account(fundedCreatorKeyJson.account_id)
     ]);
 
+    if(!isAccountCreatedByThisCall) {
+        // If someone is using a recovery method that involves a confirmation code (email / SMS)
+        // then we need to manually set the fundedAccountNeedsDeposit on the _existing_ SQL record
+        await sequelizeAccount.update({ fundedAccountNeedsDeposit: true });
+    }
+
     try {
         const newAccountResult = await fundingAccount.functionCall(
             FUNDED_NEW_ACCOUNT_CONTRACT_NAME,
@@ -97,7 +103,10 @@ const createFundedAccount = async (ctx) => {
             requiredUnlockBalance: NEW_FUNDED_ACCOUNT_BALANCE
         };
     } catch (e) {
-        await sequelizeAccount.destroy();
+        if(isAccountCreatedByThisCall) {
+            // Clean up SQL record if we were responsible for creating it during this API call
+            await sequelizeAccount.destroy();
+        }
 
         if (e.type === 'NotEnoughBalance') {
             setJSONErrorResponse({

diff --git a/middleware/near.js b/middleware/near.js
@@ -56,21 +56,38 @@ async function checkAccountOwnership(ctx, next) {
     return await next();
 }
 
-async function checkAccountDoesNotExist(ctx, next) {
-    const { accountId } = ctx.request.body;
-    // TODO: near-api-js should have explicit accoutn existence check
-    let remoteAccount = null;
-    try {
-        remoteAccount = await (await ctx.near.account(accountId)).state();
-    } catch (e) {
-        if (e.type === 'AccountDoesNotExist') {
-            return await next();
-        }
-        throw e;
+function createCheckAccountDoesNotExistMiddleware({ source, fieldName }) {
+    if (source !== 'body' && source !== 'params') {
+        throw new Error('invalid source for accountId provided');
     }
-    if (remoteAccount) {
-        ctx.throw(403, 'Account ' + accountId + ' already exists.');
+
+    if (!fieldName) {
+        throw new Error('Must provide a field to look for accountId in');
     }
+
+    return async function checkAccountDoesNotExist(ctx, next) {
+        let accountId;
+
+        if (source === 'body') {
+            accountId = ctx.request.body[fieldName];
+        } else {
+            accountId = ctx.params[fieldName];
+        }
+
+        // TODO: near-api-js should have explicit accoutn existence check
+        let remoteAccount = null;
+        try {
+            remoteAccount = await (await ctx.near.account(accountId)).state();
+        } catch (e) {
+            if (e.type === 'AccountDoesNotExist') {
+                return await next();
+            }
+            throw e;
+        }
+        if (remoteAccount) {
+            ctx.throw(403, 'Account ' + accountId + ' already exists.');
+        }
+    };
 }
 
 const creatorKeyJson = (() => {
@@ -146,5 +163,5 @@ module.exports = {
     fundedCreatorKeyJson,
     withNear,
     checkAccountOwnership,
-    checkAccountDoesNotExist,
+    createCheckAccountDoesNotExistMiddleware,
 };