Repository: firebase/firebase-tools
Pull request #1690 (2019-10-05 09:51:15)

Fix export users
<!--

Thank you for contributing to the Firebase community! Please fill out the form below.

Run the linter and test suite
==============================
Run `npm test` to make sure your changes compile properly and the tests all pass on your local machine. We've hooked up this repo with continuous integration to double check those things for you. 

-->


### Description

<!-- Are you fixing a bug? Implementing a new feature? Make sure we have the context around your change. Link to other relevant issues or pull requests. -->

This PR fixes a bug in `auth.export` command. When `serialExportUsers` function (used by the command) is called more than once, it adds a redundant comma, which breaks the JSON format.

The first call:

```
{"users": [
{
  "localId": "DxADx87BFyPe8HhD4dOAyuwXECu1",
  ...
```

The second call (and all consecutive):

```
{"users": [
,
{
  "localId": "DxADx87BFyPe8HhD4dOAyuwXECu1",
  ...
```

The bug is caused by a global variable `jsonSep` in an internal `_writeUsersToFile` function. I assume that it was first introduced to make JSON streamable.

I changed the code to make it generate `writeUsersToFile` shared among recursive calls.

### Scenarios Tested

<!-- Write a list of all the user journeys and edge cases you've tested. Instructions for manual testing can be found at https://github.com/firebase/firebase-tools/blob/master/.github/CONTRIBUTING.md#development-setup -->

When the package is used via Node.js API:

```
import * as tools from 'firebase-tools'
tools.auth.export(path)
```

Always produce valid JSON. Before the changes, the second `tools.auth.export` call produces malformed JSON (see the `,` as the first array element):

```
{"users": [
,
{
  "localId": "DxADx87BFyPe8HhD4dOAyuwXECu1"
},
{
  "localId": "hoW2ffPbIpW3ToLpxmk86lymT5w2"
},
{
  "localId": "z2ppbzpxMwbwpQhqTqKhrerM2rZ2"
}]}
```

