From 2f7d744ed528357dd6c4be0ac7d218970eecf12c Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Tue, 10 Dec 2024 19:07:12 -0500 Subject: [PATCH] Preserve Query in nextUrl during openid login redirect (#2140) (#2159) * Preserve Query in nextUrl during openid login redirect * Add release notes for 2.18 release (#2137) * Added Unittest Test Suite for OpenID handling unauthorized calls * Revert "Add release notes for 2.18 release (#2137)" * Fix ES-Lint issues * Fixing OIDC E2E tests and added a new one * Reverting line ending changes from last commit * Reverting changes to E2E OIDC Setup and added an alternative fix for the tests * Fix ESLint issues and try fixing new tests * Prevent Tenant Selection Popup to show during new E2E Test --------- (cherry picked from commit ded4012bf19374503c85275a3e2e0a741fe573d4) Signed-off-by: Sebastian Klein Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] Co-authored-by: Derek Ho Co-authored-by: SKLEIN3 Co-authored-by: Darshit Chanpura --- server/auth/types/openid/openid_auth.test.ts | 218 ++++++++++++++++++- server/auth/types/openid/openid_auth.ts | 24 +- server/auth/types/openid/routes.ts | 2 +- test/cypress/e2e/oidc/oidc_auth_test.spec.js | 33 ++- 4 files changed, 262 insertions(+), 15 deletions(-) diff --git a/server/auth/types/openid/openid_auth.test.ts b/server/auth/types/openid/openid_auth.test.ts index 58ed56547..2f8f0370a 100644 --- a/server/auth/types/openid/openid_auth.test.ts +++ b/server/auth/types/openid/openid_auth.test.ts @@ -15,7 +15,10 @@ import { httpServerMock } from '../../../../../../src/core/server/http/http_server.mocks'; -import { OpenSearchDashboardsRequest } from '../../../../../../src/core/server/http/router'; +import { + OpenSearchDashboardsRequest, + ResponseHeaders, +} from '../../../../../../src/core/server/http/router'; import { OpenIdAuthentication } from './openid_auth'; import { SecurityPluginConfigType } from '../../../index'; @@ -23,17 +26,26 @@ import { SecuritySessionCookie } from '../../../session/security_cookie'; import { deflateValue } from '../../../utils/compression'; import { getObjectProperties } from '../../../utils/object_properties_defined'; import { - IRouter, + AuthResult, + AuthResultParams, + AuthResultType, + AuthToolkit, CoreSetup, ILegacyClusterClient, + IRouter, SessionStorageFactory, } from '../../../../../../src/core/server'; +import { coreMock } from '../../../../../../src/core/public/mocks'; interface Logger { debug(message: string): void; + info(message: string): void; + warn(message: string): void; + error(message: string): void; + fatal(message: string): void; } @@ -334,3 +346,205 @@ describe('test OpenId authHeaderValue', () => { global.Date.now = realDateNow; }); }); + +describe('Test OpenID Unauthorized Flows', () => { + let router: IRouter; + let core: CoreSetup; + let esClient: ILegacyClusterClient; + let sessionStorageFactory: SessionStorageFactory; + + // Consistent with auth_handler_factory.test.ts + beforeEach(() => {}); + + const config = ({ + cookie: { + secure: false, + }, + openid: { + header: 'authorization', + scope: [], + extra_storage: { + cookie_prefix: 'testcookie', + additional_cookies: 5, + }, + }, + } as unknown) as SecurityPluginConfigType; + + const logger = { + debug: (message: string) => {}, + info: (message: string) => {}, + warn: (message: string) => {}, + error: (message: string) => {}, + fatal: (message: string) => {}, + }; + + const authToolkit: AuthToolkit = { + authenticated(data: AuthResultParams = {}): AuthResult { + return { + type: AuthResultType.authenticated, + state: data.state, + requestHeaders: data.requestHeaders, + responseHeaders: data.responseHeaders, + }; + }, + notHandled(): AuthResult { + return { + type: AuthResultType.notHandled, + }; + }, + redirected(headers: { location: string } & ResponseHeaders): AuthResult { + return { + type: AuthResultType.redirected, + headers, + }; + }, + }; + + test('Ensure non pageRequest returns an unauthorized response', () => { + const openIdAuthentication = new OpenIdAuthentication( + config, + sessionStorageFactory, + router, + esClient, + core, + logger + ); + + const mockRequest = httpServerMock.createRawRequest({ + url: { + pathname: '/unknownPath/', + }, + }); + const osRequest = OpenSearchDashboardsRequest.from(mockRequest); + + const mockLifecycleFactory = httpServerMock.createLifecycleResponseFactory(); + + openIdAuthentication.handleUnauthedRequest(osRequest, mockLifecycleFactory, authToolkit); + + expect(mockLifecycleFactory.unauthorized).toBeCalledTimes(1); + }); + + test('Ensure request without path redirects to default route', () => { + const mockCore = coreMock.createSetup(); + const openIdAuthentication = new OpenIdAuthentication( + config, + sessionStorageFactory, + router, + esClient, + mockCore, + logger + ); + + const mockRequest = httpServerMock.createRawRequest(); + const osRequest = OpenSearchDashboardsRequest.from(mockRequest); + + const mockLifecycleFactory = httpServerMock.createLifecycleResponseFactory(); + + const authToolKitSpy = jest.spyOn(authToolkit, 'redirected'); + + openIdAuthentication.handleUnauthedRequest(osRequest, mockLifecycleFactory, authToolkit); + + expect(authToolKitSpy).toHaveBeenCalledWith({ + location: '/auth/openid/captureUrlFragment?nextUrl=/', + 'set-cookie': + 'security_authentication=; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Path=/', + }); + }); + + test('Verify cookie is set "Secure" if configured', () => { + const mockCore = coreMock.createSetup(); + const openIdAuthentication = new OpenIdAuthentication( + { + ...config, + cookie: { + secure: true, + }, + }, + sessionStorageFactory, + router, + esClient, + mockCore, + logger + ); + + const mockRequest = httpServerMock.createRawRequest(); + const osRequest = OpenSearchDashboardsRequest.from(mockRequest); + + const mockLifecycleFactory = httpServerMock.createLifecycleResponseFactory(); + + const authToolKitSpy = jest.spyOn(authToolkit, 'redirected'); + + openIdAuthentication.handleUnauthedRequest(osRequest, mockLifecycleFactory, authToolkit); + + expect(authToolKitSpy).toHaveBeenCalledWith({ + location: '/auth/openid/captureUrlFragment?nextUrl=/', + 'set-cookie': + 'security_authentication=; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:00 GMT; Secure; HttpOnly; Path=/', + }); + }); + + test('Ensure nextUrl points to original request pathname', () => { + const mockCore = coreMock.createSetup(); + const openIdAuthentication = new OpenIdAuthentication( + config, + sessionStorageFactory, + router, + esClient, + mockCore, + logger + ); + + const mockRequest = httpServerMock.createRawRequest({ + url: { + pathname: '/app/dashboards', + }, + }); + const osRequest = OpenSearchDashboardsRequest.from(mockRequest); + + const mockLifecycleFactory = httpServerMock.createLifecycleResponseFactory(); + + const authToolKitSpy = jest.spyOn(authToolkit, 'redirected'); + + openIdAuthentication.handleUnauthedRequest(osRequest, mockLifecycleFactory, authToolkit); + + expect(authToolKitSpy).toHaveBeenCalledWith({ + location: '/auth/openid/captureUrlFragment?nextUrl=/app/dashboards', + 'set-cookie': + 'security_authentication=; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Path=/', + }); + }); + + test('Ensure nextUrl points to original request pathname including security_tenant', () => { + const mockCore = coreMock.createSetup(); + const openIdAuthentication = new OpenIdAuthentication( + config, + sessionStorageFactory, + router, + esClient, + mockCore, + logger + ); + + const mockRequest = httpServerMock.createRawRequest({ + url: { + pathname: '/app/dashboards', + search: 'security_tenant=testing', + }, + }); + const osRequest = OpenSearchDashboardsRequest.from(mockRequest); + + const mockLifecycleFactory = httpServerMock.createLifecycleResponseFactory(); + + const authToolKitSpy = jest.spyOn(authToolkit, 'redirected'); + + openIdAuthentication.handleUnauthedRequest(osRequest, mockLifecycleFactory, authToolkit); + + expect(authToolKitSpy).toHaveBeenCalledWith({ + location: `/auth/openid/captureUrlFragment?nextUrl=${escape( + '/app/dashboards?security_tenant=testing' + )}`, + 'set-cookie': + 'security_authentication=; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Path=/', + }); + }); +}); diff --git a/server/auth/types/openid/openid_auth.ts b/server/auth/types/openid/openid_auth.ts index 46c0f23a4..68303cad4 100644 --- a/server/auth/types/openid/openid_auth.ts +++ b/server/auth/types/openid/openid_auth.ts @@ -16,30 +16,29 @@ import * as fs from 'fs'; import wreck from '@hapi/wreck'; import { - Logger, - SessionStorageFactory, + AuthResult, + AuthToolkit, CoreSetup, - IRouter, ILegacyClusterClient, - OpenSearchDashboardsRequest, - LifecycleResponseFactory, - AuthToolkit, IOpenSearchDashboardsResponse, - AuthResult, + IRouter, + LifecycleResponseFactory, + Logger, + OpenSearchDashboardsRequest, + SessionStorageFactory, } from 'opensearch-dashboards/server'; import { PeerCertificate } from 'tls'; import { Server, ServerStateCookieOptions } from '@hapi/hapi'; import { ProxyAgent } from 'proxy-agent'; import { SecurityPluginConfigType } from '../../..'; import { - SecuritySessionCookie, clearOldVersionCookieValue, + SecuritySessionCookie, } from '../../../session/security_cookie'; import { OpenIdAuthRoutes } from './routes'; import { AuthenticationType } from '../authentication_type'; -import { callTokenEndpoint } from './helper'; +import { callTokenEndpoint, getExpirationDate } from './helper'; import { getObjectProperties } from '../../../utils/object_properties_defined'; -import { getExpirationDate } from './helper'; import { AuthType } from '../../../../common'; import { ExtraAuthStorageOptions, @@ -128,11 +127,14 @@ export class OpenIdAuthentication extends AuthenticationType { } private generateNextUrl(request: OpenSearchDashboardsRequest): string { - const path = getRedirectUrl({ + let path = getRedirectUrl({ request, basePath: this.coreSetup.http.basePath.serverBasePath, nextUrl: request.url.pathname || '/app/opensearch-dashboards', }); + if (request.url.search) { + path += request.url.search; + } return escape(path); } diff --git a/server/auth/types/openid/routes.ts b/server/auth/types/openid/routes.ts index b9c76de7b..d403ead19 100644 --- a/server/auth/types/openid/routes.ts +++ b/server/auth/types/openid/routes.ts @@ -101,7 +101,7 @@ export class OpenIdAuthRoutes { }, }) ), - redirectHash: schema.maybe(schema.boolean()), + redirectHash: schema.maybe(schema.string()), state: schema.maybe(schema.string()), refresh: schema.maybe(schema.string()), }, diff --git a/test/cypress/e2e/oidc/oidc_auth_test.spec.js b/test/cypress/e2e/oidc/oidc_auth_test.spec.js index 79a41807d..514a20a4b 100644 --- a/test/cypress/e2e/oidc/oidc_auth_test.spec.js +++ b/test/cypress/e2e/oidc/oidc_auth_test.spec.js @@ -17,7 +17,6 @@ * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */ - const basePath = Cypress.env('basePath') || ''; describe('Log in via OIDC', () => { @@ -60,6 +59,12 @@ describe('Log in via OIDC', () => { kcLogin(); + cy.url().then((url) => { + cy.visit(url, { + failOnStatusCode: false, + }); + }); + cy.getCookie('security_authentication').should('exist'); localStorage.setItem('opendistro::security::tenant::saved', '""'); @@ -91,6 +96,32 @@ describe('Log in via OIDC', () => { cy.get('h1').contains('Get started'); }); + it('Login to Dashboard preserving Tenant', () => { + const startUrl = `http://localhost:5601${basePath}/app/dashboards?security_tenant=private#/list`; + + cy.visit(startUrl, { + failOnStatusCode: false, + }); + + sessionStorage.setItem('opendistro::security::tenant::show_popup', 'false'); + + kcLogin(); + cy.getCookie('security_authentication').should('exist'); + + cy.url().then((url) => { + cy.visit(url, { + failOnStatusCode: false, + }); + }); + + localStorage.setItem('home:newThemeModal:show', 'false'); + + cy.get('#user-icon-btn').should('be.visible'); + cy.get('#user-icon-btn').click(); + + cy.get('#tenantName').should('have.text', 'Private'); + }); + it('Tenancy persisted after logout in OIDC', () => { cy.visit(`http://localhost:5601${basePath}/app/opensearch_dashboards_overview#/`, { failOnStatusCode: false,