Skip to content

Commit 85f2076

Browse files
committed
Support deep links inside of RelayState during IdP initiated login.
1 parent 3ee0bf2 commit 85f2076

File tree

11 files changed

+458
-33
lines changed

11 files changed

+458
-33
lines changed
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import { isInternalURL } from './is_internal_url';
8+
9+
describe('isInternalURL', () => {
10+
function commonTestCases(basePath?: string) {
11+
it('should return `true `if URL includes hash fragment', () => {
12+
const href = `${basePath}/app/kibana#/discover/New-Saved-Search`;
13+
expect(isInternalURL(href, basePath)).toBe(true);
14+
});
15+
16+
it('should return `false` if URL includes a protocol/hostname', () => {
17+
const href = `https://example.com${basePath}/app/kibana`;
18+
expect(isInternalURL(href, basePath)).toBe(false);
19+
});
20+
21+
it('should return `false` if URL includes a port', () => {
22+
const href = `http://localhost:5601${basePath}/app/kibana`;
23+
expect(isInternalURL(href, basePath)).toBe(false);
24+
});
25+
26+
it('should return `false` if URL does not specify protocol', () => {
27+
const hrefWithTwoSlashes = `//${basePath}/app/kibana`;
28+
expect(isInternalURL(hrefWithTwoSlashes)).toBe(false);
29+
30+
const hrefWithThreeSlashes = `///${basePath}/app/kibana`;
31+
expect(isInternalURL(hrefWithThreeSlashes)).toBe(false);
32+
});
33+
}
34+
35+
describe('with basePath defined', () => {
36+
const basePath = '/iqf';
37+
38+
commonTestCases(basePath);
39+
40+
it('should return `true` if URL starts with a basepath', () => {
41+
const href = `${basePath}/login`;
42+
expect(isInternalURL(href, basePath)).toBe(true);
43+
});
44+
45+
it('should return `false` if URL does not start with basePath', () => {
46+
const href = '/notbasepath/app/kibana';
47+
expect(isInternalURL(href, basePath)).toBe(false);
48+
});
49+
});
50+
51+
describe('without basePath defined', () => commonTestCases());
52+
});
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import { parse } from 'url';
8+
9+
export function isInternalURL(url: string, basePath = '') {
10+
const { protocol, hostname, port, pathname } = parse(
11+
url,
12+
false /* parseQueryString */,
13+
true /* slashesDenoteHost */
14+
);
15+
16+
// We should explicitly compare `protocol`, `port` and `hostname` to null to make sure these are not
17+
// detected in the URL at all. For example `hostname` can be empty string for Node URL parser, but
18+
// browser (because of various bwc reasons) processes URL differently (e.g. `///abc.com` - for browser
19+
// hostname is `abc.com`, but for Node hostname is an empty string i.e. everything between schema (`//`)
20+
// and the first slash that belongs to path.
21+
if (protocol !== null || hostname !== null || port !== null) {
22+
return false;
23+
}
24+
25+
return String(pathname).startsWith(basePath);
26+
}

x-pack/plugins/security/common/parse_next.ts

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66

77
import { parse } from 'url';
8+
import { isInternalURL } from './is_internal_url';
89

