From 7c8727ce057db1661209244b169f68a691c79697 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 27 Feb 2024 12:07:29 +0100 Subject: [PATCH] backend-app-api: review fixes for cookie auth Signed-off-by: Patrik Oldsberg --- .../httpAuth/httpAuthServiceFactory.ts | 23 +++++++++++-------- .../src/next/services/MockAuthService.ts | 2 +- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/packages/backend-app-api/src/services/implementations/httpAuth/httpAuthServiceFactory.ts b/packages/backend-app-api/src/services/implementations/httpAuth/httpAuthServiceFactory.ts index c261553685..db36e5bf2a 100644 --- a/packages/backend-app-api/src/services/implementations/httpAuth/httpAuthServiceFactory.ts +++ b/packages/backend-app-api/src/services/implementations/httpAuth/httpAuthServiceFactory.ts @@ -59,6 +59,10 @@ function getCookieFromRequest(req: Request) { return undefined; } +function willExpireSoon(expiresAt: Date) { + return Date.now() + FIVE_MINUTES_MS > expiresAt.getTime(); +} + const credentialsSymbol = Symbol('backstage-credentials'); const limitedCredentialsSymbol = Symbol('backstage-limited-credentials'); @@ -100,13 +104,13 @@ class DefaultHttpAuthService implements HttpAuthService { } const cookie = getCookieFromRequest(req); - if (!cookie) { - return await this.#auth.getNoneCredentials(); + if (cookie) { + return await this.#auth.authenticate(cookie, { + allowLimitedAccess: true, + }); } - return await this.#auth.authenticate(cookie, { - allowLimitedAccess: true, - }); + return await this.#auth.getNoneCredentials(); } async #getCredentials(req: RequestWithCredentials) { @@ -171,6 +175,10 @@ class DefaultHttpAuthService implements HttpAuthService { res: Response, options?: { credentials?: BackstageCredentials }, ): Promise<{ expiresAt: Date }> { + if (res.headersSent) { + throw new Error('Failed to issue user cookie, headers were already sent'); + } + let credentials: BackstageCredentials; if (options?.credentials) { if (!this.#auth.isPrincipal(options.credentials, 'user')) { @@ -184,10 +192,7 @@ class DefaultHttpAuthService implements HttpAuthService { } const existingExpiresAt = await this.#existingCookieExpiration(res.req); - if ( - existingExpiresAt && - existingExpiresAt.getTime() < Date.now() - FIVE_MINUTES_MS - ) { + if (existingExpiresAt && !willExpireSoon(existingExpiresAt)) { return { expiresAt: existingExpiresAt }; } diff --git a/packages/backend-test-utils/src/next/services/MockAuthService.ts b/packages/backend-test-utils/src/next/services/MockAuthService.ts index c0e6461245..64dc92747a 100644 --- a/packages/backend-test-utils/src/next/services/MockAuthService.ts +++ b/packages/backend-test-utils/src/next/services/MockAuthService.ts @@ -181,7 +181,7 @@ export class MockAuthService implements AuthService { token: mockCredentials.limitedUser.token( credentials.principal.userEntityRef, ), - expiresAt: new Date(Date.now() + 3600), + expiresAt: new Date(Date.now() + 3600_000), }; } }