chore: add e2e test for refresh tokens cooldown period - skip e2e (#2860)

* chore: add e2e test for refresh tokens cooldown period

* chore: fix refreshing session in e2e

* chore: fix session refresh cooldown test

* chore: fix e2e test

* Add dropped response simulation test

---------

Co-authored-by: moughxyz <mo@standardnotes.com>

packages/api/src/Domain/Http/HttpService.ts

@@ -1,6 +1,6 @@
 import { LoggerInterface, joinPaths, sleep } from '@standardnotes/utils'
 import { Environment } from '@standardnotes/models'
-import { LegacySession, Session, SessionToken } from '@standardnotes/domain-core'
+import { LegacySession, Result, Session, SessionToken } from '@standardnotes/domain-core'
 import {
   HttpStatusCode,
   HttpRequestParams,
@@ -23,10 +23,11 @@ import { HttpRequestOptions } from './HttpRequestOptions'
 export class HttpService implements HttpServiceInterface {
   private session?: Session | LegacySession
   private __latencySimulatorMs?: number
+  private __simulateNextSessionRefreshResponseDrop = false
   private declare host: string
   loggingEnabled = false
 
-  private inProgressRefreshSessionPromise?: Promise<boolean>
+  private inProgressRefreshSessionPromise?: Promise<Result<HttpResponse<SessionRefreshResponseBody>>>
   private updateMetaCallback!: (meta: HttpResponseMeta) => void
   private refreshSessionCallback!: (session: Session) => void
 
@@ -173,12 +174,19 @@ export class HttpService implements HttpServiceInterface {
       if (this.inProgressRefreshSessionPromise) {
         await this.inProgressRefreshSessionPromise
       } else {
-        this.inProgressRefreshSessionPromise = this.refreshSession()
+        const hasSessionTokenRenewedInBetweenOurRequest = httpRequest.authentication !== this.getSessionAccessToken()
-        const isSessionRefreshed = await this.inProgressRefreshSessionPromise
+        if (!hasSessionTokenRenewedInBetweenOurRequest) {
-        this.inProgressRefreshSessionPromise = undefined
+          this.inProgressRefreshSessionPromise = this.refreshSession()
+          const isSessionRefreshedResultOrError = await this.inProgressRefreshSessionPromise
+          let isSessionRefreshed = false
+          if (!isSessionRefreshedResultOrError.isFailed()) {
+            isSessionRefreshed = !isErrorResponse(isSessionRefreshedResultOrError.getValue())
+          }
+          this.inProgressRefreshSessionPromise = undefined
 
-        if (!isSessionRefreshed) {
+          if (!isSessionRefreshed) {
-          return response
+            return response
+          }
         }
       }
 
@@ -190,13 +198,13 @@ export class HttpService implements HttpServiceInterface {
     return response
   }
 
-  async refreshSession(): Promise<boolean> {
+  async refreshSession(): Promise<Result<HttpResponse<SessionRefreshResponseBody>>> {
     if (!this.session) {
-      return false
+      return Result.fail('No session to refresh')
     }
 
     if (this.session instanceof LegacySession) {
-      return false
+      return Result.fail('Cannot refresh legacy session')
     }
 
     const response = await this.post<SessionRefreshResponseBody>(Paths.v1.refreshSession, {
@@ -204,8 +212,13 @@ export class HttpService implements HttpServiceInterface {
       refresh_token: this.session.refreshToken.value,
     })
 
+    if (this.__simulateNextSessionRefreshResponseDrop) {
+      this.__simulateNextSessionRefreshResponseDrop = false
+      return Result.fail('Simulating a dropped response')
+    }
+
     if (isErrorResponse(response)) {
-      return false
+      return Result.ok(response)
     }
 
     if (response.meta) {
@@ -217,7 +230,7 @@ export class HttpService implements HttpServiceInterface {
       response.data.session.access_expiration,
     )
     if (accessTokenOrError.isFailed()) {
-      return false
+      return Result.fail(accessTokenOrError.getError())
     }
 
     const accessToken = accessTokenOrError.getValue()
@@ -227,21 +240,21 @@ export class HttpService implements HttpServiceInterface {
       response.data.session.refresh_expiration,
     )
     if (refreshTokenOrError.isFailed()) {
-      return false
+      return Result.fail(refreshTokenOrError.getError())
     }
 
     const refreshToken = refreshTokenOrError.getValue()
 
     const sessionOrError = Session.create(accessToken, refreshToken, response.data.session.readonly_access)
     if (sessionOrError.isFailed()) {
-      return false
+      return Result.fail(sessionOrError.getError())
     }
 
     this.setSession(sessionOrError.getValue())
 
     this.refreshSessionCallback(this.session)
 
-    return true
+    return Result.ok(response)
   }
 
   private params(inParams: Record<string | number | symbol, unknown>): HttpRequestParams {

packages/api/src/Domain/Http/HttpServiceInterface.ts

@@ -1,7 +1,8 @@
-import { LegacySession, Session } from '@standardnotes/domain-core'
+import { LegacySession, Result, Session } from '@standardnotes/domain-core'
 import { HttpRequest, HttpRequestParams, HttpResponse, HttpResponseMeta } from '@standardnotes/responses'
 
 import { HttpRequestOptions } from './HttpRequestOptions'
+import { SessionRefreshResponseBody } from '../Response'
 
 export interface HttpServiceInterface {
   setHost(host: string): void
@@ -16,7 +17,7 @@ export interface HttpServiceInterface {
   runHttp<T>(httpRequest: HttpRequest): Promise<HttpResponse<T>>
 
   setSession(session: Session | LegacySession): void
-  refreshSession(): Promise<boolean>
+  refreshSession(): Promise<Result<HttpResponse<SessionRefreshResponseBody>>>
   setCallbacks(
     updateMetaCallback: (meta: HttpResponseMeta) => void,
     refreshSessionCallback: (session: Session) => void,

packages/snjs/lib/Services/Api/ApiService.ts

@@ -408,7 +408,12 @@ export class LegacyApiService
     }
   }
 
-  async refreshSession(): Promise<HttpResponse<SessionRenewalResponse>> {
+  /**
+   * @deprecated
+   *
+   * This function should be replaced with @standardnotes/api's `HttpService::refreshSession` function.
+   */
+  async deprecatedRefreshSessionOnlyUsedInE2eTests(): Promise<HttpResponse<SessionRenewalResponse>> {
     const preprocessingError = this.preprocessingError()
     if (preprocessingError) {
       return preprocessingError

packages/snjs/lib/Services/Session/SessionManager.ts

@@ -874,7 +874,14 @@ export class SessionManager
     const willRefreshTokenExpireSoon = refreshTokenExpiration.getTime() - Date.now() < ThirtyMinutes
 
     if (willAccessTokenExpireSoon || willRefreshTokenExpireSoon) {
-      return this.httpService.refreshSession()
+      const refreshSessionResultOrError = await this.httpService.refreshSession()
+      if (refreshSessionResultOrError.isFailed()) {
+        return false
+      }
+
+      const refreshSessionResult = refreshSessionResultOrError.getValue()
+
+      return isErrorResponse(refreshSessionResult)
     }
 
     return false

packages/snjs/mocha/session.test.js

@@ -68,7 +68,7 @@ describe('server session', function () {
       password: password,
     })
 
-    const response = await application.legacyApi.refreshSession()
+    const response = await application.legacyApi.deprecatedRefreshSessionOnlyUsedInE2eTests()
 
     expect(response.status).to.equal(200)
     expect(response.data.session.access_token).to.be.a('string')
@@ -178,7 +178,7 @@ describe('server session', function () {
     expect(sessionFromStorage.refreshExpiration).to.equal(sessionFromApiService.refreshToken.expiresAt)
     expect(sessionFromStorage.readonlyAccess).to.equal(sessionFromApiService.isReadOnly())
 
-    await application.legacyApi.refreshSession()
+    await application.legacyApi.deprecatedRefreshSessionOnlyUsedInE2eTests()
 
     const updatedSessionFromStorage = await getSessionFromStorage(application)
     const updatedSessionFromApiService = application.legacyApi.getSession()
@@ -407,7 +407,7 @@ describe('server session', function () {
 
     await sleepUntilSessionExpires(application, false)
 
-    const refreshSessionResponse = await application.legacyApi.refreshSession()
+    const refreshSessionResponse = await application.legacyApi.deprecatedRefreshSessionOnlyUsedInE2eTests()
 
     expect(refreshSessionResponse.status).to.equal(400)
     /**
@@ -452,7 +452,7 @@ describe('server session', function () {
     })
     application.sessions.initializeFromDisk()
 
-    const refreshSessionResponse = await application.legacyApi.refreshSession()
+    const refreshSessionResponse = await application.legacyApi.deprecatedRefreshSessionOnlyUsedInE2eTests()
 
     expect(refreshSessionResponse.status).to.equal(400)
     expect(refreshSessionResponse.data.error.tag).to.equal('invalid-refresh-token')
@@ -470,7 +470,7 @@ describe('server session', function () {
       password: password,
     })
 
-    const refreshPromise = application.legacyApi.refreshSession()
+    const refreshPromise = application.legacyApi.deprecatedRefreshSessionOnlyUsedInE2eTests()
     const syncResponse = await application.legacyApi.sync([])
 
     expect(syncResponse.data.error).to.be.ok
@@ -481,6 +481,57 @@ describe('server session', function () {
     await refreshPromise
   })
 
+  it('should tell the client to refresh the token if one is used during the cooldown period after a refresh', async function () {
+    await Factory.registerUserToApplication({
+      application: application,
+      email: email,
+      password: password,
+    })
+
+    const mimickApplyingSessionFromTheServerUnsuccessfully = () => {}
+    const originalSetSessionFn = application.http.setSession
+    const originalRefreshSessionCallbackFn = application.http.refreshSessionCallback
+    application.http.setSession = mimickApplyingSessionFromTheServerUnsuccessfully
+    application.http.refreshSessionCallback = mimickApplyingSessionFromTheServerUnsuccessfully
+
+    const refreshResultOrError = await application.http.refreshSession()
+    expect(refreshResultOrError.isFailed()).to.equal(false)
+
+    const refreshResult = refreshResultOrError.getValue()
+    expect(isErrorResponse(refreshResult)).to.equal(false)
+
+    const secondRefreshResultOrErrorWithNotAppliedSession = await application.http.refreshSession()
+    expect(secondRefreshResultOrErrorWithNotAppliedSession.isFailed()).to.equal(false)
+
+    const secondRefreshResultWithNotAppliedSession = secondRefreshResultOrErrorWithNotAppliedSession.getValue()
+    expect(isErrorResponse(secondRefreshResultWithNotAppliedSession)).to.equal(false)
+
+    application.http.setSession = originalSetSessionFn
+    application.http.refreshSessionCallback = originalRefreshSessionCallbackFn
+  })
+
+  it('if session renewal response is dropped, next sync with server should return a 498 and successfully renew the session', async function () {
+    await Factory.registerUserToApplication({
+      application: application,
+      email: email,
+      password: password,
+    })
+
+    await sleepUntilSessionExpires(application)
+
+    const refreshSpy = sinon.spy(application.http, 'refreshSession')
+
+    /**
+     * With this sync, we expect refreshSession to be called twice, once where the response is dropped,
+     * and the other time where the request succeeds
+     */
+    application.http.__simulateNextSessionRefreshResponseDrop = true
+    await application.sync.sync(syncOptions)
+    await application.sync.sync(syncOptions)
+
+    expect(refreshSpy.callCount).to.equal(2)
+  })
+
   it('notes should be synced as expected after refreshing a session', async function () {
     await Factory.registerUserToApplication({
       application: application,