From d17c115ec28d973b6a52d1af1fdbea05871d3f0e Mon Sep 17 00:00:00 2001 From: Alankarsharma Date: Tue, 30 Apr 2024 21:30:38 +0530 Subject: [PATCH 1/8] Bug fix Signed-off-by: Alankarsharma --- server/auth/types/openid/openid_auth.ts | 4 ++-- server/auth/types/openid/routes.ts | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/server/auth/types/openid/openid_auth.ts b/server/auth/types/openid/openid_auth.ts index b67e174c8..f350f1144 100644 --- a/server/auth/types/openid/openid_auth.ts +++ b/server/auth/types/openid/openid_auth.ts @@ -272,7 +272,7 @@ export class OpenIdAuthentication extends AuthenticationType { return false; } - if (cookie.expiryTime > Date.now()) { + if (cookie.credentials.expiryTime > Date.now()) { return true; } @@ -296,8 +296,8 @@ export class OpenIdAuthentication extends AuthenticationType { cookie.credentials = { authHeaderValueExtra: true, refresh_token: refreshTokenResponse.refreshToken, + expiryTime: getExpirationDate(refreshTokenResponse), }; - cookie.expiryTime = getExpirationDate(refreshTokenResponse); setExtraAuthStorage( request, diff --git a/server/auth/types/openid/routes.ts b/server/auth/types/openid/routes.ts index c23e26b1f..84e3bee42 100644 --- a/server/auth/types/openid/routes.ts +++ b/server/auth/types/openid/routes.ts @@ -195,9 +195,10 @@ export class OpenIdAuthRoutes { username: user.username, credentials: { authHeaderValueExtra: true, + expiryTime: getExpirationDate(tokenResponse), }, authType: AuthType.OPEN_ID, - expiryTime: getExpirationDate(tokenResponse), + expiryTime: Date.now() + this.config.session.ttl, }; if (this.config.openid?.refresh_tokens && tokenResponse.refreshToken) { Object.assign(sessionStorage.credentials, { From 7774f82691cd32d239d901b13ab2f2287da56846 Mon Sep 17 00:00:00 2001 From: Alankarsharma Date: Tue, 30 Apr 2024 21:32:33 +0530 Subject: [PATCH 2/8] Update cookie expiry as well Signed-off-by: Alankarsharma --- server/auth/types/openid/openid_auth.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/server/auth/types/openid/openid_auth.ts b/server/auth/types/openid/openid_auth.ts index f350f1144..04c205414 100644 --- a/server/auth/types/openid/openid_auth.ts +++ b/server/auth/types/openid/openid_auth.ts @@ -298,6 +298,7 @@ export class OpenIdAuthentication extends AuthenticationType { refresh_token: refreshTokenResponse.refreshToken, expiryTime: getExpirationDate(refreshTokenResponse), }; + cookie.expiryTime = Date.now() + this.config.session.ttl; setExtraAuthStorage( request, From 9fe50db209d52d1c8c505779d782084e66a46923 Mon Sep 17 00:00:00 2001 From: Alankarsharma Date: Tue, 7 May 2024 18:38:54 +0530 Subject: [PATCH 3/8] Lint issue fix Signed-off-by: Alankarsharma --- server/auth/types/openid/routes.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/auth/types/openid/routes.ts b/server/auth/types/openid/routes.ts index 84e3bee42..b0483057f 100644 --- a/server/auth/types/openid/routes.ts +++ b/server/auth/types/openid/routes.ts @@ -195,7 +195,7 @@ export class OpenIdAuthRoutes { username: user.username, credentials: { authHeaderValueExtra: true, - expiryTime: getExpirationDate(tokenResponse), + expiryTime: getExpirationDate(tokenResponse), }, authType: AuthType.OPEN_ID, expiryTime: Date.now() + this.config.session.ttl, From 1440b4f1b6aaa597f80b821175e88e0941173ab4 Mon Sep 17 00:00:00 2001 From: Alankarsharma Date: Wed, 8 May 2024 08:59:48 +0530 Subject: [PATCH 4/8] fixed test case Signed-off-by: Alankarsharma --- server/auth/types/openid/openid_auth.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/auth/types/openid/openid_auth.test.ts b/server/auth/types/openid/openid_auth.test.ts index c8cc839e7..82fefa7c9 100644 --- a/server/auth/types/openid/openid_auth.test.ts +++ b/server/auth/types/openid/openid_auth.test.ts @@ -236,7 +236,8 @@ describe('test OpenId authHeaderValue', () => { authType: 'openid', }; - expect(await openIdAuthentication.isValidCookie(testCookie, {})).toBe(true); + // As ID token is expired and no refresh token value is passed, it will return false + expect(await openIdAuthentication.isValidCookie(testCookie, {})).toBe(false); global.Date.now = realDateNow; }); From 9d170d6b2969939fbdbc74a2c70207ca35fc7b3e Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Sun, 9 Jun 2024 15:12:05 -0400 Subject: [PATCH 5/8] Add test for refresh token workflow in OIDC Signed-off-by: Craig Perkins --- server/auth/types/openid/openid_auth.test.ts | 66 +++++++++++++++++++- server/auth/types/openid/openid_auth.ts | 4 +- server/auth/types/openid/routes.ts | 3 +- 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/server/auth/types/openid/openid_auth.test.ts b/server/auth/types/openid/openid_auth.test.ts index c8cc839e7..4583d638c 100644 --- a/server/auth/types/openid/openid_auth.test.ts +++ b/server/auth/types/openid/openid_auth.test.ts @@ -37,6 +37,12 @@ interface Logger { fatal(message: string): void; } +const mockClient = { post: jest.fn() }; + +jest.mock('@hapi/wreck', () => ({ + defaults: jest.fn(() => mockClient), +})); + describe('test OpenId authHeaderValue', () => { let router: IRouter; let core: CoreSetup; @@ -208,7 +214,7 @@ describe('test OpenId authHeaderValue', () => { expect(wreckHttpsOptions.passphrase).toBeUndefined(); }); - test('Ensure expiryTime is being used to test validity of cookie', async () => { + test('Ensure accessToken expiryTime is being used to test validity of cookie', async () => { const realDateNow = Date.now.bind(global.Date); const dateNowStub = jest.fn(() => 0); global.Date.now = dateNowStub; @@ -229,9 +235,9 @@ describe('test OpenId authHeaderValue', () => { const testCookie: SecuritySessionCookie = { credentials: { authHeaderValue: 'Bearer eyToken', - expiry_time: -1, + expiryTime: 200, }, - expiryTime: 2000, + expiryTime: 10000, username: 'admin', authType: 'openid', }; @@ -240,6 +246,60 @@ describe('test OpenId authHeaderValue', () => { global.Date.now = realDateNow; }); + test('Ensure refreshToken workflow is called if current time is after access token expiry, but before session expiry', async () => { + const realDateNow = Date.now.bind(global.Date); + const dateNowStub = jest.fn(() => 300); + global.Date.now = dateNowStub; + const oidcConfig: unknown = { + openid: { + header: 'authorization', + scope: [], + extra_storage: { + cookie_prefix: 'testcookie', + additional_cookies: 0, + }, + }, + }; + + const openIdAuthentication = new OpenIdAuthentication( + oidcConfig as SecurityPluginConfigType, + sessionStorageFactory, + router, + esClient, + core, + logger + ); + const testCookie: SecuritySessionCookie = { + credentials: { + authHeaderValue: 'Bearer eyToken', + expiryTime: 200, + refresh_token: 'refreshToken', + }, + expiryTime: 10000, + username: 'admin', + authType: 'openid', + }; + + const mockRequestPayload = JSON.stringify({ + grant_type: 'refresh_token', + client_id: 'clientId', + client_secret: 'clientSecret', + refresh_token: 'refreshToken', + }); + const mockResponsePayload = JSON.stringify({ + id_token: '.eyJleHAiOiIwLjUifQ.', // Translates to {"exp":"0.5"} + access_token: 'accessToken', + refresh_token: 'refreshToken', + }); + mockClient.post.mockResolvedValue({ + res: { statusCode: 200 }, + payload: mockResponsePayload, + }); + + expect(await openIdAuthentication.isValidCookie(testCookie, {})).toBe(true); + global.Date.now = realDateNow; + }); + test('getKeepAliveExpiry', () => { const oidcConfig: unknown = { openid: { diff --git a/server/auth/types/openid/openid_auth.ts b/server/auth/types/openid/openid_auth.ts index b67e174c8..f350f1144 100644 --- a/server/auth/types/openid/openid_auth.ts +++ b/server/auth/types/openid/openid_auth.ts @@ -272,7 +272,7 @@ export class OpenIdAuthentication extends AuthenticationType { return false; } - if (cookie.expiryTime > Date.now()) { + if (cookie.credentials.expiryTime > Date.now()) { return true; } @@ -296,8 +296,8 @@ export class OpenIdAuthentication extends AuthenticationType { cookie.credentials = { authHeaderValueExtra: true, refresh_token: refreshTokenResponse.refreshToken, + expiryTime: getExpirationDate(refreshTokenResponse), }; - cookie.expiryTime = getExpirationDate(refreshTokenResponse); setExtraAuthStorage( request, diff --git a/server/auth/types/openid/routes.ts b/server/auth/types/openid/routes.ts index 51ac1a85d..8634f09e2 100644 --- a/server/auth/types/openid/routes.ts +++ b/server/auth/types/openid/routes.ts @@ -195,9 +195,10 @@ export class OpenIdAuthRoutes { username: user.username, credentials: { authHeaderValueExtra: true, + expiryTime: getExpirationDate(tokenResponse), }, authType: AuthType.OPEN_ID, - expiryTime: getExpirationDate(tokenResponse), + expiryTime: Date.now() + this.config.session.ttl, }; if (this.config.openid?.refresh_tokens && tokenResponse.refreshToken) { Object.assign(sessionStorage.credentials, { From 8dc6f372c71292c36758eae60e58218383c31beb Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 10 Jun 2024 08:55:04 -0400 Subject: [PATCH 6/8] Fix assertion Signed-off-by: Craig Perkins --- server/auth/types/openid/openid_auth.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/auth/types/openid/openid_auth.test.ts b/server/auth/types/openid/openid_auth.test.ts index 480934dcc..6b83a4a2c 100644 --- a/server/auth/types/openid/openid_auth.test.ts +++ b/server/auth/types/openid/openid_auth.test.ts @@ -242,8 +242,8 @@ describe('test OpenId authHeaderValue', () => { authType: 'openid', }; - // As ID token is expired and no refresh token value is passed, it will return false - expect(await openIdAuthentication.isValidCookie(testCookie, {})).toBe(false); + // Credentials are valid because 0 < 200 + expect(await openIdAuthentication.isValidCookie(testCookie, {})).toBe(true); global.Date.now = realDateNow; }); From da4b14db75121dec240cd5f10e9448a1bdc5dc41 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 10 Jun 2024 09:55:29 -0400 Subject: [PATCH 7/8] Update getKeepAliveExpiry logic for OIDC Signed-off-by: Craig Perkins --- server/auth/types/openid/openid_auth.test.ts | 9 ++++++++- server/auth/types/openid/openid_auth.ts | 3 +-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/server/auth/types/openid/openid_auth.test.ts b/server/auth/types/openid/openid_auth.test.ts index 6b83a4a2c..b4f86e002 100644 --- a/server/auth/types/openid/openid_auth.test.ts +++ b/server/auth/types/openid/openid_auth.test.ts @@ -302,10 +302,16 @@ describe('test OpenId authHeaderValue', () => { }); test('getKeepAliveExpiry', () => { + const realDateNow = Date.now.bind(global.Date); + const dateNowStub = jest.fn(() => 300); + global.Date.now = dateNowStub; const oidcConfig: unknown = { openid: { scope: [], }, + session: { + ttl: 3600, + }, }; const openIdAuthentication = new OpenIdAuthentication( @@ -323,6 +329,7 @@ describe('test OpenId authHeaderValue', () => { expiryTime: 1000, }; - expect(openIdAuthentication.getKeepAliveExpiry(testCookie, {})).toBe(1000); + expect(openIdAuthentication.getKeepAliveExpiry(testCookie, {})).toBe(3900); + global.Date.now = realDateNow; }); }); diff --git a/server/auth/types/openid/openid_auth.ts b/server/auth/types/openid/openid_auth.ts index f350f1144..3292b68e1 100644 --- a/server/auth/types/openid/openid_auth.ts +++ b/server/auth/types/openid/openid_auth.ts @@ -250,12 +250,11 @@ export class OpenIdAuthentication extends AuthenticationType { }; } - // OIDC expiry time is set by the IDP and refreshed via refreshTokens getKeepAliveExpiry( cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest ): number { - return cookie.expiryTime!; + return Date.now() + this.config.session.ttl; } // TODO: Add token expiration check here From f6d7d1fb90fda32113ecafb34469ee5b242c88e1 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 10 Jun 2024 14:19:01 -0400 Subject: [PATCH 8/8] Add check to ensure mockClient.post is called Signed-off-by: Craig Perkins --- server/auth/types/openid/openid_auth.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/server/auth/types/openid/openid_auth.test.ts b/server/auth/types/openid/openid_auth.test.ts index b4f86e002..58ed56547 100644 --- a/server/auth/types/openid/openid_auth.test.ts +++ b/server/auth/types/openid/openid_auth.test.ts @@ -298,6 +298,7 @@ describe('test OpenId authHeaderValue', () => { }); expect(await openIdAuthentication.isValidCookie(testCookie, {})).toBe(true); + expect(mockClient.post).toBeCalledTimes(1); global.Date.now = realDateNow; });