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 URL fragment during SAML handshake. #44513

Merged
merged 23 commits into from
Oct 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
71f2980
Preserve URL fragment during SAML handshake.
azasypkin Aug 30, 2019
c891059
Merge branch 'master' into issue-18392-saml-hash
azasypkin Sep 4, 2019
a3e32fd
Merge branch 'master' into issue-18392-saml-hash
azasypkin Sep 5, 2019
ca8942a
Merge branch 'master' into issue-18392-saml-hash
azasypkin Sep 5, 2019
19e6fb3
Merge branch 'master' into issue-18392-saml-hash
azasypkin Sep 5, 2019
1f7faff
Sync with upstream and use route URLs that are relative to root. Use …
azasypkin Sep 5, 2019
017f7bb
Merge branch 'master' into issue-18392-saml-hash
azasypkin Sep 12, 2019
f564987
Review#1: get rid of `authc` part in SAML API routes.
azasypkin Sep 12, 2019
776ca98
Review#1: capture basepath and space ID.
azasypkin Sep 12, 2019
8270f19
Review#1: migrate to alternative solution to store full URL in the co…
azasypkin Sep 12, 2019
6febc2c
Review#1: use server base path instead of the request scoped one for …
azasypkin Sep 13, 2019
432d7bb
Merge branch 'master' into issue-18392-saml-hash
azasypkin Sep 13, 2019
ff90be4
Properly handle max redirect URL size.
azasypkin Sep 13, 2019
184c977
Merge branch 'master' into issue-18392-saml-hash
azasypkin Sep 16, 2019
3302508
Merge branch 'master' into issue-18392-saml-hash
azasypkin Sep 17, 2019
50fada9
Update tests.
azasypkin Sep 17, 2019
170ee51
Merge branch 'master' into issue-18392-saml-hash
elasticmachine Sep 20, 2019
a38be2e
Merge branch 'master' into issue-18392-saml-hash
elasticmachine Sep 23, 2019
a361167
Merge branch 'master' into issue-18392-saml-hash
azasypkin Oct 8, 2019
2b88f7b
Use `serverBasePath` provided by the core instead of the one provided…
azasypkin Oct 8, 2019
dca6431
Apply suggestions from code review
azasypkin Oct 9, 2019
0a86b77
Merge branch 'master' into issue-18392-saml-hash
azasypkin Oct 9, 2019
8b51a1c
Make sure redirect URL fragment always starts with `#`.
azasypkin Oct 9, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion docs/user/security/authentication/index.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ xpack.security.authc.saml.realm: realm-name
+
[source,yaml]
--------------------------------------------------------------------------------
server.xsrf.whitelist: [/api/security/v1/saml]
server.xsrf.whitelist: [/api/security/saml/callback]
--------------------------------------------------------------------------------

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_.
Expand All @@ -119,6 +119,21 @@ The order of `saml` and `basic` is important. Users who open {kib} will go throu

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

[float]
===== SAML and long URLs

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.
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,
or dropped completely, and you might experience sporadic failures during SAML authentication.

To remedy this issue, you can decrease the maximum
size of the URL that {kib} is allowed to store during the SAML handshake. The default value is 2KB.

[source,yaml]
--------------------------------------------------------------------------------
xpack.security.authc.saml.maxRedirectURLSize: 1kb
--------------------------------------------------------------------------------

[[oidc]]
==== OpenID Connect Single Sign-On

