diff --git a/app/client/aclui/AccessRules.ts b/app/client/aclui/AccessRules.ts index 9ce68b81..5cb9cf79 100644 --- a/app/client/aclui/AccessRules.ts +++ b/app/client/aclui/AccessRules.ts @@ -146,6 +146,7 @@ export class AccessRules extends Disposable { {ruleIndex: -1, value: 'user.Origin'}, {ruleIndex: -1, value: 'user.SessionID'}, {ruleIndex: -1, value: 'user.IsLoggedIn'}, + {ruleIndex: -1, value: 'user.UserRef'}, ]; for (const [i, rule] of rules.entries()) { const tableId = use(rule.tableId); diff --git a/app/common/LoginSessionAPI.ts b/app/common/LoginSessionAPI.ts index 84e1412e..805f0c1e 100644 --- a/app/common/LoginSessionAPI.ts +++ b/app/common/LoginSessionAPI.ts @@ -8,10 +8,11 @@ export interface UserProfile { loginMethod?: 'Google'|'Email + Password'|'External'; } -// User profile including user id. All information in it should +// User profile including user id and user ref. All information in it should // have been validated against database. export interface FullUser extends UserProfile { id: number; + ref?: string|null; // Not filled for anonymous users. allowGoogleLogin?: boolean; // when present, specifies whether logging in via Google is possible. } diff --git a/app/gen-server/entity/User.ts b/app/gen-server/entity/User.ts index c34612ac..59a55d74 100644 --- a/app/gen-server/entity/User.ts +++ b/app/gen-server/entity/User.ts @@ -1,6 +1,7 @@ import {UserOptions} from 'app/common/UserAPI'; import {nativeValues} from 'app/gen-server/lib/values'; -import {BaseEntity, Column, Entity, JoinTable, ManyToMany, OneToMany, OneToOne, +import {makeId} from 'app/server/lib/idUtils'; +import {BaseEntity, BeforeInsert, Column, Entity, JoinTable, ManyToMany, OneToMany, OneToOne, PrimaryGeneratedColumn} from "typeorm"; import {Group} from "./Group"; @@ -50,6 +51,19 @@ export class User extends BaseEntity { @Column({name: 'connect_id', type: String, nullable: true}) public connectId: string | null; + /** + * Unique reference for this user. Primarily used as an ownership key in a cell metadata (comments). + */ + @Column({name: 'ref', type: String, nullable: false}) + public ref: string; + + @BeforeInsert() + public async beforeInsert() { + if (!this.ref) { + this.ref = makeId(); + } + } + /** * Get user's email. Returns undefined if logins has not been joined, or no login * is available diff --git a/app/gen-server/lib/HomeDBManager.ts b/app/gen-server/lib/HomeDBManager.ts index b97cc422..93f3649c 100644 --- a/app/gen-server/lib/HomeDBManager.ts +++ b/app/gen-server/lib/HomeDBManager.ts @@ -419,6 +419,15 @@ export class HomeDBManager extends EventEmitter { throw new Error(`Cannot testGetId(${name})`); } + /** + * For tests only. Get user's unique reference by name. + */ + public async testGetRef(name: string): Promise { + const user = await User.findOne({where: {name}}); + if (user) { return user.ref; } + throw new Error(`Cannot testGetRef(${name})`); + } + /** * Clear all user preferences associated with the given email addresses. * For use in tests. @@ -2537,6 +2546,7 @@ export class HomeDBManager extends EventEmitter { const login = new Login(); login.displayEmail = login.email = ANONYMOUS_USER_EMAIL; user.logins = [login]; + user.ref = ''; return user; } @@ -3917,6 +3927,9 @@ export class HomeDBManager extends EventEmitter { cond = cond.orWhere(`gu3.user_id = ${users}`); // Support the special "everyone" user. const everyoneId = this._specialUserIds[EVERYONE_EMAIL]; + if (everyoneId === undefined) { + throw new Error("Special user id for EVERYONE_EMAIL not found"); + } cond = cond.orWhere(`gu0.user_id = ${everyoneId}`); cond = cond.orWhere(`gu1.user_id = ${everyoneId}`); cond = cond.orWhere(`gu2.user_id = ${everyoneId}`); @@ -3925,6 +3938,9 @@ export class HomeDBManager extends EventEmitter { // Support also the special anonymous user. Currently, by convention, sharing a // resource with anonymous should make it listable. const anonId = this._specialUserIds[ANONYMOUS_USER_EMAIL]; + if (anonId === undefined) { + throw new Error("Special user id for ANONYMOUS_USER_EMAIL not found"); + } cond = cond.orWhere(`gu0.user_id = ${anonId}`); cond = cond.orWhere(`gu1.user_id = ${anonId}`); cond = cond.orWhere(`gu2.user_id = ${anonId}`); diff --git a/app/gen-server/migration/1663851423064-UserUUID.ts b/app/gen-server/migration/1663851423064-UserUUID.ts new file mode 100644 index 00000000..ba0e71b1 --- /dev/null +++ b/app/gen-server/migration/1663851423064-UserUUID.ts @@ -0,0 +1,33 @@ +import {User} from 'app/gen-server/entity/User'; +import {makeId} from 'app/server/lib/idUtils'; +import {MigrationInterface, QueryRunner, TableColumn} from "typeorm"; + +export class UserUUID1663851423064 implements MigrationInterface { + public async up(queryRunner: QueryRunner): Promise { + // Add ref column, for now make it nullable and not unique, we + // first need to put a value in it for all existing users. + await queryRunner.addColumn("users", new TableColumn({ + name: "ref", + type: 'varchar', + isNullable: true, + isUnique: false, + })); + + // Updating so many rows in a multiple queries is not ideal. We will send updates in chunks. + // 300 seems to be a good number, for 24k rows we have 80 queries. + const userList = await queryRunner.manager.createQueryBuilder() + .select("users") + .from(User, "users") + .getMany(); + userList.forEach(u => u.ref = makeId()); + await queryRunner.manager.save(userList, { chunk: 300 }); + + // We are not making this column unique yet, because it can fail + // if there are some old workers still running, and any new user + // is created. We will make it unique in a later migration. + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.dropColumn("users", "ref"); + } +} diff --git a/app/gen-server/migration/1664528376930-UserRefUnique.txt b/app/gen-server/migration/1664528376930-UserRefUnique.txt new file mode 100644 index 00000000..27536042 --- /dev/null +++ b/app/gen-server/migration/1664528376930-UserRefUnique.txt @@ -0,0 +1,37 @@ +import {User} from 'app/gen-server/entity/User'; +import {makeId} from 'app/server/lib/idUtils'; +import {MigrationInterface, QueryRunner} from "typeorm"; + +export class UserRefUnique1664528376930 implements MigrationInterface { + public async up(queryRunner: QueryRunner): Promise { + // This is second part of migration 1663851423064-UserUUID, that makes + // the ref column unique. + + // Update users that don't have unique ref set. + const userList = await queryRunner.manager.createQueryBuilder() + .select("users") + .from(User, "users") + .where("ref is null") + .getMany(); + userList.forEach(u => u.ref = makeId()); + await queryRunner.manager.save(userList, {chunk: 300}); + + // Mark column as unique and non-nullable. + const users = (await queryRunner.getTable('users'))!; + const oldRef = users.findColumnByName('ref')!; + const newRef = oldRef.clone(); + newRef.isUnique = true; + newRef.isNullable = false; + await queryRunner.changeColumn('users', oldRef, newRef); + } + + public async down(queryRunner: QueryRunner): Promise { + // Mark column as non unique and nullable. + const users = (await queryRunner.getTable('users'))!; + const oldRef = users.findColumnByName('ref')!; + const newRef = oldRef.clone(); + newRef.isUnique = false; + newRef.isNullable = true; + await queryRunner.changeColumn('users', oldRef, newRef); + } +} diff --git a/app/server/lib/Authorizer.ts b/app/server/lib/Authorizer.ts index 1f544302..4f6e0d48 100644 --- a/app/server/lib/Authorizer.ts +++ b/app/server/lib/Authorizer.ts @@ -528,33 +528,46 @@ export interface Authorizer { getCachedAuth(): DocAuthResult; } +export interface DocAuthorizerOptions { + dbManager: HomeDBManager; + key: DocAuthKey; + openMode: OpenDocMode; + linkParameters: Record; + userRef?: string|null; + docAuth?: DocAuthResult; + profile?: UserProfile; +} + /** * * Handle authorization for a single document and user. * */ export class DocAuthorizer implements Authorizer { + public readonly openMode: OpenDocMode; + public readonly linkParameters: Record; constructor( - private _dbManager: HomeDBManager, - private _key: DocAuthKey, - public readonly openMode: OpenDocMode, - public readonly linkParameters: Record, - private _docAuth?: DocAuthResult, - private _profile?: UserProfile + private _options: DocAuthorizerOptions ) { + this.openMode = _options.openMode; + this.linkParameters = _options.linkParameters; } public getUserId(): number { - return this._key.userId; + return this._options.key.userId; } public getUser(): FullUser|null { - return this._profile ? {id: this.getUserId(), ...this._profile} : null; + return this._options.profile ? { + id: this.getUserId(), + ref: this._options.userRef, + ...this._options.profile + } : null; } public getDocId(): string { // We've been careful to require urlId === docId, see DocManager. - return this._key.urlId; + return this._options.key.urlId; } public getLinkParameters(): Record { @@ -562,18 +575,18 @@ export class DocAuthorizer implements Authorizer { } public async getDoc(): Promise { - return this._dbManager.getDoc(this._key); + return this._options.dbManager.getDoc(this._options.key); } public async assertAccess(role: 'viewers'|'editors'|'owners'): Promise { - const docAuth = await this._dbManager.getDocAuthCached(this._key); - this._docAuth = docAuth; + const docAuth = await this._options.dbManager.getDocAuthCached(this._options.key); + this._options.docAuth = docAuth; assertAccess(role, docAuth, {openMode: this.openMode}); } public getCachedAuth(): DocAuthResult { - if (!this._docAuth) { throw Error('no cached authentication'); } - return this._docAuth; + if (!this._options.docAuth) { throw Error('no cached authentication'); } + return this._options.docAuth; } } diff --git a/app/server/lib/Client.ts b/app/server/lib/Client.ts index b3b87592..fc780daa 100644 --- a/app/server/lib/Client.ts +++ b/app/server/lib/Client.ts @@ -87,6 +87,7 @@ export class Client { private _profile: UserProfile|null = null; private _userId: number|null = null; private _userName: string|null = null; + private _userRef: string|null = null; private _firstLoginAt: Date|null = null; private _isAnonymous: boolean = false; private _nextSeqId: number = 0; // Next sequence-ID for messages sent to the client @@ -388,25 +389,26 @@ export class Client { return this._userId; } + public getCachedUserRef(): string|null { + return this._userRef; + } + // Returns the userId for profile.email, or null when profile is not set; with caching. public async getUserId(dbManager: HomeDBManager): Promise { if (!this._userId) { - if (this._profile) { - const user = await this._fetchUser(dbManager); - this._userId = (user && user.id) || null; - this._userName = (user && user.name) || null; - this._isAnonymous = this._userId && dbManager.getAnonymousUserId() === this._userId || false; - this._firstLoginAt = (user && user.firstLoginAt) || null; - } else { - this._userId = dbManager.getAnonymousUserId(); - this._userName = 'Anonymous'; - this._isAnonymous = true; - this._firstLoginAt = null; - } + await this._refreshUser(dbManager); } return this._userId; } + // Returns the userRef for profile.email, or null when profile is not set; with caching. + public async getUserRef(dbManager: HomeDBManager): Promise { + if (!this._userRef) { + await this._refreshUser(dbManager); + } + return this._userRef; + } + // Returns the userId for profile.email, or throws 403 error when profile is not set. public async requireUserId(dbManager: HomeDBManager): Promise { const userId = await this.getUserId(dbManager); @@ -431,6 +433,22 @@ export class Client { return meta; } + private async _refreshUser(dbManager: HomeDBManager) { + if (this._profile) { + const user = await this._fetchUser(dbManager); + this._userId = (user && user.id) || null; + this._userName = (user && user.name) || null; + this._isAnonymous = this._userId && dbManager.getAnonymousUserId() === this._userId || false; + this._firstLoginAt = (user && user.firstLoginAt) || null; + this._userRef = user?.ref ?? null; + } else { + this._userId = dbManager.getAnonymousUserId(); + this._userName = 'Anonymous'; + this._isAnonymous = true; + this._firstLoginAt = null; + } + } + /** * Processes a request from a client. All requests from a client get a response, at least to * indicate success or failure. diff --git a/app/server/lib/DocManager.ts b/app/server/lib/DocManager.ts index f6fe3da6..545dc64d 100644 --- a/app/server/lib/DocManager.ts +++ b/app/server/lib/DocManager.ts @@ -261,7 +261,7 @@ export class DocManager extends EventEmitter { * `doc` - the object with metadata tables. */ public async openDoc(client: Client, docId: string, - mode: OpenDocMode = 'default', + openMode: OpenDocMode = 'default', linkParameters: Record = {}): Promise { let auth: Authorizer; const dbManager = this._homeDbManager; @@ -271,6 +271,7 @@ export class DocManager extends EventEmitter { const org = client.getOrg(); if (!org) { throw new Error('Documents can only be opened in the context of a specific organization'); } const userId = await client.getUserId(dbManager) || dbManager.getAnonymousUserId(); + const userRef = await client.getUserRef(dbManager); // We use docId in the key, and disallow urlId, so we can be sure that we are looking at the // right doc when we re-query the DB over the life of the websocket. @@ -284,7 +285,15 @@ export class DocManager extends EventEmitter { // than a docId. throw new Error(`openDoc expected docId ${docAuth.docId} not urlId ${docId}`); } - auth = new DocAuthorizer(dbManager, key, mode, linkParameters, docAuth, client.getProfile() || undefined); + auth = new DocAuthorizer({ + dbManager, + key, + openMode, + linkParameters, + userRef, + docAuth, + profile: client.getProfile() || undefined + }); } else { log.debug(`DocManager.openDoc not using authorization for ${docId} because GRIST_SINGLE_USER`); auth = new DummyAuthorizer('owners', docId); @@ -302,7 +311,7 @@ export class DocManager extends EventEmitter { // If opening in (pre-)fork mode, check if it is appropriate to treat the user as // an owner for granular access purposes. - if (mode === 'fork') { + if (openMode === 'fork') { if (await activeDoc.canForkAsOwner(docSession)) { // Mark the session specially and flush any cached access // information. It is easier to make this a property of the diff --git a/app/server/lib/DocSession.ts b/app/server/lib/DocSession.ts index 932c20ab..b9961ecb 100644 --- a/app/server/lib/DocSession.ts +++ b/app/server/lib/DocSession.ts @@ -122,15 +122,17 @@ export function getDocSessionUser(docSession: OptDocSession): FullUser|null { const user = getUser(docSession.req); const email = user.loginEmail; if (email) { - return {id: user.id, name: user.name, email}; + return {id: user.id, name: user.name, email, ref: user.ref}; } } if (docSession.client) { const id = docSession.client.getCachedUserId(); + const ref = docSession.client.getCachedUserRef(); const profile = docSession.client.getProfile(); if (id && profile) { return { id, + ref, ...profile }; } diff --git a/app/server/lib/GranularAccess.ts b/app/server/lib/GranularAccess.ts index 90c74b4c..fd1cfdb7 100644 --- a/app/server/lib/GranularAccess.ts +++ b/app/server/lib/GranularAccess.ts @@ -1608,6 +1608,7 @@ export class GranularAccess implements GranularAccessForBundle { user.Origin = docSession.req?.get('origin') || null; user.SessionID = isAnonymous ? `a${getDocSessionAltSessionId(docSession)}` : `u${user.UserID}`; user.IsLoggedIn = !isAnonymous; + user.UserRef = fullUser?.ref || null; // Empty string should be treated as null. if (this._ruler.ruleCollection.ruleError && !this._recoveryMode) { // It is important to signal that the doc is in an unexpected state, @@ -2600,6 +2601,7 @@ export class User implements UserInfo { public Origin: string | null = null; public LinkKey: Record = {}; public Email: string | null = null; + public UserRef: string | null = null; [attribute: string]: any; constructor(_info: Record = {}) { diff --git a/sandbox/grist/test_completion.py b/sandbox/grist/test_completion.py index 682978d8..f72d0784 100644 --- a/sandbox/grist/test_completion.py +++ b/sandbox/grist/test_completion.py @@ -10,6 +10,7 @@ class TestCompletion(test_engine.EngineTestCase): user = { 'Name': 'Foo', 'UserID': 1, + 'UserRef': '1', 'StudentInfo': ['Students', 1], 'LinkKey': {}, 'Origin': None, @@ -103,6 +104,7 @@ class TestCompletion(test_engine.EngineTestCase): ('user.SessionID', "'u1'"), ('user.StudentInfo', 'Students[1]'), ('user.UserID', '1'), + ('user.UserRef', "'1'"), ] ) # Should follow user attribute references and autocomplete those types. @@ -130,6 +132,7 @@ class TestCompletion(test_engine.EngineTestCase): 'Email': 'baro@example.com', 'LinkKey': {}, 'UserID': 2, + 'UserRef': '2', 'Access': 'owners', 'SessionID': 'u2', 'IsLoggedIn': True @@ -145,6 +148,7 @@ class TestCompletion(test_engine.EngineTestCase): ('user.Origin', 'None'), ('user.SessionID', "'u2'"), ('user.UserID', '2'), + ('user.UserRef', "'2'"), ] ) self.assertEqual( diff --git a/sandbox/grist/test_renames.py b/sandbox/grist/test_renames.py index e07dbf90..51c1f990 100644 --- a/sandbox/grist/test_renames.py +++ b/sandbox/grist/test_renames.py @@ -323,6 +323,7 @@ class TestRenames(test_engine.EngineTestCase): user = { 'Name': 'Foo', 'UserID': 1, + 'UserRef': '1', 'LinkKey': {}, 'Origin': None, 'Email': 'foo@example.com', diff --git a/sandbox/grist/test_trigger_formulas.py b/sandbox/grist/test_trigger_formulas.py index ae52a2fb..b7c3dd2f 100644 --- a/sandbox/grist/test_trigger_formulas.py +++ b/sandbox/grist/test_trigger_formulas.py @@ -565,6 +565,7 @@ class TestTriggerFormulas(test_engine.EngineTestCase): user1 = { 'Name': 'Foo Bar', 'UserID': 1, + 'UserRef': '1', 'StudentInfo': ['Students', 1], 'LinkKey': {}, 'Origin': None, @@ -576,6 +577,7 @@ class TestTriggerFormulas(test_engine.EngineTestCase): user2 = { 'Name': 'Baz Qux', 'UserID': 2, + 'UserRef': '2', 'StudentInfo': ['Students', 1], 'LinkKey': {}, 'Origin': None, diff --git a/sandbox/grist/test_user.py b/sandbox/grist/test_user.py index ebaf10c5..6dbc20fb 100644 --- a/sandbox/grist/test_user.py +++ b/sandbox/grist/test_user.py @@ -14,6 +14,7 @@ class TestUser(test_engine.EngineTestCase): 'Name': 'Foo Bar', 'Email': 'email@example.com', 'UserID': 1, + 'UserRef': '1', 'LinkKey': { 'Param1': 'Param1Value', 'Param2': 'Param2Value' @@ -42,6 +43,7 @@ class TestUser(test_engine.EngineTestCase): 'Name': None, 'Email': 'email@getgrist.com', 'UserID': 1, + 'UserRef': '1', 'LinkKey': { 'Param1': 'Param1Value', 'Param2': 'Param2Value' diff --git a/sandbox/grist/user.py b/sandbox/grist/user.py index f31ca1cc..f6d3f37d 100644 --- a/sandbox/grist/user.py +++ b/sandbox/grist/user.py @@ -13,6 +13,7 @@ the following fields: - Access: string or None - UserID: integer or None + - UserRef: string or None - Email: string or None - Name: string or None - Origin: string or None @@ -43,7 +44,7 @@ class User(object): """ def __init__(self, data, tables, is_sample=False): for attr in ('Access', 'UserID', 'Email', 'Name', 'Origin', 'SessionID', - 'IsLoggedIn'): + 'IsLoggedIn', 'UserRef'): setattr(self, attr, data[attr]) self.LinkKey = LinkKey(data['LinkKey'])