Merge pull request #373 from near/test-message-content

Rainbow bridge / Twilio message size limit & message content fixes

middleware/2fa.js

@@ -109,12 +109,14 @@ const sendCode = async (ctx, method, twoFactorMethod, requestId = -1, accountId
 
     await twoFactorMethod.update({ securityCode, requestId });
 
+    // Safe: assumes anything other than SMS should be HTML escaped, and that args should be shortened for SMS
+    const isForSmsDelivery = method.kind === TWO_FACTOR_AUTH_KINDS.PHONE;
+
     const deliveryOpts = {
         kind: method.kind,
         destination: escapeHtml(method.detail)
     };
 
-
     if (requestId === -1) {
         // No requestId means this is a brand new 2fa verification, not transactions being approved
         return sendMessageTo2faDestination({
@@ -150,6 +152,7 @@ const sendCode = async (ctx, method, twoFactorMethod, requestId = -1, accountId
                 messageContent: getAddingFullAccessKeyMessageContent({
                     accountId,
                     publicKey,
+                    isForSmsDelivery,
                     request,
                     securityCode,
                 })
@@ -169,6 +172,7 @@ const sendCode = async (ctx, method, twoFactorMethod, requestId = -1, accountId
                 accountId,
                 request,
                 securityCode,
+                isForSmsDelivery
             })
         }
     });

middleware/2faMessageContent.js

@@ -2,40 +2,71 @@ const hex = require('hexer');
 const nearAPI = require('near-api-js');
 const escapeHtml = require('escape-html');
 
+const TRUNCATE_ARGUMENTS_LENGTH = 247;
+
 const fmtNear = (amount) => nearAPI.utils.format.formatNearAmount(amount, 4) + 'Ⓝ';
 
-const formatArgs = (args) => {
+const formatArgs = (args, isForSmsDelivery = false) => {
+    let output = '';
+
     const argsBuffer = Buffer.from(args, 'base64');
     try {
         const jsonString = argsBuffer.toString('utf-8');
-        const json = JSON.parse(jsonString);
-        if (json.amount) json.amount = fmtNear(json.amount);
-        if (json.deposit) json.deposit = fmtNear(json.deposit);
-        return JSON.stringify(json);
-    } catch(e) {
+        const parsed = JSON.parse(jsonString);
+
+        // Composing new obj in specific order so `amount` and `deposit` will likely be in the first 250chars
+        const { amount, deposit, ...json } = parsed;
+
+        const formattedNear = {
+            ...Object.hasOwnProperty.call(parsed, 'amount') && { amount: fmtNear(amount) },
+            ...Object.hasOwnProperty.call(parsed, 'deposit') && { deposit: fmtNear(deposit) },
+        };
+
+        output = JSON.stringify({
+            ...formattedNear,
+            ...json
+        });
+    } catch (e) {
         // Cannot parse JSON, do hex dump
-        return hex(argsBuffer);
+        output = hex(argsBuffer);
     }
+
+    if (isForSmsDelivery && output.length >= TRUNCATE_ARGUMENTS_LENGTH) {
+        // Twilio SMS limits total message size to 1600chars...make best effort to keep total size small enough
+        output = output.slice(0, TRUNCATE_ARGUMENTS_LENGTH) + '...';
+    }
+
+    return output;
 };
 
-const formatAction = (receiver_id, { type, method_name, args, deposit, amount, public_key, permission }) => {
+const formatAction = (receiver_id, { type, method_name, args, deposit, amount, public_key, permission }, isForSmsDelivery = false) => {
+    let output;
+
     switch (type) {
     case 'FunctionCall':
-        return escapeHtml(`Calling method: ${ method_name } in contract: ${ receiver_id } with amount ${ deposit ? fmtNear(deposit) : '0' } and with args ${formatArgs(args)}`);
+        output = `Calling method: ${ method_name } in contract: ${ receiver_id } with amount ${ deposit ? fmtNear(deposit) : '0' } and with args ${formatArgs(args, isForSmsDelivery)}`;
+        break;
     case 'Transfer':
-        return escapeHtml(`Transferring ${ fmtNear(amount) } to: ${ receiver_id }`);
+        output =  `Transferring ${ fmtNear(amount) } to: ${ receiver_id }`;
+        break;
     case 'Stake':
-        return escapeHtml(`Staking: ${ fmtNear(amount) } to validator: ${ receiver_id }`);
+        output =  `Staking: ${ fmtNear(amount) } to validator: ${ receiver_id }`;
+        break;
     case 'AddKey':
         if (permission) {
             const { allowance, receiver_id, method_names } = permission;
             const methodsMessage = method_names && method_names.length > 0 ? `${method_names.join(', ')} methods` : 'any method';
-            return escapeHtml(`Adding key ${ public_key } limited to call ${methodsMessage} on ${receiver_id} and spend up to ${fmtNear(allowance)} on gas`);
+            output = `Adding key ${ public_key } limited to call ${methodsMessage} on ${receiver_id} and spend up to ${fmtNear(allowance)} on gas`;
+        } else {
+            output =  `Adding key ${ public_key } with FULL ACCESS to account`;
         }
-        return escapeHtml(`Adding key ${ public_key } with FULL ACCESS to account`);
+        break;
     case 'DeleteKey':
-        return escapeHtml(`Deleting key ${ public_key }`);
+        output = `Deleting key ${ public_key }`;
+        break;
     }
+
+    return isForSmsDelivery ? output :  escapeHtml(output);
 };
 
 function getSecurityCodeText(securityCode, requestDetails) {
@@ -57,10 +88,10 @@ function getVerify2faMethodMessageContent({ accountId, destination, securityCode
     };
 }
 
-function getAddingFullAccessKeyMessageContent({ accountId, publicKey, request, securityCode }) {
+function getAddingFullAccessKeyMessageContent({ accountId, publicKey, request, securityCode, isForSmsDelivery}) {
     const { receiver_id, actions } = request;
 
-    const requestDetails = actions.map(action => formatAction( receiver_id, action));
+    const requestDetails = actions.map(action => formatAction(receiver_id, action, isForSmsDelivery));
 
     const subject = 'Confirm Transaction WARNING - Adding FULL ACCESS KEY to Account: ' + accountId;
     const text = `
@@ -79,10 +110,10 @@ If you'd like to proceed, enter this security code: ${securityCode}
     };
 }
 
-function getConfirmTransactionMessageContent({ accountId, request, securityCode }) {
+function getConfirmTransactionMessageContent({ accountId, request, securityCode, isForSmsDelivery }) {
     const { receiver_id, actions } = request;
 
-    const requestDetails = actions.map(action => formatAction(receiver_id, action));
+    const requestDetails = actions.map(action => formatAction(receiver_id, action, isForSmsDelivery));
 
     return {
         subject: `Confirm Transaction from: ${accountId} to: ${receiver_id}`,
@@ -95,5 +126,7 @@ function getConfirmTransactionMessageContent({ accountId, request, securityCode
 module.exports = {
     getVerify2faMethodMessageContent,
     getConfirmTransactionMessageContent,
-    getAddingFullAccessKeyMessageContent
+    getAddingFullAccessKeyMessageContent,
+    formatArgs,
+    formatAction
 };

test/messages/acceptanceTestOutputs/confirmTransactions2fa/confirmTransactions2fa-sms.txt

@@ -0,0 +1,31 @@
+Text content for SMS and rich text incapable email clients:
+-----------------------------------
+
+NEAR Wallet security code: 123456
+
+Important: By entering this code, you are authorizing the following transactions:
+Calling method: DoAThingWithShortJSONArgs in contract: testreceiveraccount with amount 100,000Ⓝ and with args {"amount":"1,000Ⓝ","deposit":"100,000Ⓝ","someArg":"shortArg"}
+Calling method: DoAThingWithLongJSONArgs in contract: testreceiveraccount with amount 18,000,000Ⓝ and with args {"amount":"110,000Ⓝ","deposit":"103,000Ⓝ","someArg":"longestArgsAreTheBestArgs","someArg2":"longestArgsAreTheBestArgs","someArg3":"longestArgsAreTheBestArgs","someArg4":"longestArgsAreTheBestArgs","someArg5":"longestArgsAreTheBestArgs","someArg6":...
+Transferring 10.5Ⓝ to: testreceiveraccount
+Staking: 1.5Ⓝ to validator: testreceiveraccount
+Deleting key fakePublicKeyIsTheBestPublicKey
+Adding key fakePublicKeyIsTheBestPublicKey limited to call doSomething, doAnotherThing methods on an-account-id and spend up to 0Ⓝ on gas
+Adding key fakePublicKeyIsTheBestPublicKey limited to call any method on an-account-id and spend up to 0Ⓝ on gas
+
+-----------------------------------
+
+Subject (included in email only):
+-----------------------------------                
+Confirm Transaction from: exampleaccount3456 to: testreceiveraccount
+-----------------------------------
+
+Request Details (included in email only):
+-----------------------------------
+Calling method: DoAThingWithShortJSONArgs in contract: testreceiveraccount with amount 100,000Ⓝ and with args {"amount":"1,000Ⓝ","deposit":"100,000Ⓝ","someArg":"shortArg"}
+Calling method: DoAThingWithLongJSONArgs in contract: testreceiveraccount with amount 18,000,000Ⓝ and with args {"amount":"110,000Ⓝ","deposit":"103,000Ⓝ","someArg":"longestArgsAreTheBestArgs","someArg2":"longestArgsAreTheBestArgs","someArg3":"longestArgsAreTheBestArgs","someArg4":"longestArgsAreTheBestArgs","someArg5":"longestArgsAreTheBestArgs","someArg6":...
+Transferring 10.5Ⓝ to: testreceiveraccount
+Staking: 1.5Ⓝ to validator: testreceiveraccount
+Deleting key fakePublicKeyIsTheBestPublicKey
+Adding key fakePublicKeyIsTheBestPublicKey limited to call doSomething, doAnotherThing methods on an-account-id and spend up to 0Ⓝ on gas
+Adding key fakePublicKeyIsTheBestPublicKey limited to call any method on an-account-id and spend up to 0Ⓝ on gas
+-----------------------------------

test/messages/acceptanceTestOutputs/confirmTransactions2fa/confirmTransactions2fa.html

@@ -213,7 +213,13 @@
                     <!-- The content goes here -->
                     <td style="font-size:16px; color:#3b3b3b; line-height:24px;" class="normal-font-family md-content">
                         <p>Important: By entering this code, you are authorizing the following transaction:</p>
-<p>Calling method: DoAThingWithShortJSONArgs in contract: testreceiveraccount with amount 100,000Ⓝ and with args {&quot;someArg&quot;:&quot;shortArg&quot;,&quot;deposit&quot;:&quot;100,000Ⓝ&quot;,&quot;amount&quot;:&quot;1,000Ⓝ&quot;},Calling method: DoAThingWithLongJSONArgs in contract: testreceiveraccount with amount 18,000,000Ⓝ and with args {&quot;someArg&quot;:&quot;longestArgsAreTheBestArgs&quot;,&quot;someArg2&quot;:&quot;longestArgsAreTheBestArgs&quot;,&quot;someArg3&quot;:&quot;longestArgsAreTheBestArgs&quot;,&quot;someArg4&quot;:&quot;longestArgsAreTheBestArgs&quot;,&quot;someArg5&quot;:&quot;longestArgsAreTheBestArgs&quot;,&quot;someArg6&quot;:&quot;longestArgsAreTheBestArgs&quot;,&quot;deposit&quot;:&quot;103,000Ⓝ&quot;,&quot;amount&quot;:&quot;110,000Ⓝ&quot;},Transferring 10.5Ⓝ to: testreceiveraccount,Staking: 1.5Ⓝ to validator: testreceiveraccount,Deleting key fakePublicKeyIsTheBestPublicKey,Adding key fakePublicKeyIsTheBestPublicKey limited to call doSomething, doAnotherThing methods on an-account-id and spend up to 0Ⓝ on gas,Adding key fakePublicKeyIsTheBestPublicKey limited to call any method on an-account-id and spend up to 0Ⓝ on gas</p>
+<p>Calling method: DoAThingWithShortJSONArgs in contract: testreceiveraccount with amount 100,000Ⓝ and with args {&quot;amount&quot;:&quot;1,000Ⓝ&quot;,&quot;deposit&quot;:&quot;100,000Ⓝ&quot;,&quot;someArg&quot;:&quot;shortArg&quot;}<br>
+Calling method: DoAThingWithLongJSONArgs in contract: testreceiveraccount with amount 18,000,000Ⓝ and with args {&quot;amount&quot;:&quot;110,000Ⓝ&quot;,&quot;deposit&quot;:&quot;103,000Ⓝ&quot;,&quot;someArg&quot;:&quot;longestArgsAreTheBestArgs&quot;,&quot;someArg2&quot;:&quot;longestArgsAreTheBestArgs&quot;,&quot;someArg3&quot;:&quot;longestArgsAreTheBestArgs&quot;,&quot;someArg4&quot;:&quot;longestArgsAreTheBestArgs&quot;,&quot;someArg5&quot;:&quot;longestArgsAreTheBestArgs&quot;,&quot;someArg6&quot;:&quot;longestArgsAreTheBestArgs&quot;}<br>
+Transferring 10.5Ⓝ to: testreceiveraccount<br>
+Staking: 1.5Ⓝ to validator: testreceiveraccount<br>
+Deleting key fakePublicKeyIsTheBestPublicKey<br>
+Adding key fakePublicKeyIsTheBestPublicKey limited to call doSomething, doAnotherThing methods on an-account-id and spend up to 0Ⓝ on gas<br>
+Adding key fakePublicKeyIsTheBestPublicKey limited to call any method on an-account-id and spend up to 0Ⓝ on gas</p>
 <p><span style="background:#F6F6F6; border-radius:5px; color:#0B70CE; display:block; font-size:18px; font-weight:700; margin:30px 0; padding:10px 20px; text-align:center; ">123456</span></p>
                     </td>
                   </tr>

test/messages/acceptanceTestOutputs/confirmTransactions2fa/confirmTransactions2fa.txt

@@ -4,8 +4,8 @@ Text content for SMS and rich text incapable email clients:
 NEAR Wallet security code: 123456
 
 Important: By entering this code, you are authorizing the following transactions:
-Calling method: DoAThingWithShortJSONArgs in contract: testreceiveraccount with amount 100,000Ⓝ and with args {"someArg":"shortArg","deposit":"100,000Ⓝ","amount":"1,000Ⓝ"}
-Calling method: DoAThingWithLongJSONArgs in contract: testreceiveraccount with amount 18,000,000Ⓝ and with args {"someArg":"longestArgsAreTheBestArgs","someArg2":"longestArgsAreTheBestArgs","someArg3":"longestArgsAreTheBestArgs","someArg4":"longestArgsAreTheBestArgs","someArg5":"longestArgsAreTheBestArgs","someArg6":"longestArgsAreTheBestArgs","deposit":"103,000Ⓝ","amount":"110,000Ⓝ"}
+Calling method: DoAThingWithShortJSONArgs in contract: testreceiveraccount with amount 100,000Ⓝ and with args {"amount":"1,000Ⓝ","deposit":"100,000Ⓝ","someArg":"shortArg"}
+Calling method: DoAThingWithLongJSONArgs in contract: testreceiveraccount with amount 18,000,000Ⓝ and with args {"amount":"110,000Ⓝ","deposit":"103,000Ⓝ","someArg":"longestArgsAreTheBestArgs","someArg2":"longestArgsAreTheBestArgs","someArg3":"longestArgsAreTheBestArgs","someArg4":"longestArgsAreTheBestArgs","someArg5":"longestArgsAreTheBestArgs","someArg6":"longestArgsAreTheBestArgs"}
 Transferring 10.5Ⓝ to: testreceiveraccount
 Staking: 1.5Ⓝ to validator: testreceiveraccount
 Deleting key fakePublicKeyIsTheBestPublicKey
@@ -21,8 +21,8 @@ Confirm Transaction from: exampleaccount3456 to: testreceiveraccount
 
 Request Details (included in email only):
 -----------------------------------
-Calling method: DoAThingWithShortJSONArgs in contract: testreceiveraccount with amount 100,000Ⓝ and with args {"someArg":"shortArg","deposit":"100,000Ⓝ","amount":"1,000Ⓝ"}
-Calling method: DoAThingWithLongJSONArgs in contract: testreceiveraccount with amount 18,000,000Ⓝ and with args {"someArg":"longestArgsAreTheBestArgs","someArg2":"longestArgsAreTheBestArgs","someArg3":"longestArgsAreTheBestArgs","someArg4":"longestArgsAreTheBestArgs","someArg5":"longestArgsAreTheBestArgs","someArg6":"longestArgsAreTheBestArgs","deposit":"103,000Ⓝ","amount":"110,000Ⓝ"}
+Calling method: DoAThingWithShortJSONArgs in contract: testreceiveraccount with amount 100,000Ⓝ and with args {"amount":"1,000Ⓝ","deposit":"100,000Ⓝ","someArg":"shortArg"}
+Calling method: DoAThingWithLongJSONArgs in contract: testreceiveraccount with amount 18,000,000Ⓝ and with args {"amount":"110,000Ⓝ","deposit":"103,000Ⓝ","someArg":"longestArgsAreTheBestArgs","someArg2":"longestArgsAreTheBestArgs","someArg3":"longestArgsAreTheBestArgs","someArg4":"longestArgsAreTheBestArgs","someArg5":"longestArgsAreTheBestArgs","someArg6":"longestArgsAreTheBestArgs"}
 Transferring 10.5Ⓝ to: testreceiveraccount
 Staking: 1.5Ⓝ to validator: testreceiveraccount
 Deleting key fakePublicKeyIsTheBestPublicKey