Expand Down
24 changes: 16 additions & 8 deletions renovate.json5
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,22 @@
'@types/tinycolor2',
],
},
{
groupSlug: 'xml2js',
groupName: 'xml2js related packages',
packageNames: [
'xml2js',
'@types/xml2js',
],
},
{
groupSlug: 'xml-crypto',
groupName: 'xml-crypto related packages',
packageNames: [
'xml-crypto',
'@types/xml-crypto',
],
},
{
groupSlug: 'intl-relativeformat',
groupName: 'intl-relativeformat related packages',
Expand Down Expand Up @@ -919,14 +935,6 @@
'@types/parse-link-header',
],
},
{
groupSlug: 'xml2js',
groupName: 'xml2js related packages',
packageNames: [
'xml2js',
'@types/xml2js',
],
},
{
packagePatterns: [
'^@kbn/.*',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ kibana_vars=(
xpack.security.authc.providers
xpack.security.authc.oidc.realm
xpack.security.authc.saml.realm
xpack.security.authc.saml.maxRedirectURLSize
xpack.security.cookieName
xpack.security.enabled
xpack.security.encryptionKey
Expand Down
4 changes: 3 additions & 1 deletion x-pack/legacy/plugins/security/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { SecureSavedObjectsClientWrapper } from './server/lib/saved_objects_clie
import { deepFreeze } from './server/lib/deep_freeze';
import { createOptionalPlugin } from '../../server/lib/optional_plugin';
import { KibanaRequest } from '../../../../src/core/server';
import { createCSPRuleString } from '../../../../src/legacy/server/csp';

export const security = (kibana) => new kibana.Plugin({
id: 'security',
Expand Down Expand Up @@ -128,17 +129,18 @@ export const security = (kibana) => new kibana.Plugin({
throw new Error('New Platform XPack Security plugin is not available.');
}

const config = server.config();
const xpackMainPlugin = server.plugins.xpack_main;
const xpackInfo = xpackMainPlugin.info;
securityPlugin.registerLegacyAPI({
xpackInfo,
isSystemAPIRequest: server.plugins.kibana.systemApi.isSystemApiRequest.bind(
server.plugins.kibana.systemApi
),
cspRules: createCSPRuleString(config.get('csp.rules')),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: if we still need this we'll include that into NP blockers for Security plugin.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Included as blocker, for now we agreed with Platform to provide this through legacy API shim and hopefully get support for "lightweight" apps by 8.0 so that we don't need to set CSP manually anymore.

});

const plugin = this;
const config = server.config();
const xpackInfoFeature = xpackInfo.feature(plugin.id);

// Register a function that is called whenever the xpack info changes,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,90 +257,4 @@ describe('Authentication routes', () => {
expect(response).to.eql({ username: 'user' });
});
});

describe('SAML assertion consumer service endpoint', () => {
let samlAcsRoute;
let request;

beforeEach(() => {
samlAcsRoute = serverStub.route
.withArgs(sinon.match({ path: '/api/security/v1/saml' }))
.firstCall
.args[0];

request = requestFixture({ payload: { SAMLResponse: 'saml-response-xml' } });
});

it('correctly defines route.', async () => {
expect(samlAcsRoute.method).to.be('POST');
expect(samlAcsRoute.path).to.be('/api/security/v1/saml');
expect(samlAcsRoute.handler).to.be.a(Function);
expect(samlAcsRoute.config).to.eql({
auth: false,
validate: {
payload: Joi.object({
SAMLResponse: Joi.string().required(),
RelayState: Joi.string().allow('')
})
}
});
});

it('returns 500 if authentication throws unhandled exception.', async () => {
const unhandledException = new Error('Something went wrong.');
loginStub.throws(unhandledException);

const response = await samlAcsRoute.handler(request, hStub);

sinon.assert.notCalled(hStub.redirect);
expect(response.isBoom).to.be(true);
expect(response.output.payload).to.eql({
statusCode: 500,
error: 'Internal Server Error',
message: 'An internal server error occurred'
});
});

it('returns 401 if authentication fails.', async () => {
const failureReason = new Error('Something went wrong.');
loginStub.resolves(AuthenticationResult.failed(failureReason));

const response = await samlAcsRoute.handler(request, hStub);

sinon.assert.notCalled(hStub.redirect);
expect(response.isBoom).to.be(true);
expect(response.message).to.be(failureReason.message);
expect(response.output.statusCode).to.be(401);
});

it('returns 401 if authentication is not handled.', async () => {
loginStub.resolves(AuthenticationResult.notHandled());

const response = await samlAcsRoute.handler(request, hStub);

sinon.assert.notCalled(hStub.redirect);
expect(response.isBoom).to.be(true);
expect(response.message).to.be('Unauthorized');
expect(response.output.statusCode).to.be(401);
});

it('returns 401 if authentication completes with unexpected result.', async () => {
loginStub.resolves(AuthenticationResult.succeeded({}));

const response = await samlAcsRoute.handler(request, hStub);

sinon.assert.notCalled(hStub.redirect);
expect(response.isBoom).to.be(true);
expect(response.message).to.be('Unauthorized');
expect(response.output.statusCode).to.be(401);
});

it('redirects if required by the authentication process.', async () => {
loginStub.resolves(AuthenticationResult.redirectTo('http://redirect-to/path'));

await samlAcsRoute.handler(request, hStub);

sinon.assert.calledWithExactly(hStub.redirect, 'http://redirect-to/path');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -58,37 +58,6 @@ export function initAuthenticateApi({ authc: { login, logout }, config }, server
}
});

server.route({
method: 'POST',
path: '/api/security/v1/saml',
config: {
auth: false,
validate: {
payload: Joi.object({
SAMLResponse: Joi.string().required(),
RelayState: Joi.string().allow('')
})
}
},
async handler(request, h) {
try {
// When authenticating using SAML we _expect_ to redirect to the SAML Identity provider.
const authenticationResult = await login(KibanaRequest.from(request), {
provider: 'saml',
value: { samlResponse: request.payload.SAMLResponse }
});

if (authenticationResult.redirected()) {
return h.redirect(authenticationResult.redirectURL);
}

return Boom.unauthorized(authenticationResult.error);
} catch (err) {
return wrapError(err);
}
}
});

/**
* The route should be configured as a redirect URI in OP when OpenID Connect implicit flow
* is used, so that we can extract authentication response from URL fragment and send it to
Expand Down
2 changes: 2 additions & 0 deletions x-pack/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@
"@types/tar-fs": "^1.16.1",
"@types/tinycolor2": "^1.4.1",
"@types/uuid": "^3.4.4",
"@types/xml2js": "^0.4.5",
"@types/xml-crypto": "^1.4.0",
"abab": "^1.0.4",
"ansicolors": "0.3.2",
"axios": "^0.19.0",
Expand Down
18 changes: 18 additions & 0 deletions x-pack/plugins/security/server/authentication/index.mock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { Authentication } from '.';

export const authenticationMock = {
create: (): jest.Mocked<Authentication> => ({
login: jest.fn(),
createAPIKey: jest.fn(),
getCurrentUser: jest.fn(),
invalidateAPIKey: jest.fn(),
isAuthenticated: jest.fn(),
logout: jest.fn(),
}),
};
12 changes: 10 additions & 2 deletions x-pack/plugins/security/server/authentication/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { UnwrapPromise } from '@kbn/utility-types';
import {
IClusterClient,
CoreSetup,
Expand All @@ -20,8 +21,13 @@ export { canRedirectRequest } from './can_redirect_request';
export { Authenticator, ProviderLoginAttempt } from './authenticator';
export { AuthenticationResult } from './authentication_result';
export { DeauthenticationResult } from './deauthentication_result';
export { OIDCAuthenticationFlow } from './providers';
export { CreateAPIKeyResult } from './api_keys';
export { OIDCAuthenticationFlow, SAMLLoginStep } from './providers';
export {
CreateAPIKeyResult,
InvalidateAPIKeyResult,
CreateAPIKeyParams,
InvalidateAPIKeyParams,
} from './api_keys';

interface SetupAuthenticationParams {
core: CoreSetup;
Expand All @@ -31,6 +37,8 @@ interface SetupAuthenticationParams {
getLegacyAPI(): LegacyAPI;
}

export type Authentication = UnwrapPromise<ReturnType<typeof setupAuthentication>>;

export async function setupAuthentication({
core,
clusterClient,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export {
} from './base';
export { BasicAuthenticationProvider } from './basic';
export { KerberosAuthenticationProvider } from './kerberos';
export { SAMLAuthenticationProvider, isSAMLRequestQuery } from './saml';
export { SAMLAuthenticationProvider, isSAMLRequestQuery, SAMLLoginStep } from './saml';
export { TokenAuthenticationProvider } from './token';
export { OIDCAuthenticationProvider, OIDCAuthenticationFlow } from './oidc';
export { PKIAuthenticationProvider } from './pki';
Loading