coveralls: 
[![Coverage Status](https://coveralls.io/builds/26130022/badge)](https://coveralls.io/builds/26130022)

Coverage decreased (-0.02%) to 65.069% when pulling **899b7ee2b99628be375c2e80656647f038c5f824 on kossnocorp:fix-export-users** into **b0d3b8f26367bbe66eb7f8f13d1a81d6342656dd on firebase:master**.

samtstern: @kossnocorp thanks for your contribution!  Assigning this one to @bkendall who reviewed code on this path recently.
kossnocorp: @bkendall thank you for the review, I'll address that.
bkendall: Thanks for the PR!

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 77c5428cf..2766fe26c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,2 +1,3 @@
 * Firestore Emulator now serves WebChannel traffic on the same port as gRPC.
-* Fix bug where emulators could not find free ports on Windows Subsystem for Linux.
\ No newline at end of file
+* Fix bug where emulators could not find free ports on Windows Subsystem for Linux.
+* Fixes invalid JSON output in `auth.export` within a scripting environment.
diff --git a/src/accountExporter.js b/src/accountExporter.js
index 8feb99435..f5502c820 100644
--- a/src/accountExporter.js
+++ b/src/accountExporter.js
@@ -119,7 +119,7 @@ var validateOptions = function(options, fileName) {
   return exportOptions;
 };
 
-var _writeUsersToFile = (function() {
+var _createWriteUsersToFile = function() {
   var jsonSep = "";
   return function(userList, format, writeStream) {
     userList.map(function(user) {
@@ -136,9 +136,12 @@ var _writeUsersToFile = (function() {
       }
     });
   };
-})();
+};
 
 var serialExportUsers = function(projectId, options) {
+  if (!options.writeUsersToFile) {
+    options.writeUsersToFile = _createWriteUsersToFile();
+  }
   var postBody = {
     targetProjectId: projectId,
     maxResults: options.batchSize,
@@ -156,7 +159,7 @@ var serialExportUsers = function(projectId, options) {
     .then(function(ret) {
       var userList = ret.body.users;
       if (userList && userList.length > 0) {
-        _writeUsersToFile(userList, options.format, options.writeStream);
+        options.writeUsersToFile(userList, options.format, options.writeStream);
         utils.logSuccess("Exported " + userList.length + " account(s) successfully.");
         // The identitytoolkit API do not return a nextPageToken value
         // consistently when the last page is reached


Test patch:
diff --git a/src/test/accountExporter.spec.js b/src/test/accountExporter.spec.js
index d55f2fc87..b6ebe9ad9 100644
--- a/src/test/accountExporter.spec.js
+++ b/src/test/accountExporter.spec.js
@@ -120,42 +120,7 @@ describe("accountExporter", function() {
       for (var i = 0; i < 20; i++) {
         trailingCommas.push(",");
       }
-      nock("https://www.googleapis.com")
-        .post("/identitytoolkit/v3/relyingparty/downloadAccount", {
-          maxResults: 3,
-          targetProjectId: "test-project-id",
-        })
-        .reply(200, {
-          users: userList.slice(0, 3),
-          nextPageToken: "3",
-        })
-        .post("/identitytoolkit/v3/relyingparty/downloadAccount", {
-          maxResults: 3,
-          nextPageToken: "3",
-          targetProjectId: "test-project-id",
-        })
-        .reply(200, {
-          users: userList.slice(3, 6),
-          nextPageToken: "6",
-        })
-        .post("/identitytoolkit/v3/relyingparty/downloadAccount", {
-          maxResults: 3,
-          nextPageToken: "6",
-          targetProjectId: "test-project-id",
-        })
-        .reply(200, {
-          users: userList.slice(6, 7),
-          nextPageToken: "7",
-        })
-        .post("/identitytoolkit/v3/relyingparty/downloadAccount", {
-          maxResults: 3,
-          nextPageToken: "7",
-          targetProjectId: "test-project-id",
-        })
-        .reply(200, {
-          users: [],
-          nextPageToken: "7",
-        });
+      mockAllUsersRequests();
 
       return serialExportUsers("test-project-id", {
         format: "csv",
@@ -225,5 +190,77 @@ describe("accountExporter", function() {
         expect(spyWrite.getCall(0).args[0]).to.eq(expectedEntry + "," + os.EOL);
       });
     });
+
+    it("should not emit redundant comma in JSON on consecutive calls", function() {
+      mockAllUsersRequests();
+
+      const correctString =
+        '{\n  "localId": "0",\n  "email": "test0@test.org",\n  "displayName": "John Tester0"\n}';
+
+      const firstWriteSpy = sinon.spy();
+      return serialExportUsers("test-project-id", {
+        format: "JSON",
+        batchSize: 3,
+        writeStream: { write: firstWriteSpy, end: function() {} },
+      }).then(function() {
+        expect(firstWriteSpy.args[0][0]).to.be.eq(
+          correctString,
+          "The first call did not emit the correct string"
+        );
+
+        mockAllUsersRequests();
+
+        const secondWriteSpy = sinon.spy();
+        return serialExportUsers("test-project-id", {
+          format: "JSON",
+          batchSize: 3,
+          writeStream: { write: secondWriteSpy, end: function() {} },
+        }).then(() => {
+          expect(secondWriteSpy.args[0][0]).to.be.eq(
+            correctString,
+            "The second call did not emit the correct string"
+          );
+        });
+      });
+    });
+
+    function mockAllUsersRequests() {
+      nock("https://www.googleapis.com")
+        .post("/identitytoolkit/v3/relyingparty/downloadAccount", {
+          maxResults: 3,
+          targetProjectId: "test-project-id",
+        })
+        .reply(200, {
+          users: userList.slice(0, 3),
+          nextPageToken: "3",
+        })
+        .post("/identitytoolkit/v3/relyingparty/downloadAccount", {
+          maxResults: 3,
+          nextPageToken: "3",
+          targetProjectId: "test-project-id",
+        })
+        .reply(200, {
+          users: userList.slice(3, 6),
+          nextPageToken: "6",
+        })
+        .post("/identitytoolkit/v3/relyingparty/downloadAccount", {
+          maxResults: 3,
+          nextPageToken: "6",
+          targetProjectId: "test-project-id",
+        })
+        .reply(200, {
+          users: userList.slice(6, 7),
+          nextPageToken: "7",
+        })
+        .post("/identitytoolkit/v3/relyingparty/downloadAccount", {
+          maxResults: 3,
+          nextPageToken: "7",
+          targetProjectId: "test-project-id",
+        })
+        .reply(200, {
+          users: [],
+          nextPageToken: "7",
+        });
+    }
   });
 });