Fix cookie expiry issues from IDP/JWT auth methods, disables keepalive for JWT/IDP

server/auth/types/authentication_type.test.ts

@@ -13,11 +13,23 @@
  *   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 { SecuritySessionCookie } from '../../session/security_cookie';
 
 class DummyAuthType extends AuthenticationType {
+  authNotRequired(request: OpenSearchDashboardsRequest): boolean {
+    return false;
+  }
   buildAuthHeaderFromCookie() {}
   getAdditionalAuthHeader() {}
   handleUnauthedRequest() {}
@@ -33,6 +45,46 @@ class DummyAuthType extends AuthenticationType {
   resolveTenant(): Promise<string | undefined> {
     return Promise.resolve('dummy_tenant');
   }
+  public supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise<boolean> {
+    return Promise.resolve(true);
+  }
+}
+
+// Implementation of SessionStorage using browser's sessionStorage
+class BrowserSessionStorage<T> implements SessionStorage<T> {
+  private readonly storageKey: string;
+
+  constructor(storageKey: string) {
+    this.storageKey = storageKey;
+  }
+
+  async get(): Promise<T | null> {
+    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<T> implements SessionStorageFactory<T> {
+  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<T> {
+    return new BrowserSessionStorage<T>(this.storageKey);
+  }
 }
 
 describe('test tenant header', () => {
@@ -106,4 +158,52 @@ 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 */

server/auth/types/authentication_type.ts

@@ -159,8 +159,8 @@ export abstract class AuthenticationType implements IAuthenticationType {
       }
 
       // extend session expiration time
-      if (this.config.session.keepalive) {
-        cookie!.expiryTime = Date.now() + this.config.session.ttl;
+      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!);
       }
       // cookie is valid
@@ -277,6 +277,7 @@ export abstract class AuthenticationType implements IAuthenticationType {
     request: OpenSearchDashboardsRequest,
     authInfo: any
   ): SecuritySessionCookie;
+  public abstract supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise<boolean>;
   public abstract isValidCookie(
     cookie: SecuritySessionCookie,
     request: OpenSearchDashboardsRequest

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<boolean> {
+    return true;
+  }
+
   async getAdditionalAuthHeader(
     request: OpenSearchDashboardsRequest<unknown, unknown, unknown, any>
   ): Promise<any> {

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<boolean> {
+    return false;
+  }
+
   handleUnauthedRequest(
     request: OpenSearchDashboardsRequest,
     response: LifecycleResponseFactory,

server/auth/types/multiple/multi_auth.ts

@@ -22,7 +22,10 @@ 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 +133,18 @@ export class MultipleAuthentication extends AuthenticationType {
     return {};
   }
 
+  public async supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise<boolean> {
+    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

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;
+  });
 });

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<boolean> {
+    return false;
+  }
+
   // TODO: Add token expiration check here
   async isValidCookie(
     cookie: SecuritySessionCookie,
@@ -260,13 +265,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 +294,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,

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, {

server/auth/types/proxy/proxy_auth.ts

@@ -73,6 +73,10 @@ export class ProxyAuthentication extends AuthenticationType {
       : false;
   }
 
+  public async supportsKeepAlive(request: OpenSearchDashboardsRequest): Promise<boolean> {
+    return true;
+  }
+
   async getAdditionalAuthHeader(request: OpenSearchDashboardsRequest): Promise<any> {
     const authHeaders: any = {};
     const customProxyHeader = this.config.proxycache?.proxy_header;

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<boolean> {
+    return false;
+  }
+
   getCookie(request: OpenSearchDashboardsRequest, authInfo: any): SecuritySessionCookie {
     const authorizationHeaderValue: string = request.headers[
       SamlAuthentication.AUTH_HEADER_NAME