From 7a4172ad117eb27fca44cf476b67b5263662cc88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20S=C3=B3jko?= Date: Thu, 14 Mar 2024 11:11:17 +0100 Subject: [PATCH] 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 --- packages/api/src/Domain/Http/HttpService.ts | 45 +++++++++----- .../src/Domain/Http/HttpServiceInterface.ts | 5 +- packages/snjs/lib/Services/Api/ApiService.ts | 7 ++- .../lib/Services/Session/SessionManager.ts | 9 ++- packages/snjs/mocha/session.test.js | 61 +++++++++++++++++-- 5 files changed, 102 insertions(+), 25 deletions(-) diff --git a/packages/api/src/Domain/Http/HttpService.ts b/packages/api/src/Domain/Http/HttpService.ts index 24d0e2be6b8..3d10bdddd29 100644 --- a/packages/api/src/Domain/Http/HttpService.ts +++ b/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 + private inProgressRefreshSessionPromise?: Promise>> 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 isSessionRefreshed = await this.inProgressRefreshSessionPromise - this.inProgressRefreshSessionPromise = undefined - - if (!isSessionRefreshed) { - return response + const hasSessionTokenRenewedInBetweenOurRequest = httpRequest.authentication !== this.getSessionAccessToken() + if (!hasSessionTokenRenewedInBetweenOurRequest) { + this.inProgressRefreshSessionPromise = this.refreshSession() + const isSessionRefreshedResultOrError = await this.inProgressRefreshSessionPromise + let isSessionRefreshed = false + if (!isSessionRefreshedResultOrError.isFailed()) { + isSessionRefreshed = !isErrorResponse(isSessionRefreshedResultOrError.getValue()) + } + this.inProgressRefreshSessionPromise = undefined + + if (!isSessionRefreshed) { + return response + } } } @@ -190,13 +198,13 @@ export class HttpService implements HttpServiceInterface { return response } - async refreshSession(): Promise { + async refreshSession(): Promise>> { 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(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): HttpRequestParams { diff --git a/packages/api/src/Domain/Http/HttpServiceInterface.ts b/packages/api/src/Domain/Http/HttpServiceInterface.ts index 421fa347c50..5736b75aa50 100644 --- a/packages/api/src/Domain/Http/HttpServiceInterface.ts +++ b/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(httpRequest: HttpRequest): Promise> setSession(session: Session | LegacySession): void - refreshSession(): Promise + refreshSession(): Promise>> setCallbacks( updateMetaCallback: (meta: HttpResponseMeta) => void, refreshSessionCallback: (session: Session) => void, diff --git a/packages/snjs/lib/Services/Api/ApiService.ts b/packages/snjs/lib/Services/Api/ApiService.ts index 2e2bd4dd489..d127e68f5ba 100644 --- a/packages/snjs/lib/Services/Api/ApiService.ts +++ b/packages/snjs/lib/Services/Api/ApiService.ts @@ -408,7 +408,12 @@ export class LegacyApiService } } - async refreshSession(): Promise> { + /** + * @deprecated + * + * This function should be replaced with @standardnotes/api's `HttpService::refreshSession` function. + */ + async deprecatedRefreshSessionOnlyUsedInE2eTests(): Promise> { const preprocessingError = this.preprocessingError() if (preprocessingError) { return preprocessingError diff --git a/packages/snjs/lib/Services/Session/SessionManager.ts b/packages/snjs/lib/Services/Session/SessionManager.ts index 72cbcb4da53..920e6a104cc 100644 --- a/packages/snjs/lib/Services/Session/SessionManager.ts +++ b/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 diff --git a/packages/snjs/mocha/session.test.js b/packages/snjs/mocha/session.test.js index ce9bd2ce430..3ca733e193d 100644 --- a/packages/snjs/mocha/session.test.js +++ b/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,