From 183b502f4bb02f4cc87bd082323aa2b8bbac7d78 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 8 Feb 2024 17:42:49 -0500 Subject: [PATCH 01/26] Fix SAML timeout issues when keepalive is true Signed-off-by: Derek Ho --- server/auth/types/authentication_type.ts | 4 +++- server/auth/types/saml/routes.ts | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/server/auth/types/authentication_type.ts b/server/auth/types/authentication_type.ts index 3fef38175..eb15b3971 100755 --- a/server/auth/types/authentication_type.ts +++ b/server/auth/types/authentication_type.ts @@ -143,6 +143,8 @@ export abstract class AuthenticationType implements IAuthenticationType { cookie = undefined; } + console.log(cookie) + if (!cookie || !(await this.isValidCookie(cookie, request))) { // clear cookie this.sessionStorageFactory.asScoped(request).clear(); @@ -160,7 +162,7 @@ export abstract class AuthenticationType implements IAuthenticationType { // extend session expiration time if (this.config.session.keepalive) { - cookie!.expiryTime = Date.now() + this.config.session.ttl; + cookie!.expiryTime = Math.max(Date.now() + this.config.session.ttl, cookie.expiryTime || 0); this.sessionStorageFactory.asScoped(request).set(cookie!); } // cookie is valid diff --git a/server/auth/types/saml/routes.ts b/server/auth/types/saml/routes.ts index 87605d65e..64e2aa84f 100644 --- a/server/auth/types/saml/routes.ts +++ b/server/auth/types/saml/routes.ts @@ -84,6 +84,7 @@ export class SamlAuthRoutes { redirectHash: request.query.redirectHash === 'true', }, }; + console.log('saml login cookie' + JSON.stringify(cookie)) this.sessionStorageFactory.asScoped(request).set(cookie); return response.redirected({ headers: { @@ -113,6 +114,7 @@ export class SamlAuthRoutes { let redirectHash: boolean = false; try { const cookie = await this.sessionStorageFactory.asScoped(request).get(); + console.log('acs' + JSON.stringify(cookie)) if (cookie) { requestId = cookie.saml?.requestId || ''; nextUrl = @@ -142,16 +144,20 @@ export class SamlAuthRoutes { credentials.authorization ); + console.log('creds' + JSON.stringify(credentials), 'user' + JSON.stringify(user)) + let expiryTime = Date.now() + this.config.session.ttl; const [headerEncoded, payloadEncoded, signature] = credentials.authorization.split('.'); if (!payloadEncoded) { context.security_plugin.logger.error('JWT token payload not found'); } const tokenPayload = JSON.parse(Buffer.from(payloadEncoded, 'base64').toString()); + console.log(tokenPayload) if (tokenPayload.exp) { expiryTime = parseInt(tokenPayload.exp, 10) * 1000; } + console.log(expiryTime) const cookie: SecuritySessionCookie = { username: user.username, From a1848b0982e057887048dffa1b34cb8b974bb730 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 8 Feb 2024 17:54:31 -0500 Subject: [PATCH 02/26] lint Signed-off-by: Derek Ho --- server/auth/types/authentication_type.ts | 2 -- server/auth/types/saml/routes.ts | 7 +------ 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/server/auth/types/authentication_type.ts b/server/auth/types/authentication_type.ts index eb15b3971..c0f93a085 100755 --- a/server/auth/types/authentication_type.ts +++ b/server/auth/types/authentication_type.ts @@ -143,8 +143,6 @@ export abstract class AuthenticationType implements IAuthenticationType { cookie = undefined; } - console.log(cookie) - if (!cookie || !(await this.isValidCookie(cookie, request))) { // clear cookie this.sessionStorageFactory.asScoped(request).clear(); diff --git a/server/auth/types/saml/routes.ts b/server/auth/types/saml/routes.ts index 64e2aa84f..6e628bdb4 100644 --- a/server/auth/types/saml/routes.ts +++ b/server/auth/types/saml/routes.ts @@ -84,7 +84,6 @@ export class SamlAuthRoutes { redirectHash: request.query.redirectHash === 'true', }, }; - console.log('saml login cookie' + JSON.stringify(cookie)) this.sessionStorageFactory.asScoped(request).set(cookie); return response.redirected({ headers: { @@ -114,7 +113,6 @@ export class SamlAuthRoutes { let redirectHash: boolean = false; try { const cookie = await this.sessionStorageFactory.asScoped(request).get(); - console.log('acs' + JSON.stringify(cookie)) if (cookie) { requestId = cookie.saml?.requestId || ''; nextUrl = @@ -144,20 +142,17 @@ export class SamlAuthRoutes { credentials.authorization ); - console.log('creds' + JSON.stringify(credentials), 'user' + JSON.stringify(user)) - let expiryTime = Date.now() + this.config.session.ttl; const [headerEncoded, payloadEncoded, signature] = credentials.authorization.split('.'); if (!payloadEncoded) { context.security_plugin.logger.error('JWT token payload not found'); } const tokenPayload = JSON.parse(Buffer.from(payloadEncoded, 'base64').toString()); - console.log(tokenPayload) if (tokenPayload.exp) { expiryTime = parseInt(tokenPayload.exp, 10) * 1000; } - console.log(expiryTime) + console.log(expiryTime); const cookie: SecuritySessionCookie = { username: user.username, From 9afd31efa3c096145c3af88db988d1e52d43a983 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Fri, 9 Feb 2024 10:33:48 -0500 Subject: [PATCH 03/26] Fix oidc flow Signed-off-by: Derek Ho --- server/auth/types/openid/openid_auth.ts | 7 +++---- server/auth/types/openid/routes.ts | 3 +-- server/auth/types/saml/routes.ts | 1 - 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/server/auth/types/openid/openid_auth.ts b/server/auth/types/openid/openid_auth.ts index 4d59ea632..46225f0cd 100644 --- a/server/auth/types/openid/openid_auth.ts +++ b/server/auth/types/openid/openid_auth.ts @@ -260,13 +260,12 @@ export class OpenIdAuthentication extends AuthenticationType { cookie.authType !== this.type || !cookie.username || !cookie.expiryTime || - (!cookie.credentials?.authHeaderValue && !this.getExtraAuthStorageValue(request, cookie)) || - !cookie.credentials?.expires_at + (!cookie.credentials?.authHeaderValue && !this.getExtraAuthStorageValue(request, cookie)) ) { return false; } - if (cookie.credentials?.expires_at > Date.now()) { + if (cookie.expiryTime > Date.now()) { return true; } @@ -290,8 +289,8 @@ export class OpenIdAuthentication extends AuthenticationType { cookie.credentials = { authHeaderValueExtra: true, refresh_token: refreshTokenResponse.refreshToken, - expires_at: getExpirationDate(refreshTokenResponse), // expiresIn is in second }; + cookie.expiryTime = getExpirationDate(refreshTokenResponse); setExtraAuthStorage( request, diff --git a/server/auth/types/openid/routes.ts b/server/auth/types/openid/routes.ts index a9b84e75c..a8dc2e2b1 100644 --- a/server/auth/types/openid/routes.ts +++ b/server/auth/types/openid/routes.ts @@ -196,10 +196,9 @@ export class OpenIdAuthRoutes { username: user.username, credentials: { authHeaderValueExtra: true, - expires_at: getExpirationDate(tokenResponse), }, authType: AuthType.OPEN_ID, - expiryTime: Date.now() + this.config.session.ttl, + expiryTime: getExpirationDate(tokenResponse), }; if (this.config.openid?.refresh_tokens && tokenResponse.refreshToken) { Object.assign(sessionStorage.credentials, { diff --git a/server/auth/types/saml/routes.ts b/server/auth/types/saml/routes.ts index 6e628bdb4..87605d65e 100644 --- a/server/auth/types/saml/routes.ts +++ b/server/auth/types/saml/routes.ts @@ -152,7 +152,6 @@ export class SamlAuthRoutes { if (tokenPayload.exp) { expiryTime = parseInt(tokenPayload.exp, 10) * 1000; } - console.log(expiryTime); const cookie: SecuritySessionCookie = { username: user.username, From 1da97392876b0675904d9a24217b7c574bd8b37b Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Mon, 12 Feb 2024 19:19:02 -0500 Subject: [PATCH 04/26] Add keep alive test Signed-off-by: Derek Ho --- server/auth/types/authentication_type.test.ts | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/server/auth/types/authentication_type.test.ts b/server/auth/types/authentication_type.test.ts index d3f6026bb..53b65dac6 100644 --- a/server/auth/types/authentication_type.test.ts +++ b/server/auth/types/authentication_type.test.ts @@ -16,8 +16,20 @@ import { SecurityPluginConfigType } from '../..'; import { AuthenticationType } from './authentication_type'; import { httpServerMock } from '../../../../../src/core/server/mocks'; +import { + SessionStorageFactory, + SessionStorage, + OpenSearchDashboardsRequest, +} from '../../../../../src/core/server'; +import { SecuritySessionCookie } from '../../session/security_cookie'; + +const mockedNow = 0; +Date.now = jest.fn(() => mockedNow); class DummyAuthType extends AuthenticationType { + authNotRequired(request: OpenSearchDashboardsRequest): boolean { + return false; + } buildAuthHeaderFromCookie() {} getAdditionalAuthHeader() {} handleUnauthedRequest() {} @@ -35,6 +47,43 @@ class DummyAuthType extends AuthenticationType { } } +// Implementation of SessionStorage using browser's sessionStorage +class BrowserSessionStorage implements SessionStorage { + private readonly storageKey: string; + + constructor(storageKey: string) { + this.storageKey = storageKey; + } + + async get(): Promise { + const storedValue = sessionStorage.getItem(this.storageKey); + return storedValue ? JSON.parse(storedValue) : null; + } + + set(sessionValue: T): void { + const serializedValue = JSON.stringify(sessionValue); + sessionStorage.setItem(this.storageKey, serializedValue); + } + + clear(): void { + sessionStorage.removeItem(this.storageKey); + } +} + +// Implementation of SessionStorageFactory using the browser's sessionStorage +class BrowserSessionStorageFactory implements SessionStorageFactory { + private readonly storageKey: string; + + constructor(storageKey: string) { + this.storageKey = storageKey; + } + + // This method returns a new instance of the browser's SessionStorage for each request + asScoped(request: OpenSearchDashboardsRequest): SessionStorage { + return new BrowserSessionStorage(this.storageKey); + } +} + describe('test tenant header', () => { const config = { multitenancy: { @@ -106,4 +155,45 @@ describe('test tenant header', () => { const result = await dummyAuthType.authHandler(request, response, toolkit); expect(result.requestHeaders.securitytenant).toEqual('dummy_tenant'); }); + + it(`keepalive should not shorten the cookie expiry`, async () => { + const keepAliveConfig = { + multitenancy: { + enabled: true, + }, + auth: { + unauthenticated_routes: [] as string[], + }, + session: { + keepalive: true, + ttl: 1000, + }, + } as SecurityPluginConfigType; + const keepAliveDummyAuth = new DummyAuthType( + keepAliveConfig, + new BrowserSessionStorageFactory('security_cookie'), + router, + esClient, + coreSetup, + logger + ); + const testCookie: SecuritySessionCookie = { + credentials: { + authHeaderValueExtra: true, + }, + expiryTime: 2000, + }; + // Set cookie + sessionStorage.setItem('security_cookie', JSON.stringify(testCookie)); + const request = httpServerMock.createOpenSearchDashboardsRequest({ + path: '/internal/v1', + }); + const response = jest.fn(); + const toolkit = { + authenticated: jest.fn((value) => value), + }; + const _ = await keepAliveDummyAuth.authHandler(request, response, toolkit); + const cookieAfterRequest = sessionStorage.getItem('security_cookie'); + expect(JSON.parse(cookieAfterRequest!).expiryTime).toBe(2000); + }); }); From cf0b8578f7b2f3a0574c01155981aa14003eef3f Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Mon, 12 Feb 2024 19:24:23 -0500 Subject: [PATCH 05/26] Lint Signed-off-by: Derek Ho --- server/auth/types/authentication_type.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/server/auth/types/authentication_type.test.ts b/server/auth/types/authentication_type.test.ts index 53b65dac6..e9cdbefdf 100644 --- a/server/auth/types/authentication_type.test.ts +++ b/server/auth/types/authentication_type.test.ts @@ -13,6 +13,9 @@ * permissions and limitations under the License. */ +/* eslint-disable max-classes-per-file */ +// This file has extra classes used for testing + import { SecurityPluginConfigType } from '../..'; import { AuthenticationType } from './authentication_type'; import { httpServerMock } from '../../../../../src/core/server/mocks'; @@ -197,3 +200,5 @@ describe('test tenant header', () => { expect(JSON.parse(cookieAfterRequest!).expiryTime).toBe(2000); }); }); + +/* eslint-enable max-classes-per-file */ From bc96a3f2039f03c6521b3c8ceeb674223956f743 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Tue, 13 Feb 2024 10:18:12 -0500 Subject: [PATCH 06/26] Add openid valid cookie test Signed-off-by: Derek Ho --- server/auth/types/openid/openid_auth.test.ts | 39 ++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/server/auth/types/openid/openid_auth.test.ts b/server/auth/types/openid/openid_auth.test.ts index 55f730481..f5952b153 100644 --- a/server/auth/types/openid/openid_auth.test.ts +++ b/server/auth/types/openid/openid_auth.test.ts @@ -207,4 +207,43 @@ describe('test OpenId authHeaderValue', () => { expect(wreckHttpsOptions.cert).toBeUndefined(); expect(wreckHttpsOptions.passphrase).toBeUndefined(); }); + + test('Ensure 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; + const customConfig = { + openid: { + pfx: 'test/certs/keyStore.p12', + certificate: 'test/certs/cert.pem', + private_key: 'test/certs/private-key.pem', + passphrase: '', + header: 'authorization', + scope: [], + }, + }; + + const openidConfig = (customConfig as unknown) as SecurityPluginConfigType; + + const openIdAuthentication = new OpenIdAuthentication( + openidConfig, + sessionStorageFactory, + router, + esClient, + core, + logger + ); + const testCookie: SecuritySessionCookie = { + credentials: { + authHeaderValue: 'Bearer eyToken', + expiry_time: -1, + }, + expiryTime: 2000, + username: 'admin', + authType: 'openid', + }; + + expect(await openIdAuthentication.isValidCookie(testCookie, {})).toBe(true); + global.Date.now = realDateNow; + }); }); From 5883c809f0b89a12b7420ff884388545d94ff18f Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Tue, 13 Feb 2024 12:16:53 -0500 Subject: [PATCH 07/26] Push up stale work Signed-off-by: Derek Ho --- server/auth/types/authentication_type.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/server/auth/types/authentication_type.test.ts b/server/auth/types/authentication_type.test.ts index e9cdbefdf..f9eceee1f 100644 --- a/server/auth/types/authentication_type.test.ts +++ b/server/auth/types/authentication_type.test.ts @@ -26,9 +26,6 @@ import { } from '../../../../../src/core/server'; import { SecuritySessionCookie } from '../../session/security_cookie'; -const mockedNow = 0; -Date.now = jest.fn(() => mockedNow); - class DummyAuthType extends AuthenticationType { authNotRequired(request: OpenSearchDashboardsRequest): boolean { return false; @@ -160,6 +157,10 @@ describe('test tenant header', () => { }); it(`keepalive should not shorten the cookie expiry`, async () => { + const realDateNow = Date.now.bind(global.Date); + const dateNowStub = jest.fn(() => 0); + global.Date.now = dateNowStub; + const keepAliveConfig = { multitenancy: { enabled: true, @@ -198,6 +199,7 @@ describe('test tenant header', () => { const _ = await keepAliveDummyAuth.authHandler(request, response, toolkit); const cookieAfterRequest = sessionStorage.getItem('security_cookie'); expect(JSON.parse(cookieAfterRequest!).expiryTime).toBe(2000); + global.Date.now = realDateNow; }); }); From 278c40d295dba69c1dabfcda32b9f12cb321b765 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Tue, 13 Feb 2024 14:08:28 -0500 Subject: [PATCH 08/26] remove const assignment Signed-off-by: Derek Ho --- server/auth/types/authentication_type.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/auth/types/authentication_type.test.ts b/server/auth/types/authentication_type.test.ts index f9eceee1f..8897c622b 100644 --- a/server/auth/types/authentication_type.test.ts +++ b/server/auth/types/authentication_type.test.ts @@ -196,7 +196,7 @@ describe('test tenant header', () => { const toolkit = { authenticated: jest.fn((value) => value), }; - const _ = await keepAliveDummyAuth.authHandler(request, response, toolkit); + await keepAliveDummyAuth.authHandler(request, response, toolkit); const cookieAfterRequest = sessionStorage.getItem('security_cookie'); expect(JSON.parse(cookieAfterRequest!).expiryTime).toBe(2000); global.Date.now = realDateNow; From 364827fb13ecd66fa64257776e03442c352897d6 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Tue, 13 Feb 2024 17:04:30 -0500 Subject: [PATCH 09/26] Introduce abstract method to block jwt implementations from keep alive Signed-off-by: Derek Ho --- server/auth/types/authentication_type.test.ts | 3 +++ server/auth/types/authentication_type.ts | 3 ++- server/auth/types/basic/basic_auth.ts | 4 ++++ server/auth/types/jwt/jwt_auth.ts | 5 +++++ server/auth/types/multiple/multi_auth.ts | 14 +++++++++++++- server/auth/types/openid/openid_auth.ts | 5 +++++ server/auth/types/proxy/proxy_auth.ts | 4 ++++ server/auth/types/saml/saml_auth.ts | 5 +++++ 8 files changed, 41 insertions(+), 2 deletions(-) diff --git a/server/auth/types/authentication_type.test.ts b/server/auth/types/authentication_type.test.ts index 8897c622b..82a7c8a56 100644 --- a/server/auth/types/authentication_type.test.ts +++ b/server/auth/types/authentication_type.test.ts @@ -45,6 +45,9 @@ class DummyAuthType extends AuthenticationType { resolveTenant(): Promise { return Promise.resolve('dummy_tenant'); } + public supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise { + return Promise.resolve(true); + } } // Implementation of SessionStorage using browser's sessionStorage diff --git a/server/auth/types/authentication_type.ts b/server/auth/types/authentication_type.ts index c0f93a085..36c61ea32 100755 --- a/server/auth/types/authentication_type.ts +++ b/server/auth/types/authentication_type.ts @@ -159,7 +159,7 @@ export abstract class AuthenticationType implements IAuthenticationType { } // extend session expiration time - if (this.config.session.keepalive) { + if (this.supportsKeepAlive(request) && this.config.session.keepalive) { cookie!.expiryTime = Math.max(Date.now() + this.config.session.ttl, cookie.expiryTime || 0); this.sessionStorageFactory.asScoped(request).set(cookie!); } @@ -277,6 +277,7 @@ export abstract class AuthenticationType implements IAuthenticationType { request: OpenSearchDashboardsRequest, authInfo: any ): SecuritySessionCookie; + public abstract supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise; public abstract isValidCookie( cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest diff --git a/server/auth/types/basic/basic_auth.ts b/server/auth/types/basic/basic_auth.ts index af7a8727f..3e14b5f03 100644 --- a/server/auth/types/basic/basic_auth.ts +++ b/server/auth/types/basic/basic_auth.ts @@ -64,6 +64,10 @@ export class BasicAuthentication extends AuthenticationType { return request.headers[AUTH_HEADER_NAME] ? true : false; } + public async supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise { + return true; + } + async getAdditionalAuthHeader( request: OpenSearchDashboardsRequest ): Promise { diff --git a/server/auth/types/jwt/jwt_auth.ts b/server/auth/types/jwt/jwt_auth.ts index 43f17708c..57a6ee279 100644 --- a/server/auth/types/jwt/jwt_auth.ts +++ b/server/auth/types/jwt/jwt_auth.ts @@ -175,6 +175,11 @@ export class JwtAuthentication extends AuthenticationType { ); } + // JWT auth types should get expiry from the JWT, and not override this value + public async supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise { + return false; + } + handleUnauthedRequest( request: OpenSearchDashboardsRequest, response: LifecycleResponseFactory, diff --git a/server/auth/types/multiple/multi_auth.ts b/server/auth/types/multiple/multi_auth.ts index 8763eaa4f..b8328c7f6 100644 --- a/server/auth/types/multiple/multi_auth.ts +++ b/server/auth/types/multiple/multi_auth.ts @@ -22,7 +22,7 @@ import { LifecycleResponseFactory, AuthToolkit, } from '../../../../opensearch-dashboards/server'; -import { OpenSearchDashboardsResponse } from '../../../../../../src/core/server/http/router'; +import { OpenSearchDashboardsRequest, OpenSearchDashboardsResponse } from '../../../../../../src/core/server/http/router'; import { SecurityPluginConfigType } from '../../..'; import { AuthenticationType, IAuthenticationType } from '../authentication_type'; import { ANONYMOUS_AUTH_LOGIN, AuthType, LOGIN_PAGE_URI } from '../../../../common'; @@ -130,6 +130,18 @@ export class MultipleAuthentication extends AuthenticationType { return {}; } + public async supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise { + const cookie = await this.sessionStorageFactory.asScoped(request).get(); + const reqAuthType = cookie?.authType?.toLowerCase(); + + if (reqAuthType && this.authHandlers.has(reqAuthType)) { + return this.authHandlers.get(reqAuthType)!.supportsKeepAlive(request) + } else { + // default to true + return true; + } + } + async isValidCookie( cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest diff --git a/server/auth/types/openid/openid_auth.ts b/server/auth/types/openid/openid_auth.ts index 46225f0cd..3f51fa405 100644 --- a/server/auth/types/openid/openid_auth.ts +++ b/server/auth/types/openid/openid_auth.ts @@ -251,6 +251,11 @@ export class OpenIdAuthentication extends AuthenticationType { }; } + // JWT auth types should get expiry from the JWT, and not override this value + public async supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise { + return false; + } + // TODO: Add token expiration check here async isValidCookie( cookie: SecuritySessionCookie, diff --git a/server/auth/types/proxy/proxy_auth.ts b/server/auth/types/proxy/proxy_auth.ts index 346553a50..6309c05a0 100644 --- a/server/auth/types/proxy/proxy_auth.ts +++ b/server/auth/types/proxy/proxy_auth.ts @@ -73,6 +73,10 @@ export class ProxyAuthentication extends AuthenticationType { : false; } + public async supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise { + return true; + } + async getAdditionalAuthHeader(request: OpenSearchDashboardsRequest): Promise { const authHeaders: any = {}; const customProxyHeader = this.config.proxycache?.proxy_header; diff --git a/server/auth/types/saml/saml_auth.ts b/server/auth/types/saml/saml_auth.ts index 23c1ba9c2..6d2da965e 100644 --- a/server/auth/types/saml/saml_auth.ts +++ b/server/auth/types/saml/saml_auth.ts @@ -130,6 +130,11 @@ export class SamlAuthentication extends AuthenticationType { return {}; } + // JWT auth types should get expiry from the JWT, and not override this value + public async supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise { + return false; + } + getCookie(request: OpenSearchDashboardsRequest, authInfo: any): SecuritySessionCookie { const authorizationHeaderValue: string = request.headers[ SamlAuthentication.AUTH_HEADER_NAME From 8e3f4342c3bc0759bb34096b5096e7a3492cb6a0 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Tue, 13 Feb 2024 17:12:55 -0500 Subject: [PATCH 10/26] Lint Signed-off-by: Derek Ho --- server/auth/types/multiple/multi_auth.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/server/auth/types/multiple/multi_auth.ts b/server/auth/types/multiple/multi_auth.ts index b8328c7f6..701f192f1 100644 --- a/server/auth/types/multiple/multi_auth.ts +++ b/server/auth/types/multiple/multi_auth.ts @@ -22,7 +22,10 @@ import { LifecycleResponseFactory, AuthToolkit, } from '../../../../opensearch-dashboards/server'; -import { OpenSearchDashboardsRequest, OpenSearchDashboardsResponse } from '../../../../../../src/core/server/http/router'; +import { + OpenSearchDashboardsRequest, + OpenSearchDashboardsResponse, +} from '../../../../../../src/core/server/http/router'; import { SecurityPluginConfigType } from '../../..'; import { AuthenticationType, IAuthenticationType } from '../authentication_type'; import { ANONYMOUS_AUTH_LOGIN, AuthType, LOGIN_PAGE_URI } from '../../../../common'; @@ -135,7 +138,7 @@ export class MultipleAuthentication extends AuthenticationType { const reqAuthType = cookie?.authType?.toLowerCase(); if (reqAuthType && this.authHandlers.has(reqAuthType)) { - return this.authHandlers.get(reqAuthType)!.supportsKeepAlive(request) + return this.authHandlers.get(reqAuthType)!.supportsKeepAlive(request); } else { // default to true return true; From cf2b4ae5dcd56b52ac9da388741f55418be6d876 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Tue, 13 Feb 2024 17:41:37 -0500 Subject: [PATCH 11/26] Fix compile issues Signed-off-by: Derek Ho --- server/auth/types/multiple/multi_auth.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/server/auth/types/multiple/multi_auth.ts b/server/auth/types/multiple/multi_auth.ts index 701f192f1..2f7bb35cf 100644 --- a/server/auth/types/multiple/multi_auth.ts +++ b/server/auth/types/multiple/multi_auth.ts @@ -17,15 +17,12 @@ import { SessionStorageFactory, IRouter, ILegacyClusterClient, - OpenSearchDashboardsRequest, Logger, LifecycleResponseFactory, - AuthToolkit, -} from '../../../../opensearch-dashboards/server'; -import { OpenSearchDashboardsRequest, - OpenSearchDashboardsResponse, -} from '../../../../../../src/core/server/http/router'; + AuthToolkit, +} from 'opensearch-dashboards/server'; +import { OpenSearchDashboardsResponse } from '../../../../../../src/core/server/http/router'; import { SecurityPluginConfigType } from '../../..'; import { AuthenticationType, IAuthenticationType } from '../authentication_type'; import { ANONYMOUS_AUTH_LOGIN, AuthType, LOGIN_PAGE_URI } from '../../../../common'; From 52fd304f3681468d516adaaa9eacee73b1c9064e Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 15 Feb 2024 11:20:32 -0500 Subject: [PATCH 12/26] Fix warnings Signed-off-by: Derek Ho --- server/auth/types/authentication_type.ts | 7 ++++++- server/auth/types/basic/basic_auth.ts | 2 +- server/auth/types/proxy/proxy_auth.ts | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/server/auth/types/authentication_type.ts b/server/auth/types/authentication_type.ts index 36c61ea32..db148a9c3 100755 --- a/server/auth/types/authentication_type.ts +++ b/server/auth/types/authentication_type.ts @@ -159,7 +159,7 @@ export abstract class AuthenticationType implements IAuthenticationType { } // extend session expiration time - if (this.supportsKeepAlive(request) && this.config.session.keepalive) { + if (await this.supportsKeepAlive(request) && this.config.session.keepalive) { cookie!.expiryTime = Math.max(Date.now() + this.config.session.ttl, cookie.expiryTime || 0); this.sessionStorageFactory.asScoped(request).set(cookie!); } @@ -287,5 +287,10 @@ export abstract class AuthenticationType implements IAuthenticationType { response: LifecycleResponseFactory, toolkit: AuthToolkit ): IOpenSearchDashboardsResponse | AuthResult; + public abstract requestIncludesAuthInfo(request: OpenSearchDashboardsRequest): boolean; + public abstract buildAuthHeaderFromCookie( + cookie: SecuritySessionCookie, + request: OpenSearchDashboardsRequest + ): any public abstract init(): Promise; } diff --git a/server/auth/types/basic/basic_auth.ts b/server/auth/types/basic/basic_auth.ts index 3e14b5f03..85b3f2c05 100644 --- a/server/auth/types/basic/basic_auth.ts +++ b/server/auth/types/basic/basic_auth.ts @@ -137,7 +137,7 @@ export class BasicAuthentication extends AuthenticationType { } } - buildAuthHeaderFromCookie(cookie: SecuritySessionCookie): any { + buildAuthHeaderFromCookie(cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest): any { if (this.config.auth.anonymous_auth_enabled && cookie.isAnonymousAuth) { return {}; } diff --git a/server/auth/types/proxy/proxy_auth.ts b/server/auth/types/proxy/proxy_auth.ts index 6309c05a0..c35b9c8dd 100644 --- a/server/auth/types/proxy/proxy_auth.ts +++ b/server/auth/types/proxy/proxy_auth.ts @@ -139,7 +139,7 @@ export class ProxyAuthentication extends AuthenticationType { } } - buildAuthHeaderFromCookie(cookie: SecuritySessionCookie): any { + buildAuthHeaderFromCookie(cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest): any { const authHeaders: any = {}; if (get(cookie.credentials, this.userHeaderName)) { authHeaders[this.userHeaderName] = cookie.credentials[this.userHeaderName]; From b9528e8b7ef1f5921dd1d7bbec13a1cf08314a25 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 15 Feb 2024 11:28:49 -0500 Subject: [PATCH 13/26] Lint Signed-off-by: Derek Ho --- server/auth/types/authentication_type.ts | 4 ++-- server/auth/types/basic/basic_auth.ts | 5 ++++- server/auth/types/proxy/proxy_auth.ts | 5 ++++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/server/auth/types/authentication_type.ts b/server/auth/types/authentication_type.ts index db148a9c3..79c5216ba 100755 --- a/server/auth/types/authentication_type.ts +++ b/server/auth/types/authentication_type.ts @@ -159,7 +159,7 @@ export abstract class AuthenticationType implements IAuthenticationType { } // extend session expiration time - if (await this.supportsKeepAlive(request) && this.config.session.keepalive) { + if ((await this.supportsKeepAlive(request)) && this.config.session.keepalive) { cookie!.expiryTime = Math.max(Date.now() + this.config.session.ttl, cookie.expiryTime || 0); this.sessionStorageFactory.asScoped(request).set(cookie!); } @@ -291,6 +291,6 @@ export abstract class AuthenticationType implements IAuthenticationType { public abstract buildAuthHeaderFromCookie( cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest - ): any + ): any; public abstract init(): Promise; } diff --git a/server/auth/types/basic/basic_auth.ts b/server/auth/types/basic/basic_auth.ts index 85b3f2c05..260348e39 100644 --- a/server/auth/types/basic/basic_auth.ts +++ b/server/auth/types/basic/basic_auth.ts @@ -137,7 +137,10 @@ export class BasicAuthentication extends AuthenticationType { } } - buildAuthHeaderFromCookie(cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest): any { + buildAuthHeaderFromCookie( + cookie: SecuritySessionCookie, + request: OpenSearchDashboardsRequest + ): any { if (this.config.auth.anonymous_auth_enabled && cookie.isAnonymousAuth) { return {}; } diff --git a/server/auth/types/proxy/proxy_auth.ts b/server/auth/types/proxy/proxy_auth.ts index c35b9c8dd..761837901 100644 --- a/server/auth/types/proxy/proxy_auth.ts +++ b/server/auth/types/proxy/proxy_auth.ts @@ -139,7 +139,10 @@ export class ProxyAuthentication extends AuthenticationType { } } - buildAuthHeaderFromCookie(cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest): any { + buildAuthHeaderFromCookie( + cookie: SecuritySessionCookie, + request: OpenSearchDashboardsRequest + ): any { const authHeaders: any = {}; if (get(cookie.credentials, this.userHeaderName)) { authHeaders[this.userHeaderName] = cookie.credentials[this.userHeaderName]; From 1202368ec0cb19dcfddaf565a293b873f630d003 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Fri, 16 Feb 2024 11:33:35 -0500 Subject: [PATCH 14/26] Add test for JWT and fix warnings Signed-off-by: Derek Ho --- server/auth/types/authentication_type.test.ts | 50 +--- server/auth/types/authentication_type.ts | 2 +- server/auth/types/jwt/jwt_auth.ts | 7 +- server/auth/types/jwt/jwt_helper.test.ts | 215 +++++++++++++++--- server/auth/types/jwt/jwt_helper.ts | 34 +++ 5 files changed, 225 insertions(+), 83 deletions(-) create mode 100644 server/auth/types/jwt/jwt_helper.ts diff --git a/server/auth/types/authentication_type.test.ts b/server/auth/types/authentication_type.test.ts index 82a7c8a56..3cd73fa43 100644 --- a/server/auth/types/authentication_type.test.ts +++ b/server/auth/types/authentication_type.test.ts @@ -74,7 +74,7 @@ class BrowserSessionStorage implements SessionStorage { } // Implementation of SessionStorageFactory using the browser's sessionStorage -class BrowserSessionStorageFactory implements SessionStorageFactory { +export class BrowserSessionStorageFactory implements SessionStorageFactory { private readonly storageKey: string; constructor(storageKey: string) { @@ -158,52 +158,4 @@ describe('test tenant header', () => { const result = await dummyAuthType.authHandler(request, response, toolkit); expect(result.requestHeaders.securitytenant).toEqual('dummy_tenant'); }); - - it(`keepalive should not shorten the cookie expiry`, async () => { - const realDateNow = Date.now.bind(global.Date); - const dateNowStub = jest.fn(() => 0); - global.Date.now = dateNowStub; - - const keepAliveConfig = { - multitenancy: { - enabled: true, - }, - auth: { - unauthenticated_routes: [] as string[], - }, - session: { - keepalive: true, - ttl: 1000, - }, - } as SecurityPluginConfigType; - const keepAliveDummyAuth = new DummyAuthType( - keepAliveConfig, - new BrowserSessionStorageFactory('security_cookie'), - router, - esClient, - coreSetup, - logger - ); - const testCookie: SecuritySessionCookie = { - credentials: { - authHeaderValueExtra: true, - }, - expiryTime: 2000, - }; - // Set cookie - sessionStorage.setItem('security_cookie', JSON.stringify(testCookie)); - const request = httpServerMock.createOpenSearchDashboardsRequest({ - path: '/internal/v1', - }); - const response = jest.fn(); - const toolkit = { - authenticated: jest.fn((value) => value), - }; - await keepAliveDummyAuth.authHandler(request, response, toolkit); - const cookieAfterRequest = sessionStorage.getItem('security_cookie'); - expect(JSON.parse(cookieAfterRequest!).expiryTime).toBe(2000); - global.Date.now = realDateNow; - }); }); - -/* eslint-enable max-classes-per-file */ diff --git a/server/auth/types/authentication_type.ts b/server/auth/types/authentication_type.ts index 79c5216ba..867954364 100755 --- a/server/auth/types/authentication_type.ts +++ b/server/auth/types/authentication_type.ts @@ -160,7 +160,7 @@ export abstract class AuthenticationType implements IAuthenticationType { // extend session expiration time if ((await this.supportsKeepAlive(request)) && this.config.session.keepalive) { - cookie!.expiryTime = Math.max(Date.now() + this.config.session.ttl, cookie.expiryTime || 0); + cookie!.expiryTime = Date.now() + this.config.session.ttl; this.sessionStorageFactory.asScoped(request).set(cookie!); } // cookie is valid diff --git a/server/auth/types/jwt/jwt_auth.ts b/server/auth/types/jwt/jwt_auth.ts index 57a6ee279..62c9979db 100644 --- a/server/auth/types/jwt/jwt_auth.ts +++ b/server/auth/types/jwt/jwt_auth.ts @@ -35,6 +35,7 @@ import { getExtraAuthStorageValue, setExtraAuthStorage, } from '../../../session/cookie_splitter'; +import { getExpirationDate } from './jwt_helper'; export const JWT_DEFAULT_EXTRA_STORAGE_OPTIONS: ExtraAuthStorageOptions = { cookiePrefix: 'security_authentication_jwt', @@ -154,13 +155,17 @@ export class JwtAuthentication extends AuthenticationType { this.getBearerToken(request) || '', this.getExtraAuthStorageOptions() ); + return { username: authInfo.user_name, credentials: { authHeaderValueExtra: true, }, authType: this.type, - expiryTime: Date.now() + this.config.session.ttl, + expiryTime: getExpirationDate( + this.getBearerToken(request), + Date.now() + this.config.session.ttl + ), }; } diff --git a/server/auth/types/jwt/jwt_helper.test.ts b/server/auth/types/jwt/jwt_helper.test.ts index 73dd0e0ab..82267a459 100644 --- a/server/auth/types/jwt/jwt_helper.test.ts +++ b/server/auth/types/jwt/jwt_helper.test.ts @@ -14,7 +14,7 @@ */ import { getAuthenticationHandler } from '../../auth_handler_factory'; -import { JWT_DEFAULT_EXTRA_STORAGE_OPTIONS } from './jwt_auth'; +import { JWT_DEFAULT_EXTRA_STORAGE_OPTIONS, JwtAuthentication } from './jwt_auth'; import { CoreSetup, ILegacyClusterClient, @@ -27,41 +27,43 @@ import { SecuritySessionCookie } from '../../../session/security_cookie'; import { SecurityPluginConfigType } from '../../../index'; import { httpServerMock } from '../../../../../../src/core/server/http/http_server.mocks'; import { deflateValue } from '../../../utils/compression'; +import { getExpirationDate } from './jwt_helper'; -describe('test jwt auth library', () => { - const router: Partial = { post: (body) => {} }; - const core = { - http: { - basePath: { - serverBasePath: '/', - }, +const router: Partial = { post: (body) => {} }; +const core = { + http: { + basePath: { + serverBasePath: '/', }, - } as CoreSetup; - let esClient: ILegacyClusterClient; - const sessionStorageFactory: SessionStorageFactory = { - asScoped: jest.fn().mockImplementation(() => { - return { - server: { - states: { - add: jest.fn(), - }, + }, +} as CoreSetup; +let esClient: ILegacyClusterClient; + +const sessionStorageFactory: SessionStorageFactory = { + asScoped: jest.fn().mockImplementation(() => { + return { + server: { + states: { + add: jest.fn(), }, - }; - }), - }; - let logger: Logger; - - const cookieConfig: Partial = { - cookie: { - secure: false, - name: 'test_cookie_name', - password: 'secret', - ttl: 60 * 60 * 1000, - domain: null, - isSameSite: false, - }, - }; + }, + }; + }), +}; +let logger: Logger; +const cookieConfig: Partial = { + cookie: { + secure: false, + name: 'test_cookie_name', + password: 'secret', + ttl: 60 * 60 * 1000, + domain: null, + isSameSite: false, + }, +}; + +describe('test jwt auth library', () => { function getTestJWTAuthenticationHandlerWithConfig(config: SecurityPluginConfigType) { return getAuthenticationHandler( 'jwt', @@ -200,4 +202,153 @@ describe('test jwt auth library', () => { expect(headers).toEqual(expectedHeaders); }); +}); // re-import JWTAuth to change cookie splitter to a no-op + +/* eslint-disable no-shadow, @typescript-eslint/no-var-requires */ describe('JWT Expiry Tests', () => { + const setExtraAuthStorageMock = jest.fn(); + jest.resetModules(); + jest.doMock('../../../session/cookie_splitter', () => ({ + setExtraAuthStorage: setExtraAuthStorageMock, + })); + const { JwtAuthentication } = require('./jwt_auth'); + + const realDateNow = Date.now.bind(global.Date); + const dateNowStub = jest.fn(() => 0); + global.Date.now = dateNowStub; + const coreSetup = jest.fn(); + + afterAll(() => { + global.Date.now = realDateNow; + }); + + test('getExpirationDate', () => { + expect(getExpirationDate(undefined, 1000)).toBe(1000); // undefined + expect(getExpirationDate('', 1000)).toBe(1000); // empty string + expect(getExpirationDate('Bearer ', 1000)).toBe(1000); // empty token + expect(getExpirationDate('Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9', 1000)).toBe(1000); // malformed token with one part + expect( + getExpirationDate( + 'Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwiZXhwIjoxMzAwODE5MzgwMCwibmFtZSI6IkpvaG4gRG9lIiwicm9sZXMiOiJhZG1pbiJ9.ciW9WWtIaA-QJqy0flPSfMNQfGs9GEFqcNFY_LqrdII', + 1000 + ) + ).toBe(1000); // JWT with very far expiry defaults to lower value (ttl) + expect( + getExpirationDate( + 'Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwiZXhwIjo5MjA4NjkyMDAsIm5hbWUiOiJKb2huIERvZSIsInJvbGVzIjoiYWRtaW4ifQ.q8CtMfAeWOGDCGZ8UB8IIV-YM9hkDS8-pq0DSXh965I', + 920869200001 + ) + ).toBe(920869200000); // JWT expiry is lower than the default + expect( + getExpirationDate( + 'Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwibmFtZSI6IkpvaG4gRG9lIiwicm9sZXMiOiJhZG1pbiJ9.YDDoAKtA6wXd09zZ0aIUEt_IFvOwUd3rk4fW5aNppHM', + 1000 + ) + ).toBe(1000); // JWT doesn't include a exp claim + }); + + test('JWT auth type sets expiryTime of cookie JWT exp less than ttl', async () => { + const keepAliveConfig = { + multitenancy: { + enabled: false, + }, + auth: { + unauthenticated_routes: [] as string[], + }, + session: { + keepalive: true, + ttl: Infinity, + }, + jwt: { + url_param: 'awesome', + header: 'AUTHORIZATION', + extra_storage: { + cookie_prefix: 'testcookie', + additional_cookies: 2, + }, + }, + } as SecurityPluginConfigType; + + const jwtAuth = new JwtAuthentication( + keepAliveConfig, + sessionStorageFactory, + router, + esClient, + coreSetup, + logger + ); + + const requestWithHeaders = httpServerMock.createOpenSearchDashboardsRequest({ + path: '/internal/v1', + headers: { + authorization: + 'Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwiZXhwIjo5MjA4NjkyMDAsIm5hbWUiOiJKb2huIERvZSIsInJvbGVzIjoiYWRtaW4ifQ.q8CtMfAeWOGDCGZ8UB8IIV-YM9hkDS8-pq0DSXh965I', + }, + }); + const cookieFromHeaders = jwtAuth.getCookie(requestWithHeaders, {}); + expect(cookieFromHeaders.expiryTime!).toBe(920869200000); + + const requestWithJWTInUrl = httpServerMock.createOpenSearchDashboardsRequest({ + path: '/internal/v1', + query: { + awesome: + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwiZXhwIjo5MjA4NjkyMDAsIm5hbWUiOiJKb2huIERvZSIsInJvbGVzIjoiYWRtaW4ifQ.q8CtMfAeWOGDCGZ8UB8IIV-YM9hkDS8-pq0DSXh965I', + }, + }); + const cookieFromURL = jwtAuth.getCookie(requestWithJWTInUrl, {}); + expect(cookieFromURL.expiryTime!).toBe(920869200000); + }); + + test('JWT auth type sets expiryTime of cookie ttl less than JWT exp', async () => { + const keepAliveConfig = { + multitenancy: { + enabled: false, + }, + auth: { + unauthenticated_routes: [] as string[], + }, + session: { + keepalive: true, + ttl: 1000, + }, + jwt: { + url_param: 'awesome', + header: 'AUTHORIZATION', + extra_storage: { + cookie_prefix: 'testcookie', + additional_cookies: 2, + }, + }, + } as SecurityPluginConfigType; + + const jwtAuth = new JwtAuthentication( + keepAliveConfig, + sessionStorageFactory, + router, + esClient, + coreSetup, + logger + ); + + const requestWithHeaders = httpServerMock.createOpenSearchDashboardsRequest({ + path: '/internal/v1', + headers: { + authorization: + 'Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwiZXhwIjo5MjA4NjkyMDAsIm5hbWUiOiJKb2huIERvZSIsInJvbGVzIjoiYWRtaW4ifQ.q8CtMfAeWOGDCGZ8UB8IIV-YM9hkDS8-pq0DSXh965I', + }, + }); + const cookieFromHeaders = jwtAuth.getCookie(requestWithHeaders, {}); + expect(cookieFromHeaders.expiryTime!).toBe(1000); + + const requestWithJWTInUrl = httpServerMock.createOpenSearchDashboardsRequest({ + path: '/internal/v1', + query: { + awesome: + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwiZXhwIjo5MjA4NjkyMDAsIm5hbWUiOiJKb2huIERvZSIsInJvbGVzIjoiYWRtaW4ifQ.q8CtMfAeWOGDCGZ8UB8IIV-YM9hkDS8-pq0DSXh965I', + }, + }); + const cookieFromURL = jwtAuth.getCookie(requestWithJWTInUrl, {}); + expect(cookieFromURL.expiryTime!).toBe(1000); + }); + + /* eslint-enable no-shadow, @typescript-eslint/no-var-requires */ }); diff --git a/server/auth/types/jwt/jwt_helper.ts b/server/auth/types/jwt/jwt_helper.ts new file mode 100644 index 000000000..8837a3039 --- /dev/null +++ b/server/auth/types/jwt/jwt_helper.ts @@ -0,0 +1,34 @@ +/* + * Copyright OpenSearch Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +export function getExpirationDate(authHeader: string | undefined, defaultExpiry: number) { + if (!authHeader) { + return defaultExpiry; + } else if (authHeader.startsWith('Bearer ')) { + // Extract the token part by splitting the string and taking the second part + const token = authHeader.split(' ')[1]; + const parts = token.split('.'); + if (parts.length !== 3) { + return defaultExpiry; + } + const claim = JSON.parse(Buffer.from(parts[1], 'base64').toString()); + if (claim.exp) { + return Math.min(claim.exp * 1000, defaultExpiry); + } + return defaultExpiry; + } else { + return defaultExpiry; + } +} From 934ca7603a5b6b890e8e6b1fff95ed07bafd8f0f Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Fri, 16 Feb 2024 11:40:32 -0500 Subject: [PATCH 15/26] Remove unused testing code Signed-off-by: Derek Ho --- server/auth/types/authentication_type.test.ts | 92 ++++++++++--------- 1 file changed, 47 insertions(+), 45 deletions(-) diff --git a/server/auth/types/authentication_type.test.ts b/server/auth/types/authentication_type.test.ts index 3cd73fa43..a06af70fd 100644 --- a/server/auth/types/authentication_type.test.ts +++ b/server/auth/types/authentication_type.test.ts @@ -13,17 +13,10 @@ * permissions and limitations under the License. */ -/* eslint-disable max-classes-per-file */ -// This file has extra classes used for testing - import { SecurityPluginConfigType } from '../..'; import { AuthenticationType } from './authentication_type'; import { httpServerMock } from '../../../../../src/core/server/mocks'; -import { - SessionStorageFactory, - SessionStorage, - OpenSearchDashboardsRequest, -} from '../../../../../src/core/server'; +import { OpenSearchDashboardsRequest } from '../../../../../src/core/server'; import { SecuritySessionCookie } from '../../session/security_cookie'; class DummyAuthType extends AuthenticationType { @@ -50,43 +43,6 @@ class DummyAuthType extends AuthenticationType { } } -// Implementation of SessionStorage using browser's sessionStorage -class BrowserSessionStorage implements SessionStorage { - private readonly storageKey: string; - - constructor(storageKey: string) { - this.storageKey = storageKey; - } - - async get(): Promise { - const storedValue = sessionStorage.getItem(this.storageKey); - return storedValue ? JSON.parse(storedValue) : null; - } - - set(sessionValue: T): void { - const serializedValue = JSON.stringify(sessionValue); - sessionStorage.setItem(this.storageKey, serializedValue); - } - - clear(): void { - sessionStorage.removeItem(this.storageKey); - } -} - -// Implementation of SessionStorageFactory using the browser's sessionStorage -export class BrowserSessionStorageFactory implements SessionStorageFactory { - private readonly storageKey: string; - - constructor(storageKey: string) { - this.storageKey = storageKey; - } - - // This method returns a new instance of the browser's SessionStorage for each request - asScoped(request: OpenSearchDashboardsRequest): SessionStorage { - return new BrowserSessionStorage(this.storageKey); - } -} - describe('test tenant header', () => { const config = { multitenancy: { @@ -158,4 +114,50 @@ describe('test tenant header', () => { const result = await dummyAuthType.authHandler(request, response, toolkit); expect(result.requestHeaders.securitytenant).toEqual('dummy_tenant'); }); + + it(`keepalive should not shorten the cookie expiry`, async () => { + const realDateNow = Date.now.bind(global.Date); + const dateNowStub = jest.fn(() => 0); + global.Date.now = dateNowStub; + + const keepAliveConfig = { + multitenancy: { + enabled: true, + }, + auth: { + unauthenticated_routes: [] as string[], + }, + session: { + keepalive: true, + ttl: 1000, + }, + } as SecurityPluginConfigType; + const keepAliveDummyAuth = new DummyAuthType( + keepAliveConfig, + new BrowserSessionStorageFactory('security_cookie'), + router, + esClient, + coreSetup, + logger + ); + const testCookie: SecuritySessionCookie = { + credentials: { + authHeaderValueExtra: true, + }, + expiryTime: 2000, + }; + // Set cookie + sessionStorage.setItem('security_cookie', JSON.stringify(testCookie)); + const request = httpServerMock.createOpenSearchDashboardsRequest({ + path: '/internal/v1', + }); + const response = jest.fn(); + const toolkit = { + authenticated: jest.fn((value) => value), + }; + await keepAliveDummyAuth.authHandler(request, response, toolkit); + const cookieAfterRequest = sessionStorage.getItem('security_cookie'); + expect(JSON.parse(cookieAfterRequest!).expiryTime).toBe(2000); + global.Date.now = realDateNow; + }); }); From 804cce78e642f3e303a6e3b5b88ae1028860f723 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Fri, 16 Feb 2024 11:43:48 -0500 Subject: [PATCH 16/26] Remove test after PR changed Signed-off-by: Derek Ho --- server/auth/types/authentication_type.test.ts | 47 ------------------- 1 file changed, 47 deletions(-) diff --git a/server/auth/types/authentication_type.test.ts b/server/auth/types/authentication_type.test.ts index a06af70fd..3b592f309 100644 --- a/server/auth/types/authentication_type.test.ts +++ b/server/auth/types/authentication_type.test.ts @@ -17,7 +17,6 @@ import { SecurityPluginConfigType } from '../..'; import { AuthenticationType } from './authentication_type'; import { httpServerMock } from '../../../../../src/core/server/mocks'; import { OpenSearchDashboardsRequest } from '../../../../../src/core/server'; -import { SecuritySessionCookie } from '../../session/security_cookie'; class DummyAuthType extends AuthenticationType { authNotRequired(request: OpenSearchDashboardsRequest): boolean { @@ -114,50 +113,4 @@ describe('test tenant header', () => { const result = await dummyAuthType.authHandler(request, response, toolkit); expect(result.requestHeaders.securitytenant).toEqual('dummy_tenant'); }); - - it(`keepalive should not shorten the cookie expiry`, async () => { - const realDateNow = Date.now.bind(global.Date); - const dateNowStub = jest.fn(() => 0); - global.Date.now = dateNowStub; - - const keepAliveConfig = { - multitenancy: { - enabled: true, - }, - auth: { - unauthenticated_routes: [] as string[], - }, - session: { - keepalive: true, - ttl: 1000, - }, - } as SecurityPluginConfigType; - const keepAliveDummyAuth = new DummyAuthType( - keepAliveConfig, - new BrowserSessionStorageFactory('security_cookie'), - router, - esClient, - coreSetup, - logger - ); - const testCookie: SecuritySessionCookie = { - credentials: { - authHeaderValueExtra: true, - }, - expiryTime: 2000, - }; - // Set cookie - sessionStorage.setItem('security_cookie', JSON.stringify(testCookie)); - const request = httpServerMock.createOpenSearchDashboardsRequest({ - path: '/internal/v1', - }); - const response = jest.fn(); - const toolkit = { - authenticated: jest.fn((value) => value), - }; - await keepAliveDummyAuth.authHandler(request, response, toolkit); - const cookieAfterRequest = sessionStorage.getItem('security_cookie'); - expect(JSON.parse(cookieAfterRequest!).expiryTime).toBe(2000); - global.Date.now = realDateNow; - }); }); From 529ded150a21270bf4dac1e4322c2536938ec8b4 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Mon, 19 Feb 2024 15:48:20 -0500 Subject: [PATCH 17/26] Refactor keep alive for JWT and no-op for SAML, OIDC Signed-off-by: Derek Ho --- server/auth/types/authentication_type.test.ts | 3 --- server/auth/types/authentication_type.ts | 9 ++++++--- server/auth/types/basic/basic_auth.ts | 9 +++++---- server/auth/types/jwt/jwt_auth.ts | 6 +++--- server/auth/types/multiple/multi_auth.ts | 10 ++++------ server/auth/types/openid/openid_auth.ts | 6 +++--- server/auth/types/proxy/proxy_auth.ts | 4 ++-- server/auth/types/saml/saml_auth.ts | 6 +++--- 8 files changed, 26 insertions(+), 27 deletions(-) diff --git a/server/auth/types/authentication_type.test.ts b/server/auth/types/authentication_type.test.ts index 3b592f309..fdf0397cf 100644 --- a/server/auth/types/authentication_type.test.ts +++ b/server/auth/types/authentication_type.test.ts @@ -37,9 +37,6 @@ class DummyAuthType extends AuthenticationType { resolveTenant(): Promise { return Promise.resolve('dummy_tenant'); } - public supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise { - return Promise.resolve(true); - } } describe('test tenant header', () => { diff --git a/server/auth/types/authentication_type.ts b/server/auth/types/authentication_type.ts index 867954364..5501dd88c 100755 --- a/server/auth/types/authentication_type.ts +++ b/server/auth/types/authentication_type.ts @@ -159,8 +159,8 @@ export abstract class AuthenticationType implements IAuthenticationType { } // extend session expiration time - if ((await this.supportsKeepAlive(request)) && this.config.session.keepalive) { - cookie!.expiryTime = Date.now() + this.config.session.ttl; + if (this.config.session.keepalive) { + cookie!.expiryTime = this.getKeepAliveExpiry(cookie!, request); this.sessionStorageFactory.asScoped(request).set(cookie!); } // cookie is valid @@ -277,7 +277,10 @@ export abstract class AuthenticationType implements IAuthenticationType { request: OpenSearchDashboardsRequest, authInfo: any ): SecuritySessionCookie; - public abstract supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise; + public abstract getKeepAliveExpiry( + cookie: SecuritySessionCookie, + request: OpenSearchDashboardsRequest, + ): number; public abstract isValidCookie( cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest diff --git a/server/auth/types/basic/basic_auth.ts b/server/auth/types/basic/basic_auth.ts index 260348e39..80034b64d 100644 --- a/server/auth/types/basic/basic_auth.ts +++ b/server/auth/types/basic/basic_auth.ts @@ -64,10 +64,6 @@ export class BasicAuthentication extends AuthenticationType { return request.headers[AUTH_HEADER_NAME] ? true : false; } - public async supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise { - return true; - } - async getAdditionalAuthHeader( request: OpenSearchDashboardsRequest ): Promise { @@ -105,6 +101,11 @@ export class BasicAuthentication extends AuthenticationType { ); } + getKeepAliveExpiry(cookie: SecuritySessionCookie, + request: OpenSearchDashboardsRequest): number { + return Date.now() + this.config.session.ttl; + } + handleUnauthedRequest( request: OpenSearchDashboardsRequest, response: LifecycleResponseFactory, diff --git a/server/auth/types/jwt/jwt_auth.ts b/server/auth/types/jwt/jwt_auth.ts index 62c9979db..bd3aaee3c 100644 --- a/server/auth/types/jwt/jwt_auth.ts +++ b/server/auth/types/jwt/jwt_auth.ts @@ -180,9 +180,9 @@ export class JwtAuthentication extends AuthenticationType { ); } - // JWT auth types should get expiry from the JWT, and not override this value - public async supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise { - return false; + getKeepAliveExpiry(cookie: SecuritySessionCookie, + request: OpenSearchDashboardsRequest): number { + return getExpirationDate(this.buildAuthHeaderFromCookie(cookie,request)[this.authHeaderName], Date.now() + this.config.session.ttl); } handleUnauthedRequest( diff --git a/server/auth/types/multiple/multi_auth.ts b/server/auth/types/multiple/multi_auth.ts index 2f7bb35cf..55160596b 100644 --- a/server/auth/types/multiple/multi_auth.ts +++ b/server/auth/types/multiple/multi_auth.ts @@ -130,15 +130,13 @@ export class MultipleAuthentication extends AuthenticationType { return {}; } - public async supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise { - const cookie = await this.sessionStorageFactory.asScoped(request).get(); + getKeepAliveExpiry(cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest): number { const reqAuthType = cookie?.authType?.toLowerCase(); - if (reqAuthType && this.authHandlers.has(reqAuthType)) { - return this.authHandlers.get(reqAuthType)!.supportsKeepAlive(request); + return this.authHandlers.get(reqAuthType)!.getKeepAliveExpiry(cookie, request); } else { - // default to true - return true; + // default to TTL setting + return Date.now() + this.config.session.ttl } } diff --git a/server/auth/types/openid/openid_auth.ts b/server/auth/types/openid/openid_auth.ts index 3f51fa405..b23d06157 100644 --- a/server/auth/types/openid/openid_auth.ts +++ b/server/auth/types/openid/openid_auth.ts @@ -251,9 +251,9 @@ export class OpenIdAuthentication extends AuthenticationType { }; } - // JWT auth types should get expiry from the JWT, and not override this value - public async supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise { - return false; + // OIDC expiry time is set by the IDP and refreshed via refreshTokens + getKeepAliveExpiry(cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest): number { + return cookie.expiryTime! } // TODO: Add token expiration check here diff --git a/server/auth/types/proxy/proxy_auth.ts b/server/auth/types/proxy/proxy_auth.ts index 761837901..4e571f4fd 100644 --- a/server/auth/types/proxy/proxy_auth.ts +++ b/server/auth/types/proxy/proxy_auth.ts @@ -73,8 +73,8 @@ export class ProxyAuthentication extends AuthenticationType { : false; } - public async supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise { - return true; + public getKeepAliveExpiry(cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest): number { + return Date.now() + this.config.session.ttl; } async getAdditionalAuthHeader(request: OpenSearchDashboardsRequest): Promise { diff --git a/server/auth/types/saml/saml_auth.ts b/server/auth/types/saml/saml_auth.ts index 6d2da965e..9461e7187 100644 --- a/server/auth/types/saml/saml_auth.ts +++ b/server/auth/types/saml/saml_auth.ts @@ -130,9 +130,9 @@ export class SamlAuthentication extends AuthenticationType { return {}; } - // JWT auth types should get expiry from the JWT, and not override this value - public async supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise { - return false; + // SAML expiry time is set by the IDP and returned via the security backend. Keep alive should not modify this value. + public getKeepAliveExpiry(cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest): number { + return cookie.expiryTime! } getCookie(request: OpenSearchDashboardsRequest, authInfo: any): SecuritySessionCookie { From 0549a52cba9fa2075ac7ea9d0f8a7c1268d18c51 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Mon, 19 Feb 2024 15:52:10 -0500 Subject: [PATCH 18/26] Lint Signed-off-by: Derek Ho --- server/auth/types/authentication_type.ts | 2 +- server/auth/types/basic/basic_auth.ts | 3 +-- server/auth/types/jwt/jwt_auth.ts | 8 +++++--- server/auth/types/multiple/multi_auth.ts | 7 +++++-- server/auth/types/openid/openid_auth.ts | 7 +++++-- server/auth/types/proxy/proxy_auth.ts | 5 ++++- server/auth/types/saml/saml_auth.ts | 7 +++++-- 7 files changed, 26 insertions(+), 13 deletions(-) diff --git a/server/auth/types/authentication_type.ts b/server/auth/types/authentication_type.ts index 5501dd88c..b48409cb1 100755 --- a/server/auth/types/authentication_type.ts +++ b/server/auth/types/authentication_type.ts @@ -279,7 +279,7 @@ export abstract class AuthenticationType implements IAuthenticationType { ): SecuritySessionCookie; public abstract getKeepAliveExpiry( cookie: SecuritySessionCookie, - request: OpenSearchDashboardsRequest, + request: OpenSearchDashboardsRequest ): number; public abstract isValidCookie( cookie: SecuritySessionCookie, diff --git a/server/auth/types/basic/basic_auth.ts b/server/auth/types/basic/basic_auth.ts index 80034b64d..b502339c8 100644 --- a/server/auth/types/basic/basic_auth.ts +++ b/server/auth/types/basic/basic_auth.ts @@ -101,8 +101,7 @@ export class BasicAuthentication extends AuthenticationType { ); } - getKeepAliveExpiry(cookie: SecuritySessionCookie, - request: OpenSearchDashboardsRequest): number { + getKeepAliveExpiry(cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest): number { return Date.now() + this.config.session.ttl; } diff --git a/server/auth/types/jwt/jwt_auth.ts b/server/auth/types/jwt/jwt_auth.ts index bd3aaee3c..da1c13dc6 100644 --- a/server/auth/types/jwt/jwt_auth.ts +++ b/server/auth/types/jwt/jwt_auth.ts @@ -180,9 +180,11 @@ export class JwtAuthentication extends AuthenticationType { ); } - getKeepAliveExpiry(cookie: SecuritySessionCookie, - request: OpenSearchDashboardsRequest): number { - return getExpirationDate(this.buildAuthHeaderFromCookie(cookie,request)[this.authHeaderName], Date.now() + this.config.session.ttl); + getKeepAliveExpiry(cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest): number { + return getExpirationDate( + this.buildAuthHeaderFromCookie(cookie, request)[this.authHeaderName], + Date.now() + this.config.session.ttl + ); } handleUnauthedRequest( diff --git a/server/auth/types/multiple/multi_auth.ts b/server/auth/types/multiple/multi_auth.ts index 55160596b..2bcc5e1ba 100644 --- a/server/auth/types/multiple/multi_auth.ts +++ b/server/auth/types/multiple/multi_auth.ts @@ -130,13 +130,16 @@ export class MultipleAuthentication extends AuthenticationType { return {}; } - getKeepAliveExpiry(cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest): number { + getKeepAliveExpiry( + cookie: SecuritySessionCookie, + request: OpenSearchDashboardsRequest + ): number { const reqAuthType = cookie?.authType?.toLowerCase(); if (reqAuthType && this.authHandlers.has(reqAuthType)) { return this.authHandlers.get(reqAuthType)!.getKeepAliveExpiry(cookie, request); } else { // default to TTL setting - return Date.now() + this.config.session.ttl + return Date.now() + this.config.session.ttl; } } diff --git a/server/auth/types/openid/openid_auth.ts b/server/auth/types/openid/openid_auth.ts index b23d06157..61088dfd3 100644 --- a/server/auth/types/openid/openid_auth.ts +++ b/server/auth/types/openid/openid_auth.ts @@ -252,8 +252,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! + getKeepAliveExpiry( + cookie: SecuritySessionCookie, + request: OpenSearchDashboardsRequest + ): number { + return cookie.expiryTime!; } // TODO: Add token expiration check here diff --git a/server/auth/types/proxy/proxy_auth.ts b/server/auth/types/proxy/proxy_auth.ts index 4e571f4fd..b2f0c4019 100644 --- a/server/auth/types/proxy/proxy_auth.ts +++ b/server/auth/types/proxy/proxy_auth.ts @@ -73,7 +73,10 @@ export class ProxyAuthentication extends AuthenticationType { : false; } - public getKeepAliveExpiry(cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest): number { + public getKeepAliveExpiry( + cookie: SecuritySessionCookie, + request: OpenSearchDashboardsRequest + ): number { return Date.now() + this.config.session.ttl; } diff --git a/server/auth/types/saml/saml_auth.ts b/server/auth/types/saml/saml_auth.ts index 9461e7187..1a58efb1a 100644 --- a/server/auth/types/saml/saml_auth.ts +++ b/server/auth/types/saml/saml_auth.ts @@ -131,8 +131,11 @@ export class SamlAuthentication extends AuthenticationType { } // SAML expiry time is set by the IDP and returned via the security backend. Keep alive should not modify this value. - public getKeepAliveExpiry(cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest): number { - return cookie.expiryTime! + public getKeepAliveExpiry( + cookie: SecuritySessionCookie, + request: OpenSearchDashboardsRequest + ): number { + return cookie.expiryTime!; } getCookie(request: OpenSearchDashboardsRequest, authInfo: any): SecuritySessionCookie { From ace02e27d5b75e7a2f283bced7d7d57d516a49f3 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Mon, 19 Feb 2024 16:09:15 -0500 Subject: [PATCH 19/26] Refactor test for readability Signed-off-by: Derek Ho --- server/auth/types/jwt/jwt_helper.test.ts | 41 +++++++++--------------- 1 file changed, 15 insertions(+), 26 deletions(-) diff --git a/server/auth/types/jwt/jwt_helper.test.ts b/server/auth/types/jwt/jwt_helper.test.ts index 82267a459..3061d0e08 100644 --- a/server/auth/types/jwt/jwt_helper.test.ts +++ b/server/auth/types/jwt/jwt_helper.test.ts @@ -29,6 +29,14 @@ import { httpServerMock } from '../../../../../../src/core/server/http/http_serv import { deflateValue } from '../../../utils/compression'; import { getExpirationDate } from './jwt_helper'; +// TODO: add dependency to a JWT decode/encode library for easier test writing and reading +const JWT_TEST = + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwiZXhwIjo5MjA4NjkyMDAsIm5hbWUiOiJKb2huIERvZSIsInJvbGVzIjoiYWRtaW4ifQ.q8CtMfAeWOGDCGZ8UB8IIV-YM9hkDS8-pq0DSXh965I'; // A test JWT used for testing various scenarios +const JWT_TEST_NO_EXP = + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwibmFtZSI6IkpvaG4gRG9lIiwicm9sZXMiOiJhZG1pbiJ9.YDDoAKtA6wXd09zZ0aIUEt_IFvOwUd3rk4fW5aNppHM'; // A test JWT with no exp claim +const JWT_TEST_FAR_EXP = + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwiZXhwIjoxMzAwODE5MzgwMCwibmFtZSI6IkpvaG4gRG9lIiwicm9sZXMiOiJhZG1pbiJ9.ciW9WWtIaA-QJqy0flPSfMNQfGs9GEFqcNFY_LqrdII'; // A test JWT with a far off exp claim + const router: Partial = { post: (body) => {} }; const core = { http: { @@ -226,24 +234,9 @@ describe('test jwt auth library', () => { expect(getExpirationDate('', 1000)).toBe(1000); // empty string expect(getExpirationDate('Bearer ', 1000)).toBe(1000); // empty token expect(getExpirationDate('Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9', 1000)).toBe(1000); // malformed token with one part - expect( - getExpirationDate( - 'Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwiZXhwIjoxMzAwODE5MzgwMCwibmFtZSI6IkpvaG4gRG9lIiwicm9sZXMiOiJhZG1pbiJ9.ciW9WWtIaA-QJqy0flPSfMNQfGs9GEFqcNFY_LqrdII', - 1000 - ) - ).toBe(1000); // JWT with very far expiry defaults to lower value (ttl) - expect( - getExpirationDate( - 'Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwiZXhwIjo5MjA4NjkyMDAsIm5hbWUiOiJKb2huIERvZSIsInJvbGVzIjoiYWRtaW4ifQ.q8CtMfAeWOGDCGZ8UB8IIV-YM9hkDS8-pq0DSXh965I', - 920869200001 - ) - ).toBe(920869200000); // JWT expiry is lower than the default - expect( - getExpirationDate( - 'Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwibmFtZSI6IkpvaG4gRG9lIiwicm9sZXMiOiJhZG1pbiJ9.YDDoAKtA6wXd09zZ0aIUEt_IFvOwUd3rk4fW5aNppHM', - 1000 - ) - ).toBe(1000); // JWT doesn't include a exp claim + expect(getExpirationDate(`Bearer ${JWT_TEST_FAR_EXP}`, 1000)).toBe(1000); // JWT with very far expiry defaults to lower value (ttl) + expect(getExpirationDate(`Bearer ${JWT_TEST}`, 920869200001)).toBe(920869200000); // JWT expiry is lower than the default + expect(getExpirationDate(`Bearer ${JWT_TEST_NO_EXP}`, 1000)).toBe(1000); // JWT doesn't include a exp claim }); test('JWT auth type sets expiryTime of cookie JWT exp less than ttl', async () => { @@ -280,8 +273,7 @@ describe('test jwt auth library', () => { const requestWithHeaders = httpServerMock.createOpenSearchDashboardsRequest({ path: '/internal/v1', headers: { - authorization: - 'Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwiZXhwIjo5MjA4NjkyMDAsIm5hbWUiOiJKb2huIERvZSIsInJvbGVzIjoiYWRtaW4ifQ.q8CtMfAeWOGDCGZ8UB8IIV-YM9hkDS8-pq0DSXh965I', + authorization: `Bearer ${JWT_TEST}`, }, }); const cookieFromHeaders = jwtAuth.getCookie(requestWithHeaders, {}); @@ -290,8 +282,7 @@ describe('test jwt auth library', () => { const requestWithJWTInUrl = httpServerMock.createOpenSearchDashboardsRequest({ path: '/internal/v1', query: { - awesome: - 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwiZXhwIjo5MjA4NjkyMDAsIm5hbWUiOiJKb2huIERvZSIsInJvbGVzIjoiYWRtaW4ifQ.q8CtMfAeWOGDCGZ8UB8IIV-YM9hkDS8-pq0DSXh965I', + awesome: JWT_TEST, }, }); const cookieFromURL = jwtAuth.getCookie(requestWithJWTInUrl, {}); @@ -332,8 +323,7 @@ describe('test jwt auth library', () => { const requestWithHeaders = httpServerMock.createOpenSearchDashboardsRequest({ path: '/internal/v1', headers: { - authorization: - 'Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwiZXhwIjo5MjA4NjkyMDAsIm5hbWUiOiJKb2huIERvZSIsInJvbGVzIjoiYWRtaW4ifQ.q8CtMfAeWOGDCGZ8UB8IIV-YM9hkDS8-pq0DSXh965I', + authorization: `Bearer ${JWT_TEST}`, }, }); const cookieFromHeaders = jwtAuth.getCookie(requestWithHeaders, {}); @@ -342,8 +332,7 @@ describe('test jwt auth library', () => { const requestWithJWTInUrl = httpServerMock.createOpenSearchDashboardsRequest({ path: '/internal/v1', query: { - awesome: - 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwiZXhwIjo5MjA4NjkyMDAsIm5hbWUiOiJKb2huIERvZSIsInJvbGVzIjoiYWRtaW4ifQ.q8CtMfAeWOGDCGZ8UB8IIV-YM9hkDS8-pq0DSXh965I', + awesome: JWT_TEST, }, }); const cookieFromURL = jwtAuth.getCookie(requestWithJWTInUrl, {}); From 6b76c4c2cf784fdb270a44b6809e3e4982cf5195 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Mon, 19 Feb 2024 17:02:59 -0500 Subject: [PATCH 20/26] Add tests for each auth type Signed-off-by: Derek Ho --- server/auth/types/basic/basic_auth.test.ts | 78 +++++++++++++++++++ server/auth/types/jwt/jwt_auth.ts | 1 + server/auth/types/jwt/jwt_helper.test.ts | 66 ++++++++++++++++ server/auth/types/multiple/multi_auth.test.ts | 78 +++++++++++++++++++ server/auth/types/openid/openid_auth.test.ts | 32 ++++++++ server/auth/types/proxy/proxy_auth.test.ts | 78 +++++++++++++++++++ server/auth/types/saml/saml_auth.test.ts | 24 ++++++ 7 files changed, 357 insertions(+) create mode 100644 server/auth/types/basic/basic_auth.test.ts create mode 100644 server/auth/types/multiple/multi_auth.test.ts create mode 100644 server/auth/types/proxy/proxy_auth.test.ts diff --git a/server/auth/types/basic/basic_auth.test.ts b/server/auth/types/basic/basic_auth.test.ts new file mode 100644 index 000000000..216b672af --- /dev/null +++ b/server/auth/types/basic/basic_auth.test.ts @@ -0,0 +1,78 @@ +/* + * Copyright OpenSearch Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +import { httpServerMock } from '../../../../../../src/core/server/http/http_server.mocks'; + +import { SecurityPluginConfigType } from '../../../index'; +import { SecuritySessionCookie } from '../../../session/security_cookie'; +import { + IRouter, + CoreSetup, + ILegacyClusterClient, + Logger, + SessionStorageFactory, +} from '../../../../../../src/core/server'; +import { BasicAuthentication } from './basic_auth'; + +describe('Basic auth tests', () => { + let router: IRouter; + let core: CoreSetup; + let esClient: ILegacyClusterClient; + let sessionStorageFactory: SessionStorageFactory; + let logger: Logger; + + // Consistent with auth_handler_factory.test.ts + beforeEach(() => {}); + + const config = ({ + saml: { + extra_storage: { + cookie_prefix: 'testcookie', + additional_cookies: 5, + }, + }, + session: { + ttl: 1000, + }, + } as unknown) as SecurityPluginConfigType; + + test('getKeepAliveExpiry', () => { + const realDateNow = Date.now.bind(global.Date); + const dateNowStub = jest.fn(() => 0); + global.Date.now = dateNowStub; + const proxyAuthentication = new BasicAuthentication( + config, + sessionStorageFactory, + router, + esClient, + core, + logger + ); + + const cookie: SecuritySessionCookie = { + credentials: { + authHeaderValueExtra: true, + }, + expiryTime: 0, + }; + + const request = httpServerMock.createOpenSearchDashboardsRequest({ + path: '/internal/v1', + }); + + expect(proxyAuthentication.getKeepAliveExpiry(cookie, request)).toBe(1000); + global.Date.now = realDateNow; + }); +}); diff --git a/server/auth/types/jwt/jwt_auth.ts b/server/auth/types/jwt/jwt_auth.ts index da1c13dc6..30def6044 100644 --- a/server/auth/types/jwt/jwt_auth.ts +++ b/server/auth/types/jwt/jwt_auth.ts @@ -181,6 +181,7 @@ export class JwtAuthentication extends AuthenticationType { } getKeepAliveExpiry(cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest): number { + console.log(this.buildAuthHeaderFromCookie(cookie, request)[this.authHeaderName]); return getExpirationDate( this.buildAuthHeaderFromCookie(cookie, request)[this.authHeaderName], Date.now() + this.config.session.ttl diff --git a/server/auth/types/jwt/jwt_helper.test.ts b/server/auth/types/jwt/jwt_helper.test.ts index 3061d0e08..b6e68efe9 100644 --- a/server/auth/types/jwt/jwt_helper.test.ts +++ b/server/auth/types/jwt/jwt_helper.test.ts @@ -37,6 +37,9 @@ const JWT_TEST_NO_EXP = const JWT_TEST_FAR_EXP = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwiZXhwIjoxMzAwODE5MzgwMCwibmFtZSI6IkpvaG4gRG9lIiwicm9sZXMiOiJhZG1pbiJ9.ciW9WWtIaA-QJqy0flPSfMNQfGs9GEFqcNFY_LqrdII'; // A test JWT with a far off exp claim +const JWT_TEST_NEAR_EXP = + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJleGFtcGxlLmNvbSIsInN1YiI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwiZXhwIjo1MCwibmFtZSI6IkpvaG4gRG9lIiwicm9sZXMiOiJhZG1pbiJ9.96_h7V_OrO-bHzhh1DUIOJ2_J2sEI8y--cjBOBonk2o'; // A test JWT with exp claim of 50 + const router: Partial = { post: (body) => {} }; const core = { http: { @@ -339,5 +342,68 @@ describe('test jwt auth library', () => { expect(cookieFromURL.expiryTime!).toBe(1000); }); + test('getKeepAliveExpiry', () => { + const keepAliveConfig = { + multitenancy: { + enabled: false, + }, + auth: { + unauthenticated_routes: [] as string[], + }, + session: { + keepalive: true, + ttl: 100000, + }, + jwt: { + url_param: 'awesome', + header: 'AUTHORIZATION', + extra_storage: { + cookie_prefix: 'testcookie', + additional_cookies: 2, + }, + }, + } as SecurityPluginConfigType; + + const jwtAuth = new JwtAuthentication( + keepAliveConfig, + sessionStorageFactory, + router, + esClient, + coreSetup, + logger + ); + + const requestWithHeaders = httpServerMock.createOpenSearchDashboardsRequest({ + path: '/internal/v1', + headers: { + authorization: `Bearer ${JWT_TEST}`, + }, + }); + + const cookie: SecuritySessionCookie = { + credentials: {}, + expiryTime: 1000, + }; + + // Mock the method with a JWT with far exp + jest.spyOn(jwtAuth, 'buildAuthHeaderFromCookie').mockReturnValue({ + authorization: `Bearer ${JWT_TEST_FAR_EXP}`, + }); + + // getKeepAliveExpiry takes on the value of the ttl, since it is less than the exp claim * 1000 + expect(jwtAuth.getKeepAliveExpiry(cookie, requestWithHeaders)).toBe(100000); + + // Mock the method with a JWT with near exp + jest.spyOn(jwtAuth, 'buildAuthHeaderFromCookie').mockReturnValue({ + authorization: `Bearer ${JWT_TEST_NEAR_EXP}`, + }); + + // getKeepAliveExpiry takes on the value of the exp claim * 1000, since it is less than the ttl + expect(jwtAuth.getKeepAliveExpiry(cookie, requestWithHeaders)).toBe(50000); + + // Restore the original method implementation after the test + jwtAuth.buildAuthHeaderFromCookie.mockRestore(); + }); + /* eslint-enable no-shadow, @typescript-eslint/no-var-requires */ }); diff --git a/server/auth/types/multiple/multi_auth.test.ts b/server/auth/types/multiple/multi_auth.test.ts new file mode 100644 index 000000000..841082eb5 --- /dev/null +++ b/server/auth/types/multiple/multi_auth.test.ts @@ -0,0 +1,78 @@ +/* + * Copyright OpenSearch Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +import { httpServerMock } from '../../../../../../src/core/server/http/http_server.mocks'; + +import { OpenSearchDashboardsRequest } from '../../../../../../src/core/server/http/router'; + +import { SecurityPluginConfigType } from '../../../index'; +import { SecuritySessionCookie } from '../../../session/security_cookie'; +import { deflateValue } from '../../../utils/compression'; +import { + IRouter, + CoreSetup, + ILegacyClusterClient, + Logger, + SessionStorageFactory, +} from '../../../../../../src/core/server'; +import { MultipleAuthentication } from './multi_auth'; + +describe('Multi auth tests', () => { + let router: IRouter; + let core: CoreSetup; + let esClient: ILegacyClusterClient; + let sessionStorageFactory: SessionStorageFactory; + let logger: Logger; + + // Consistent with auth_handler_factory.test.ts + beforeEach(() => {}); + + const config = ({ + session: { + ttl: 1000, + }, + auth: { + type: 'basic', + }, + } as unknown) as SecurityPluginConfigType; + + test('getKeepAliveExpiry', () => { + const realDateNow = Date.now.bind(global.Date); + const dateNowStub = jest.fn(() => 0); + global.Date.now = dateNowStub; + const proxyAuthentication = new MultipleAuthentication( + config, + sessionStorageFactory, + router, + esClient, + core, + logger + ); + + const cookie: SecuritySessionCookie = { + credentials: { + authHeaderValueExtra: true, + }, + expiryTime: 1000, + }; + + const request = httpServerMock.createOpenSearchDashboardsRequest({ + path: '/internal/v1', + }); + + expect(proxyAuthentication.getKeepAliveExpiry(cookie, request)).toBe(1000); // Multi auth using basic auth's implementation + global.Date.now = realDateNow; + }); +}); diff --git a/server/auth/types/openid/openid_auth.test.ts b/server/auth/types/openid/openid_auth.test.ts index f5952b153..f64e1ae9d 100644 --- a/server/auth/types/openid/openid_auth.test.ts +++ b/server/auth/types/openid/openid_auth.test.ts @@ -246,4 +246,36 @@ describe('test OpenId authHeaderValue', () => { expect(await openIdAuthentication.isValidCookie(testCookie, {})).toBe(true); global.Date.now = realDateNow; }); + + test('getKeepAliveExpiry', () => { + const customConfig = { + openid: { + pfx: 'test/certs/keyStore.p12', + certificate: 'test/certs/cert.pem', + private_key: 'test/certs/private-key.pem', + passphrase: '', + header: 'authorization', + scope: [], + }, + }; + + const openidConfig = (customConfig as unknown) as SecurityPluginConfigType; + + const openIdAuthentication = new OpenIdAuthentication( + openidConfig, + sessionStorageFactory, + router, + esClient, + core, + logger + ); + const testCookie: SecuritySessionCookie = { + credentials: { + authHeaderValue: 'Bearer eyToken', + }, + expiryTime: 1000, + }; + + expect(openIdAuthentication.getKeepAliveExpiry(testCookie, {})).toBe(1000); + }); }); diff --git a/server/auth/types/proxy/proxy_auth.test.ts b/server/auth/types/proxy/proxy_auth.test.ts new file mode 100644 index 000000000..babda58ea --- /dev/null +++ b/server/auth/types/proxy/proxy_auth.test.ts @@ -0,0 +1,78 @@ +/* + * Copyright OpenSearch Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +import { httpServerMock } from '../../../../../../src/core/server/http/http_server.mocks'; + +import { SecurityPluginConfigType } from '../../../index'; +import { SecuritySessionCookie } from '../../../session/security_cookie'; +import { + IRouter, + CoreSetup, + ILegacyClusterClient, + Logger, + SessionStorageFactory, +} from '../../../../../../src/core/server'; +import { ProxyAuthentication } from './proxy_auth'; + +describe('Proxy auth tests', () => { + let router: IRouter; + let core: CoreSetup; + let esClient: ILegacyClusterClient; + let sessionStorageFactory: SessionStorageFactory; + let logger: Logger; + + // Consistent with auth_handler_factory.test.ts + beforeEach(() => {}); + + const config = ({ + saml: { + extra_storage: { + cookie_prefix: 'testcookie', + additional_cookies: 5, + }, + }, + session: { + ttl: 1000, + }, + } as unknown) as SecurityPluginConfigType; + + test('getKeepAliveExpiry', () => { + const realDateNow = Date.now.bind(global.Date); + const dateNowStub = jest.fn(() => 0); + global.Date.now = dateNowStub; + const proxyAuthentication = new ProxyAuthentication( + config, + sessionStorageFactory, + router, + esClient, + core, + logger + ); + + const cookie: SecuritySessionCookie = { + credentials: { + authHeaderValueExtra: true, + }, + expiryTime: 1000, + }; + + const request = httpServerMock.createOpenSearchDashboardsRequest({ + path: '/internal/v1', + }); + + expect(proxyAuthentication.getKeepAliveExpiry(cookie, request)).toBe(1000); + global.Date.now = realDateNow; + }); +}); diff --git a/server/auth/types/saml/saml_auth.test.ts b/server/auth/types/saml/saml_auth.test.ts index 355d8b28c..7b1b47112 100644 --- a/server/auth/types/saml/saml_auth.test.ts +++ b/server/auth/types/saml/saml_auth.test.ts @@ -115,4 +115,28 @@ describe('test SAML authHeaderValue', () => { expect(headers).toEqual(expectedHeaders); }); + + test('getKeepAliveExpiry', () => { + const samlAuthentication = new SamlAuthentication( + config, + sessionStorageFactory, + router, + esClient, + core, + logger + ); + + const cookie: SecuritySessionCookie = { + credentials: { + authHeaderValueExtra: true, + }, + expiryTime: 1000, + }; + + const request = httpServerMock.createOpenSearchDashboardsRequest({ + path: '/internal/v1', + }); + + expect(samlAuthentication.getKeepAliveExpiry(cookie, request)).toBe(1000); + }); }); From 275d5692581418a5fb0698403d8e43aba205a78e Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Mon, 19 Feb 2024 17:07:12 -0500 Subject: [PATCH 21/26] Fix test to make it clear that it is taking value from date.now + ttl Signed-off-by: Derek Ho --- server/auth/types/multiple/multi_auth.test.ts | 2 +- server/auth/types/proxy/proxy_auth.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/auth/types/multiple/multi_auth.test.ts b/server/auth/types/multiple/multi_auth.test.ts index 841082eb5..8cde43580 100644 --- a/server/auth/types/multiple/multi_auth.test.ts +++ b/server/auth/types/multiple/multi_auth.test.ts @@ -65,7 +65,7 @@ describe('Multi auth tests', () => { credentials: { authHeaderValueExtra: true, }, - expiryTime: 1000, + expiryTime: 0, }; const request = httpServerMock.createOpenSearchDashboardsRequest({ diff --git a/server/auth/types/proxy/proxy_auth.test.ts b/server/auth/types/proxy/proxy_auth.test.ts index babda58ea..2f1dd6ab8 100644 --- a/server/auth/types/proxy/proxy_auth.test.ts +++ b/server/auth/types/proxy/proxy_auth.test.ts @@ -65,7 +65,7 @@ describe('Proxy auth tests', () => { credentials: { authHeaderValueExtra: true, }, - expiryTime: 1000, + expiryTime: 0, }; const request = httpServerMock.createOpenSearchDashboardsRequest({ From 40878d88bf5037fb122500fecead5d1849cc2c78 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 21 Feb 2024 09:02:31 -0500 Subject: [PATCH 22/26] Remove console log Signed-off-by: Derek Ho --- server/auth/types/jwt/jwt_auth.ts | 1 - server/auth/types/jwt/jwt_helper.test.ts | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/auth/types/jwt/jwt_auth.ts b/server/auth/types/jwt/jwt_auth.ts index 30def6044..da1c13dc6 100644 --- a/server/auth/types/jwt/jwt_auth.ts +++ b/server/auth/types/jwt/jwt_auth.ts @@ -181,7 +181,6 @@ export class JwtAuthentication extends AuthenticationType { } getKeepAliveExpiry(cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest): number { - console.log(this.buildAuthHeaderFromCookie(cookie, request)[this.authHeaderName]); return getExpirationDate( this.buildAuthHeaderFromCookie(cookie, request)[this.authHeaderName], Date.now() + this.config.session.ttl diff --git a/server/auth/types/jwt/jwt_helper.test.ts b/server/auth/types/jwt/jwt_helper.test.ts index b6e68efe9..1d32a755b 100644 --- a/server/auth/types/jwt/jwt_helper.test.ts +++ b/server/auth/types/jwt/jwt_helper.test.ts @@ -215,7 +215,8 @@ describe('test jwt auth library', () => { }); }); // re-import JWTAuth to change cookie splitter to a no-op -/* eslint-disable no-shadow, @typescript-eslint/no-var-requires */ describe('JWT Expiry Tests', () => { +/* eslint-disable no-shadow, @typescript-eslint/no-var-requires */ +describe('JWT Expiry Tests', () => { const setExtraAuthStorageMock = jest.fn(); jest.resetModules(); jest.doMock('../../../session/cookie_splitter', () => ({ From 30ff1694974a92e15931697b05cdec0e6ca9e428 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 21 Feb 2024 11:52:41 -0500 Subject: [PATCH 23/26] PR feedback Signed-off-by: Derek Ho --- server/auth/types/authentication_type.ts | 11 +++++++---- server/auth/types/basic/basic_auth.ts | 4 ---- server/auth/types/jwt/jwt_helper.ts | 4 +--- server/auth/types/proxy/proxy_auth.ts | 7 ------- 4 files changed, 8 insertions(+), 18 deletions(-) diff --git a/server/auth/types/authentication_type.ts b/server/auth/types/authentication_type.ts index b48409cb1..0afb1978d 100755 --- a/server/auth/types/authentication_type.ts +++ b/server/auth/types/authentication_type.ts @@ -266,6 +266,13 @@ export abstract class AuthenticationType implements IAuthenticationType { }); } + public getKeepAliveExpiry( + cookie: SecuritySessionCookie, + request: OpenSearchDashboardsRequest + ): number { + return Date.now() + this.config.session.ttl; + }; + isPageRequest(request: OpenSearchDashboardsRequest) { const path = request.url.pathname || '/'; return path.startsWith('/app/') || path === '/' || path.startsWith('/goto/'); @@ -277,10 +284,6 @@ export abstract class AuthenticationType implements IAuthenticationType { request: OpenSearchDashboardsRequest, authInfo: any ): SecuritySessionCookie; - public abstract getKeepAliveExpiry( - cookie: SecuritySessionCookie, - request: OpenSearchDashboardsRequest - ): number; public abstract isValidCookie( cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest diff --git a/server/auth/types/basic/basic_auth.ts b/server/auth/types/basic/basic_auth.ts index b502339c8..f21f86827 100644 --- a/server/auth/types/basic/basic_auth.ts +++ b/server/auth/types/basic/basic_auth.ts @@ -101,10 +101,6 @@ export class BasicAuthentication extends AuthenticationType { ); } - getKeepAliveExpiry(cookie: SecuritySessionCookie, request: OpenSearchDashboardsRequest): number { - return Date.now() + this.config.session.ttl; - } - handleUnauthedRequest( request: OpenSearchDashboardsRequest, response: LifecycleResponseFactory, diff --git a/server/auth/types/jwt/jwt_helper.ts b/server/auth/types/jwt/jwt_helper.ts index 8837a3039..e5d35487e 100644 --- a/server/auth/types/jwt/jwt_helper.ts +++ b/server/auth/types/jwt/jwt_helper.ts @@ -27,8 +27,6 @@ export function getExpirationDate(authHeader: string | undefined, defaultExpiry: if (claim.exp) { return Math.min(claim.exp * 1000, defaultExpiry); } - return defaultExpiry; - } else { - return defaultExpiry; } + return defaultExpiry; } diff --git a/server/auth/types/proxy/proxy_auth.ts b/server/auth/types/proxy/proxy_auth.ts index b2f0c4019..b3a97d5ed 100644 --- a/server/auth/types/proxy/proxy_auth.ts +++ b/server/auth/types/proxy/proxy_auth.ts @@ -73,13 +73,6 @@ export class ProxyAuthentication extends AuthenticationType { : false; } - public getKeepAliveExpiry( - cookie: SecuritySessionCookie, - request: OpenSearchDashboardsRequest - ): number { - return Date.now() + this.config.session.ttl; - } - async getAdditionalAuthHeader(request: OpenSearchDashboardsRequest): Promise { const authHeaders: any = {}; const customProxyHeader = this.config.proxycache?.proxy_header; From 4c1e448b83b99bde4364d7cccdf2e26506860a00 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 21 Feb 2024 12:04:41 -0500 Subject: [PATCH 24/26] Lint Signed-off-by: Derek Ho --- server/auth/types/authentication_type.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/auth/types/authentication_type.ts b/server/auth/types/authentication_type.ts index 0afb1978d..8bde376ac 100755 --- a/server/auth/types/authentication_type.ts +++ b/server/auth/types/authentication_type.ts @@ -271,7 +271,7 @@ export abstract class AuthenticationType implements IAuthenticationType { request: OpenSearchDashboardsRequest ): number { return Date.now() + this.config.session.ttl; - }; + } isPageRequest(request: OpenSearchDashboardsRequest) { const path = request.url.pathname || '/'; From 1c542fc6d858659030fa605fbdb8c44eb53ca36a Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 22 Feb 2024 10:11:22 -0500 Subject: [PATCH 25/26] Test fixes Signed-off-by: Derek Ho --- server/auth/types/basic/basic_auth.test.ts | 13 ++------ server/auth/types/jwt/jwt_helper.test.ts | 30 ++++--------------- server/auth/types/multiple/multi_auth.test.ts | 6 ---- server/auth/types/openid/openid_auth.test.ts | 22 +++----------- server/auth/types/proxy/proxy_auth.test.ts | 9 ------ 5 files changed, 12 insertions(+), 68 deletions(-) diff --git a/server/auth/types/basic/basic_auth.test.ts b/server/auth/types/basic/basic_auth.test.ts index 216b672af..4dfb38e86 100644 --- a/server/auth/types/basic/basic_auth.test.ts +++ b/server/auth/types/basic/basic_auth.test.ts @@ -33,20 +33,11 @@ describe('Basic auth tests', () => { let sessionStorageFactory: SessionStorageFactory; let logger: Logger; - // Consistent with auth_handler_factory.test.ts - beforeEach(() => {}); - - const config = ({ - saml: { - extra_storage: { - cookie_prefix: 'testcookie', - additional_cookies: 5, - }, - }, + const config = { session: { ttl: 1000, }, - } as unknown) as SecurityPluginConfigType; + } as SecurityPluginConfigType; test('getKeepAliveExpiry', () => { const realDateNow = Date.now.bind(global.Date); diff --git a/server/auth/types/jwt/jwt_helper.test.ts b/server/auth/types/jwt/jwt_helper.test.ts index 1d32a755b..f82621a62 100644 --- a/server/auth/types/jwt/jwt_helper.test.ts +++ b/server/auth/types/jwt/jwt_helper.test.ts @@ -244,13 +244,7 @@ describe('JWT Expiry Tests', () => { }); test('JWT auth type sets expiryTime of cookie JWT exp less than ttl', async () => { - const keepAliveConfig = { - multitenancy: { - enabled: false, - }, - auth: { - unauthenticated_routes: [] as string[], - }, + const infiniteTTLConfig = { session: { keepalive: true, ttl: Infinity, @@ -266,7 +260,7 @@ describe('JWT Expiry Tests', () => { } as SecurityPluginConfigType; const jwtAuth = new JwtAuthentication( - keepAliveConfig, + infiniteTTLConfig, sessionStorageFactory, router, esClient, @@ -294,13 +288,7 @@ describe('JWT Expiry Tests', () => { }); test('JWT auth type sets expiryTime of cookie ttl less than JWT exp', async () => { - const keepAliveConfig = { - multitenancy: { - enabled: false, - }, - auth: { - unauthenticated_routes: [] as string[], - }, + const lowTTLConfig = { session: { keepalive: true, ttl: 1000, @@ -316,7 +304,7 @@ describe('JWT Expiry Tests', () => { } as SecurityPluginConfigType; const jwtAuth = new JwtAuthentication( - keepAliveConfig, + lowTTLConfig, sessionStorageFactory, router, esClient, @@ -344,13 +332,7 @@ describe('JWT Expiry Tests', () => { }); test('getKeepAliveExpiry', () => { - const keepAliveConfig = { - multitenancy: { - enabled: false, - }, - auth: { - unauthenticated_routes: [] as string[], - }, + const jwtConfig = { session: { keepalive: true, ttl: 100000, @@ -366,7 +348,7 @@ describe('JWT Expiry Tests', () => { } as SecurityPluginConfigType; const jwtAuth = new JwtAuthentication( - keepAliveConfig, + jwtConfig, sessionStorageFactory, router, esClient, diff --git a/server/auth/types/multiple/multi_auth.test.ts b/server/auth/types/multiple/multi_auth.test.ts index 8cde43580..685b06cc7 100644 --- a/server/auth/types/multiple/multi_auth.test.ts +++ b/server/auth/types/multiple/multi_auth.test.ts @@ -15,11 +15,8 @@ import { httpServerMock } from '../../../../../../src/core/server/http/http_server.mocks'; -import { OpenSearchDashboardsRequest } from '../../../../../../src/core/server/http/router'; - import { SecurityPluginConfigType } from '../../../index'; import { SecuritySessionCookie } from '../../../session/security_cookie'; -import { deflateValue } from '../../../utils/compression'; import { IRouter, CoreSetup, @@ -36,9 +33,6 @@ describe('Multi auth tests', () => { let sessionStorageFactory: SessionStorageFactory; let logger: Logger; - // Consistent with auth_handler_factory.test.ts - beforeEach(() => {}); - const config = ({ session: { ttl: 1000, diff --git a/server/auth/types/openid/openid_auth.test.ts b/server/auth/types/openid/openid_auth.test.ts index f64e1ae9d..c8cc839e7 100644 --- a/server/auth/types/openid/openid_auth.test.ts +++ b/server/auth/types/openid/openid_auth.test.ts @@ -212,21 +212,14 @@ describe('test OpenId authHeaderValue', () => { const realDateNow = Date.now.bind(global.Date); const dateNowStub = jest.fn(() => 0); global.Date.now = dateNowStub; - const customConfig = { + const oidcConfig: unknown = { openid: { - pfx: 'test/certs/keyStore.p12', - certificate: 'test/certs/cert.pem', - private_key: 'test/certs/private-key.pem', - passphrase: '', - header: 'authorization', scope: [], }, }; - const openidConfig = (customConfig as unknown) as SecurityPluginConfigType; - const openIdAuthentication = new OpenIdAuthentication( - openidConfig, + oidcConfig as SecurityPluginConfigType, sessionStorageFactory, router, esClient, @@ -248,21 +241,14 @@ describe('test OpenId authHeaderValue', () => { }); test('getKeepAliveExpiry', () => { - const customConfig = { + const oidcConfig: unknown = { openid: { - pfx: 'test/certs/keyStore.p12', - certificate: 'test/certs/cert.pem', - private_key: 'test/certs/private-key.pem', - passphrase: '', - header: 'authorization', scope: [], }, }; - const openidConfig = (customConfig as unknown) as SecurityPluginConfigType; - const openIdAuthentication = new OpenIdAuthentication( - openidConfig, + oidcConfig as SecurityPluginConfigType, sessionStorageFactory, router, esClient, diff --git a/server/auth/types/proxy/proxy_auth.test.ts b/server/auth/types/proxy/proxy_auth.test.ts index 2f1dd6ab8..c25eaf645 100644 --- a/server/auth/types/proxy/proxy_auth.test.ts +++ b/server/auth/types/proxy/proxy_auth.test.ts @@ -33,16 +33,7 @@ describe('Proxy auth tests', () => { let sessionStorageFactory: SessionStorageFactory; let logger: Logger; - // Consistent with auth_handler_factory.test.ts - beforeEach(() => {}); - const config = ({ - saml: { - extra_storage: { - cookie_prefix: 'testcookie', - additional_cookies: 5, - }, - }, session: { ttl: 1000, }, From d426aae95767c5e7dce38b5c507b83aa56fe845d Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 22 Feb 2024 10:41:54 -0500 Subject: [PATCH 26/26] Fix variable names Signed-off-by: Derek Ho --- server/auth/types/basic/basic_auth.test.ts | 4 ++-- server/auth/types/multiple/multi_auth.test.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/auth/types/basic/basic_auth.test.ts b/server/auth/types/basic/basic_auth.test.ts index 4dfb38e86..4698db354 100644 --- a/server/auth/types/basic/basic_auth.test.ts +++ b/server/auth/types/basic/basic_auth.test.ts @@ -43,7 +43,7 @@ describe('Basic auth tests', () => { const realDateNow = Date.now.bind(global.Date); const dateNowStub = jest.fn(() => 0); global.Date.now = dateNowStub; - const proxyAuthentication = new BasicAuthentication( + const basicAuthentication = new BasicAuthentication( config, sessionStorageFactory, router, @@ -63,7 +63,7 @@ describe('Basic auth tests', () => { path: '/internal/v1', }); - expect(proxyAuthentication.getKeepAliveExpiry(cookie, request)).toBe(1000); + expect(basicAuthentication.getKeepAliveExpiry(cookie, request)).toBe(1000); global.Date.now = realDateNow; }); }); diff --git a/server/auth/types/multiple/multi_auth.test.ts b/server/auth/types/multiple/multi_auth.test.ts index 685b06cc7..fb61d4ed8 100644 --- a/server/auth/types/multiple/multi_auth.test.ts +++ b/server/auth/types/multiple/multi_auth.test.ts @@ -46,7 +46,7 @@ describe('Multi auth tests', () => { const realDateNow = Date.now.bind(global.Date); const dateNowStub = jest.fn(() => 0); global.Date.now = dateNowStub; - const proxyAuthentication = new MultipleAuthentication( + const multiAuthentication = new MultipleAuthentication( config, sessionStorageFactory, router, @@ -66,7 +66,7 @@ describe('Multi auth tests', () => { path: '/internal/v1', }); - expect(proxyAuthentication.getKeepAliveExpiry(cookie, request)).toBe(1000); // Multi auth using basic auth's implementation + expect(multiAuthentication.getKeepAliveExpiry(cookie, request)).toBe(1000); // Multi auth using basic auth's implementation global.Date.now = realDateNow; }); });