Skip to content

Commit da745fa

Browse files
azasypkinazasypkin
authored
[7.x] Preserve URL fragment during SAML handshake. (#47742)
1 parent bd9225f commit da745fa

27 files changed

Lines changed: 1616 additions & 564 deletions

File tree

‎docs/user/security/authentication/index.asciidoc‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ xpack.security.authc.saml.realm: realm-name
100100
+
101101
[source,yaml]
102102
--------------------------------------------------------------------------------
103-
server.xsrf.whitelist: [/api/security/v1/saml]
103+
server.xsrf.whitelist: [/api/security/saml/callback]
104104
--------------------------------------------------------------------------------
105105

106106
Users will be able to log in to {kib} via SAML Single Sign-On by navigating directly to the {kib} URL. Users who aren't authenticated are redirected to the Identity Provider for login. Most Identity Providers maintain a long-lived session—users who logged in to a different application using the same Identity Provider in the same browser are automatically authenticated. An exception is if {es} or the Identity Provider is configured to force user to re-authenticate. This login scenario is called _Service Provider initiated login_.
@@ -119,6 +119,21 @@ The order of `saml` and `basic` is important. Users who open {kib} will go throu
119119

120120
Basic authentication is supported _only_ if `basic` authentication provider is explicitly declared in `xpack.security.authc.providers` setting in addition to `saml`.
121121

122+
[float]
123+
===== SAML and long URLs
124+
125+
At the beginning of the SAML handshake, {kib} stores the initial URL in the session cookie, so it can redirect the user back to that URL after successful SAML authentication.
126+
If the URL is long, the session cookie might exceed the maximum size supported by the browser--typically 4KB for all cookies per domain. When this happens, the session cookie is truncated,
127+
or dropped completely, and you might experience sporadic failures during SAML authentication.
128+
129+
To remedy this issue, you can decrease the maximum
130+
size of the URL that {kib} is allowed to store during the SAML handshake. The default value is 2KB.
131+
132+
[source,yaml]
133+
--------------------------------------------------------------------------------
134+
xpack.security.authc.saml.maxRedirectURLSize: 1kb
135+
--------------------------------------------------------------------------------
136+
122137
[[oidc]]
123138
==== OpenID Connect Single Sign-On
124139

‎src/dev/build/tasks/os_packages/docker_generator/resources/bin/kibana-docker‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ kibana_vars=(
175175
xpack.security.authc.providers
176176
xpack.security.authc.oidc.realm
177177
xpack.security.authc.saml.realm
178+
xpack.security.authc.saml.maxRedirectURLSize
178179
xpack.security.cookieName
179180
xpack.security.enabled
180181
xpack.security.encryptionKey

‎x-pack/legacy/plugins/security/index.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import { SecureSavedObjectsClientWrapper } from './server/lib/saved_objects_clie
3232
import { deepFreeze } from './server/lib/deep_freeze';
3333
import { createOptionalPlugin } from '../../server/lib/optional_plugin';
3434
import { KibanaRequest } from '../../../../src/core/server';
35+
import { createCSPRuleString } from '../../../../src/legacy/server/csp';
3536

3637
export const security = (kibana) => new kibana.Plugin({
3738
id: 'security',
@@ -155,6 +156,7 @@ export const security = (kibana) => new kibana.Plugin({
155156
isSystemAPIRequest: server.plugins.kibana.systemApi.isSystemApiRequest.bind(
156157
server.plugins.kibana.systemApi
157158
),
159+
cspRules: createCSPRuleString(config.get('csp.rules')),
158160
});
159161

160162
const plugin = this;

‎x-pack/legacy/plugins/security/server/routes/api/v1/__tests__/authenticate.js‎

Lines changed: 0 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -257,90 +257,4 @@ describe('Authentication routes', () => {
257257
expect(response).to.eql({ username: 'user' });
258258
});
259259
});
260-
261-
describe('SAML assertion consumer service endpoint', () => {
262-
let samlAcsRoute;
263-
let request;
264-
265-
beforeEach(() => {
266-
samlAcsRoute = serverStub.route
267-
.withArgs(sinon.match({ path: '/api/security/v1/saml' }))
268-
.firstCall
269-
.args[0];
270-
271-
request = requestFixture({ payload: { SAMLResponse: 'saml-response-xml' } });
272-
});
273-
274-
it('correctly defines route.', async () => {
275-
expect(samlAcsRoute.method).to.be('POST');
276-
expect(samlAcsRoute.path).to.be('/api/security/v1/saml');
277-
expect(samlAcsRoute.handler).to.be.a(Function);
278-
expect(samlAcsRoute.config).to.eql({
279-
auth: false,
280-
validate: {
281-
payload: Joi.object({
282-
SAMLResponse: Joi.string().required(),
283-
RelayState: Joi.string().allow('')
284-
})
285-
}
286-
});
287-
});
288-
289-
it('returns 500 if authentication throws unhandled exception.', async () => {
290-
const unhandledException = new Error('Something went wrong.');
291-
loginStub.throws(unhandledException);
292-
293-
const response = await samlAcsRoute.handler(request, hStub);
294-
295-
sinon.assert.notCalled(hStub.redirect);
296-
expect(response.isBoom).to.be(true);
297-
expect(response.output.payload).to.eql({
298-
statusCode: 500,
299-
error: 'Internal Server Error',
300-
message: 'An internal server error occurred'
301-
});
302-
});
303-
304-
it('returns 401 if authentication fails.', async () => {
305-
const failureReason = new Error('Something went wrong.');
306-
loginStub.resolves(AuthenticationResult.failed(failureReason));
307-
308-
const response = await samlAcsRoute.handler(request, hStub);
309-
310-
sinon.assert.notCalled(hStub.redirect);
311-
expect(response.isBoom).to.be(true);
312-
expect(response.message).to.be(failureReason.message);
313-
expect(response.output.statusCode).to.be(401);
314-
});
315-
316-
it('returns 401 if authentication is not handled.', async () => {
317-
loginStub.resolves(AuthenticationResult.notHandled());
318-
319-
const response = await samlAcsRoute.handler(request, hStub);
320-
321-
sinon.assert.notCalled(hStub.redirect);
322-
expect(response.isBoom).to.be(true);
323-
expect(response.message).to.be('Unauthorized');
324-
expect(response.output.statusCode).to.be(401);
325-
});
326-
327-
it('returns 401 if authentication completes with unexpected result.', async () => {
328-
loginStub.resolves(AuthenticationResult.succeeded({}));
329-
330-
const response = await samlAcsRoute.handler(request, hStub);
331-
332-
sinon.assert.notCalled(hStub.redirect);
333-
expect(response.isBoom).to.be(true);
334-
expect(response.message).to.be('Unauthorized');
335-
expect(response.output.statusCode).to.be(401);
336-
});
337-
338-
it('redirects if required by the authentication process.', async () => {
339-
loginStub.resolves(AuthenticationResult.redirectTo('http://redirect-to/path'));
340-
341-
await samlAcsRoute.handler(request, hStub);
342-
343-
sinon.assert.calledWithExactly(hStub.redirect, 'http://redirect-to/path');
344-
});
345-
});
346260
});

