Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve Query in nextUrl during openid login redirect #2140

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
## Version 2.18.0 Release Notes
KleinSebastian marked this conversation as resolved.
Show resolved Hide resolved

Compatible with OpenSearch and OpenSearch Dashboards version 2.18.0

### Enhancements
* Add JWT authentication type to MultipleAuthentication ([#2107](https://github.com/opensearch-project/security-dashboards-plugin/pull/2107))

### Bug Fixes
* Update category label for security plugin ([#2121](https://github.com/opensearch-project/security-dashboards-plugin/pull/2121))
* Fix button label ([#2130](https://github.com/opensearch-project/security-dashboards-plugin/pull/2130))
215 changes: 213 additions & 2 deletions server/auth/types/openid/openid_auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,27 @@

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;
Expand Down Expand Up @@ -334,3 +342,206 @@
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',() =>{

Check failure on line 482 in server/auth/types/openid/openid_auth.test.ts

View workflow job for this annotation

GitHub Actions / Run unit tests (ubuntu-latest)

Replace `()·=>` with `·()·=>·`

Check failure on line 482 in server/auth/types/openid/openid_auth.test.ts

View workflow job for this annotation

GitHub Actions / Run unit tests (windows-latest)

Replace `()·=>` with `·()·=>·`
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');


Check failure on line 504 in server/auth/types/openid/openid_auth.test.ts

View workflow job for this annotation

GitHub Actions / Run unit tests (ubuntu-latest)

Delete `⏎`

Check failure on line 504 in server/auth/types/openid/openid_auth.test.ts

View workflow job for this annotation

GitHub Actions / Run unit tests (windows-latest)

Delete `␍⏎`
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=/',
});
})

Check failure on line 512 in server/auth/types/openid/openid_auth.test.ts

View workflow job for this annotation

GitHub Actions / Run unit tests (ubuntu-latest)

Insert `;`

Check failure on line 512 in server/auth/types/openid/openid_auth.test.ts

View workflow job for this annotation

GitHub Actions / Run unit tests (windows-latest)

Insert `;`

test('Ensure nextUrl points to original request pathname including security_tenant',() =>{

Check failure on line 514 in server/auth/types/openid/openid_auth.test.ts

View workflow job for this annotation

GitHub Actions / Run unit tests (ubuntu-latest)

Replace `()·=>` with `·()·=>·`

Check failure on line 514 in server/auth/types/openid/openid_auth.test.ts

View workflow job for this annotation

GitHub Actions / Run unit tests (windows-latest)

Replace `()·=>` with `·()·=>·`
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'

Check failure on line 528 in server/auth/types/openid/openid_auth.test.ts

View workflow job for this annotation

GitHub Actions / Run unit tests (ubuntu-latest)

Insert `,`

Check failure on line 528 in server/auth/types/openid/openid_auth.test.ts

View workflow job for this annotation

GitHub Actions / Run unit tests (windows-latest)

Insert `,`
},
});
const osRequest = OpenSearchDashboardsRequest.from(mockRequest);

const mockLifecycleFactory = httpServerMock.createLifecycleResponseFactory();

const authToolKitSpy = jest.spyOn(authToolkit, 'redirected');

Check failure on line 535 in server/auth/types/openid/openid_auth.test.ts

View workflow job for this annotation

GitHub Actions / Run unit tests (ubuntu-latest)

Delete `⏎`


Check failure on line 537 in server/auth/types/openid/openid_auth.test.ts

View workflow job for this annotation

GitHub Actions / Run unit tests (windows-latest)

Delete `␍⏎`
openIdAuthentication.handleUnauthedRequest(osRequest, mockLifecycleFactory, authToolkit);

expect(authToolKitSpy).toHaveBeenCalledWith({
location: `/auth/openid/captureUrlFragment?nextUrl=${escape("/app/dashboards?security_tenant=testing")}`,

Check failure on line 541 in server/auth/types/openid/openid_auth.test.ts

View workflow job for this annotation

GitHub Actions / Run unit tests (ubuntu-latest)

Replace `"/app/dashboards?security_tenant=testing"` with `⏎········'/app/dashboards?security_tenant=testing'⏎······`

Check failure on line 541 in server/auth/types/openid/openid_auth.test.ts

View workflow job for this annotation

GitHub Actions / Run unit tests (windows-latest)

Replace `"/app/dashboards?security_tenant=testing"` with `␍⏎········'/app/dashboards?security_tenant=testing'␍⏎······`
'set-cookie':
'security_authentication=; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Path=/',
});
})

Check failure on line 545 in server/auth/types/openid/openid_auth.test.ts

View workflow job for this annotation

GitHub Actions / Run unit tests (ubuntu-latest)

Replace `⏎` with `;`

Check failure on line 545 in server/auth/types/openid/openid_auth.test.ts

View workflow job for this annotation

GitHub Actions / Run unit tests (windows-latest)

Replace `␍⏎` with `;`

});
24 changes: 13 additions & 11 deletions server/auth/types/openid/openid_auth.ts
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
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
Loading