910
export function parseNext(href: string, basePath = '') {
1011
const { query, hash } = parse(href, true);
@@ -20,23 +21,8 @@ export function parseNext(href: string, basePath = '') {
2021
}
2122

2223
// validate that `next` is not attempting a redirect to somewhere
23-
// outside of this Kibana install
24-
const { protocol, hostname, port, pathname } = parse(
25-
next,
26-
false /* parseQueryString */,
27-
true /* slashesDenoteHost */
28-
);
29-
30-
// We should explicitly compare `protocol`, `port` and `hostname` to null to make sure these are not
31-
// detected in the URL at all. For example `hostname` can be empty string for Node URL parser, but
32-
// browser (because of various bwc reasons) processes URL differently (e.g. `///abc.com` - for browser
33-
// hostname is `abc.com`, but for Node hostname is an empty string i.e. everything between schema (`//`)
34-
// and the first slash that belongs to path.
35-
if (protocol !== null || hostname !== null || port !== null) {
36-
return `${basePath}/`;
37-
}
38-
39-
if (!String(pathname).startsWith(basePath)) {
24+
// outside of this Kibana install.
25+
if (!isInternalURL(next, basePath)) {
4026
return `${basePath}/`;
4127
}
4228

x-pack/plugins/security/server/authentication/providers/saml.test.ts

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,131 @@ describe('SAMLAuthenticationProvider', () => {
231231
);
232232
});
233233

234+
describe('IdP initiated login', () => {
235+
beforeEach(() => {
236+
mockOptions = mockAuthenticationProviderOptions({ name: 'saml' });
237+
mockOptions.basePath.get.mockReturnValue(mockOptions.basePath.serverBasePath);
238+
239+
const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
240+
mockScopedClusterClient.callAsCurrentUser.mockImplementation(() =>
241+
Promise.resolve(mockAuthenticatedUser())
242+
);
243+
mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient);
244+
245+
mockOptions.client.callAsInternalUser.mockResolvedValue({
246+
username: 'user',
247+
access_token: 'valid-token',
248+
refresh_token: 'valid-refresh-token',
249+
});
250+
251+
provider = new SAMLAuthenticationProvider(mockOptions, {
252+
realm: 'test-realm',
253+
maxRedirectURLSize: new ByteSizeValue(100),
254+
useRelayStateDeepLink: true,
255+
});
256+
});
257+
258+
it('redirects to the home page if `useRelayStateDeepLink` is set to `false`.', async () => {
259+
provider = new SAMLAuthenticationProvider(mockOptions, {
260+
realm: 'test-realm',
261+
maxRedirectURLSize: new ByteSizeValue(100),
262+
useRelayStateDeepLink: false,
263+
});
264+
265+
await expect(
266+
provider.login(httpServerMock.createKibanaRequest({ headers: {} }), {
267+
type: SAMLLogin.LoginWithSAMLResponse,
268+
samlResponse: 'saml-response-xml',
269+
relayState: '/mock-server-basepath/app/some-app#some-deep-link',
270+
})
271+
).resolves.toEqual(
272+
AuthenticationResult.redirectTo('/mock-server-basepath/', {
273+
state: {
274+
username: 'user',
275+
accessToken: 'valid-token',
276+
refreshToken: 'valid-refresh-token',
277+
realm: 'test-realm',
278+
},
279+
})
280+
);
281+
});
282+
283+
it('redirects to the home page if `relayState` is not specified.', async () => {
284+
await expect(
285+
provider.login(httpServerMock.createKibanaRequest({ headers: {} }), {
286+
type: SAMLLogin.LoginWithSAMLResponse,
287+
samlResponse: 'saml-response-xml',
288+
})
289+
).resolves.toEqual(
290+
AuthenticationResult.redirectTo('/mock-server-basepath/', {
291+
state: {
292+
username: 'user',
293+
accessToken: 'valid-token',
294+
refreshToken: 'valid-refresh-token',
295+
realm: 'test-realm',
296+
},
297+
})
298+
);
299+
});
300+
301+
it('redirects to the home page if `relayState` includes external URL', async () => {
302+
await expect(
303+
provider.login(httpServerMock.createKibanaRequest({ headers: {} }), {
304+
type: SAMLLogin.LoginWithSAMLResponse,
305+
samlResponse: 'saml-response-xml',
306+
relayState: 'https://evil.com/mock-server-basepath/app/some-app#some-deep-link',
307+
})
308+
).resolves.toEqual(
309+
AuthenticationResult.redirectTo('/mock-server-basepath/', {
310+
state: {
311+
username: 'user',
312+
accessToken: 'valid-token',
313+
refreshToken: 'valid-refresh-token',
314+
realm: 'test-realm',
315+
},
316+
})
317+
);
318+
});
319+
320+
it('redirects to the home page if `relayState` includes URL that starts with double slashes', async () => {
321+
await expect(
322+
provider.login(httpServerMock.createKibanaRequest({ headers: {} }), {
323+
type: SAMLLogin.LoginWithSAMLResponse,
324+
samlResponse: 'saml-response-xml',
325+
relayState: '//mock-server-basepath/app/some-app#some-deep-link',
326+
})
327+
).resolves.toEqual(
328+
AuthenticationResult.redirectTo('/mock-server-basepath/', {
329+
state: {
330+
username: 'user',
331+
accessToken: 'valid-token',
332+
refreshToken: 'valid-refresh-token',
333+
realm: 'test-realm',
334+
},
335+
})
336+
);
337+
});
338+
339+
it('redirects to the URL from the relay state.', async () => {
340+
await expect(
341+
provider.login(httpServerMock.createKibanaRequest({ headers: {} }), {
342+
type: SAMLLogin.LoginWithSAMLResponse,
343+
samlResponse: 'saml-response-xml',
344+
relayState: '/mock-server-basepath/app/some-app#some-deep-link',
345+
})
346+
).resolves.toEqual(
347+
AuthenticationResult.redirectTo('/mock-server-basepath/app/some-app#some-deep-link', {
348+
state: {
349+
username: 'user',
350+
accessToken: 'valid-token',
351+
refreshToken: 'valid-refresh-token',
352+
realm: 'test-realm',
353+
},
354+
})
355+
);
356+
});
357+
});
358+
234359
describe('IdP initiated login with existing session', () => {
235360
it('returns `notHandled` if new SAML Response is rejected.', async () => {
236361
const request = httpServerMock.createKibanaRequest({ headers: {} });
@@ -377,6 +502,71 @@ describe('SAMLAuthenticationProvider', () => {
377502
});
378503
});
379504

505+
it(`redirects to the URL from relay state if new SAML Response is for the same user if ${description}.`, async () => {
506+
const request = httpServerMock.createKibanaRequest({ headers: {} });
507+
const state = {
508+
username: 'user',
509+
accessToken: 'existing-token',
510+
refreshToken: 'existing-refresh-token',
511+
realm: 'test-realm',
512+
};
513+
const authorization = `Bearer ${state.accessToken}`;
514+
515+
const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
516+
mockScopedClusterClient.callAsCurrentUser.mockImplementation(() => response);
517+
mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient);
518+
519+
mockOptions.client.callAsInternalUser.mockResolvedValue({
520+
username: 'user',
521+
access_token: 'new-valid-token',
522+
refresh_token: 'new-valid-refresh-token',
523+
});
524+
525+
mockOptions.tokens.invalidate.mockResolvedValue(undefined);
526+
527+
provider = new SAMLAuthenticationProvider(mockOptions, {
528+
realm: 'test-realm',
529+
maxRedirectURLSize: new ByteSizeValue(100),
530+
useRelayStateDeepLink: true,
531+
});
532+
533+
await expect(
534+
provider.login(
535+
request,
536+
{
537+
type: SAMLLogin.LoginWithSAMLResponse,
538+
samlResponse: 'saml-response-xml',
539+
relayState: '/mock-server-basepath/app/some-app#some-deep-link',
540+
},
541+
state
542+
)
543+
).resolves.toEqual(
544+
AuthenticationResult.redirectTo('/mock-server-basepath/app/some-app#some-deep-link', {
545+
state: {
546+
username: 'user',
547+
accessToken: 'new-valid-token',
548+
refreshToken: 'new-valid-refresh-token',
549+
realm: 'test-realm',
550+
},
551+
})
552+
);
553+
554+
expectAuthenticateCall(mockOptions.client, { headers: { authorization } });
555+
556+
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith(
557+
'shield.samlAuthenticate',
558+
{
559+
body: { ids: [], content: 'saml-response-xml', realm: 'test-realm' },
560+
}
561+
);
562+
563+
expect(mockOptions.tokens.invalidate).toHaveBeenCalledTimes(1);
564+
expect(mockOptions.tokens.invalidate).toHaveBeenCalledWith({
565+
accessToken: state.accessToken,
566+
refreshToken: state.refreshToken,
567+
});
568+
});
569+
380570
it(`redirects to \`overwritten_session\` if new SAML Response is for the another user if ${description}.`, async () => {
381571
const request = httpServerMock.createKibanaRequest({ headers: {} });
382572
const state = {

0 commit comments

Comments
 (0)