Skip to content

Commit 99ef78e

Browse files
anthonykhoaanthonykhoa
authored
Merge branch 'master' into arango
2 parents bd35628 + 642b583 commit 99ef78e

File tree

8 files changed

+164
-27
lines changed

8 files changed

+164
-27
lines changed

‎database/elasticsearch/elastic.js‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ es.createAccount = async (account) => {
6363
};
6464

6565
es.deleteAccount = async (account) => {
66-
if (!account.username || !account.password) {
66+
if (!account.username) {
6767
logger.error("Account data is invalid");
6868
throw new Error("Account data is invalid");
6969
}
@@ -79,6 +79,7 @@ es.deleteAccount = async (account) => {
7979
null,
8080
authorization
8181
);
82+
const r3 = await sendESRequest(`/${account.username}-*`, "DELETE");
8283
const err = r1.error || r2.error;
8384
if (err || !r1.found || !r2.found) {
8485
logger.error("Deleting Elasticsearch user failed");
@@ -90,13 +91,18 @@ es.deleteAccount = async (account) => {
9091
};
9192

9293
es.checkAccount = async (account) => {
93-
if (!account.username || !account.password || !account.email) {
94+
const username = account.username;
95+
if (!username) {
9496
logger.error("Account data is invalid");
9597
throw new Error("Account data is invalid");
9698
}
97-
const index = account.username + "-example";
98-
const r1 = await sendFetch(`${ES_HOST}/${index}`, "GET", null, authorization);
99-
if (r1[index]) return true;
99+
100+
const r1 = await sendFetch(`${ES_HOST}/_security/user/${username}`, "GET", null, authorization);
101+
logger.info(
102+
`Checking Elasticsearch account for ${username} result:`,
103+
!!r1[username]
104+
);
105+
if (r1[username]) return true;
100106
return false;
101107
};
102108

‎database/elasticsearch/elastic.test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ describe("testing es check account function", () => {
159159
fetch.mockReturnValue(
160160
Promise.resolve({
161161
json: () => {
162-
return { "testuser-example": {} };
162+
return { testuser: {} };
163163
},
164164
})
165165
);

‎database/postgres/pg.js‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,14 @@ pgModule.createPgAccount = async (user) => {
4949
}
5050
};
5151

52-
pgModule.deletePgAccount = async (username) => {
53-
if (!username) return;
52+
pgModule.deletePgAccount = async (account) => {
53+
if (!account.username) return;
54+
const { username } = account;
5455
try {
55-
await client.query(`DROP DATABASE IF EXISTS $1`, [username]);
56-
await client.query(`DROP USER IF EXISTS $1`, [username]);
56+
const sqlQuery1 = escape(`DROP DATABASE %s;`, username);
57+
const sqlQuery2 = escape(`DROP USER %s;`, username);
58+
await client.query(sqlQuery1);
59+
await client.query(sqlQuery2);
5760
} catch (err) {
5861
logger.error(err);
5962
throw new Error(
@@ -63,12 +66,10 @@ pgModule.deletePgAccount = async (username) => {
6366
};
6467

6568
pgModule.userHasPgAccount = async (username) => {
66-
logger.info(`checking to see if ${username} has a pg account`);
6769
const result = await client.query(`SELECT 1 FROM pg_roles WHERE rolname=$1`, [
6870
username,
6971
]);
70-
71-
logger.info(`result: ${result.rowCount}`);
72+
logger.info(`Checking pg account for ${username} result: ${result.rowCount}`);
7273
return Boolean(result.rowCount);
7374
};
7475

‎database/postgres/pg.test.js‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -84,28 +84,19 @@ describe("Test PG DB", () => {
8484
describe("Test deletePgAccount", () => {
8585
it("it should execute all queries if required arguments are passed into deletePgAccount", async () => {
8686
mockClient.query.mockReturnValue(Promise.resolve());
87-
await deletePgAccount("username");
87+
await deletePgAccount({ username: "username" });
8888
expect(mockClient.query).toHaveBeenCalledTimes(2);
89-
expect(mockClient.query.mock.calls[0]).toEqual([
90-
`DROP DATABASE IF EXISTS $1`,
91-
["username"],
92-
]);
93-
expect(mockClient.query.mock.calls[1]).toEqual([
94-
`DROP USER IF EXISTS $1`,
95-
["username"],
96-
]);
9789
});
9890
it("it should not execute any queries in deletePgAccount if required arguments are not passed in", async () => {
99-
await deletePgAccount();
91+
await deletePgAccount({});
10092
expect(mockClient.query).toHaveBeenCalledTimes(0);
10193
});
10294
it("it should check if logger.error is called at throw of deletePgAccount", async () => {
10395
try {
10496
await mockClient.query.mockReturnValue(Promise.reject());
105-
const resDeletePgAccount = await deletePgAccount(
106-
"username",
107-
"password"
108-
);
97+
const resDeletePgAccount = await deletePgAccount({
98+
username: "username",
99+
});
109100
expect(resDeletePgAccount).rejects.toThrow();
110101
} catch (err) {
111102
expect(logger.error).toHaveBeenCalledTimes(1);

‎lib/util.js‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
const { Op } = require("sequelize");
2+
const db = require("../sequelize/db");
3+
const pg = require("../database/postgres/pg");
4+
const es = require("../database/elasticsearch/elastic");
5+
const logger = require("./log")(__filename);
6+
7+
const util = {};
8+
9+
util.cleanAnonymous = async () => {
10+
logger.info("Checking expired anonymous accounts...");
11+
try {
12+
const { Accounts } = db.getModels();
13+
const userAccount = await Accounts.findAll({
14+
attributes: ["id", "username"],
15+
where: {
16+
[Op.and]: [
17+
{
18+
createdAt: {
19+
[Op.lt]: new Date(new Date() - 5 * 24 * 60 * 60 * 1000),
20+
},
21+
},
22+
{ email: null },
23+
],
24+
},
25+
});
26+
logger.info(`${userAccount.length} expired accounts are found`);
27+
return Promise.all(
28+
userAccount.map(async (e) => {
29+
const pgDbExists = await pg.userHasPgAccount(e.username);
30+
if (pgDbExists) await pg.deletePgAccount(e);
31+
const esDbExists = await es.checkAccount(e);
32+
if (esDbExists) await es.deleteAccount(e);
33+
return await e.destroy();
34+
})
35+
).then(() => {
36+
logger.info("Cleaning expired accounts has completed");
37+
const timer = setTimeout(util.cleanAnonymous, 24 * 60 * 60 * 1000);
38+
return {
39+
stop: () => {
40+
clearTimeout(timer);
41+
},
42+
};
43+
});
44+
} catch (err) {
45+
logger.error("Cleaning expired accounts has failed", err);
46+
}
47+
};
48+
49+
module.exports = util;

‎lib/util.test.js‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
jest.mock("sequelize");
2+
jest.mock("../sequelize/db");
3+
jest.mock("../database/postgres/pg");
4+
jest.mock("../database/elasticsearch/elastic");
5+
jest.mock("./log");
6+
7+
const sequelize = require("sequelize");
8+
const db = require("../sequelize/db");
9+
const pg = require("../database/postgres/pg");
10+
const es = require("../database/elasticsearch/elastic");
11+
12+
sequelize.Op = { and: "and", lt: "lt" };
13+
const Accounts = {
14+
findAll: jest.fn(),
15+
};
16+
db.getModels = () => {
17+
return { Accounts: Accounts };
18+
};
19+
pg.deletePgAccount = jest.fn();
20+
es.deleteAccount = jest.fn();
21+
const logGen = require("./log");
22+
const logger = {
23+
info: jest.fn(),
24+
error: jest.fn(),
25+
};
26+
logGen.mockReturnValue(logger);
27+
28+
const util = require("./util");
29+
30+
describe("Testing cleanAnonymous function", () => {
31+
beforeEach(() => {
32+
jest.clearAllMocks();
33+
});
34+
it("should call findAll with correct queries", async () => {
35+
// Hi future engineers, I wrote some strict test here because I thought
36+
// that it's important to protect this query. Someone can accidently
37+
// clear up the whole user account database by making a little change.
38+
// If you are changing this, please do so with caution and plentiful of tests.
39+
Accounts.findAll.mockReturnValue([]);
40+
await util.cleanAnonymous();
41+
const args = Accounts.findAll.mock.calls[0][0];
42+
expect(args.attributes[0]).toEqual("id");
43+
expect(args.attributes[1]).toEqual("username");
44+
expect(Number(args.where.and[0].createdAt.lt)).toBeLessThan(
45+
Number(new Date(new Date() - 5 * 24 * 60 * 60 * 1000 + 1))
46+
);
47+
expect(args.where.and[1].email).toBeNull();
48+
});
49+
it("should not call database functions if expired accounts are found but database does not exist", async () => {
50+
Accounts.findAll.mockReturnValue([{ destroy: () => {} }]);
51+
pg.userHasPgAccount = () => false;
52+
es.checkAccount = () => false;
53+
await util.cleanAnonymous();
54+
expect(pg.deletePgAccount).not.toHaveBeenCalled();
55+
expect(es.deleteAccount).not.toHaveBeenCalled();
56+
});
57+
it("should call database functions if expired accounts are found", async () => {
58+
Accounts.findAll.mockReturnValue([{ destroy: () => {} }]);
59+
pg.userHasPgAccount = () => true;
60+
es.checkAccount = () => true;
61+
await util.cleanAnonymous();
62+
expect(pg.deletePgAccount).toHaveBeenCalled();
63+
expect(es.deleteAccount).toHaveBeenCalled();
64+
});
65+
it("should call logger.error if cleaning fails", async () => {
66+
Accounts.findAll.mockImplementation(() => {
67+
throw new Error("error");
68+
});
69+
await util.cleanAnonymous();
70+
expect(logger.error).toHaveBeenCalled();
71+
});
72+
});

‎src/server.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
const express = require("express");
22
const logger = require("../lib/log")(__filename);
3+
const util = require("../lib/util");
34
const dbModule = require("../sequelize/db");
45
const session = require("express-session");
56
const pgModule = require("../database/postgres/pg");
@@ -16,8 +17,11 @@ const { database } = require("./routes/renderRoutes");
1617
require("dotenv").config();
1718
let server = null;
1819
let app = null;
20+
1921
const arangoModule = require("../database/arango/arango");
2022

23+
let cleaner = null;
24+
2125
const getApp = () => {
2226
return app;
2327
};
@@ -27,6 +31,8 @@ const startServer = async (portNumber) => {
2731
await pgModule.startPGDB();
2832
await arangoModule.startArangoDB();
2933

34+
cleaner = await util.cleanAnonymous();
35+
3036
return new Promise((resolve, reject) => {
3137
app = express();
3238
app.set("view engine", "ejs");
@@ -79,6 +85,7 @@ const startServer = async (portNumber) => {
7985

8086
const stopServer = () => {
8187
return new Promise((resolve, reject) => {
88+
cleaner.stop();
8289
dbModule.close();
8390
pgModule.closePGDB();
8491
arangoModule.closeArangoDB();

‎src/server.test.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ jest.mock("express");
44
jest.mock("mailgun-js");
55
jest.mock("../sequelize/db");
66
jest.mock("../database/postgres/pg");
7+
jest.mock("../database/elasticsearch/elastic");
78

89
const express = require("express");
910
const dbModule = require("../sequelize/db");
11+
const util = require("../lib/util");
1012
const userRoutes = require("./routes/userRoutes");
1113
const renderRoutes = require("./routes/renderRoutes");
1214

@@ -25,6 +27,15 @@ const { startServer, stopServer, getApp } = require("./server");
2527

2628
dbModule.start = jest.fn();
2729
dbModule.close = jest.fn();
30+
dbModule.getModels = () => {
31+
return {
32+
Accounts: {
33+
findAll: () => {
34+
return [];
35+
},
36+
},
37+
};
38+
};
2839

2940
const app = {
3041
set: () => {},

0 commit comments

Comments
 (0)