Server tests to validate we don't pollute the session store when using an API key (#2246)

## Context

Making Rest API calls pollutes session store uselessly.

To be exact, this issue has been addressed recently: e6a3351


## Proposed solution

As the issue is already addressed, I suggest to add non-regression tests
to ensure this won't happen anymore.

Handling anonymous sessions is trickier and will be done in a future PR.

## Has this been tested?

<!-- Put an `x` in the box that applies: -->

- [x] 👍 yes, I added tests to the test suite
- [ ] 💭 no, because this PR is a draft and still needs work
- [ ] 🙅 no, because this is not relevant here
- [ ] 🙋 no, because I need help <!-- Detail how we can help you -->

Author: fflorent

app/server/lib/Authorizer.ts

@@ -9,15 +9,16 @@ import { User } from "app/gen-server/entity/User";
 import { HomeDBManager } from "app/gen-server/lib/homedb/HomeDBManager";
 import { DocAuthResult, HomeDBAuth } from "app/gen-server/lib/homedb/Interfaces";
 import { AccessTokenInfo } from "app/server/lib/AccessTokens";
-import { forceSessionChange, getSessionProfiles, getSessionUser, getSignInStatus, linkOrgWithEmail, SessionObj,
-  SessionUserObj, SignInStatus } from "app/server/lib/BrowserSession";
+import {
+  forceSessionChange, generateAltSessionID, getSessionProfiles,
+  getSessionUser, getSignInStatus, linkOrgWithEmail, SessionObj, SessionUserObj, SignInStatus,
+} from "app/server/lib/BrowserSession";
 import { expressWrap } from "app/server/lib/expressWrap";
 import { RequestWithOrg } from "app/server/lib/extractOrg";
 import { GristServer } from "app/server/lib/GristServer";
 import { COOKIE_MAX_AGE,
   cookieName as sessionCookieName, getAllowedOrgForSessionID, getCookieDomain } from "app/server/lib/gristSessions";
 import { getBootKey } from "app/server/lib/gristSettings";
-import { makeId } from "app/server/lib/idUtils";
 import log from "app/server/lib/log";
 import { IPermitStore, Permit } from "app/server/lib/Permit";
 import { allowHost, buildXForwardedForHeader, getOriginUrl, optStringParam } from "app/server/lib/requestUtils";
@@ -490,11 +491,10 @@ export async function addRequestUser(
   // access-token requests are GETs for attachments (no trigger formulas) and
   // boot-key requests are admin-only. The alternative — threading authDone through
   // the IdentityResult — would complicate the interface for no practical benefit.
-  if (!skipSession) {
+  if (!skipSession && !identity.hasApiKey) {
     const session = mreq.session;
     if (session && !session.altSessionId) {
-      session.altSessionId = makeId();
-      forceSessionChange(session);
+      generateAltSessionID(session);
     }
     mreq.altSessionId = session?.altSessionId;
   }

app/server/lib/BrowserSession.ts

@@ -1,6 +1,7 @@
 import { normalizeEmail } from "app/common/emails";
 import { UserProfile } from "app/common/LoginSessionAPI";
 import { SessionStore } from "app/server/lib/gristSessions";
+import { makeId } from "app/server/lib/idUtils";
 import log from "app/server/lib/log";
 import { fromCallback } from "app/server/lib/serverUtils";
 
@@ -87,7 +88,23 @@ export interface SessionOIDCInfo {
   idToken?: string;
 }
 
-// Make an artificial change to a session to encourage express-session to set a cookie.
+/**
+ * Create a default alternative session id for use in documents.
+ *
+ * This sessionID is also used client side on formulas (`user.SessionID`) when the user is anonymous
+ *
+ * Also force express-session to persist the sessionin the store.
+ */
+export function generateAltSessionID(session: SessionObj) {
+  session.altSessionId = makeId();
+  forceSessionChange(session);
+}
+
+/**
+ * Make an artificial change to a session to encourage express-session to set a cookie.
+ *
+ * This way, express-session will persist the session in the store.
+ */
 export function forceSessionChange(session: SessionObj) {
   session.alive = Number(session.alive || 0) + 1;
 }

app/server/lib/gristSessions.ts

@@ -28,6 +28,7 @@ export interface SessionStore {
   setAsync(sid: string, session: any): Promise<void>;
   clearAsync(): Promise<void>;
   close(): Promise<void>;
+  lengthAsync(): Promise<number>;
 }
 
 /**

test/server/lib/Authorizer.ts

@@ -49,6 +49,15 @@ async function activateServer(home: FlexServer, docManager: DocManager) {
   serverUrl = home.getOwnUrl();
 }
 
+/**
+ * Count the number of sessions in the session store (whenever we use SQLite or Redis).
+ */
+function countSessions(flexServer: FlexServer = server): Promise<number> {
+  const store = flexServer["_sessionStore"];
+  return store.lengthAsync();
+}
+
+const anon = configForUser("Anonymous");
 const chimpy = configForUser("Chimpy");
 const charon = configForUser("Charon");
 
@@ -151,6 +160,26 @@ describe("Authorizer", function() {
     assert.equal(resp.status, 404);
   });
 
+  it("does not generate sessions when using an API key to avoid polluting " +
+    "the store session needlessly", async function() {
+    const nbSessionsBefore = await countSessions();
+    const resp = await axios.get(`${serverUrl}/api/orgs`, chimpy);
+    const nbSessionsAfter = await countSessions();
+    assert.equal(resp.status, 200);
+    assert.equal(nbSessionsAfter, nbSessionsBefore, "No new session should have been created during the API call");
+  });
+
+  // This checks we create an altSessionID for Anonymous users. Useful in particular to give them
+  // rights through Access Rules.
+  it("does generate a session when calling the API as anonymous", async function() {
+    const nbSessionsBefore = await countSessions(server);
+    const resp = await axios.get(`${serverUrl}/api/orgs`, anon);
+
+    const nbSessionsAfter = await countSessions(server);
+    assert.equal(resp.status, 200);
+    assert.equal(nbSessionsAfter, nbSessionsBefore + 1, "A new session should have been created during the API call");
+  });
+
   it("websocket allows openDoc for viewer", async function() {
     const cli = await openClient(server, "chimpy@getgrist.com", "pr");
     cli.ignoreTrivialActions();