‎x-pack/legacy/plugins/security/server/routes/api/v1/authenticate.js‎

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -58,37 +58,6 @@ export function initAuthenticateApi({ authc: { login, logout }, config }, server
5858
}
5959
});
6060

61-
server.route({
62-
method: 'POST',
63-
path: '/api/security/v1/saml',
64-
config: {
65-
auth: false,
66-
validate: {
67-
payload: Joi.object({
68-
SAMLResponse: Joi.string().required(),
69-
RelayState: Joi.string().allow('')
70-
})
71-
}
72-
},
73-
async handler(request, h) {
74-
try {
75-
// When authenticating using SAML we _expect_ to redirect to the SAML Identity provider.
76-
const authenticationResult = await login(KibanaRequest.from(request), {
77-
provider: 'saml',
78-
value: { samlResponse: request.payload.SAMLResponse }
79-
});
80-
81-
if (authenticationResult.redirected()) {
82-
return h.redirect(authenticationResult.redirectURL);
83-
}
84-
85-
return Boom.unauthorized(authenticationResult.error);
86-
} catch (err) {
87-
return wrapError(err);
88-
}
89-
}
90-
});
91-
9261
/**
9362
* The route should be configured as a redirect URI in OP when OpenID Connect implicit flow
9463
* is used, so that we can extract authentication response from URL fragment and send it to

‎x-pack/package.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,8 @@
107107
"@types/tar-fs": "^1.16.1",
108108
"@types/tinycolor2": "^1.4.1",
109109
"@types/uuid": "^3.4.4",
110+
"@types/xml2js": "^0.4.5",
111+
"@types/xml-crypto": "^1.4.0",
110112
"abab": "^1.0.4",
111113
"ansicolors": "0.3.2",
112114
"axios": "^0.19.0",

‎x-pack/plugins/security/server/authentication/index.mock.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
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 { Authentication } from '.';
8+
9+
export const authenticationMock = {
10+
create: (): jest.Mocked<Authentication> => ({
11+
login: jest.fn(),
12+
createAPIKey: jest.fn(),
13+
getCurrentUser: jest.fn(),
14+
invalidateAPIKey: jest.fn(),
15+
isAuthenticated: jest.fn(),
16+
logout: jest.fn(),
17+
}),
18+
};

‎x-pack/plugins/security/server/authentication/index.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* or more contributor license agreements. Licensed under the Elastic License;
44
* you may not use this file except in compliance with the Elastic License.
55
*/
6+
import { UnwrapPromise } from '@kbn/utility-types';
67
import {
78
IClusterClient,
89
CoreSetup,
@@ -20,8 +21,13 @@ export { canRedirectRequest } from './can_redirect_request';
2021
export { Authenticator, ProviderLoginAttempt } from './authenticator';
2122
export { AuthenticationResult } from './authentication_result';
2223
export { DeauthenticationResult } from './deauthentication_result';
23-
export { OIDCAuthenticationFlow } from './providers';
24-
export { CreateAPIKeyResult } from './api_keys';
24+
export { OIDCAuthenticationFlow, SAMLLoginStep } from './providers';
25+
export {
26+
CreateAPIKeyResult,
27+
InvalidateAPIKeyResult,
28+
CreateAPIKeyParams,
29+
InvalidateAPIKeyParams,
30+
} from './api_keys';
2531

2632
interface SetupAuthenticationParams {
2733
core: CoreSetup;
@@ -31,6 +37,8 @@ interface SetupAuthenticationParams {
3137
getLegacyAPI(): LegacyAPI;
3238
}
3339

40+
export type Authentication = UnwrapPromise<ReturnType<typeof setupAuthentication>>;
41+
3442
export async function setupAuthentication({
3543
core,
3644
clusterClient,

‎x-pack/plugins/security/server/authentication/providers/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export {
1111
} from './base';
1212
export { BasicAuthenticationProvider } from './basic';
1313
export { KerberosAuthenticationProvider } from './kerberos';
14-
export { SAMLAuthenticationProvider, isSAMLRequestQuery } from './saml';
14+
export { SAMLAuthenticationProvider, isSAMLRequestQuery, SAMLLoginStep } from './saml';
1515
export { TokenAuthenticationProvider } from './token';
1616
export { OIDCAuthenticationProvider, OIDCAuthenticationFlow } from './oidc';
1717
export { PKIAuthenticationProvider } from './pki';

0 commit comments

Comments
 (0)