Skip to content

Commit

Permalink
Preserve Query in nextUrl during openid login redirect (#2140) (#2159)
Browse files Browse the repository at this point in the history 
* 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 ded4012)

Signed-off-by: Sebastian Klein <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Derek Ho <[email protected]>
Co-authored-by: SKLEIN3 <[email protected]>
Co-authored-by: Darshit Chanpura <[email protected]>
  • Loading branch information
@opensearch-trigger-bot @github-actions
@derek-ho @DarshitChanpura
5 people authored Dec 11, 2024
1 parent 9e22dbd commit 2f7d744
Show file tree
Hide file tree
Showing 4 changed files with 262 additions and 15 deletions.
218 changes: 216 additions & 2 deletions server/auth/types/openid/openid_auth.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,37 @@

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';
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;
}

Expand Down Expand Up @@ -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<SecuritySessionCookie>;

// 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=/',
});
});
});
24 changes: 13 additions & 11 deletions server/auth/types/openid/openid_auth.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion server/auth/types/openid/routes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
},
Expand Down
33 changes: 32 additions & 1 deletion test/cypress/e2e/oidc/oidc_auth_test.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

const basePath = Cypress.env('basePath') || '';

describe('Log in via OIDC', () => {
Expand Down Expand Up @@ -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', '""');
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 2f7d744

Please sign in to comment.