diff --git a/app/gen-server/ApiServer.ts b/app/gen-server/ApiServer.ts index 7d283f2f..1db45714 100644 --- a/app/gen-server/ApiServer.ts +++ b/app/gen-server/ApiServer.ts @@ -429,7 +429,7 @@ export class ApiServer { throw new ApiError('Name expected in the body', 400); } const name = req.body.name; - await this._dbManager.updateUserName(userId, name); + await this._dbManager.updateUser(userId, { name }); res.sendStatus(200); })); diff --git a/app/gen-server/lib/homedb/HomeDBManager.ts b/app/gen-server/lib/homedb/HomeDBManager.ts index e70884d1..b510c21b 100644 --- a/app/gen-server/lib/homedb/HomeDBManager.ts +++ b/app/gen-server/lib/homedb/HomeDBManager.ts @@ -56,7 +56,7 @@ import { readJson } from 'app/gen-server/sqlUtils'; import {appSettings} from 'app/server/lib/AppSettings'; -import {getOrCreateConnection} from 'app/server/lib/dbUtils'; +import {createNewConnection, getOrCreateConnection} from 'app/server/lib/dbUtils'; import {makeId} from 'app/server/lib/idUtils'; import log from 'app/server/lib/log'; import {Permit} from 'app/server/lib/Permit'; @@ -70,6 +70,7 @@ import { Brackets, Connection, DatabaseType, + DataSourceOptions, EntityManager, ObjectLiteral, SelectQueryBuilder, @@ -248,7 +249,6 @@ export type BillingOptions = Partial { this._connection = await getOrCreateConnection(); - this._dbType = this._connection.driver.options.type; + } + + public async createNewConnection(overrideConf?: Partial): Promise { + this._connection = await createNewConnection(overrideConf); } // make sure special users and workspaces are available @@ -461,10 +468,6 @@ export class HomeDBManager extends EventEmitter { } } - public async updateUserName(userId: number, name: string) { - return this._usersManager.updateUserName(userId, name); - } - public async updateUserOptions(userId: number, props: Partial) { return this._usersManager.updateUserOptions(userId, props); } @@ -472,14 +475,14 @@ export class HomeDBManager extends EventEmitter { /** * @see UsersManager.prototype.getUserByLoginWithRetry */ - public async getUserByLoginWithRetry(email: string, options: GetUserOptions = {}): Promise { + public async getUserByLoginWithRetry(email: string, options: GetUserOptions = {}): Promise { return this._usersManager.getUserByLoginWithRetry(email, options); } /** * @see UsersManager.prototype.getUserByLogin */ - public async getUserByLogin(email: string, options: GetUserOptions = {}): Promise { + public async getUserByLogin(email: string, options: GetUserOptions = {}): Promise { return this._usersManager.getUserByLogin(email, options); } @@ -4362,7 +4365,6 @@ export class HomeDBManager extends EventEmitter { }); return verifyEntity(orgQuery); } - } // Return a QueryResult reflecting the output of a query builder. diff --git a/app/gen-server/lib/homedb/Interfaces.ts b/app/gen-server/lib/homedb/Interfaces.ts index 0802d96a..22eec249 100644 --- a/app/gen-server/lib/homedb/Interfaces.ts +++ b/app/gen-server/lib/homedb/Interfaces.ts @@ -34,7 +34,7 @@ export type NonGuestGroup = Group & { name: roles.NonGuestRole }; export type Resource = Organization|Workspace|Document; -export type RunInTransaction = ( +export type RunInTransaction = ( transaction: EntityManager|undefined, - op: ((manager: EntityManager) => Promise) -) => Promise; + op: ((manager: EntityManager) => Promise) +) => Promise; diff --git a/app/gen-server/lib/homedb/UsersManager.ts b/app/gen-server/lib/homedb/UsersManager.ts index e070273a..a219fa23 100644 --- a/app/gen-server/lib/homedb/UsersManager.ts +++ b/app/gen-server/lib/homedb/UsersManager.ts @@ -97,7 +97,7 @@ export class UsersManager { public async testClearUserPrefs(emails: string[]) { return await this._connection.transaction(async manager => { for (const email of emails) { - const user = await this.getUserByLogin(email, {manager}); + const user = await this.getExistingUserByLogin(email, manager); if (user) { await manager.delete(Pref, {userId: user.id}); } @@ -116,7 +116,7 @@ export class UsersManager { */ public getAnonymousUserId(): number { const id = this._specialUserIds[ANONYMOUS_USER_EMAIL]; - if (!id) { throw new Error("Anonymous user not available"); } + if (!id) { throw new Error("'Anonymous' user not available"); } return id; } @@ -125,7 +125,7 @@ export class UsersManager { */ public getPreviewerUserId(): number { const id = this._specialUserIds[PREVIEWER_EMAIL]; - if (!id) { throw new Error("Previewer user not available"); } + if (!id) { throw new Error("'Previewer' user not available"); } return id; } @@ -134,7 +134,7 @@ export class UsersManager { */ public getEveryoneUserId(): number { const id = this._specialUserIds[EVERYONE_EMAIL]; - if (!id) { throw new Error("'everyone' user not available"); } + if (!id) { throw new Error("'Everyone' user not available"); } return id; } @@ -143,7 +143,7 @@ export class UsersManager { */ public getSupportUserId(): number { const id = this._specialUserIds[SUPPORT_EMAIL]; - if (!id) { throw new Error("'support' user not available"); } + if (!id) { throw new Error("'Support' user not available"); } return id; } @@ -221,9 +221,6 @@ export class UsersManager { profile, manager }); - if (!newUser) { - throw new ApiError("Unable to create user", 500); - } // No need to survey this user. newUser.isFirstTimeUser = false; await newUser.save(); @@ -286,13 +283,7 @@ export class UsersManager { return { user, isWelcomed }; } - public async updateUserName(userId: number, name: string) { - const user = await User.findOne({where: {id: userId}}); - if (!user) { throw new ApiError("unable to find user", 400); } - user.name = name; - await user.save(); - } - + // TODO: rather use the updateUser() method, if that makes sense? public async updateUserOptions(userId: number, props: Partial) { const user = await User.findOne({where: {id: userId}}); if (!user) { throw new ApiError("unable to find user", 400); } @@ -321,7 +312,7 @@ export class UsersManager { // for an email key conflict failure. This is in case our transaction conflicts with a peer // doing the same thing. This is quite likely if the first page visited by a previously // unseen user fires off multiple api calls. - public async getUserByLoginWithRetry(email: string, options: GetUserOptions = {}): Promise { + public async getUserByLoginWithRetry(email: string, options: GetUserOptions = {}): Promise { try { return await this.getUserByLogin(email, options); } catch (e) { @@ -361,10 +352,10 @@ export class UsersManager { * unset/outdated fields of an existing record. * */ - public async getUserByLogin(email: string, options: GetUserOptions = {}): Promise { + public async getUserByLogin(email: string, options: GetUserOptions = {}) { const {manager: transaction, profile, userOptions} = options; const normalizedEmail = normalizeEmail(email); - const userByLogin = await this._runInTransaction(transaction, async manager => { + return await this._runInTransaction(transaction, async manager => { let needUpdate = false; const userQuery = manager.createQueryBuilder() .select('user') @@ -473,9 +464,8 @@ export class UsersManager { // In principle this could be optimized, but this is simpler to maintain. user = await userQuery.getOne(); } - return user; + return user!; }); - return userByLogin; } /** @@ -520,6 +510,63 @@ export class UsersManager { }; } + public async initializeSpecialIds(): Promise { + await this._maybeCreateSpecialUserId({ + email: ANONYMOUS_USER_EMAIL, + name: "Anonymous" + }); + await this._maybeCreateSpecialUserId({ + email: PREVIEWER_EMAIL, + name: "Preview" + }); + await this._maybeCreateSpecialUserId({ + email: EVERYONE_EMAIL, + name: "Everyone" + }); + await this._maybeCreateSpecialUserId({ + email: SUPPORT_EMAIL, + name: "Support" + }); + } + + /** + * + * Take a list of user profiles coming from the client's session, correlate + * them with Users and Logins in the database, and construct full profiles + * with user ids, standardized display emails, pictures, and anonymous flags. + * + */ + public async completeProfiles(profiles: UserProfile[]): Promise { + if (profiles.length === 0) { return []; } + const qb = this._connection.createQueryBuilder() + .select('logins') + .from(Login, 'logins') + .leftJoinAndSelect('logins.user', 'user') + .where('logins.email in (:...emails)', {emails: profiles.map(profile => normalizeEmail(profile.email))}); + const completedProfiles: {[email: string]: FullUser} = {}; + for (const login of await qb.getMany()) { + completedProfiles[login.email] = { + id: login.user.id, + email: login.displayEmail, + name: login.user.name, + picture: login.user.picture, + anonymous: login.user.id === this.getAnonymousUserId(), + locale: login.user.options?.locale + }; + } + return profiles.map(profile => completedProfiles[normalizeEmail(profile.email)]) + .filter(fullProfile => fullProfile); + } + + /** + * ================================== + * + * Below methods are public but not exposed by HomeDBManager + * + * They are meant to be used internally (i.e. by homedb/ modules) + * + */ + // Looks up the emails in the permission delta and adds them to the users map in // the delta object. // Returns a QueryResult based on the validity of the passed in PermissionDelta object. @@ -589,25 +636,6 @@ export class UsersManager { }; } - public async initializeSpecialIds(): Promise { - await this._maybeCreateSpecialUserId({ - email: ANONYMOUS_USER_EMAIL, - name: "Anonymous" - }); - await this._maybeCreateSpecialUserId({ - email: PREVIEWER_EMAIL, - name: "Preview" - }); - await this._maybeCreateSpecialUserId({ - email: EVERYONE_EMAIL, - name: "Everyone" - }); - await this._maybeCreateSpecialUserId({ - email: SUPPORT_EMAIL, - name: "Support" - }); - } - /** * Check for anonymous user, either encoded directly as an id, or as a singular * profile (this case arises during processing of the session/access/all endpoint @@ -684,34 +712,6 @@ export class UsersManager { return members; } - /** - * - * Take a list of user profiles coming from the client's session, correlate - * them with Users and Logins in the database, and construct full profiles - * with user ids, standardized display emails, pictures, and anonymous flags. - * - */ - public async completeProfiles(profiles: UserProfile[]): Promise { - if (profiles.length === 0) { return []; } - const qb = this._connection.createQueryBuilder() - .select('logins') - .from(Login, 'logins') - .leftJoinAndSelect('logins.user', 'user') - .where('logins.email in (:...emails)', {emails: profiles.map(profile => normalizeEmail(profile.email))}); - const completedProfiles: {[email: string]: FullUser} = {}; - for (const login of await qb.getMany()) { - completedProfiles[login.email] = { - id: login.user.id, - email: login.displayEmail, - name: login.user.name, - picture: login.user.picture, - anonymous: login.user.id === this.getAnonymousUserId(), - locale: login.user.options?.locale - }; - } - return profiles.map(profile => completedProfiles[normalizeEmail(profile.email)]) - .filter(profile => profile); - } // For the moment only the support user can add both everyone@ and anon@ to a // resource, since that allows spam. TODO: enhance or remove. @@ -735,7 +735,7 @@ export class UsersManager { // user if a bunch of servers start simultaneously and the user doesn't exist // yet. const user = await this.getUserByLoginWithRetry(profile.email, {profile}); - if (user) { id = this._specialUserIds[profile.email] = user.id; } + id = this._specialUserIds[profile.email] = user.id; } if (!id) { throw new Error(`Could not find or create user ${profile.email}`); } return id; diff --git a/app/server/companion.ts b/app/server/companion.ts index bad8092c..ab454cb5 100644 --- a/app/server/companion.ts +++ b/app/server/companion.ts @@ -166,10 +166,6 @@ export function addSiteCommand(program: commander.Command, const profile = {email, name: email}; const db = await getHomeDBManager(); const user = await db.getUserByLogin(email, {profile}); - if (!user) { - // This should not happen. - throw new Error('failed to create user'); - } db.unwrapQueryResult(await db.addOrg(user, { name: domain, domain, diff --git a/app/server/lib/TestLogin.ts b/app/server/lib/TestLogin.ts index 12ef87a3..ca85b47c 100644 --- a/app/server/lib/TestLogin.ts +++ b/app/server/lib/TestLogin.ts @@ -25,10 +25,8 @@ export async function getTestLoginSystem(): Promise { if (process.env.TEST_SUPPORT_API_KEY) { const dbManager = gristServer.getHomeDBManager(); const user = await dbManager.getUserByLogin(SUPPORT_EMAIL); - if (user) { - user.apiKey = process.env.TEST_SUPPORT_API_KEY; - await user.save(); - } + user.apiKey = process.env.TEST_SUPPORT_API_KEY; + await user.save(); } return "test-login"; }, diff --git a/app/server/lib/dbUtils.ts b/app/server/lib/dbUtils.ts index 4a823640..754d774e 100644 --- a/app/server/lib/dbUtils.ts +++ b/app/server/lib/dbUtils.ts @@ -45,38 +45,53 @@ export async function updateDb(connection?: Connection) { await synchronizeProducts(connection, true); } +export function getConnectionName() { + return process.env.TYPEORM_NAME || 'default'; +} + /** * Get a connection to db if one exists, or create one. Serialized to * avoid duplication. */ const connectionMutex = new Mutex(); + +async function buildConnection(overrideConf?: Partial) { + const settings = getTypeORMSettings(overrideConf); + const connection = await createConnection(settings); + // When using Sqlite, set a busy timeout of 3s to tolerate a little + // interference from connections made by tests. Logging doesn't show + // any particularly slow queries, but bad luck is possible. + // This doesn't affect when Postgres is in use. It also doesn't have + // any impact when there is a single connection to the db, as is the + // case when Grist is run as a single process. + if (connection.driver.options.type === 'sqlite') { + await connection.query('PRAGMA busy_timeout = 3000'); + } + return connection; +} + export async function getOrCreateConnection(): Promise { - return connectionMutex.runExclusive(async() => { + return connectionMutex.runExclusive(async () => { try { // If multiple servers are started within the same process, we // share the database connection. This saves locking trouble // with Sqlite. - const connection = getConnection(); - return connection; + return getConnection(getConnectionName()); } catch (e) { if (!String(e).match(/ConnectionNotFoundError/)) { throw e; } - const connection = await createConnection(getTypeORMSettings()); - // When using Sqlite, set a busy timeout of 3s to tolerate a little - // interference from connections made by tests. Logging doesn't show - // any particularly slow queries, but bad luck is possible. - // This doesn't affect when Postgres is in use. It also doesn't have - // any impact when there is a single connection to the db, as is the - // case when Grist is run as a single process. - if (connection.driver.options.type === 'sqlite') { - await connection.query('PRAGMA busy_timeout = 3000'); - } - return connection; + return buildConnection(); } }); } +export async function createNewConnection(overrideConf?: Partial): Promise { + return connectionMutex.runExclusive(async () => { + return buildConnection(overrideConf); + }); +} + export async function runMigrations(connection: Connection) { // on SQLite, migrations fail if we don't temporarily disable foreign key // constraint checking. This is because for sqlite typeorm copies each @@ -103,7 +118,7 @@ export async function undoLastMigration(connection: Connection) { // Replace the old janky ormconfig.js file, which was always a source of // pain to use since it wasn't properly integrated into the typescript // project. -export function getTypeORMSettings(): DataSourceOptions { +export function getTypeORMSettings(overrideConf?: Partial): DataSourceOptions { // If we have a redis server available, tell typeorm. Then any queries built with // .cache() called on them will be cached via redis. // We use a separate environment variable for the moment so that we don't have to @@ -120,7 +135,7 @@ export function getTypeORMSettings(): DataSourceOptions { } : undefined; return { - "name": process.env.TYPEORM_NAME || "default", + "name": getConnectionName(), "type": (process.env.TYPEORM_TYPE as any) || "sqlite", // officially, TYPEORM_CONNECTION - // but if we use that, this file will never // be read, and we can't configure @@ -144,5 +159,6 @@ export function getTypeORMSettings(): DataSourceOptions { ], ...JSON.parse(process.env.TYPEORM_EXTRA || "{}"), ...cache, + ...overrideConf, }; } diff --git a/stubs/app/server/server.ts b/stubs/app/server/server.ts index c7ccc7be..921def02 100644 --- a/stubs/app/server/server.ts +++ b/stubs/app/server/server.ts @@ -79,10 +79,6 @@ async function setupDb() { } const profile = {email, name: email}; const user = await db.getUserByLogin(email, {profile}); - if (!user) { - // This should not happen. - throw new Error('failed to create GRIST_DEFAULT_EMAIL user'); - } db.unwrapQueryResult(await db.addOrg(user, { name: org, domain: org, diff --git a/test/gen-server/ApiServer.ts b/test/gen-server/ApiServer.ts index 2f651604..b79355ca 100644 --- a/test/gen-server/ApiServer.ts +++ b/test/gen-server/ApiServer.ts @@ -49,9 +49,9 @@ describe('ApiServer', function() { homeUrl = await server.start(['home', 'docs']); dbManager = server.dbManager; - chimpyRef = await dbManager.getUserByLogin(chimpyEmail).then((user) => user!.ref); - kiwiRef = await dbManager.getUserByLogin(kiwiEmail).then((user) => user!.ref); - charonRef = await dbManager.getUserByLogin(charonEmail).then((user) => user!.ref); + chimpyRef = await dbManager.getUserByLogin(chimpyEmail).then((user) => user.ref); + kiwiRef = await dbManager.getUserByLogin(kiwiEmail).then((user) => user.ref); + charonRef = await dbManager.getUserByLogin(charonEmail).then((user) => user.ref); // Listen to user count updates and add them to an array. dbManager.on('userChange', ({org, countBefore, countAfter}: UserChange) => { @@ -2070,8 +2070,7 @@ describe('ApiServer', function() { // create a new user const profile = {email: 'meep@getgrist.com', name: 'Meep'}; const user = await dbManager.getUserByLogin('meep@getgrist.com', {profile}); - assert(user); - const userId = user!.id; + const userId = user.id; // set up an api key await dbManager.connection.query("update users set api_key = 'api_key_for_meep' where id = $1", [userId]); @@ -2111,11 +2110,10 @@ describe('ApiServer', function() { const userBlank = await dbManager.getUserByLogin('blank@getgrist.com', {profile: {email: 'blank@getgrist.com', name: ''}}); - assert(userBlank); - await dbManager.connection.query("update users set api_key = 'api_key_for_blank' where id = $1", [userBlank!.id]); + await dbManager.connection.query("update users set api_key = 'api_key_for_blank' where id = $1", [userBlank.id]); // check that user can delete themselves - resp = await axios.delete(`${homeUrl}/api/users/${userBlank!.id}`, + resp = await axios.delete(`${homeUrl}/api/users/${userBlank.id}`, {data: {name: ""}, ...configForUser("blank")}); assert.equal(resp.status, 200); diff --git a/test/gen-server/ApiServerAccess.ts b/test/gen-server/ApiServerAccess.ts index 33abe90b..a95494f5 100644 --- a/test/gen-server/ApiServerAccess.ts +++ b/test/gen-server/ApiServerAccess.ts @@ -61,9 +61,9 @@ describe('ApiServerAccess', function() { } ); dbManager = server.dbManager; - chimpyRef = await dbManager.getUserByLogin(chimpyEmail).then((user) => user!.ref); - kiwiRef = await dbManager.getUserByLogin(kiwiEmail).then((user) => user!.ref); - charonRef = await dbManager.getUserByLogin(charonEmail).then((user) => user!.ref); + chimpyRef = await dbManager.getUserByLogin(chimpyEmail).then((user) => user.ref); + kiwiRef = await dbManager.getUserByLogin(kiwiEmail).then((user) => user.ref); + charonRef = await dbManager.getUserByLogin(charonEmail).then((user) => user.ref); // Listen to user count updates and add them to an array. dbManager.on('userChange', ({org, countBefore, countAfter}: UserChange) => { if (countBefore === countAfter) { return; } diff --git a/test/gen-server/ApiServerBugs.ts b/test/gen-server/ApiServerBugs.ts index f8ef2112..dd1ad183 100644 --- a/test/gen-server/ApiServerBugs.ts +++ b/test/gen-server/ApiServerBugs.ts @@ -32,7 +32,7 @@ describe('ApiServerBugs', function() { server = new TestServer(this); homeUrl = await server.start(); dbManager = server.dbManager; - userRef = (email) => server.dbManager.getUserByLogin(email).then((user) => user!.ref); + userRef = (email) => server.dbManager.getUserByLogin(email).then((user) => user.ref); }); after(async function() { diff --git a/test/gen-server/lib/HomeDBManager.ts b/test/gen-server/lib/HomeDBManager.ts index 77778fef..a22c5547 100644 --- a/test/gen-server/lib/HomeDBManager.ts +++ b/test/gen-server/lib/HomeDBManager.ts @@ -33,14 +33,14 @@ describe('HomeDBManager', function() { it('can find existing user by email', async function() { const user = await home.getUserByLogin('chimpy@getgrist.com'); - assert.equal(user!.name, 'Chimpy'); + assert.equal(user.name, 'Chimpy'); }); it('can create new user by email, with personal org', async function() { const profile = {email: 'unseen@getgrist.com', name: 'Unseen'}; const user = await home.getUserByLogin('unseen@getgrist.com', {profile}); - assert.equal(user!.name, 'Unseen'); - const orgs = await home.getOrgs(user!.id, null); + assert.equal(user.name, 'Unseen'); + const orgs = await home.getOrgs(user.id, null); assert.isAtLeast(orgs.data!.length, 1); assert.equal(orgs.data![0].name, 'Personal'); assert.equal(orgs.data![0].owner.name, 'Unseen'); @@ -65,37 +65,37 @@ describe('HomeDBManager', function() { // log in without a name let user = await home.getUserByLogin('unseen2@getgrist.com'); // name is blank - assert.equal(user!.name, ''); + assert.equal(user.name, ''); // log in with a name const profile: UserProfile = {email: 'unseen2@getgrist.com', name: 'Unseen2'}; user = await home.getUserByLogin('unseen2@getgrist.com', {profile}); // name is now set - assert.equal(user!.name, 'Unseen2'); + assert.equal(user.name, 'Unseen2'); // log in without a name user = await home.getUserByLogin('unseen2@getgrist.com'); // name is still set - assert.equal(user!.name, 'Unseen2'); + assert.equal(user.name, 'Unseen2'); // no picture yet - assert.equal(user!.picture, null); + assert.equal(user.picture, null); // log in with picture link profile.picture = 'http://picture.pic'; user = await home.getUserByLogin('unseen2@getgrist.com', {profile}); // now should have a picture link - assert.equal(user!.picture, 'http://picture.pic'); + assert.equal(user.picture, 'http://picture.pic'); // log in without picture user = await home.getUserByLogin('unseen2@getgrist.com'); // should still have picture link - assert.equal(user!.picture, 'http://picture.pic'); + assert.equal(user.picture, 'http://picture.pic'); }); it('can add an org', async function() { const user = await home.getUserByLogin('chimpy@getgrist.com'); - const orgId = (await home.addOrg(user!, {name: 'NewOrg', domain: 'novel-org'}, teamOptions)).data!; - const org = await home.getOrg({userId: user!.id}, orgId); + const orgId = (await home.addOrg(user, {name: 'NewOrg', domain: 'novel-org'}, teamOptions)).data!; + const org = await home.getOrg({userId: user.id}, orgId); assert.equal(org.data!.name, 'NewOrg'); assert.equal(org.data!.domain, 'novel-org'); assert.equal(org.data!.billingAccount.product.name, TEAM_PLAN); - await home.deleteOrg({userId: user!.id}, orgId); + await home.deleteOrg({userId: user.id}, orgId); }); it('creates default plan if defined', async function() { @@ -104,28 +104,28 @@ describe('HomeDBManager', function() { try { // Set the default product to be the free plan. process.env.GRIST_DEFAULT_PRODUCT = FREE_PLAN; - let orgId = (await home.addOrg(user!, {name: 'NewOrg', domain: 'novel-org'}, { + let orgId = (await home.addOrg(user, {name: 'NewOrg', domain: 'novel-org'}, { setUserAsOwner: false, useNewPlan: true, // omit plan, to use a default one (teamInitial) // it will either be 'stub' or anything set in GRIST_DEFAULT_PRODUCT })).data!; - let org = await home.getOrg({userId: user!.id}, orgId); + let org = await home.getOrg({userId: user.id}, orgId); assert.equal(org.data!.name, 'NewOrg'); assert.equal(org.data!.domain, 'novel-org'); assert.equal(org.data!.billingAccount.product.name, FREE_PLAN); - await home.deleteOrg({userId: user!.id}, orgId); + await home.deleteOrg({userId: user.id}, orgId); // Now remove the default product, and check that the default plan is used. delete process.env.GRIST_DEFAULT_PRODUCT; - orgId = (await home.addOrg(user!, {name: 'NewOrg', domain: 'novel-org'}, { + orgId = (await home.addOrg(user, {name: 'NewOrg', domain: 'novel-org'}, { setUserAsOwner: false, useNewPlan: true, })).data!; - org = await home.getOrg({userId: user!.id}, orgId); + org = await home.getOrg({userId: user.id}, orgId); assert.equal(org.data!.billingAccount.product.name, STUB_PLAN); - await home.deleteOrg({userId: user!.id}, orgId); + await home.deleteOrg({userId: user.id}, orgId); } finally { oldEnv.restore(); } @@ -134,17 +134,17 @@ describe('HomeDBManager', function() { it('cannot duplicate a domain', async function() { const user = await home.getUserByLogin('chimpy@getgrist.com'); const domain = 'repeated-domain'; - const result = await home.addOrg(user!, {name: `${domain}!`, domain}, teamOptions); + const result = await home.addOrg(user, {name: `${domain}!`, domain}, teamOptions); const orgId = result.data!; assert.equal(result.status, 200); - await assert.isRejected(home.addOrg(user!, {name: `${domain}!`, domain}, teamOptions), + await assert.isRejected(home.addOrg(user, {name: `${domain}!`, domain}, teamOptions), /Domain already in use/); - await home.deleteOrg({userId: user!.id}, orgId); + await home.deleteOrg({userId: user.id}, orgId); }); it('cannot add an org with a (blacklisted) dodgy domain', async function() { const user = await home.getUserByLogin('chimpy@getgrist.com'); - const userId = user!.id; + const userId = user.id; const misses = [ 'thing!', ' thing', 'ww', 'docs-999', 'o-99', '_domainkey', 'www', 'api', 'thissubdomainiswaytoolongmyfriendyoushouldrethinkitoratleastsummarizeit', @@ -154,13 +154,13 @@ describe('HomeDBManager', function() { 'thing', 'jpl', 'xyz', 'appel', '123', '1google' ]; for (const domain of misses) { - const result = await home.addOrg(user!, {name: `${domain}!`, domain}, teamOptions); + const result = await home.addOrg(user, {name: `${domain}!`, domain}, teamOptions); assert.equal(result.status, 400); const org = await home.getOrg({userId}, domain); assert.equal(org.status, 404); } for (const domain of hits) { - const result = await home.addOrg(user!, {name: `${domain}!`, domain}, teamOptions); + const result = await home.addOrg(user, {name: `${domain}!`, domain}, teamOptions); assert.equal(result.status, 200); const org = await home.getOrg({userId}, domain); assert.equal(org.status, 200); @@ -189,7 +189,7 @@ describe('HomeDBManager', function() { // Fetch the doc and check that the updatedAt value is as expected. const kiwi = await home.getUserByLogin('kiwi@getgrist.com'); - const resp1 = await home.getOrgWorkspaces({userId: kiwi!.id}, primatelyOrgId); + const resp1 = await home.getOrgWorkspaces({userId: kiwi.id}, primatelyOrgId); assert.equal(resp1.status, 200); // Check that the apples metadata is as expected. updatedAt should have been set @@ -209,7 +209,7 @@ describe('HomeDBManager', function() { // Check that the shark metadata is as expected. updatedAt should have been set // to 2004. usage should be set. - const resp2 = await home.getOrgWorkspaces({userId: kiwi!.id}, fishOrgId); + const resp2 = await home.getOrgWorkspaces({userId: kiwi.id}, fishOrgId); assert.equal(resp2.status, 200); const shark = resp2.data![0].docs.find((doc: any) => doc.name === 'Shark'); assert.equal(shark!.updatedAt.toISOString(), setDateISO2); @@ -340,7 +340,7 @@ describe('HomeDBManager', function() { it('can fork docs', async function() { const user1 = await home.getUserByLogin('kiwi@getgrist.com'); - const user1Id = user1!.id; + const user1Id = user1.id; const orgId = await home.testGetId('Fish') as number; const doc1Id = await home.testGetId('Shark') as string; const scope = {userId: user1Id, urlId: doc1Id}; @@ -393,7 +393,7 @@ describe('HomeDBManager', function() { // Now fork "Shark" as Chimpy, and check that Kiwi's forks aren't listed. const user2 = await home.getUserByLogin('chimpy@getgrist.com'); - const user2Id = user2!.id; + const user2Id = user2.id; const resp4 = await home.getOrgWorkspaces({userId: user2Id}, orgId); const resp4Doc = resp4.data![0].docs.find((d: any) => d.name === 'Shark'); assert.deepEqual(resp4Doc!.forks, []); diff --git a/test/gen-server/lib/emails.ts b/test/gen-server/lib/emails.ts index aae3957c..bff255ae 100644 --- a/test/gen-server/lib/emails.ts +++ b/test/gen-server/lib/emails.ts @@ -19,7 +19,7 @@ describe('emails', function() { beforeEach(async function() { this.timeout(5000); server = new TestServer(this); - ref = (email: string) => server.dbManager.getUserByLogin(email).then((user) => user!.ref); + ref = (email: string) => server.dbManager.getUserByLogin(email).then((user) => user.ref); serverUrl = await server.start(); }); diff --git a/test/gen-server/lib/homedb/UsersManager.ts b/test/gen-server/lib/homedb/UsersManager.ts new file mode 100644 index 00000000..736f1710 --- /dev/null +++ b/test/gen-server/lib/homedb/UsersManager.ts @@ -0,0 +1,1033 @@ +import { isAffirmative } from 'app/common/gutil'; +import { FullUser, UserProfile } from 'app/common/LoginSessionAPI'; +import { ANONYMOUS_USER_EMAIL, EVERYONE_EMAIL, PREVIEWER_EMAIL, UserOptions } from 'app/common/UserAPI'; +import { AclRuleOrg } from 'app/gen-server/entity/AclRule'; +import { Document } from 'app/gen-server/entity/Document'; +import { Group } from 'app/gen-server/entity/Group'; +import { Login } from 'app/gen-server/entity/Login'; +import { Organization } from 'app/gen-server/entity/Organization'; +import { Pref } from 'app/gen-server/entity/Pref'; +import { User } from 'app/gen-server/entity/User'; +import { Workspace } from 'app/gen-server/entity/Workspace'; +import { HomeDBManager } from 'app/gen-server/lib/homedb/HomeDBManager'; +import { GetUserOptions, NonGuestGroup, Resource } from 'app/gen-server/lib/homedb/Interfaces'; +import { SUPPORT_EMAIL, UsersManager } from 'app/gen-server/lib/homedb/UsersManager'; +import { updateDb } from 'app/server/lib/dbUtils'; +import { prepareDatabase } from 'test/server/lib/helpers/PrepareDatabase'; +import { EnvironmentSnapshot } from 'test/server/testUtils'; +import { getDatabase } from 'test/testUtils'; + +import log from 'app/server/lib/log'; +import { assert } from 'chai'; +import Sinon, { SinonSandbox, SinonSpy } from 'sinon'; +import { EntityManager } from 'typeorm'; +import winston from 'winston'; + +import fs from 'fs/promises'; +import { tmpdir } from 'os'; +import path from 'path'; +import { dereferenceConnection } from 'test/gen-server/seed'; + +const username = process.env.USER || "nobody"; +const tmpDirPrefix = path.join(tmpdir(), `grist_test_${username}_userendpoint_`); +// it is sometimes useful in debugging to turn off automatic cleanup of sqlite databases. +const noCleanup = isAffirmative(process.env.NO_CLEANUP); + +describe('UsersManager', function () { + this.timeout('30s'); + let tmpDir: string; + + before(async function () { + tmpDir = await fs.mkdtemp(tmpDirPrefix); + }); + + after(async function () { + if (!noCleanup) { + await fs.rm(tmpDir, {recursive: true}); + } + }); + + describe('static method', function () { + /** + * Create a simple iterator of integer starting from 0 which is incremented every time we call next() + */ + function* makeUserIdIterator() { + for (let i = 0; i < 500; i++) { + yield i; + } + } + + /** + * Create a table of users. + * @param nbUsers The number of users to create + * @param [userIdIterator=makeIdxIterator()] An iterator used to create users' id, + * which keep track of the increment accross calls. + * Pass your own iterator if you want to call this methods several times and keep the id unique. + * If omitted, create its own iterator that starts from 0. + */ + function makeUsers(nbUsers: number, userIdIterator = makeUserIdIterator()): User[] { + return Array(nbUsers).fill(null).map(() => { + const user = new User(); + const itItem = userIdIterator.next(); + if (itItem.done) { + throw new Error('Excessive number of users created'); + } + user.id = itItem.value; + user.name = `User ${itItem.value}`; + return user; + }); + } + + /** + * Populate passed resources with members + * @param resources The Resources + * @param nbUsersByResource The number of users to create for each resources (each one is unique) + * @returns The Resources and their respective members + */ + function populateResourcesWithMembers( + resources: Resource[], nbUsersByResource: number, makeResourceGrpName?: (idx: number) => string + ): Map { + const membersByResource = new Map(); + const idxIterator = makeUserIdIterator(); + for (const [idx, resource] of resources.entries()) { + const aclRule = new AclRuleOrg(); + const group = new Group(); + if (makeResourceGrpName) { + group.name = makeResourceGrpName(idx); + } + const members = makeUsers(nbUsersByResource, idxIterator); + group.memberUsers = members; + aclRule.group = group; + resource.aclRules = [ + aclRule + ]; + membersByResource.set(resource, members); + } + return membersByResource; + } + + /** + * Populate a resource with members and return the members + */ + function populateSingleResourceWithMembers(resource: Resource, nbUsers: number) { + const membersByResource = populateResourcesWithMembers([resource], nbUsers); + return membersByResource.get(resource)!; + } + + describe('getResourceUsers()', function () { + it('should return all users from a single organization ACL', function () { + const resource = new Organization(); + const expectedUsers = populateSingleResourceWithMembers(resource, 5); + + const result = UsersManager.getResourceUsers(resource); + + assert.deepEqual(result, expectedUsers); + }); + + it('should return all users from all resources ACL', function () { + const resources: Resource[] = [new Organization(), new Workspace(), new Document()]; + const membersByResource = populateResourcesWithMembers(resources, 5); + + const result = UsersManager.getResourceUsers(resources); + + assert.deepEqual(result, [...membersByResource.values()].flat()); + }); + + it('should deduplicate the results', function () { + const resources: Resource[] = [new Organization(), new Workspace()]; + const membersByResource = populateResourcesWithMembers(resources, 1); + const usersList = [...membersByResource.values()]; + const expectedResult = usersList.flat(); + + const duplicateUser = new User(); + duplicateUser.id = usersList[1][0].id; + usersList[0].unshift(duplicateUser); + + const result = UsersManager.getResourceUsers(resources); + + assert.deepEqual(result, expectedResult); + }); + + it('should return users matching group names from all resources ACL', function () { + const someOrg = new Organization(); + const someWorkspace = new Workspace(); + const someDoc = new Document(); + const resources: Resource[] = [someOrg, someWorkspace, someDoc]; + const allGroupNames = ['OrgGrp', 'WorkspaceGrp', 'DocGrp']; + const membersByResource = populateResourcesWithMembers(resources, 5, i => allGroupNames[i]); + const filteredGroupNames = [ 'WorkspaceGrp', 'DocGrp' ]; + + const result = UsersManager.getResourceUsers(resources, filteredGroupNames); + + const expectedResult = [...membersByResource.get(someWorkspace)!, ...membersByResource.get(someDoc)!]; + assert.deepEqual(result, expectedResult, 'should discard the users from the first resource'); + }); + }); + + describe('getUsersWithRole()', function () { + function makeGroups(groupDefinition: {[k in NonGuestGroup['name']]?: User[] | undefined}){ + const entries = Object.entries(groupDefinition) as [NonGuestGroup['name'], User[] | undefined][]; + + return entries.map(([groupName, users], index) => { + const group = new Group() as NonGuestGroup; + group.id = index; + group.name = groupName; + if (users) { + group.memberUsers = users; + } + return group; + }); + } + + it('should retrieve no users if passed groups do not contain any', function () { + const groups = makeGroups({ + 'members': undefined + }); + + const result = UsersManager.getUsersWithRole(groups); + + assert.deepEqual(result, new Map([['members', undefined]] as any)); + }); + + it('should retrieve users of passed groups', function () { + const idxIt = makeUserIdIterator(); + const groupsUsersMap = { + 'editors': makeUsers(3, idxIt), + 'owners': makeUsers(4, idxIt), + 'members': makeUsers(5, idxIt), + 'viewers': [] + }; + const groups = makeGroups(groupsUsersMap); + + const result = UsersManager.getUsersWithRole(groups); + + assert.deepEqual(result, new Map(Object.entries(groupsUsersMap))); + }); + + it('should exclude users of given IDs', function () { + const groupUsersMap = { + 'editors': makeUsers(5), + }; + const excludedUsersId = [1, 2, 3, 4]; + const expectedUsers = [groupUsersMap.editors[0]]; + const groups = makeGroups(groupUsersMap); + + const result = UsersManager.getUsersWithRole(groups, excludedUsersId); + + assert.deepEqual(result, new Map([ ['editors', expectedUsers] ])); + }); + }); + }); + + describe('class method', function () { + const NON_EXISTING_USER_ID = 10001337; + let env: EnvironmentSnapshot; + let db: HomeDBManager; + let sandbox: SinonSandbox; + const uniqueLocalPart = new Set(); + + /** + * Works around lacks of type narrowing after asserting the value is defined. + * This is fixed in latest versions of @types/chai + * + * FIXME: once upgrading @types/chai to 4.3.17 or higher, remove this function which would not be usefull anymore + */ + function assertExists(value?: T, message?: string): asserts value is T { + assert.exists(value, message); + } + + function ensureUnique(localPart: string) { + if (uniqueLocalPart.has(localPart)) { + throw new Error('passed localPart is already used elsewhere'); + } + uniqueLocalPart.add(localPart); + return localPart; + } + + function createUniqueUser(uniqueEmailLocalPart: string, options?: GetUserOptions) { + ensureUnique(uniqueEmailLocalPart); + return getOrCreateUser(uniqueEmailLocalPart, options); + } + + async function getOrCreateUser(localPart: string, options?: GetUserOptions) { + return db.getUserByLogin(makeEmail(localPart), options); + } + + function makeEmail(localPart: string) { + return localPart + '@getgrist.com'; + } + + async function getPersonalOrg(user: User) { + return db.getOrg({userId: user.id}, user.personalOrg.id); + } + + async function withDataBase(dbName: string, cb: (db: HomeDBManager) => Promise) { + const database = path.join(tmpDir, dbName + '.db'); + const localDb = new HomeDBManager(); + await localDb.createNewConnection({ name: dbName, database }); + await updateDb(localDb.connection); + try { + await cb(localDb); + } finally { + await localDb.connection.destroy(); + // HACK: This is a weird case, we have established a different connection + // but the existing connection is also impacted. + // A found workaround consist in destroying and creating again the connection. + // + // TODO: Check whether using DataSource would help and avoid this hack. + await db.connection.destroy(); + await db.createNewConnection(); + dereferenceConnection(dbName); + } + } + + function disableLoggingLevel(method: T) { + return sandbox.stub(log, method); + } + + /** + * Make a user profile. + * @param localPart A unique local part of the email (also used for the other fields). + */ + function makeProfile(localPart: string): UserProfile { + ensureUnique(localPart); + return { + email: makeEmail(localPart), + name: `NewUser ${localPart}`, + connectId: `ConnectId-${localPart}`, + picture: `https://mypic.com/${localPart}.png` + }; + } + + + before(async function () { + if (process.env.TYPEORM_TYPE === "postgres") { + this.skip(); + } + env = new EnvironmentSnapshot(); + await prepareDatabase(tmpDir); + db = await getDatabase(); + }); + + after(async function () { + env?.restore(); + }); + + beforeEach(function () { + sandbox = Sinon.createSandbox(); + }); + + afterEach(function () { + sandbox.restore(); + }); + + describe('Special User Ids', function () { + const ANONYMOUS_USER_ID = 6; + const PREVIEWER_USER_ID = 7; + const EVERYONE_USER_ID = 8; + const SUPPORT_USER_ID = 5; + it('getAnonymousUserId() should retrieve anonymous user id', function () { + assert.strictEqual(db.getAnonymousUserId(), ANONYMOUS_USER_ID); + }); + + it('getPreviewerUserId() should retrieve previewer user id', function () { + assert.strictEqual(db.getPreviewerUserId(), PREVIEWER_USER_ID); + }); + + it("getEveryoneUserId() should retrieve 'everyone' user id", function () { + assert.strictEqual(db.getEveryoneUserId(), EVERYONE_USER_ID); + }); + + it("getSupportUserId() should retrieve 'support' user id", function () { + assert.strictEqual(db.getSupportUserId(), SUPPORT_USER_ID); + }); + + describe('Without special id initialization', function () { + it('should throw an error', async function () { + await withDataBase('without-special-ids', async function (localDb) { + assert.throws(() => localDb.getAnonymousUserId(), "'Anonymous' user not available"); + assert.throws(() => localDb.getPreviewerUserId(), "'Previewer' user not available"); + assert.throws(() => localDb.getEveryoneUserId(), "'Everyone' user not available"); + assert.throws(() => localDb.getSupportUserId(), "'Support' user not available"); + }); + }); + }); + }); + + describe('getUserByKey()', function () { + it('should return the user given their API Key', async function () { + const user = await db.getUserByKey('api_key_for_chimpy'); + + assert.strictEqual(user?.name, 'Chimpy', 'should retrieve Chimpy by their API key'); + assert.strictEqual(user?.logins?.[0].email, 'chimpy@getgrist.com'); + }); + + it('should return undefined if no user matches the API key', async function () { + const user = await db.getUserByKey('non-existing API key'); + + assert.strictEqual(user, undefined); + }); + }); + + describe('getUser()', async function () { + it('should retrieve a user by their ID', async function () { + const user = await db.getUser(db.getSupportUserId()); + assertExists(user, 'Should have returned a user'); + assert.strictEqual(user.name, 'Support'); + assert.strictEqual(user.loginEmail, SUPPORT_EMAIL); + assert.notExists(user.prefs, "should not have retrieved user's prefs"); + }); + + it('should retrieve a user along with their prefs with `includePrefs` set to true', async function () { + const expectedUser = await createUniqueUser('getuser-userwithprefs'); + const user = await db.getUser(expectedUser.id, {includePrefs: true}); + assertExists(user, "Should have retrieved the user"); + assertExists(user.loginEmail); + assert.strictEqual(user.loginEmail, expectedUser.loginEmail); + assert.isTrue(Array.isArray(user.prefs), "should not have retrieved user's prefs"); + assert.deepEqual(user.prefs, [{ + userId: expectedUser.id, + orgId: expectedUser.personalOrg.id, + prefs: { showGristTour: true } as any + }]); + }); + + it('should return undefined when the id is not found', async function () { + assert.isUndefined(await db.getUser(NON_EXISTING_USER_ID)); + }); + }); + + describe('getFullUser()', function () { + it('should return the support user', async function () { + const supportId = db.getSupportUserId(); + + const user = await db.getFullUser(supportId); + + const expectedResult: FullUser = { + isSupport: true, + email: SUPPORT_EMAIL, + id: supportId, + name: 'Support' + }; + assert.deepInclude(user, expectedResult); + assert.notOk(user.anonymous, 'anonymous property should be falsy'); + }); + + it('should return the anonymous user', async function () { + const anonId = db.getAnonymousUserId(); + + const user = await db.getFullUser(anonId); + + const expectedResult: FullUser = { + anonymous: true, + email: ANONYMOUS_USER_EMAIL, + id: anonId, + name: 'Anonymous', + }; + assert.deepInclude(user, expectedResult); + assert.notOk(user.isSupport, 'support property should be falsy'); + }); + + it('should reject when user is not found', async function () { + await assert.isRejected(db.getFullUser(NON_EXISTING_USER_ID), "unable to find user"); + }); + }); + + describe('makeFullUser()', function () { + const someUserDisplayEmail = 'SomeUser@getgrist.com'; + const normalizedSomeUserEmail = 'someuser@getgrist.com'; + const someUserLocale = 'en-US'; + const SOME_USER_ID = 42; + const prefWithOrg: Pref = { + prefs: {placeholder: 'pref-with-org'}, + orgId: 43, + user: new User(), + userId: SOME_USER_ID, + }; + const prefWithoutOrg: Pref = { + prefs: {placeholder: 'pref-without-org'}, + orgId: null, + user: new User(), + userId: SOME_USER_ID + }; + + + function makeSomeUser() { + return User.create({ + id: SOME_USER_ID, + ref: 'some ref', + name: 'some user', + picture: 'https://grist.com/mypic', + options: { + locale: someUserLocale + }, + logins: [ + Login.create({ + userId: SOME_USER_ID, + email: normalizedSomeUserEmail, + displayEmail: someUserDisplayEmail, + }), + ], + prefs: [ + prefWithOrg, + prefWithoutOrg + ] + }); + } + + it('creates a FullUser from a User entity', function () { + const input = makeSomeUser(); + + const fullUser = db.makeFullUser(input); + + assert.deepEqual(fullUser, { + id: SOME_USER_ID, + email: someUserDisplayEmail, + loginEmail: normalizedSomeUserEmail, + name: input.name, + picture: input.picture, + ref: input.ref, + locale: someUserLocale, + prefs: prefWithoutOrg.prefs + }); + }); + + it('sets `anonymous` property to true for anon@getgrist.com', function () { + const anon = db.getAnonymousUser(); + + const fullUser = db.makeFullUser(anon); + + assert.isTrue(fullUser.anonymous, "`anonymous` property should be set to true"); + assert.notOk(fullUser.isSupport, "`isSupport` should be falsy"); + }); + + it('sets `isSupport` property to true for support account', async function () { + const support = await db.getUser(db.getSupportUserId()); + + const fullUser = db.makeFullUser(support!); + + assert.isTrue(fullUser.isSupport, "`isSupport` property should be set to true"); + assert.notOk(fullUser.anonymous, "`anonymouse` should be falsy"); + }); + + it('should throw when no displayEmail exist for this user', function () { + const input = makeSomeUser(); + input.logins[0].displayEmail = ''; + + assert.throws(() => db.makeFullUser(input), "unable to find mandatory user email"); + + input.logins = []; + assert.throws(() => db.makeFullUser(input), "unable to find mandatory user email"); + }); + }); + + describe('ensureExternalUser()', function () { + let managerSaveSpy: SinonSpy; + let userSaveSpy: SinonSpy; + + beforeEach(function () { + managerSaveSpy = sandbox.spy(EntityManager.prototype, 'save'); + userSaveSpy = sandbox.spy(User.prototype, 'save'); + }); + + async function checkUserInfo(profile: UserProfile) { + const user = await db.getExistingUserByLogin(profile.email); + assertExists(user, "the new user should be in database"); + assert.deepInclude(user, { + isFirstTimeUser: false, + name: profile.name, + picture: profile.picture + }); + assert.exists(user.logins?.[0]); + assert.deepInclude(user.logins[0], { + email: profile.email.toLowerCase(), + displayEmail: profile.email + }); + return user; + } + + it('should not do anything if the user already exists and is up to date', async function () { + await db.ensureExternalUser({ + name: 'Chimpy', + email: 'chimpy@getgrist.com', + }); + + assert.isFalse(userSaveSpy.called, 'user.save() should not have been called'); + assert.isFalse(managerSaveSpy.called, 'manager.save() should not have been called'); + }); + + it('should save an unknown user', async function () { + const profile = makeProfile('ensureExternalUser-saves-an-unknown-user'); + await db.ensureExternalUser(profile); + assert.isTrue(userSaveSpy.called, 'user.save() should have been called'); + assert.isTrue(managerSaveSpy.called, 'manager.save() should have been called'); + + await checkUserInfo(profile); + }); + + it('should update a user if they already exist in database', async function () { + const oldProfile = makeProfile('ensureexternaluser-updates-an-existing-user_old'); + + await db.ensureExternalUser(oldProfile); + + let oldUser = await db.getExistingUserByLogin(oldProfile.email); + assertExists(oldUser); + + const newProfile = { + ...makeProfile('ensureexternaluser-updates-an-existing-user_new'), + connectId: oldProfile.connectId, + }; + + await db.ensureExternalUser(newProfile); + + oldUser = await db.getExistingUserByLogin(oldProfile.email); + assert.notExists(oldUser, 'we should not retrieve the user given their old email address'); + + await checkUserInfo(newProfile); + }); + + it('should normalize email address', async function() { + const profile = makeProfile('ENSUREEXTERNALUSER-NORMALIZES-email-address'); + + await db.ensureExternalUser(profile); + + const user = await checkUserInfo(profile); + assert.equal(user.logins[0].email, profile.email.toLowerCase(), 'the email should be lowercase'); + assert.equal(user.logins[0].displayEmail, profile.email, 'the display email should keep the original case'); + }); + }); + + describe('updateUser()', function () { + let emitSpy: SinonSpy; + + before(function () { + emitSpy = Sinon.spy(); + db.on('firstLogin', emitSpy); + }); + + after(function () { + db.off('firstLogin', emitSpy); + }); + + afterEach(function () { + emitSpy.resetHistory(); + }); + + function checkNoEventEmitted() { + assert.equal(emitSpy.callCount, 0, 'No event should have been emitted'); + } + + it('should reject when user is not found', async function () { + disableLoggingLevel('debug'); + + const promise = db.updateUser(NON_EXISTING_USER_ID, {name: 'foobar'}); + + await assert.isRejected(promise, 'unable to find user'); + checkNoEventEmitted(); + }); + + it('should update a user name', async function () { + const emailLocalPart = 'updateUser-should-update-user-name'; + const createdUser = await createUniqueUser(emailLocalPart); + assert.equal(createdUser.name, ''); + const userName = 'user name'; + + await db.updateUser(createdUser.id, {name: userName}); + + checkNoEventEmitted(); + const updatedUser = await getOrCreateUser(emailLocalPart); + assert.equal(updatedUser.name, userName); + }); + + it('should not emit any event when isFirstTimeUser value has not changed', async function () { + const localPart = 'updateuser-should-not-emit-when-isfirsttimeuser-not-changed'; + const createdUser = await createUniqueUser(localPart); + assert.equal(createdUser.isFirstTimeUser, true); + + await db.updateUser(createdUser.id, {isFirstTimeUser: true}); + + checkNoEventEmitted(); + }); + + it('should emit "firstLogin" event when isFirstTimeUser value has been toggled to false', async function () { + const localPart = 'updateuser-emits-firstlogin'; + const userName = 'user name'; + const newUser = await createUniqueUser(localPart); + assert.equal(newUser.isFirstTimeUser, true); + + await db.updateUser(newUser.id, {isFirstTimeUser: false, name: userName}); + assert.equal(emitSpy.callCount, 1, '"firstLogin" event should have been emitted'); + + const fullUserFromEvent = emitSpy.firstCall.args[0]; + assertExists(fullUserFromEvent, 'a FullUser object should be passed with the "firstLogin" event'); + assert.equal(fullUserFromEvent.name, userName); + assert.equal(fullUserFromEvent.email, makeEmail(localPart)); + + const updatedUser = await getOrCreateUser(localPart); + assert.equal(updatedUser.isFirstTimeUser, false, 'the user is not considered as being first time user anymore'); + }); + }); + + describe('updateUserOptions()', function () { + it('should reject when user is not found', async function () { + disableLoggingLevel('debug'); + + const promise = db.updateUserOptions(NON_EXISTING_USER_ID, {}); + + await assert.isRejected(promise, 'unable to find user'); + }); + + it('should update user options', async function () { + const localPart = 'updateuseroptions-updates-user-options'; + const createdUser = await createUniqueUser(localPart); + + assert.notExists(createdUser.options); + + const options: UserOptions = {locale: 'fr', authSubject: 'subject', isConsultant: true, allowGoogleLogin: true}; + await db.updateUserOptions(createdUser.id, options); + + const updatedUser = await getOrCreateUser(localPart); + assertExists(updatedUser.options); + assert.deepEqual(updatedUser.options, options); + }); + }); + + describe('getExistingUserByLogin()', function () { + it('should return an existing user', async function () { + const retrievedUser = await db.getExistingUserByLogin(PREVIEWER_EMAIL); + assertExists(retrievedUser); + + assert.equal(retrievedUser.id, db.getPreviewerUserId()); + assert.equal(retrievedUser.name, 'Preview'); + }); + + it('should normalize the passed user email', async function () { + const retrievedUser = await db.getExistingUserByLogin(PREVIEWER_EMAIL.toUpperCase()); + + assertExists(retrievedUser); + }); + + it('should return undefined when the user is not found', async function () { + const nonExistingEmail = 'i-dont-exist@getgrist.com'; + + const retrievedUser = await db.getExistingUserByLogin(nonExistingEmail); + + assert.isUndefined(retrievedUser); + }); + }); + + describe('getUserByLogin()', function () { + it('should create a user when none exist with the corresponding email', async function () { + const localPart = ensureUnique('getuserbylogin-creates-user-when-not-already-exists'); + const email = makeEmail(localPart); + sandbox.useFakeTimers(42_000); + assert.notExists(await db.getExistingUserByLogin(email)); + + const user = await db.getUserByLogin(makeEmail(localPart.toUpperCase())); + + assert.isTrue(user.isFirstTimeUser, 'should be marked as first time user'); + assert.equal(user.loginEmail, email); + assert.equal(user.logins[0].displayEmail, makeEmail(localPart.toUpperCase())); + assert.equal(user.name, ''); + // FIXME: why is user.lastConnectionAt actually a string and not a Date? + // FIXME: is it consistent that user.lastConnectionAt updated here and not firstLoginAt? + assert.equal(String(user.lastConnectionAt), '1970-01-01 00:00:42.000'); + }); + + it('should create a personnal organization for the new user', async function () { + const localPart = ensureUnique('getuserbylogin-creates-personnal-org'); + + const user = await db.getUserByLogin(makeEmail(localPart)); + + const org = await getPersonalOrg(user); + assertExists(org.data, 'should have retrieved personnal org data'); + assert.equal(org.data.name, 'Personal'); + }); + + it('should not create organizations for non-login emails', async function () { + const user = await db.getUserByLogin(EVERYONE_EMAIL); + assert.notExists(user.personalOrg); + }); + + it('should not update user information when no profile is passed', async function () { + const localPart = ensureUnique('getuserbylogin-does-not-update-without-profile'); + + const userFirstCall = await db.getUserByLogin(makeEmail(localPart)); + const userSecondCall = await db.getUserByLogin(makeEmail(localPart)); + + assert.deepEqual(userFirstCall, userSecondCall); + }); + + // FIXME: why using Sinon.useFakeTimers() makes user.lastConnectionAt a string instead of a Date + it.skip('should update lastConnectionAt only for different days', async function () { + const fakeTimer = sandbox.useFakeTimers(0); + const localPart = ensureUnique('getuserbylogin-updates-last_connection_at-for-different-days'); + let user = await db.getUserByLogin(makeEmail(localPart)); + const epochDateTime = '1970-01-01 00:00:00.000'; + assert.equal(String(user.lastConnectionAt), epochDateTime); + + await fakeTimer.tickAsync(42_000); + user = await db.getUserByLogin(makeEmail(localPart)); + assert.equal(String(user.lastConnectionAt), epochDateTime); + + await fakeTimer.tickAsync('1d'); + user = await db.getUserByLogin(makeEmail(localPart)); + assert.match(String(user.lastConnectionAt), /^1970-01-02/); + }); + + describe('when passing information to update (using `profile`)', function () { + it('should populate the firstTimeLogin and deduce the name from the email', async function () { + sandbox.useFakeTimers(42_000); + const localPart = ensureUnique('getuserbylogin-with-profile-populates-first_time_login-and-name'); + const user = await db.getUserByLogin(makeEmail(localPart), { + profile: {name: '', email: makeEmail(localPart)} + }); + assert.equal(user.name, localPart); + // FIXME: why using Sinon.useFakeTimers() makes user.firstLoginAt a string instead of a Date + assert.equal(String(user.firstLoginAt), '1970-01-01 00:00:42.000'); + }); + + it('should populate user with any passed information', async function () { + const localPart = ensureUnique('getuserbylogin-with-profile-populates-user-with-passed-info_OLD'); + await db.getUserByLogin(makeEmail(localPart)); + const originalNormalizedLoginEmail = makeEmail(localPart.toLowerCase()); + + const profile = makeProfile(makeEmail('getuserbylogin-with-profile-populates-user-with-passed-info_NEW')); + const userOptions: UserOptions = {authSubject: 'my-auth-subject'}; + + const updatedUser = await db.getUserByLogin(makeEmail(localPart), { profile, userOptions }); + assert.deepInclude(updatedUser, { + name: profile.name, + connectId: profile.connectId, + picture: profile.picture, + }); + assert.deepInclude(updatedUser.logins[0], { + displayEmail: profile.email, + email: originalNormalizedLoginEmail, + }); + assert.deepInclude(updatedUser.options, { + authSubject: userOptions.authSubject, + }); + }); + }); + }); + + describe('getUserByLoginWithRetry()', async function () { + async function ensureGetUserByLoginWithRetryWorks(localPart: string) { + const email = makeEmail(localPart); + const user = await db.getUserByLoginWithRetry(email); + assertExists(user); + assert.equal(user.loginEmail, email); + } + + function makeQueryFailedError() { + const error = new Error() as any; + error.name = 'QueryFailedError'; + error.detail = 'Key (email)=whatever@getgrist.com already exists'; + return error; + } + + it('should work just like getUserByLogin', async function () { + await ensureGetUserByLoginWithRetryWorks( ensureUnique('getuserbyloginwithretry-works-like-getuserbylogin')); + }); + + it('should make a second attempt on special error', async function () { + sandbox.stub(UsersManager.prototype, 'getUserByLogin') + .onFirstCall().throws(makeQueryFailedError()) + .callThrough(); + await ensureGetUserByLoginWithRetryWorks('getuserbyloginwithretry-makes-a-single-retry'); + }); + + it('should reject after 2 attempts', async function () { + const secondError = makeQueryFailedError(); + sandbox.stub(UsersManager.prototype, 'getUserByLogin') + .onFirstCall().throws(makeQueryFailedError()) + .onSecondCall().throws(secondError) + .callThrough(); + + const email = makeEmail(ensureUnique('getuserbyloginwithretry-rejects-after-2-attempts')); + const promise = db.getUserByLoginWithRetry(email); + await assert.isRejected(promise); + await promise.catch(err => assert.equal(err, secondError)); + }); + + it('should reject immediately if the error is not a QueryFailedError', async function () { + const errorMsg = 'my error'; + sandbox.stub(UsersManager.prototype, 'getUserByLogin') + .onFirstCall().rejects(new Error(errorMsg)) + .callThrough(); + + const email = makeEmail(ensureUnique('getuserbyloginwithretry-rejects-immediately-when-not-queryfailederror')); + const promise = db.getUserByLoginWithRetry(email); + await assert.isRejected(promise, errorMsg); + }); + }); + + describe('deleteUser()', function () { + function userHasPrefs(userId: number, manager: EntityManager) { + return manager.exists(Pref, { where: { userId: userId }}); + } + + function userHasGroupUsers(userId: number, manager: EntityManager) { + return manager.exists('group_users', { where: {user_id: userId} }); + } + + async function assertUserStillExistsInDb(userId: number) { + assert.exists(await db.getUser(userId)); + } + + it('should refuse to delete the account of someone else', async function () { + const userToDelete = await createUniqueUser('deleteuser-refuses-for-someone-else'); + + const promise = db.deleteUser({userId: 2}, userToDelete.id); + + await assert.isRejected(promise, 'not permitted to delete this user'); + await assertUserStillExistsInDb(userToDelete.id); + }); + + it('should refuse to delete a non existing account', async function () { + disableLoggingLevel('debug'); + + const promise = db.deleteUser({userId: NON_EXISTING_USER_ID}, NON_EXISTING_USER_ID); + + await assert.isRejected(promise, 'user not found'); + }); + + it('should refuse to delete the account if the passed name is not matching', async function () { + disableLoggingLevel('debug'); + const localPart = 'deleteuser-refuses-if-name-not-matching'; + + const userToDelete = await createUniqueUser(localPart, { + profile: { + name: 'someone to delete', + email: makeEmail(localPart), + } + }); + + const promise = db.deleteUser({userId: userToDelete.id}, userToDelete.id, 'wrong name'); + + await assert.isRejected(promise); + await promise.catch(e => assert.match(e.message, /user name did not match/)); + await assertUserStillExistsInDb(userToDelete.id); + }); + + it('should remove the user and cleanup their info and personal organization', async function () { + const localPart = 'deleteuser-removes-user-and-cleanups-info'; + const userToDelete = await createUniqueUser(localPart, { + profile: { + name: 'someone to delete', + email: makeEmail(localPart), + } + }); + + assertExists(await getPersonalOrg(userToDelete)); + + await db.connection.transaction(async (manager) => { + assert.isTrue(await userHasGroupUsers(userToDelete.id, manager)); + assert.isTrue(await userHasPrefs(userToDelete.id, manager)); + }); + + await db.deleteUser({userId: userToDelete.id}, userToDelete.id); + + assert.notExists(await db.getUser(userToDelete.id)); + assert.deepEqual(await getPersonalOrg(userToDelete), { errMessage: 'organization not found', status: 404 }); + + await db.connection.transaction(async (manager) => { + assert.isFalse(await userHasGroupUsers(userToDelete.id, manager)); + assert.isFalse(await userHasPrefs(userToDelete.id, manager)); + }); + }); + + it("should remove the user when passed name corresponds to the user's name", async function () { + const userName = 'someone to delete'; + const localPart = 'deleteuser-removes-user-when-name-matches'; + const userToDelete = await createUniqueUser(localPart, { + profile: { + name: userName, + email: makeEmail(localPart), + } + }); + + const promise = db.deleteUser({userId: userToDelete.id}, userToDelete.id, userName); + + await assert.isFulfilled(promise); + }); + }); + + describe('initializeSpecialIds()', function () { + it('should initialize special ids', async function () { + return withDataBase('test-special-ids', async (localDb) => { + const specialAccounts = [ + {name: "Support", email: SUPPORT_EMAIL}, + {name: "Anonymous", email: ANONYMOUS_USER_EMAIL}, + {name: "Preview", email: PREVIEWER_EMAIL}, + {name: "Everyone", email: EVERYONE_EMAIL} + ]; + for (const {email} of specialAccounts) { + assert.notExists(await localDb.getExistingUserByLogin(email)); + } + + await localDb.initializeSpecialIds(); + + for (const {name, email} of specialAccounts) { + const res = await localDb.getExistingUserByLogin(email); + assertExists(res); + assert.equal(res.name, name); + } + }); + }); + }); + + describe('completeProfiles()', function () { + it('should return an empty array if no profiles are provided', async function () { + const res = await db.completeProfiles([]); + assert.deepEqual(res, []); + }); + + it("should complete a single user profile with looking by normalized address", async function () { + const localPart = ensureUnique('completeprofiles-with-single-profile'); + const email = makeEmail(localPart); + const emailUpperCase = email.toUpperCase(); + const profile = { + name: 'completeprofiles-with-single-profile-username', + email: makeEmail(localPart), + picture: 'https://mypic.com/me.png', + }; + const someLocale = 'fr-FR'; + const userCreated = await getOrCreateUser(localPart, { profile }); + await db.updateUserOptions(userCreated.id, {locale: someLocale}); + + const res = await db.completeProfiles([{name: 'whatever', email: emailUpperCase}]); + + assert.deepEqual(res, [{ + ...profile, + id: userCreated.id, + locale: someLocale, + anonymous: false + }]); + }); + + it('should complete several user profiles', async function () { + const localPartPrefix = ensureUnique('completeprofiles-with-several-profiles'); + const seq = Array(10).fill(null).map((_, i) => i+1); + const localParts = seq.map(i => `${localPartPrefix}_${i}`); + const usersCreated = await Promise.all( + localParts.map(localPart => getOrCreateUser(localPart)) + ); + + const res = await db.completeProfiles( + localParts.map( + localPart => ({name: 'whatever', email: makeEmail(localPart)}) + ) + ); + assert.lengthOf(res, localParts.length); + for (const [index, localPart] of localParts.entries()) { + assert.deepInclude(res[index], { + id: usersCreated[index].id, + email: makeEmail(localPart), + }); + } + }); + }); + }); +}); diff --git a/test/gen-server/lib/removedAt.ts b/test/gen-server/lib/removedAt.ts index 2e990707..219a9b35 100644 --- a/test/gen-server/lib/removedAt.ts +++ b/test/gen-server/lib/removedAt.ts @@ -258,7 +258,7 @@ describe('removedAt', function() { 'test3@getgrist.com': 'editors', } }); - const userRef = (email: string) => home.dbManager.getUserByLogin(email).then((user) => user!.ref); + const userRef = (email: string) => home.dbManager.getUserByLogin(email).then((user) => user.ref); const idTest1 = (await home.dbManager.getUserByLogin("test1@getgrist.com"))!.id; const idTest2 = (await home.dbManager.getUserByLogin("test2@getgrist.com"))!.id; const idTest3 = (await home.dbManager.getUserByLogin("test3@getgrist.com"))!.id; diff --git a/test/gen-server/seed.ts b/test/gen-server/seed.ts index d935211e..0e230660 100644 --- a/test/gen-server/seed.ts +++ b/test/gen-server/seed.ts @@ -42,7 +42,9 @@ import {User} from "app/gen-server/entity/User"; import {Workspace} from "app/gen-server/entity/Workspace"; import {EXAMPLE_WORKSPACE_NAME} from 'app/gen-server/lib/homedb/HomeDBManager'; import {Permissions} from 'app/gen-server/lib/Permissions'; -import {getOrCreateConnection, runMigrations, undoLastMigration, updateDb} from 'app/server/lib/dbUtils'; +import { + getConnectionName, getOrCreateConnection, runMigrations, undoLastMigration, updateDb +} from 'app/server/lib/dbUtils'; import {FlexServer} from 'app/server/lib/FlexServer'; import * as fse from 'fs-extra'; @@ -527,16 +529,27 @@ class Seed { // When running mocha on several test files at once, we need to reset our database connection // if it exists. This is a little ugly since it is stored globally. export async function removeConnection() { - if (getConnectionManager().connections.length > 0) { - if (getConnectionManager().connections.length > 1) { + const connections = getConnectionManager().connections; + if (connections.length > 0) { + if (connections.length > 1) { throw new Error("unexpected number of connections"); } - await getConnectionManager().connections[0].close(); - // There is still no official way to delete connections that I've found. - (getConnectionManager() as any).connectionMap = new Map(); + await connections[0].destroy(); + dereferenceConnection(getConnectionName()); } } +export function dereferenceConnection(name: string) { + // There seem to be no official way to delete connections. + // Also we should probably get rid of the use of connectionManager, which is deprecated + const connectionMgr = getConnectionManager(); + const connectionMap = (connectionMgr as any).connectionMap as Map; + if (!connectionMap.has(name)) { + throw new Error('connection with this name not found: ' + name); + } + connectionMap.delete(name); +} + export async function createInitialDb(connection?: Connection, migrateAndSeedData: boolean = true) { // In jenkins tests, we may want to reset the database to a clean // state. If so, TEST_CLEAN_DATABASE will have been set. How to diff --git a/test/gen-server/testUtils.ts b/test/gen-server/testUtils.ts index 3f7707ab..a942773a 100644 --- a/test/gen-server/testUtils.ts +++ b/test/gen-server/testUtils.ts @@ -46,7 +46,6 @@ export async function createUser(dbManager: HomeDBManager, name: string): Promis const username = name.toLowerCase(); const email = `${username}@getgrist.com`; const user = await dbManager.getUserByLogin(email, {profile: {email, name}}); - if (!user) { throw new Error('failed to create user'); } user.apiKey = `api_key_for_${username}`; await user.save(); const userHome = (await dbManager.getOrg({userId: user.id}, null)).data; diff --git a/test/nbrowser/homeUtil.ts b/test/nbrowser/homeUtil.ts index 3aa2ff37..e7b770a3 100644 --- a/test/nbrowser/homeUtil.ts +++ b/test/nbrowser/homeUtil.ts @@ -420,7 +420,7 @@ export class HomeUtil { if (this.server.isExternalServer()) { throw new Error('not supported'); } const dbManager = await this.server.getDatabase(); const user = await dbManager.getUserByLogin(email); - if (user) { await dbManager.deleteUser({userId: user.id}, user.id, user.name); } + await dbManager.deleteUser({userId: user.id}, user.id, user.name); } // Set whether this is the user's first time logging in. Requires access to the database. @@ -428,10 +428,8 @@ export class HomeUtil { if (this.server.isExternalServer()) { throw new Error('not supported'); } const dbManager = await this.server.getDatabase(); const user = await dbManager.getUserByLogin(email); - if (user) { - user.isFirstTimeUser = isFirstLogin; - await user.save(); - } + user.isFirstTimeUser = isFirstLogin; + await user.save(); } private async _initShowGristTour(email: string, showGristTour: boolean) { @@ -463,7 +461,6 @@ export class HomeUtil { const dbManager = await this.server.getDatabase(); const user = await dbManager.getUserByLogin(email); - if (!user) { return; } if (user.personalOrg) { const org = await dbManager.getOrg({userId: user.id}, user.personalOrg.id); diff --git a/test/server/fixSiteProducts.ts b/test/server/fixSiteProducts.ts index b42d5a01..353b4ac8 100644 --- a/test/server/fixSiteProducts.ts +++ b/test/server/fixSiteProducts.ts @@ -122,7 +122,7 @@ describe('fixSiteProducts', function() { assert.equal(getDefaultProductNames().teamInitial, 'stub'); const db = server.dbManager; - const user = await db.getUserByLogin(email, {profile}) as any; + const user = await db.getUserByLogin(email, {profile}); const orgId = db.unwrapQueryResult(await db.addOrg(user, { name: 'sanity-check-org', domain: 'sanity-check-org', diff --git a/test/server/lib/GranularAccess.ts b/test/server/lib/GranularAccess.ts index 87225d2d..c722de30 100644 --- a/test/server/lib/GranularAccess.ts +++ b/test/server/lib/GranularAccess.ts @@ -3279,7 +3279,7 @@ describe('GranularAccess', function() { cliOwner.flush(); let perm: PermissionDataWithExtraUsers = (await cliOwner.send("getUsersForViewAs", 0)).data; const getId = (name: string) => home.dbManager.testGetId(name) as Promise; - const getRef = (email: string) => home.dbManager.getUserByLogin(email).then(user => user!.ref); + const getRef = (email: string) => home.dbManager.getUserByLogin(email).then(user => user.ref); assert.deepEqual(perm.users, [ { id: await getId('Chimpy'), email: 'chimpy@getgrist.com', name: 'Chimpy', ref: await getRef('chimpy@getgrist.com'), diff --git a/test/server/lib/helpers/PrepareDatabase.ts b/test/server/lib/helpers/PrepareDatabase.ts index ab643a60..eff2e0bf 100644 --- a/test/server/lib/helpers/PrepareDatabase.ts +++ b/test/server/lib/helpers/PrepareDatabase.ts @@ -2,13 +2,13 @@ import path from "path"; import * as testUtils from "test/server/testUtils"; import {execFileSync} from "child_process"; -export async function prepareDatabase(tempDirectory: string) { +export async function prepareDatabase(tempDirectory: string, filename: string = 'landing.db') { // Let's create a sqlite db that we can share with servers that run in other processes, hence // not an in-memory db. Running seed.ts directly might not take in account the most recent value // for TYPEORM_DATABASE, because ormconfig.js may already have been loaded with a different // configuration (in-memory for instance). Spawning a process is one way to make sure that the // latest value prevail. - process.env.TYPEORM_DATABASE = path.join(tempDirectory, 'landing.db'); + process.env.TYPEORM_DATABASE = path.join(tempDirectory, filename); const seed = await testUtils.getBuildFile('test/gen-server/seed.js'); execFileSync('node', [seed, 'init'], { env: process.env,