Merge branch 'master' into arango

database/elasticsearch/elastic.js

@@ -63,7 +63,7 @@ es.createAccount = async (account) => {
 };
 
 es.deleteAccount = async (account) => {
-  if (!account.username || !account.password) {
+  if (!account.username) {
     logger.error("Account data is invalid");
     throw new Error("Account data is invalid");
   }
@@ -79,6 +79,7 @@ es.deleteAccount = async (account) => {
     null,
     authorization
   );
+  const r3 = await sendESRequest(`/${account.username}-*`, "DELETE");
   const err = r1.error || r2.error;
   if (err || !r1.found || !r2.found) {
     logger.error("Deleting Elasticsearch user failed");
@@ -90,13 +91,18 @@ es.deleteAccount = async (account) => {
 };
 
 es.checkAccount = async (account) => {
-  if (!account.username || !account.password || !account.email) {
+  const username = account.username;
+  if (!username) {
     logger.error("Account data is invalid");
     throw new Error("Account data is invalid");
   }
-  const index = account.username + "-example";
-  const r1 = await sendFetch(`${ES_HOST}/${index}`, "GET", null, authorization);
-  if (r1[index]) return true;
+
+  const r1 = await sendFetch(`${ES_HOST}/_security/user/${username}`, "GET", null, authorization);
+  logger.info(
+    `Checking Elasticsearch account for ${username} result:`,
+    !!r1[username]
+  );
+  if (r1[username]) return true;
   return false;
 };
 

database/elasticsearch/elastic.test.js

@@ -159,7 +159,7 @@ describe("testing es check account function", () => {
     fetch.mockReturnValue(
       Promise.resolve({
         json: () => {
-          return { "testuser-example": {} };
+          return { testuser: {} };
         },
       })
     );

database/postgres/pg.js

@@ -49,11 +49,14 @@ pgModule.createPgAccount = async (user) => {
   }
 };
 
-pgModule.deletePgAccount = async (username) => {
-  if (!username) return;
+pgModule.deletePgAccount = async (account) => {
+  if (!account.username) return;
+  const { username } = account;
   try {
-    await client.query(`DROP DATABASE IF EXISTS $1`, [username]);
-    await client.query(`DROP USER IF EXISTS $1`, [username]);
+    const sqlQuery1 = escape(`DROP DATABASE %s;`, username);
+    const sqlQuery2 = escape(`DROP USER %s;`, username);
+    await client.query(sqlQuery1);
+    await client.query(sqlQuery2);
   } catch (err) {
     logger.error(err);
     throw new Error(
@@ -63,12 +66,10 @@ pgModule.deletePgAccount = async (username) => {
 };
 
 pgModule.userHasPgAccount = async (username) => {
-  logger.info(`checking to see if ${username} has a pg account`);
   const result = await client.query(`SELECT 1 FROM pg_roles WHERE rolname=$1`, [
     username,
   ]);
-
-  logger.info(`result: ${result.rowCount}`);
+  logger.info(`Checking pg account for ${username} result: ${result.rowCount}`);
   return Boolean(result.rowCount);
 };
 

database/postgres/pg.test.js

@@ -84,28 +84,19 @@ describe("Test PG DB", () => {
     describe("Test deletePgAccount", () => {
       it("it should execute all queries if required arguments are passed into deletePgAccount", async () => {
         mockClient.query.mockReturnValue(Promise.resolve());
-        await deletePgAccount("username");
+        await deletePgAccount({ username: "username" });
         expect(mockClient.query).toHaveBeenCalledTimes(2);
-        expect(mockClient.query.mock.calls[0]).toEqual([
-          `DROP DATABASE IF EXISTS $1`,
-          ["username"],
-        ]);
-        expect(mockClient.query.mock.calls[1]).toEqual([
-          `DROP USER IF EXISTS $1`,
-          ["username"],
-        ]);
       });
       it("it should not execute any queries in deletePgAccount if required arguments are not passed in", async () => {
-        await deletePgAccount();
+        await deletePgAccount({});
         expect(mockClient.query).toHaveBeenCalledTimes(0);
       });
       it("it should check if logger.error is called at throw of deletePgAccount", async () => {
         try {
           await mockClient.query.mockReturnValue(Promise.reject());
-          const resDeletePgAccount = await deletePgAccount(
-            "username",
-            "password"
-          );
+          const resDeletePgAccount = await deletePgAccount({
+            username: "username",
+          });
           expect(resDeletePgAccount).rejects.toThrow();
         } catch (err) {
           expect(logger.error).toHaveBeenCalledTimes(1);

lib/util.js

@@ -0,0 +1,49 @@
+const { Op } = require("sequelize");
+const db = require("../sequelize/db");
+const pg = require("../database/postgres/pg");
+const es = require("../database/elasticsearch/elastic");
+const logger = require("./log")(__filename);
+
+const util = {};
+
+util.cleanAnonymous = async () => {
+  logger.info("Checking expired anonymous accounts...");
+  try {
+    const { Accounts } = db.getModels();
+    const userAccount = await Accounts.findAll({
+      attributes: ["id", "username"],
+      where: {
+        [Op.and]: [
+          {
+            createdAt: {
+              [Op.lt]: new Date(new Date() - 5 * 24 * 60 * 60 * 1000),
+            },
+          },
+          { email: null },
+        ],
+      },
+    });
+    logger.info(`${userAccount.length} expired accounts are found`);
+    return Promise.all(
+      userAccount.map(async (e) => {
+        const pgDbExists = await pg.userHasPgAccount(e.username);
+        if (pgDbExists) await pg.deletePgAccount(e);
+        const esDbExists = await es.checkAccount(e);
+        if (esDbExists) await es.deleteAccount(e);
+        return await e.destroy();
+      })
+    ).then(() => {
+      logger.info("Cleaning expired accounts has completed");
+      const timer = setTimeout(util.cleanAnonymous, 24 * 60 * 60 * 1000);
+      return {
+        stop: () => {
+          clearTimeout(timer);
+        },
+      };
+    });
+  } catch (err) {
+    logger.error("Cleaning expired accounts has failed", err);
+  }
+};
+
+module.exports = util;

lib/util.test.js

@@ -0,0 +1,72 @@
+jest.mock("sequelize");
+jest.mock("../sequelize/db");
+jest.mock("../database/postgres/pg");
+jest.mock("../database/elasticsearch/elastic");
+jest.mock("./log");
+
+const sequelize = require("sequelize");
+const db = require("../sequelize/db");
+const pg = require("../database/postgres/pg");
+const es = require("../database/elasticsearch/elastic");
+
+sequelize.Op = { and: "and", lt: "lt" };
+const Accounts = {
+  findAll: jest.fn(),
+};
+db.getModels = () => {
+  return { Accounts: Accounts };
+};
+pg.deletePgAccount = jest.fn();
+es.deleteAccount = jest.fn();
+const logGen = require("./log");
+const logger = {
+  info: jest.fn(),
+  error: jest.fn(),
+};
+logGen.mockReturnValue(logger);
+
+const util = require("./util");
+
+describe("Testing cleanAnonymous function", () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+  it("should call findAll with correct queries", async () => {
+    // Hi future engineers, I wrote some strict test here because I thought
+    // that it's important to protect this query. Someone can accidently
+    // clear up the whole user account database by making a little change.
+    // If you are changing this, please do so with caution and plentiful of tests.
+    Accounts.findAll.mockReturnValue([]);
+    await util.cleanAnonymous();
+    const args = Accounts.findAll.mock.calls[0][0];
+    expect(args.attributes[0]).toEqual("id");
+    expect(args.attributes[1]).toEqual("username");
+    expect(Number(args.where.and[0].createdAt.lt)).toBeLessThan(
+      Number(new Date(new Date() - 5 * 24 * 60 * 60 * 1000 + 1))
+    );
+    expect(args.where.and[1].email).toBeNull();
+  });
+  it("should not call database functions if expired accounts are found but database does not exist", async () => {
+    Accounts.findAll.mockReturnValue([{ destroy: () => {} }]);
+    pg.userHasPgAccount = () => false;
+    es.checkAccount = () => false;
+    await util.cleanAnonymous();
+    expect(pg.deletePgAccount).not.toHaveBeenCalled();
+    expect(es.deleteAccount).not.toHaveBeenCalled();
+  });
+  it("should call database functions if expired accounts are found", async () => {
+    Accounts.findAll.mockReturnValue([{ destroy: () => {} }]);
+    pg.userHasPgAccount = () => true;
+    es.checkAccount = () => true;
+    await util.cleanAnonymous();
+    expect(pg.deletePgAccount).toHaveBeenCalled();
+    expect(es.deleteAccount).toHaveBeenCalled();
+  });
+  it("should call logger.error if cleaning fails", async () => {
+    Accounts.findAll.mockImplementation(() => {
+      throw new Error("error");
+    });
+    await util.cleanAnonymous();
+    expect(logger.error).toHaveBeenCalled();
+  });
+});

src/server.js

@@ -1,5 +1,6 @@
 const express = require("express");
 const logger = require("../lib/log")(__filename);
+const util = require("../lib/util");
 const dbModule = require("../sequelize/db");
 const session = require("express-session");
 const pgModule = require("../database/postgres/pg");
@@ -16,8 +17,11 @@ const { database } = require("./routes/renderRoutes");
 require("dotenv").config();
 let server = null;
 let app = null;
+
 const arangoModule = require("../database/arango/arango");
 
+let cleaner = null;
+
 const getApp = () => {
   return app;
 };
@@ -27,6 +31,8 @@ const startServer = async (portNumber) => {
   await pgModule.startPGDB();
   await arangoModule.startArangoDB();
 
+  cleaner = await util.cleanAnonymous();
+
   return new Promise((resolve, reject) => {
     app = express();
     app.set("view engine", "ejs");
@@ -79,6 +85,7 @@ const startServer = async (portNumber) => {
 
 const stopServer = () => {
   return new Promise((resolve, reject) => {
+    cleaner.stop();
     dbModule.close();
     pgModule.closePGDB();
     arangoModule.closeArangoDB();

src/server.test.js

@@ -4,9 +4,11 @@ jest.mock("express");
 jest.mock("mailgun-js");
 jest.mock("../sequelize/db");
 jest.mock("../database/postgres/pg");
+jest.mock("../database/elasticsearch/elastic");
 
 const express = require("express");
 const dbModule = require("../sequelize/db");
+const util = require("../lib/util");
 const userRoutes = require("./routes/userRoutes");
 const renderRoutes = require("./routes/renderRoutes");
 
@@ -25,6 +27,15 @@ const { startServer, stopServer, getApp } = require("./server");
 
 dbModule.start = jest.fn();
 dbModule.close = jest.fn();
+dbModule.getModels = () => {
+  return {
+    Accounts: {
+      findAll: () => {
+        return [];
+      },
+    },
+  };
+};
 
 const app = {
   set: () => {},