From accd6400784652fa09c2ad53fe0c54ab147540b8 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Tue, 22 Feb 2022 10:42:06 -0500 Subject: [PATCH] (core) add a `user.SessionID` value for trigger formulas and granular access rules Summary: This makes a `user.SessionID` value available in information about the user, for use with trigger formulas and granular access rules. The ID should be constant within a browser session for anonymous user. For logged in users it simply reflects their user id. This ID makes it possible to write access rules and trigger formulas that allow different anonymous users to create, view, and edit their own records in a document. For example, you could have a brain-storming document for puns, and allow anyone to add to it (without logging in), letting people edit their own records, but not showing the records to others until they are approved by a moderator. Without something like this, we could only let anonymous people add one field of a record, and not have a secure way to let them edit that field or others in the same record. Also adds a `user.IsLoggedIn` flag in passing. Test Plan: Added a test, updated tests. The test added is a mini-moderation doc, don't use it for real because it allows users to edit their entries after a moderator has approved them. Reviewers: georgegevoian Reviewed By: georgegevoian Subscribers: dsagal Differential Revision: https://phab.getgrist.com/D3273 --- app/client/aclui/AccessRules.ts | 2 ++ app/server/lib/Authorizer.ts | 8 ++++++++ app/server/lib/BrowserSession.ts | 13 +++++++++++++ app/server/lib/Client.ts | 4 ++++ app/server/lib/DocSession.ts | 10 ++++++++++ app/server/lib/GranularAccess.ts | 9 +++++++-- sandbox/grist/test_completion.py | 12 ++++++++++-- sandbox/grist/test_renames.py | 4 +++- sandbox/grist/test_trigger_formulas.py | 8 ++++++-- sandbox/grist/test_user.py | 8 ++++++-- sandbox/grist/user.py | 5 ++++- 11 files changed, 73 insertions(+), 10 deletions(-) diff --git a/app/client/aclui/AccessRules.ts b/app/client/aclui/AccessRules.ts index 37e9faa8..e3949f65 100644 --- a/app/client/aclui/AccessRules.ts +++ b/app/client/aclui/AccessRules.ts @@ -143,6 +143,8 @@ export class AccessRules extends Disposable { {ruleIndex: -1, value: 'user.Name'}, {ruleIndex: -1, value: 'user.LinkKey.'}, {ruleIndex: -1, value: 'user.Origin'}, + {ruleIndex: -1, value: 'user.SessionID'}, + {ruleIndex: -1, value: 'user.IsLoggedIn'}, ]; for (const [i, rule] of rules.entries()) { const tableId = use(rule.tableId); diff --git a/app/server/lib/Authorizer.ts b/app/server/lib/Authorizer.ts index 37626570..f62eb874 100644 --- a/app/server/lib/Authorizer.ts +++ b/app/server/lib/Authorizer.ts @@ -11,6 +11,7 @@ import {forceSessionChange, getSessionProfiles, getSessionUser, getSignInStatus, import {RequestWithOrg} from 'app/server/lib/extractOrg'; import {COOKIE_MAX_AGE, getAllowedOrgForSessionID, getCookieDomain, cookieName as sessionCookieName} from 'app/server/lib/gristSessions'; +import {makeId} from 'app/server/lib/idUtils'; import * as log from 'app/server/lib/log'; import {IPermitStore, Permit} from 'app/server/lib/Permit'; import {allowHost, optStringParam} from 'app/server/lib/requestUtils'; @@ -30,6 +31,7 @@ export interface RequestWithLogin extends Request { userIsAuthorized?: boolean; // If userId is for "anonymous", this will be false. docAuth?: DocAuthResult; // For doc requests, the docId and the user's access level. specialPermit?: Permit; + altSessionId?: string; // a session id for use in trigger formulas and granular access rules } /** @@ -154,6 +156,12 @@ export async function addRequestUser(dbManager: HomeDBManager, permitStore: IPer // If we haven't selected a user by other means, and have profiles available in the // session, then select a user based on those profiles. const session = mreq.session; + if (session && !session.altSessionId) { + // Create a default alternative session id for use in documents. + session.altSessionId = makeId(); + forceSessionChange(session); + } + mreq.altSessionId = session?.altSessionId; if (!mreq.userId && session && session.users && session.users.length > 0 && mreq.org !== undefined) { diff --git a/app/server/lib/BrowserSession.ts b/app/server/lib/BrowserSession.ts index 83d7cb78..3ffdd789 100644 --- a/app/server/lib/BrowserSession.ts +++ b/app/server/lib/BrowserSession.ts @@ -53,6 +53,13 @@ export interface SessionObj { // This gets set to encourage express-session to set a cookie. Was a boolean in the past. alive?: number; + + altSessionId?: string; // An ID unique to the session, but which isn't related + // to the session id used to lookup the cookie. This ID + // is suitable for embedding in documents that allows + // anonymous editing (e.g. to allow the user to edit + // something they just added, without allowing the suer + // to edit other people's contributions). } // Make an artificial change to a session to encourage express-session to set a cookie. @@ -138,6 +145,7 @@ export function linkOrgWithEmail(session: SessionObj, email: string, org: string export class ScopedSession { private _sessionCache?: SessionObj; private _live: boolean; // if set, never cache session in memory. + private _altSessionId?: string; /** * Create an interface to the session identified by _sessionId, in the store identified @@ -220,6 +228,10 @@ export class ScopedSession { await this._setSession(req, session); } + public getAltSessionId(): string | undefined { + return this._altSessionId; + } + /** * Read the state of the session. */ @@ -227,6 +239,7 @@ export class ScopedSession { if (this._sessionCache) { return this._sessionCache; } const session = ((await this._sessionStore.getAsync(this._sessionId)) || {}) as SessionObj; if (!this._live) { this._sessionCache = session; } + this._altSessionId = session.altSessionId; return session; } diff --git a/app/server/lib/Client.ts b/app/server/lib/Client.ts index 047c4f53..2c809b6a 100644 --- a/app/server/lib/Client.ts +++ b/app/server/lib/Client.ts @@ -234,6 +234,10 @@ export class Client { return this._session; } + public getAltSessionId(): string|undefined { + return this._session?.getAltSessionId(); + } + public destroy() { this._destroyed = true; } diff --git a/app/server/lib/DocSession.ts b/app/server/lib/DocSession.ts index 04f15f47..932c20ab 100644 --- a/app/server/lib/DocSession.ts +++ b/app/server/lib/DocSession.ts @@ -101,6 +101,16 @@ export function getDocSessionUserId(docSession: OptDocSession): number|null { return null; } +export function getDocSessionAltSessionId(docSession: OptDocSession): string|null { + if (docSession.req) { + return docSession.req.altSessionId || null; + } + if (docSession.client) { + return docSession.client.getAltSessionId() || null; + } + return null; +} + /** * Get as much of user profile as we can (id, name, email). */ diff --git a/app/server/lib/GranularAccess.ts b/app/server/lib/GranularAccess.ts index 535bffa3..3ff8a2cb 100644 --- a/app/server/lib/GranularAccess.ts +++ b/app/server/lib/GranularAccess.ts @@ -22,7 +22,8 @@ import { HomeDBManager } from 'app/gen-server/lib/HomeDBManager'; import { GristObjCode } from 'app/plugin/GristData'; import { compileAclFormula } from 'app/server/lib/ACLFormula'; import { DocClients } from 'app/server/lib/DocClients'; -import { getDocSessionAccess, getDocSessionUser, OptDocSession } from 'app/server/lib/DocSession'; +import { getDocSessionAccess, getDocSessionAltSessionId, getDocSessionUser, + OptDocSession } from 'app/server/lib/DocSession'; import * as log from 'app/server/lib/log'; import { IPermissionInfo, PermissionInfo, PermissionSetWithContext } from 'app/server/lib/PermissionInfo'; import { TablePermissionSetWithContext } from 'app/server/lib/PermissionInfo'; @@ -1356,7 +1357,9 @@ export class GranularAccess implements GranularAccessForBundle { } const user = new User(); user.Access = access; - user.UserID = fullUser?.id || null; + const isAnonymous = fullUser?.id === this._homeDbManager?.getAnonymousUserId() || + fullUser?.id === null; + user.UserID = (!isAnonymous && fullUser?.id) || null; user.Email = fullUser?.email || null; user.Name = fullUser?.name || null; // If viewed from a websocket, collect any link parameters included. @@ -1365,6 +1368,8 @@ export class GranularAccess implements GranularAccessForBundle { // Include origin info if accessed via the rest api. // TODO: could also get this for websocket access, just via a different route. user.Origin = docSession.req?.get('origin') || null; + user.SessionID = isAnonymous ? `a${getDocSessionAltSessionId(docSession)}` : `u${user.UserID}`; + user.IsLoggedIn = !isAnonymous; if (this._ruler.ruleCollection.ruleError && !this._recoveryMode) { // It is important to signal that the doc is in an unexpected state, diff --git a/sandbox/grist/test_completion.py b/sandbox/grist/test_completion.py index 17cdc9e2..4a3aac54 100644 --- a/sandbox/grist/test_completion.py +++ b/sandbox/grist/test_completion.py @@ -10,7 +10,9 @@ class TestCompletion(test_engine.EngineTestCase): 'LinkKey': {}, 'Origin': None, 'Email': 'foo@example.com', - 'Access': 'owners' + 'Access': 'owners', + 'SessionID': 'u1', + 'IsLoggedIn': True } def setUp(self): @@ -85,9 +87,11 @@ class TestCompletion(test_engine.EngineTestCase): [ 'user.Access', 'user.Email', + 'user.IsLoggedIn', 'user.LinkKey', 'user.Name', 'user.Origin', + 'user.SessionID', 'user.StudentInfo', 'user.UserID' ] @@ -114,16 +118,20 @@ class TestCompletion(test_engine.EngineTestCase): 'Email': 'baro@example.com', 'LinkKey': {}, 'UserID': 2, - 'Access': 'owners' + 'Access': 'owners', + 'SessionID': 'u2', + 'IsLoggedIn': True } self.assertEqual( self.engine.autocomplete("user.", "Schools", "lastModified", user2), [ 'user.Access', 'user.Email', + 'user.IsLoggedIn', 'user.LinkKey', 'user.Name', 'user.Origin', + 'user.SessionID', 'user.UserID' ] ) diff --git a/sandbox/grist/test_renames.py b/sandbox/grist/test_renames.py index 0af60c7a..de8b92bd 100644 --- a/sandbox/grist/test_renames.py +++ b/sandbox/grist/test_renames.py @@ -314,7 +314,9 @@ class TestRenames(test_engine.EngineTestCase): 'LinkKey': {}, 'Origin': None, 'Email': 'foo@example.com', - 'Access': 'owners' + 'Access': 'owners', + 'SessionID': 'u1', + 'IsLoggedIn': True } # Renaming a table should not leave the old name available for auto-complete. diff --git a/sandbox/grist/test_trigger_formulas.py b/sandbox/grist/test_trigger_formulas.py index c196469f..23885ce1 100644 --- a/sandbox/grist/test_trigger_formulas.py +++ b/sandbox/grist/test_trigger_formulas.py @@ -566,7 +566,9 @@ class TestTriggerFormulas(test_engine.EngineTestCase): 'LinkKey': {}, 'Origin': None, 'Email': 'foo.bar@getgrist.com', - 'Access': 'owners' + 'Access': 'owners', + 'SessionID': 'u1', + 'IsLoggedIn': True } user2 = { 'Name': 'Baz Qux', @@ -575,7 +577,9 @@ class TestTriggerFormulas(test_engine.EngineTestCase): 'LinkKey': {}, 'Origin': None, 'Email': 'baz.qux@getgrist.com', - 'Access': 'owners' + 'Access': 'owners', + 'SessionID': 'u2', + 'IsLoggedIn': True } # Use formula to store last modified by data (user name and email). Check that it works as expected. self.load_sample(self.sample) diff --git a/sandbox/grist/test_user.py b/sandbox/grist/test_user.py index a4955a64..ebaf10c5 100644 --- a/sandbox/grist/test_user.py +++ b/sandbox/grist/test_user.py @@ -19,7 +19,9 @@ class TestUser(test_engine.EngineTestCase): 'Param2': 'Param2Value' }, 'Origin': 'https://getgrist.com', - 'StudentInfo': ['Students', 1] + 'StudentInfo': ['Students', 1], + 'SessionID': 'u1', + 'IsLoggedIn': True } u = User(data, self.engine.tables) self.assertEqual(u.Name, 'Foo Bar') @@ -45,7 +47,9 @@ class TestUser(test_engine.EngineTestCase): 'Param2': 'Param2Value' }, 'Origin': 'https://getgrist.com', - 'StudentInfo': ['Students', 1] + 'StudentInfo': ['Students', 1], + 'SessionID': 'u1', + 'IsLoggedIn': True } u = User(data, self.engine.tables, is_sample=True) self.assertEqual(u.StudentInfo.id, 0) diff --git a/sandbox/grist/user.py b/sandbox/grist/user.py index c3f12a15..f31ca1cc 100644 --- a/sandbox/grist/user.py +++ b/sandbox/grist/user.py @@ -17,6 +17,8 @@ the following fields: - Name: string or None - Origin: string or None - LinkKey: dictionary + - SessionID: string or None + - IsLoggedIn: boolean Additional keys may be included, which may have a value that is either None or of type tuple with the following shape: @@ -40,7 +42,8 @@ class User(object): typed equivalents, for use by autocompletion. """ def __init__(self, data, tables, is_sample=False): - for attr in ('Access', 'UserID', 'Email', 'Name', 'Origin'): + for attr in ('Access', 'UserID', 'Email', 'Name', 'Origin', 'SessionID', + 'IsLoggedIn'): setattr(self, attr, data[attr]) self.LinkKey = LinkKey(data['LinkKey'])